Page MenuHomePhabricator

[Clang] Add __builtin_addressof_nocfi
Needs ReviewPublic

Authored by samitolvanen on Aug 20 2021, 11:59 AM.

Details

Summary

This change adds builtin_addressof_nocfi(). This built-in function
is similar to the existing
builtin_addressof(), except that with
-fsanitize=cfi, it returns the address of the function body instead of
a CFI jump table address.

__builtin_addressof_nocfi() is helpful in low-level code, such as
operating system kernels, which may need the address of a specific
function without a jump table indirection.

Link: https://github.com/ClangBuiltLinux/linux/issues/1353

Depends on D108478

Diff Detail

Event Timeline

samitolvanen created this revision.Aug 20 2021, 11:59 AM
samitolvanen requested review of this revision.Aug 20 2021, 11:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 20 2021, 11:59 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript

Here's a PoC of the built-in function that returns the address of the function body with CFI. Based on D108478.

clang/lib/CodeGen/CGExprConstant.cpp
1890–1895

fix linter warning

1891

Don't use auto here.
https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable
It's common when the type is already specified on the RHS of an assignment that's using of the casting operators that's already explicitly specialized with the type.

clang/test/CodeGen/builtin-addressof-nocfi.c
19

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?

martong removed a subscriber: martong.Aug 27 2021, 1:29 AM
samitolvanen marked 2 inline comments as done.

Fixed clang-tidy warnings, dropped an unnecessary auto.

clang/test/CodeGen/builtin-addressof-nocfi.c
19

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.