[PATCH] D14274: Add alloc_size attribute to clang

[George Burgess IV via cfe-commits]

george.burgess.iv 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) +
----------------
ahatanak wrote:
> 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.
Hm. Looks like how we handle allocsize in LLVM may be slightly broken, then, because it works with regular allocas.

Filed PR28834. Thanks for the heads-up. :)


https://reviews.llvm.org/D14274