[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi

Thu Nov 4 03:05:38 PDT 2021

ardb added a comment.

I would argue that the existing __builtin_addressof() should absorb this behavior, rather than adding a special builtin related to CFI.

As is documented for __builtin_addressof(), its intended use is in cases where the & operator may return something other than the address of the object, which is exactly the case we are dealing with here.

================
Comment at: clang/test/CodeGen/builtin-addressof-nocfi.c:18
+  // CHECK: call void @c(void ()* no_cfi @a)
+  c(__builtin_addressof_nocfi(a));
+  e();
----------------
samitolvanen wrote:
> nickdesaulniers wrote:
> > do we ever need the builtin address of a global that's NOT a function?
> > 
> > If so, we should add a test for that. If not, perhaps we don't need to waste space in every APValue?
> > do we ever need the builtin address of a global that's NOT a function?
> 
> I don't think so. This version does accept any global because it was modelled after `__builtin_addressof()`, but we could look into limiting this only for functions. Perhaps the built-in name should also be changed in that case?
> 
> > If so, we should add a test for that. If not, perhaps we don't need to waste space in every APValue?
> 
> What would be a better place for the flag? I think in Clang, changing this to support only functions would probably just need some additional Sema checks.
> > do we ever need the builtin address of a global that's NOT a function?
> 
> I don't think so. This version does accept any global because it was modelled after `__builtin_addressof()`, but we could look into limiting this only for functions. Perhaps the built-in name should also be changed in that case?
> 

Why is that necessary? That will only make it harder to use for users that want to wrap this in another macro that may be used for arbitrary symbols.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108479/new/

https://reviews.llvm.org/D108479