Skip to content

Commit 3d6f49a

Browse files
author
serge-sans-paille
committed
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
1 parent 5aa4c74 commit 3d6f49a

File tree

6 files changed

+58
-28
lines changed

6 files changed

+58
-28
lines changed

clang/lib/CodeGen/CodeGenFunction.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1301,6 +1301,11 @@ void CodeGenFunction::GenerateCode(GlobalDecl GD, llvm::Function *Fn,
13011301
FunctionArgList Args;
13021302
QualType ResTy = BuildFunctionArgList(GD, Args);
13031303

1304+
// Give a different name to inline builtin to avoid conflict with actual
1305+
// builtins.
1306+
if (FD->isInlineBuiltinDeclaration() && Fn)
1307+
Fn->setName(Fn->getName() + ".inline");
1308+
13041309
// Check if we should generate debug info for this function.
13051310
if (FD->hasAttr<NoDebugAttr>()) {
13061311
// Clear non-distinct debug info that was possibly attached to the function

clang/lib/CodeGen/CodeGenModule.cpp

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3169,6 +3169,11 @@ bool CodeGenModule::shouldEmitFunction(GlobalDecl GD) {
31693169
}
31703170
}
31713171

3172+
// Inline builtins declaration must be emitted. They often are fortified
3173+
// functions.
3174+
if (F->isInlineBuiltinDeclaration())
3175+
return true;
3176+
31723177
// PR9614. Avoid cases where the source code is lying to us. An available
31733178
// externally function should have an equivalent function somewhere else,
31743179
// but a function that calls itself through asm label/`__builtin_` trickery is
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
2+
3+
// RUN: %clang_cc1 -triple x86_64 -S -emit-llvm -o - %s | FileCheck %s
4+
//
5+
// Verifies that clang detects memcpy inline version and uses it instead of the builtin.
6+
7+
typedef unsigned long size_t;
8+
9+
// Clang requires these attributes for a function to be redefined.
10+
#define AVAILABLE_EXTERNALLY extern inline __attribute__((always_inline)) __attribute__((gnu_inline))
11+
12+
// Clang recognizes an inline builtin and renames it to prevent conflict with builtins.
13+
AVAILABLE_EXTERNALLY void *memcpy(void *a, const void *b, size_t c) {
14+
asm("# memcpy.inline marker");
15+
return __builtin_memcpy(a, b, c);
16+
}
17+
18+
// CHECK-LABEL: @foo(
19+
// CHECK-NEXT: entry:
20+
// CHECK-NEXT: [[A_ADDR_I:%.*]] = alloca i8*, align 8
21+
// CHECK-NEXT: [[B_ADDR_I:%.*]] = alloca i8*, align 8
22+
// CHECK-NEXT: [[C_ADDR_I:%.*]] = alloca i64, align 8
23+
// CHECK-NEXT: [[A_ADDR:%.*]] = alloca i8*, align 8
24+
// CHECK-NEXT: [[B_ADDR:%.*]] = alloca i8*, align 8
25+
// CHECK-NEXT: [[C_ADDR:%.*]] = alloca i64, align 8
26+
// CHECK-NEXT: store i8* [[A:%.*]], i8** [[A_ADDR]], align 8
27+
// CHECK-NEXT: store i8* [[B:%.*]], i8** [[B_ADDR]], align 8
28+
// CHECK-NEXT: store i64 [[C:%.*]], i64* [[C_ADDR]], align 8
29+
// CHECK-NEXT: [[TMP0:%.*]] = load i8*, i8** [[A_ADDR]], align 8
30+
// CHECK-NEXT: [[TMP1:%.*]] = load i8*, i8** [[B_ADDR]], align 8
31+
// CHECK-NEXT: [[TMP2:%.*]] = load i64, i64* [[C_ADDR]], align 8
32+
// CHECK-NEXT: store i8* [[TMP0]], i8** [[A_ADDR_I]], align 8
33+
// CHECK-NEXT: store i8* [[TMP1]], i8** [[B_ADDR_I]], align 8
34+
// CHECK-NEXT: store i64 [[TMP2]], i64* [[C_ADDR_I]], align 8
35+
// CHECK-NEXT: call void asm sideeffect "# memcpy.inline marker", "~{dirflag},~{fpsr},~{flags}"() #[[ATTR2:[0-9]+]], !srcloc !2
36+
// CHECK-NEXT: [[TMP3:%.*]] = load i8*, i8** [[A_ADDR_I]], align 8
37+
// CHECK-NEXT: [[TMP4:%.*]] = load i8*, i8** [[B_ADDR_I]], align 8
38+
// CHECK-NEXT: [[TMP5:%.*]] = load i64, i64* [[C_ADDR_I]], align 8
39+
// CHECK-NEXT: call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 1 [[TMP3]], i8* align 1 [[TMP4]], i64 [[TMP5]], i1 false) #[[ATTR2]]
40+
// CHECK-NEXT: ret void
41+
//
42+
void foo(void *a, const void *b, size_t c) {
43+
memcpy(a, b, c);
44+
}

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

Lines changed: 0 additions & 25 deletions
This file was deleted.

clang/test/CodeGen/memcpy-nobuiltin.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@
44
//
55
// CHECK-WITH-DECL-NOT: @llvm.memcpy
66
// CHECK-NO-DECL: @llvm.memcpy
7-
// CHECK-SELF-REF-DECL: @llvm.memcpy
7+
// CHECK-SELF-REF-DECL-LABEL: define dso_local i8* @memcpy.inline
8+
// CHECK-SELF-REF-DECL: @memcpy(
89
//
910
#include <memcpy-nobuiltin.inc>
1011
void test(void *dest, void const *from, size_t n) {

clang/test/CodeGen/pr9614.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,14 +32,14 @@ void f(void) {
3232

3333
// CHECK-LABEL: define{{.*}} void @f()
3434
// CHECK: call void @foo()
35-
// CHECK: call i32 @abs(i32 0)
35+
// CHECK: call i32 @abs(i32 %0)
3636
// CHECK: call i8* @strrchr(
3737
// CHECK: call void @llvm.prefetch.p0i8(
3838
// CHECK: call i8* @memchr(
3939
// CHECK: ret void
4040

4141
// CHECK: declare void @foo()
42-
// CHECK: declare i32 @abs(i32
4342
// CHECK: declare i8* @strrchr(i8*, i32)
4443
// CHECK: declare i8* @memchr(
44+
// CHECK: declare i32 @abs(i32
4545
// CHECK: declare void @llvm.prefetch.p0i8(

0 commit comments

Comments
 (0)