[PATCH] D14274: Add alloc_size attribute to clang

[George Burgess IV via cfe-commits]

george.burgess.iv added a comment.

> I'm wondering what the status of this patch is since someone has asked us to add support for this attribute in clang. Are you still waiting for review?


Waiting for an LGTM from Richard, though this patch currently doesn't pass all tests. One of the changes in it causes some craziness with how we codegen blocks (in e.g. objc/OpenCL). Basically, we'll end up emitting the same block definition N times, since this makes the constexpr evaluator more accurate in some cases. I hope to fix that + get back to this in the nearish future.

(Happy to accept an LGTM now, though, since the bug isn't with this patch. ;) Naturally, I'll submit after the block-related bug is fixed.)


================
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:
> 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. :)


https://reviews.llvm.org/D14274