serge-sans-paille created this revision.
serge-sans-paille added reviewers: rnk, nickdesaulniers, efriedma.
serge-sans-paille requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
It is a common practice in glibc header to provide an inline redefinition of an
existing function. It is especially the case for fortified function.
Clang currently has an imperfect approach to the problem, using a combination of
trivially recursive function detection and noinline attribute.
Simplify the logic by suffixing these functions by `.inline` during codegen, so
that they are not recognized as builtin by llvm.
After that patch, clang passes all tests from:
https://github.com/serge-sans-paille/fortify-test-suite
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D109967
Files:
clang/lib/CodeGen/CodeGenFunction.cpp
clang/lib/CodeGen/CodeGenModule.cpp
clang/test/CodeGen/memcpy-inline-builtin.c
clang/test/CodeGen/memcpy-no-nobuiltin-if-not-emitted.c
clang/test/CodeGen/memcpy-nobuiltin.c
clang/test/CodeGen/pr9614.c
Index: clang/test/CodeGen/pr9614.c
===================================================================
--- clang/test/CodeGen/pr9614.c
+++ clang/test/CodeGen/pr9614.c
@@ -32,14 +32,14 @@
// CHECK-LABEL: define{{.*}} void @f()
// CHECK: call void @foo()
-// CHECK: call i32 @abs(i32 0)
+// CHECK: call i32 @abs(i32 %0)
// CHECK: call i8* @strrchr(
// CHECK: call void @llvm.prefetch.p0i8(
// CHECK: call i8* @memchr(
// CHECK: ret void
// CHECK: declare void @foo()
-// CHECK: declare i32 @abs(i32
// CHECK: declare i8* @strrchr(i8*, i32)
// CHECK: declare i8* @memchr(
+// CHECK: declare i32 @abs(i32
// CHECK: declare void @llvm.prefetch.p0i8(
Index: clang/test/CodeGen/memcpy-nobuiltin.c
===================================================================
--- clang/test/CodeGen/memcpy-nobuiltin.c
+++ clang/test/CodeGen/memcpy-nobuiltin.c
@@ -4,7 +4,8 @@
//
// CHECK-WITH-DECL-NOT: @llvm.memcpy
// CHECK-NO-DECL: @llvm.memcpy
-// CHECK-SELF-REF-DECL: @llvm.memcpy
+// CHECK-SELF-REF-DECL-LABEL: define dso_local i8* @memcpy.inline
+// CHECK-SELF-REF-DECL: @memcpy(
//
#include <memcpy-nobuiltin.inc>
void test(void *dest, void const *from, size_t n) {
Index: clang/test/CodeGen/memcpy-no-nobuiltin-if-not-emitted.c
===================================================================
--- clang/test/CodeGen/memcpy-no-nobuiltin-if-not-emitted.c
+++ /dev/null
@@ -1,25 +0,0 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s | FileCheck %s
-//
-// Verifies that clang doesn't mark an inline builtin definition as `nobuiltin`
-// if the builtin isn't emittable.
-
-typedef unsigned long size_t;
-
-// always_inline is used so clang will emit this body. Otherwise, we need >=
-// -O1.
-#define AVAILABLE_EXTERNALLY extern inline __attribute__((always_inline)) \
- __attribute__((gnu_inline))
-
-AVAILABLE_EXTERNALLY void *memcpy(void *a, const void *b, size_t c) {
- return __builtin_memcpy(a, b, c);
-}
-
-// CHECK-LABEL: define{{.*}} void @foo
-void foo(void *a, const void *b, size_t c) {
- // Clang will always _emit_ this as memcpy. LLVM turns it into @llvm.memcpy
- // later on if optimizations are enabled.
- // CHECK: call i8* @memcpy
- memcpy(a, b, c);
-}
-
-// CHECK-NOT: nobuiltin
Index: clang/test/CodeGen/memcpy-inline-builtin.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/memcpy-inline-builtin.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -S -emit-llvm -o - %s -disable-llvm-passes | FileCheck %s
+//
+// Verifies that clang detects memcpy inline version and use it correctly.
+
+typedef unsigned long size_t;
+
+// always_inline is used so clang will emit this body. Otherwise, we need >= -O1.
+#define AVAILABLE_EXTERNALLY extern inline __attribute__((always_inline)) \
+__attribute__((gnu_inline))
+
+// Clang recognizes an inline builtin and renames it to prevent conflict with
+// builtins.
+
+AVAILABLE_EXTERNALLY void *memcpy(void *a, const void *b, size_t c) {
+ return __builtin_memcpy(a, b, c);
+}
+
+// CHECK-LABEL: define{{.*}} void @foo
+void foo(void *a, const void *b, size_t c) {
+ // CHECK: call i8* @memcpy.inline
+ memcpy(a, b, c);
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1293,8 +1293,10 @@
case MultiVersionKind::None:
llvm_unreachable("None multiversion type isn't valid here");
}
+
}
+
// Make unique name for device side static file-scope variable for HIP.
if (CGM.getContext().shouldExternalizeStaticVar(ND) &&
CGM.getLangOpts().GPURelocatableDeviceCode &&
@@ -3146,6 +3148,7 @@
if (getFunctionLinkage(GD) != llvm::Function::AvailableExternallyLinkage)
return true;
const auto *F = cast<FunctionDecl>(GD.getDecl());
+
if (CodeGenOpts.OptimizationLevel == 0 && !F->hasAttr<AlwaysInlineAttr>())
return false;
@@ -3169,6 +3172,11 @@
}
}
+ // Inline builtins declaration must be emitted. They often are fortified
+ // functions.
+ if (F->isInlineBuiltinDeclaration())
+ return true;
+
// PR9614. Avoid cases where the source code is lying to us. An available
// externally function should have an equivalent function somewhere else,
// but a function that calls itself through asm label/`__builtin_` trickery is
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -1301,6 +1301,10 @@
FunctionArgList Args;
QualType ResTy = BuildFunctionArgList(GD, Args);
+ if (FD->isInlineBuiltinDeclaration() && Fn) {
+ Fn->setName(Fn->getName() + ".inline");
+ }
+
// Check if we should generate debug info for this function.
if (FD->hasAttr<NoDebugAttr>()) {
// Clear non-distinct debug info that was possibly attached to the function
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
