[PATCH] D14274: Add alloc_size attribute to clang

[Akira Hatanaka via cfe-commits]

ahatanak added inline comments.

================
Comment at: test/CodeGenCXX/alloc-size.cpp:66
@@ +65,3 @@
+  // CHECK: ret i32 122
+  return __builtin_object_size(my_malloc<int>(), 0) +
+         __builtin_object_size(my_calloc<int>(5), 0) +
----------------
george.burgess.iv wrote:
> ahatanak wrote:
> > Is it necessary to compute __builtin_object_size in the front-end (rather than in some IR passes like instcombine) when it takes the pointer returned by a function marked alloc_size?
> > 
> > Also, is the IR optimization smart enough to get the exact object size in the following case?
> > 
> > ```
> > void foo(int a, int b) {
> >   void *p0 = my_malloc(a);
> >   g0 = __builtin_object_size(p0, 0);
> >   void *p1 = my_calloc(a, b);
> >   g1 = __builtin_object_size(p1, 1);
> > }
> > 
> > void foo1() {
> >   foo(10, 50);
> > }
> > ```
> > Is it necessary to compute __builtin_object_size in the front-end (rather than in some IR passes like instcombine)
> 
> It's necessary to try in the frontend. If that fails, we'll lower calls to __bos to `@llvm.objectsize`, and let LLVM try it.
> 
> The main reason that we care for the frontend is, among other reasons, so we can use it with the `enable_if` attribute, like so:
> 
> ```
> void strncat(char *buf, int n, const char *from)
>   __attribute__((overloadable,
>                  enable_if(__builtin_object_size(buf, 0) < n, ""),
>                  unavailable("'n' is larger than the target buffer!")));
> 
> void strncat(char *buf, int n, const char *from)
>   __attribute__((overloadable));
> 
> int main() {
>   char buf[4];
>   strncat(buf, sizeof(buf)+1, "hi"); // expected-error{{'n' is larger than the target buffer!}}
> }
> ```
> 
> 
> ----
> 
> > Also, is the IR optimization smart enough to get the exact object size in the following case?
> 
> `__builtin_object_size` *always* gets lowered to a constant by either clang or LLVM. That is, if `foo` gets inlined into `foo1`, then LLVM should be able to determine accurate values for `g0` and `g1`. If `foo` isn't inlined, then no: you'll get -1 for both.
> 
> If you'd like to see what we can and can't get, LLVM already has the `allocsize` attribute (and `@llvm.objectsize` intrinsic) in it. :)
Thanks for the explanation. I see why you want to evaluate it in the front-end.

It looks like llvm optimizes function foo before it gets inlined into function foo1 and, as a result, g0 and g1 both get -1 instead of 10 and 500. I guess llvm should defer optimizing the function until after inliner is run.


https://reviews.llvm.org/D14274