[PATCH] D108479: [Clang] Add __builtin_addressof_nocfi
Ard Biesheuvel via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
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
More information about the cfe-commits
mailing list