[llvm] [CFI][annotation] Leave alone function pointers in function annotations (PR #80173)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 31 23:50:02 PST 2024
https://github.com/yozhu updated https://github.com/llvm/llvm-project/pull/80173
>From c91a3a6f07488703d9f939a0d2477023589fc1fe Mon Sep 17 00:00:00 2001
From: YongKang Zhu <yongzhu at meta.com>
Date: Thu, 25 Jan 2024 20:37:10 -0800
Subject: [PATCH 1/2] [CFI][annotation] Leave alone function pointers in
function annotations
Function annotation, as part of llvm.metadata, is for the function itself
and doesn't apply to its corresponding jump table entry, so with CFI we
shouldn't replace function pointer in function annotation with pointer to
its corresponding jump table entry.
---
llvm/lib/Transforms/IPO/LowerTypeTests.cpp | 30 +++++++++++++++++++++-
1 file changed, 29 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
index 733f290b1bc93..f6630019eaec6 100644
--- a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
+++ b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
@@ -470,6 +470,9 @@ class LowerTypeTestsModule {
Function *WeakInitializerFn = nullptr;
+ GlobalVariable *GlobalAnnotation;
+ std::set<void *> FunctionAnnotations;
+
bool shouldExportConstantsAsAbsoluteSymbols();
uint8_t *exportTypeId(StringRef TypeId, const TypeIdLowering &TIL);
TypeIdLowering importTypeId(StringRef TypeId);
@@ -531,6 +534,10 @@ class LowerTypeTestsModule {
/// replace each use, which is a direct function call.
void replaceDirectCalls(Value *Old, Value *New);
+ bool isFunctionAnnotation(void *GV) {
+ return FunctionAnnotations.count(GV) != 0;
+ }
+
public:
LowerTypeTestsModule(Module &M, ModuleAnalysisManager &AM,
ModuleSummaryIndex *ExportSummary,
@@ -1377,8 +1384,11 @@ void LowerTypeTestsModule::replaceWeakDeclarationWithJumpTablePtr(
// (all?) targets. Switch to a runtime initializer.
SmallSetVector<GlobalVariable *, 8> GlobalVarUsers;
findGlobalVariableUsersOf(F, GlobalVarUsers);
- for (auto *GV : GlobalVarUsers)
+ for (auto *GV : GlobalVarUsers) {
+ if (GV == GlobalAnnotation)
+ continue;
moveInitializerToModuleConstructor(GV);
+ }
// Can not RAUW F with an expression that uses F. Replace with a temporary
// placeholder first.
@@ -1392,6 +1402,10 @@ void LowerTypeTestsModule::replaceWeakDeclarationWithJumpTablePtr(
// Don't use range based loop, because use list will be modified.
while (!PlaceholderFn->use_empty()) {
Use &U = *PlaceholderFn->use_begin();
+ if (isFunctionAnnotation(U.getUser())) {
+ U.set(F);
+ continue;
+ }
auto *InsertPt = dyn_cast<Instruction>(U.getUser());
assert(InsertPt && "Non-instruction users should have been eliminated");
auto *PN = dyn_cast<PHINode>(InsertPt);
@@ -1837,6 +1851,16 @@ LowerTypeTestsModule::LowerTypeTestsModule(
}
OS = TargetTriple.getOS();
ObjectFormat = TargetTriple.getObjectFormat();
+
+ // Function annotation describes or applies to function itself, and
+ // shouldn't be associated with jump table thunk generated for CFI.
+ GlobalAnnotation = M.getGlobalVariable("llvm.global.annotations");
+ auto *C = dyn_cast_or_null<Constant>(GlobalAnnotation);
+ if (C && C->getNumOperands() == 1) {
+ C = cast<Constant>(C->getOperand(0));
+ for (auto &Op : C->operands())
+ FunctionAnnotations.insert(Op.get());
+ }
}
bool LowerTypeTestsModule::runForTesting(Module &M, ModuleAnalysisManager &AM) {
@@ -1900,6 +1924,10 @@ void LowerTypeTestsModule::replaceCfiUses(Function *Old, Value *New,
if (isDirectCall(U) && (Old->isDSOLocal() || !IsJumpTableCanonical))
continue;
+ // Skip function annotation
+ if (isFunctionAnnotation(U.getUser()))
+ continue;
+
// Must handle Constants specially, we cannot call replaceUsesOfWith on a
// constant because they are uniqued.
if (auto *C = dyn_cast<Constant>(U.getUser())) {
>From a6fef79167066afdf715c6f1bb7834a9d04d575e Mon Sep 17 00:00:00 2001
From: YongKang Zhu <yongzhu at fb.com>
Date: Wed, 31 Jan 2024 23:48:21 -0800
Subject: [PATCH 2/2] Address review feedback
---
llvm/lib/Transforms/IPO/LowerTypeTests.cpp | 16 ++++-----
llvm/test/CodeGen/AArch64/cfi-annotation.test | 35 +++++++++++++++++++
2 files changed, 43 insertions(+), 8 deletions(-)
create mode 100644 llvm/test/CodeGen/AArch64/cfi-annotation.test
diff --git a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
index f6630019eaec6..e5e7a022763c0 100644
--- a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
+++ b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp
@@ -471,7 +471,7 @@ class LowerTypeTestsModule {
Function *WeakInitializerFn = nullptr;
GlobalVariable *GlobalAnnotation;
- std::set<void *> FunctionAnnotations;
+ DenseSet<Value *> FunctionAnnotations;
bool shouldExportConstantsAsAbsoluteSymbols();
uint8_t *exportTypeId(StringRef TypeId, const TypeIdLowering &TIL);
@@ -534,8 +534,8 @@ class LowerTypeTestsModule {
/// replace each use, which is a direct function call.
void replaceDirectCalls(Value *Old, Value *New);
- bool isFunctionAnnotation(void *GV) {
- return FunctionAnnotations.count(GV) != 0;
+ bool isFunctionAnnotation(Value *V) {
+ return FunctionAnnotations.count(V) != 0;
}
public:
@@ -1855,11 +1855,11 @@ LowerTypeTestsModule::LowerTypeTestsModule(
// Function annotation describes or applies to function itself, and
// shouldn't be associated with jump table thunk generated for CFI.
GlobalAnnotation = M.getGlobalVariable("llvm.global.annotations");
- auto *C = dyn_cast_or_null<Constant>(GlobalAnnotation);
- if (C && C->getNumOperands() == 1) {
- C = cast<Constant>(C->getOperand(0));
- for (auto &Op : C->operands())
- FunctionAnnotations.insert(Op.get());
+ if (GlobalAnnotation && GlobalAnnotation->getNumOperands() == 1) {
+ const ConstantArray *CA =
+ cast<ConstantArray>(GlobalAnnotation->getOperand(0));
+ for (Value *Op : CA->operands())
+ FunctionAnnotations.insert(Op);
}
}
diff --git a/llvm/test/CodeGen/AArch64/cfi-annotation.test b/llvm/test/CodeGen/AArch64/cfi-annotation.test
new file mode 100644
index 0000000000000..d1191a66ea1a5
--- /dev/null
+++ b/llvm/test/CodeGen/AArch64/cfi-annotation.test
@@ -0,0 +1,35 @@
+; RUN: echo -e "int (*fptr1)(int);" > %t.c
+; RUN: echo -e "int (*fptr2)(int);" >> %t.c
+; RUN: echo -e "__attribute__((annotate(\"test_foo\"))) int foo(int);" >> %t.c
+; RUN: echo -e "__attribute__((annotate(\"test_goo\"))) int goo(int);" >> %t.c
+; RUN: echo -e "__attribute__((annotate(\"test_bar\"))) int bar(int x) {" >> %t.c
+; RUN: echo -e " return foo(x) + goo(x);" >> %t.c
+; RUN: echo -e "}" >> %t.c
+; RUN: echo -e "int test(int x) {" >> %t.c
+; RUN: echo -e " if (x > 10) {" >> %t.c
+; RUN: echo -e " fptr1 = bar;" >> %t.c
+; RUN: echo -e " fptr2 = foo;" >> %t.c
+; RUN: echo -e " } else if (x > 0) {" >> %t.c
+; RUN: echo -e " fptr1 = bar;" >> %t.c
+; RUN: echo -e " fptr2 = goo;" >> %t.c
+; RUN: echo -e " } else {" >> %t.c
+; RUN: echo -e " fptr1 = goo;" >> %t.c
+; RUN: echo -e " fptr2 = foo;" >> %t.c
+; RUN: echo -e " }" >> %t.c
+; RUN: echo -e " return fptr1(fptr2(x));" >> %t.c
+; RUN: echo -e "}" >> %t.c
+
+; RUN: clang -target aarch64-none-linux-gnu -c -o %t.o -fPIC -flto=full \
+; RUN: -fno-sanitize-cfi-canonical-jump-tables -fsanitize=cfi-icall \
+; RUN: -fno-sanitize-ignorelist %t.c
+; RUN: ld.lld -shared -o %t.so %t.o --save-temps
+; RUN: llvm-dis %t.so.0.4.opt.bc
+
+; REM: Find the `llvm.global.annotations` symbol in `%t.so.0.4.opt.ll` and
+: REM: verify that no function annotation references CFI jump table entry.
+
+; RUN: grep llvm.global.annotations %t.so.0.4.opt.ll > %t.annotations
+; RUN: grep bar %t.annotations
+; RUN: grep foo %t.annotations
+; RUN: grep goo %t.annotations
+; RUN: not grep cfi %t.annotations
More information about the llvm-commits
mailing list