[clang] 3d6f49a - Simplify handling of builtin with inline redefinition

via cfe-commits cfe-commits at lists.llvm.org
Tue Sep 28 04:24:33 PDT 2021


Author: serge-sans-paille
Date: 2021-09-28T13:24:25+02:00
New Revision: 3d6f49a56995b845c40be5827ded5d1e3f692cec

URL: https://github.com/llvm/llvm-project/commit/3d6f49a56995b845c40be5827ded5d1e3f692cec
DIFF: https://github.com/llvm/llvm-project/commit/3d6f49a56995b845c40be5827ded5d1e3f692cec.diff

LOG: Simplify handling of builtin with inline redefinition

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

Differential Revision: https://reviews.llvm.org/D109967

Added: 
    clang/test/CodeGen/memcpy-inline-builtin.c

Modified: 
    clang/lib/CodeGen/CodeGenFunction.cpp
    clang/lib/CodeGen/CodeGenModule.cpp
    clang/test/CodeGen/memcpy-nobuiltin.c
    clang/test/CodeGen/pr9614.c

Removed: 
    clang/test/CodeGen/memcpy-no-nobuiltin-if-not-emitted.c


################################################################################
diff  --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp
index 5f63461aea96b..fbb06885ba354 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -1301,6 +1301,11 @@ void CodeGenFunction::GenerateCode(GlobalDecl GD, llvm::Function *Fn,
   FunctionArgList Args;
   QualType ResTy = BuildFunctionArgList(GD, Args);
 
+  // Give a 
diff erent name to inline builtin to avoid conflict with actual
+  // builtins.
+  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

diff  --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 9715657b4c6e1..83418014f0b5c 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3169,6 +3169,11 @@ bool CodeGenModule::shouldEmitFunction(GlobalDecl GD) {
     }
   }
 
+  // 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

diff  --git a/clang/test/CodeGen/memcpy-inline-builtin.c b/clang/test/CodeGen/memcpy-inline-builtin.c
new file mode 100644
index 0000000000000..814ce220195fb
--- /dev/null
+++ b/clang/test/CodeGen/memcpy-inline-builtin.c
@@ -0,0 +1,44 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+
+// RUN: %clang_cc1 -triple x86_64 -S -emit-llvm -o - %s | FileCheck %s
+//
+// Verifies that clang detects memcpy inline version and uses it instead of the builtin.
+
+typedef unsigned long size_t;
+
+// Clang requires these attributes for a function to be redefined.
+#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) {
+  asm("# memcpy.inline marker");
+  return __builtin_memcpy(a, b, c);
+}
+
+// CHECK-LABEL: @foo(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[A_ADDR_I:%.*]] = alloca i8*, align 8
+// CHECK-NEXT:    [[B_ADDR_I:%.*]] = alloca i8*, align 8
+// CHECK-NEXT:    [[C_ADDR_I:%.*]] = alloca i64, align 8
+// CHECK-NEXT:    [[A_ADDR:%.*]] = alloca i8*, align 8
+// CHECK-NEXT:    [[B_ADDR:%.*]] = alloca i8*, align 8
+// CHECK-NEXT:    [[C_ADDR:%.*]] = alloca i64, align 8
+// CHECK-NEXT:    store i8* [[A:%.*]], i8** [[A_ADDR]], align 8
+// CHECK-NEXT:    store i8* [[B:%.*]], i8** [[B_ADDR]], align 8
+// CHECK-NEXT:    store i64 [[C:%.*]], i64* [[C_ADDR]], align 8
+// CHECK-NEXT:    [[TMP0:%.*]] = load i8*, i8** [[A_ADDR]], align 8
+// CHECK-NEXT:    [[TMP1:%.*]] = load i8*, i8** [[B_ADDR]], align 8
+// CHECK-NEXT:    [[TMP2:%.*]] = load i64, i64* [[C_ADDR]], align 8
+// CHECK-NEXT:    store i8* [[TMP0]], i8** [[A_ADDR_I]], align 8
+// CHECK-NEXT:    store i8* [[TMP1]], i8** [[B_ADDR_I]], align 8
+// CHECK-NEXT:    store i64 [[TMP2]], i64* [[C_ADDR_I]], align 8
+// CHECK-NEXT:    call void asm sideeffect "# memcpy.inline marker", "~{dirflag},~{fpsr},~{flags}"() #[[ATTR2:[0-9]+]], !srcloc !2
+// CHECK-NEXT:    [[TMP3:%.*]] = load i8*, i8** [[A_ADDR_I]], align 8
+// CHECK-NEXT:    [[TMP4:%.*]] = load i8*, i8** [[B_ADDR_I]], align 8
+// CHECK-NEXT:    [[TMP5:%.*]] = load i64, i64* [[C_ADDR_I]], align 8
+// CHECK-NEXT:    call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 [[TMP3]], i8* align 1 [[TMP4]], i64 [[TMP5]], i1 false) #[[ATTR2]]
+// CHECK-NEXT:    ret void
+//
+void foo(void *a, const void *b, size_t c) {
+  memcpy(a, b, c);
+}

diff  --git a/clang/test/CodeGen/memcpy-no-nobuiltin-if-not-emitted.c b/clang/test/CodeGen/memcpy-no-nobuiltin-if-not-emitted.c
deleted file mode 100644
index d3a219a972d3e..0000000000000
--- a/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

diff  --git a/clang/test/CodeGen/memcpy-nobuiltin.c b/clang/test/CodeGen/memcpy-nobuiltin.c
index fb51d87413a16..cbd0ccaa2ebb3 100644
--- a/clang/test/CodeGen/memcpy-nobuiltin.c
+++ b/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) {

diff  --git a/clang/test/CodeGen/pr9614.c b/clang/test/CodeGen/pr9614.c
index c153283a83778..3674ba844aa1c 100644
--- a/clang/test/CodeGen/pr9614.c
+++ b/clang/test/CodeGen/pr9614.c
@@ -32,14 +32,14 @@ void f(void) {
 
 // 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(


        


More information about the cfe-commits mailing list