Page MenuHomePhabricator

[CodeGen] Mark inline definitions of builtins as nobuiltin only if we plan to emit them.
ClosedPublic

Authored by george.burgess.iv on Apr 14 2020, 3:16 PM.

Details

Summary

This suboptimality was encountered in Linux (see some discussion on D71082, and https://github.com/ClangBuiltLinux/linux/issues/979).

While it'd be great to have the kernel's FORTIFY working well with clang, it seems best to preserve old functionality in cases where D71082 can't emit a FORTIFY'ed wrapper of a builtin. With this patch, the memcpy in the test-case here https://reviews.llvm.org/D71082#1953975 compiles back to a mov.

Diff Detail

Event Timeline

This revision is now accepted and ready to land.Apr 14 2020, 5:47 PM
This revision was automatically updated to reflect the committed changes.

This triggers failed asserts:

$ cat dcpdecrypt.cpp
extern "C" __inline__ __attribute__((__gnu_inline__)) void *
memcpy(void *, const void *, unsigned) {}
void *memcpy(void *, const void *, unsigned);
void a() { memcpy; }
$ clang++ -c dcpdecrypt.cpp -target i686-linux-gnu
clang++: ../tools/clang/lib/AST/Decl.cpp:3436: bool clang::FunctionDecl::isInlineDefinitionExternallyVisible() const: Assertion `(doesThisDeclarationHaveABody() || willHaveBody() || hasAttr<AliasAttr>()) && "Must be
a function definition"' failed.

Thanks for the report! Looking now.

melver added a subscriber: melver.May 19 2020, 3:26 AM

This seems to be causing problems for the Linux kernel with ASan instrumentation enabled: https://lkml.kernel.org/r/20200518180513.GA114619@google.com

Still investigating if it's the kernel, KASAN, or Clang that's doing something wrong. Any pointers welcome.

Many thanks!