[PATCH RFC] add support to alloc_size attribute

Clang Frontend
1

Hi,

Attached is a proposed patch that adds support to the alloc_size attribute.

http://llvm.org/bugs/show_bug.cgi?id=10516

alloc_size.patch (6.14 KB)

2

Hi,

I’ve not looked at the patch in any detail, but I see that it doesn’t affect the __builtin_object_size calculation. Do you intend to implement that part of it too? The code implementing that is in lib/AST/ExprConstant.cpp (search for TryEvaluateBuiltinObjectSize).

Future patches should be sent to cfe-commits@ for review, not cfe-dev@.

Thanks!
Richard

3

Quick review:

+ unsigned* start = &AllocSizeArgs[0];
+ unsigned size = AllocSizeArgs.size();
+ llvm::array_pod_sort(start, start + size);

Don't you need to check for duplicates? Also, for an alloc_size
attribute which doesn't specify any indexes?

+ // Is the function argument an integer type?
+ QualType T = getFunctionOrMethodArgType(D, x).getNonReferenceType();

I don't think you want to allow applying alloc_size to arguments of type "int&".

+ unsigned x = (unsigned) ArgNum.getZExtValue();

This is unsafe; there's no guarantee ArgNum fits into a 64-bit integer
(which will cause an assert), and you're masking off the top 32 bits
of that 64-bit integer without any additional checks. You should
perform the bounds checking on the APSInt, and then perform whatever
conversion is necessary. (And if this is copied from existing cod
which does the same thing, please fix that as well.)

-Eli

4

I've not looked at the patch in any detail, but I see that it doesn't affect the __builtin_object_size calculation. Do you intend to implement that part of it too? The code implementing that is in lib/AST/ExprConstant.cpp (search for TryEvaluateBuiltinObjectSize).

You are right. This patch only recognizes the alloc_size attribute. It was not intended to be used for __builtin_object_size. Internally we use it to mark allocation sizes and generate code to prevent integer overflows (i.e., forcing malloc to return NULL if the allocation size calculation overflows).

Future patches should be sent to cfe-commits@ for review, not cfe-dev@.

Thanks for letting me know. :wink:

- xi

5

Thanks for the comments. Yes, the code is basically a clone of
handleNonNullAttr(). Probably we need some helper function to
handle both cases. :wink:

- xi