Skip to content

[CFI][annotation] Leave alone function pointers in function annotations #80173

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Feb 9, 2024
Merged

[CFI][annotation] Leave alone function pointers in function annotations #80173

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c91a3a6
[CFI][annotation] Leave alone function pointers in function annotations
Jan 26, 2024
a6fef79
Address review feedback
yozhu Feb 1, 2024
f8cfc2d
Address review feedback 2
yozhu Feb 1, 2024
b8e7d0a
Address review feedback 3
yozhu Feb 2, 2024
083cb19
Address review feedback 4
yozhu Feb 6, 2024
40eef6f
Remove unnecessary stuff from the test
yozhu Feb 7, 2024
3ddede4
Address review feedback 5
yozhu Feb 8, 2024
62eff41
Add REQUIRES to new test
yozhu Feb 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 29 additions & 1 deletion llvm/lib/Transforms/IPO/LowerTypeTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand All @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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())) {
Expand Down