This revision was automatically updated to reflect the committed changes.
Closed by commit rL368495: cfi-icall: Allow the jump table to be optionally
made non-canonical. (authored by pcc, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D65629?vs=214434&id=214468#toc
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65629/new/
https://reviews.llvm.org/D65629
Files:
cfe/trunk/docs/ControlFlowIntegrity.rst
cfe/trunk/include/clang/Basic/Attr.td
cfe/trunk/include/clang/Basic/AttrDocs.td
cfe/trunk/include/clang/Basic/CodeGenOptions.def
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Driver/SanitizerArgs.h
cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
cfe/trunk/lib/CodeGen/CodeGenModule.cpp
cfe/trunk/lib/Driver/SanitizerArgs.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Sema/SemaDeclAttr.cpp
cfe/trunk/test/CodeGen/cfi-icall-canonical-jump-tables.c
cfe/trunk/test/CodeGen/cfi-icall-cross-dso.c
cfe/trunk/test/Driver/fsanitize.c
cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
cfe/trunk/test/SemaCXX/attr-cfi-canonical-jump-table.cpp
llvm/trunk/include/llvm/Transforms/IPO/LowerTypeTests.h
llvm/trunk/lib/Transforms/IPO/CrossDSOCFI.cpp
llvm/trunk/lib/Transforms/IPO/LowerTypeTests.cpp
llvm/trunk/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
llvm/trunk/test/Transforms/LowerTypeTests/import-icall.ll
llvm/trunk/test/Transforms/ThinLTOBitcodeWriter/cfi-functions-canonical-jump-tables.ll
Index: llvm/trunk/lib/Transforms/IPO/LowerTypeTests.cpp
===================================================================
--- llvm/trunk/lib/Transforms/IPO/LowerTypeTests.cpp
+++ llvm/trunk/lib/Transforms/IPO/LowerTypeTests.cpp
@@ -230,6 +230,16 @@
Bytes[AllocByteOffset + B] |= AllocMask;
}
+bool lowertypetests::isJumpTableCanonical(Function *F) {
+ if (F->isDeclarationForLinker())
+ return false;
+ auto *CI = mdconst::extract_or_null<ConstantInt>(
+ F->getParent()->getModuleFlag("CFI Canonical Jump Tables"));
+ if (!CI || CI->getZExtValue() != 0)
+ return true;
+ return F->hasFnAttribute("cfi-canonical-jump-table");
+}
+
namespace {
struct ByteArrayInfo {
@@ -251,9 +261,12 @@
GlobalObject *GO;
size_t NTypes;
- // For functions: true if this is a definition (either in the merged module or
- // in one of the thinlto modules).
- bool IsDefinition;
+ // For functions: true if the jump table is canonical. This essentially means
+ // whether the canonical address (i.e. the symbol table entry) of the function
+ // is provided by the local jump table. This is normally the same as whether
+ // the function is defined locally, but if canonical jump tables are disabled
+ // by the user then the jump table never provides a canonical definition.
+ bool IsJumpTableCanonical;
// For functions: true if this function is either defined or used in a thinlto
// module and its jumptable entry needs to be exported to thinlto backends.
@@ -263,13 +276,13 @@
public:
static GlobalTypeMember *create(BumpPtrAllocator &Alloc, GlobalObject *GO,
- bool IsDefinition, bool IsExported,
+ bool IsJumpTableCanonical, bool IsExported,
ArrayRef<MDNode *> Types) {
auto *GTM = static_cast<GlobalTypeMember *>(Alloc.Allocate(
totalSizeToAlloc<MDNode *>(Types.size()), alignof(GlobalTypeMember)));
GTM->GO = GO;
GTM->NTypes = Types.size();
- GTM->IsDefinition = IsDefinition;
+ GTM->IsJumpTableCanonical = IsJumpTableCanonical;
GTM->IsExported = IsExported;
std::uninitialized_copy(Types.begin(), Types.end(),
GTM->getTrailingObjects<MDNode *>());
@@ -280,8 +293,8 @@
return GO;
}
- bool isDefinition() const {
- return IsDefinition;
+ bool isJumpTableCanonical() const {
+ return IsJumpTableCanonical;
}
bool isExported() const {
@@ -320,6 +333,49 @@
size_t NTargets;
};
+struct ScopedSaveAliaseesAndUsed {
+ Module &M;
+ SmallPtrSet<GlobalValue *, 16> Used, CompilerUsed;
+ std::vector<std::pair<GlobalIndirectSymbol *, Function *>> FunctionAliases;
+
+ ScopedSaveAliaseesAndUsed(Module &M) : M(M) {
+ // The users of this class want to replace all function references except
+ // for aliases and llvm.used/llvm.compiler.used with references to a jump
+ // table. We avoid replacing aliases in order to avoid introducing a double
+ // indirection (or an alias pointing to a declaration in ThinLTO mode), and
+ // we avoid replacing llvm.used/llvm.compiler.used because these global
+ // variables describe properties of the global, not the jump table (besides,
+ // offseted references to the jump table in llvm.used are invalid).
+ // Unfortunately, LLVM doesn't have a "RAUW except for these (possibly
+ // indirect) users", so what we do is save the list of globals referenced by
+ // llvm.used/llvm.compiler.used and aliases, erase the used lists, let RAUW
+ // replace the aliasees and then set them back to their original values at
+ // the end.
+ if (GlobalVariable *GV = collectUsedGlobalVariables(M, Used, false))
+ GV->eraseFromParent();
+ if (GlobalVariable *GV = collectUsedGlobalVariables(M, CompilerUsed, true))
+ GV->eraseFromParent();
+
+ for (auto &GIS : concat<GlobalIndirectSymbol>(M.aliases(), M.ifuncs())) {
+ // FIXME: This should look past all aliases not just interposable ones,
+ // see discussion on D65118.
+ if (auto *F =
+ dyn_cast<Function>(GIS.getIndirectSymbol()->stripPointerCasts()))
+ FunctionAliases.push_back({&GIS, F});
+ }
+ }
+
+ ~ScopedSaveAliaseesAndUsed() {
+ appendToUsed(M, std::vector<GlobalValue *>(Used.begin(), Used.end()));
+ appendToCompilerUsed(M, std::vector<GlobalValue *>(CompilerUsed.begin(),
+ CompilerUsed.end()));
+
+ for (auto P : FunctionAliases)
+ P.first->setIndirectSymbol(
+ ConstantExpr::getBitCast(P.second, P.first->getType()));
+ }
+};
+
class LowerTypeTestsModule {
Module &M;
@@ -387,7 +443,8 @@
uint8_t *exportTypeId(StringRef TypeId, const TypeIdLowering &TIL);
TypeIdLowering importTypeId(StringRef TypeId);
void importTypeTest(CallInst *CI);
- void importFunction(Function *F, bool isDefinition);
+ void importFunction(Function *F, bool isJumpTableCanonical,
+ std::vector<GlobalAlias *> &AliasesToErase);
BitSetInfo
buildBitSet(Metadata *TypeId,
@@ -421,7 +478,8 @@
ArrayRef<GlobalTypeMember *> Globals,
ArrayRef<ICallBranchFunnel *> ICallBranchFunnels);
- void replaceWeakDeclarationWithJumpTablePtr(Function *F, Constant *JT, bool IsDefinition);
+ void replaceWeakDeclarationWithJumpTablePtr(Function *F, Constant *JT,
+ bool IsJumpTableCanonical);
void moveInitializerToModuleConstructor(GlobalVariable *GV);
void findGlobalVariableUsersOf(Constant *C,
SmallSetVector<GlobalVariable *, 8> &Out);
@@ -433,7 +491,7 @@
/// the block. 'This's use list is expected to have at least one element.
/// Unlike replaceAllUsesWith this function skips blockaddr and direct call
/// uses.
- void replaceCfiUses(Function *Old, Value *New, bool IsDefinition);
+ void replaceCfiUses(Function *Old, Value *New, bool IsJumpTableCanonical);
/// replaceDirectCalls - Go through the uses list for this definition and
/// replace each use, which is a direct function call.
@@ -982,14 +1040,16 @@
}
// ThinLTO backend: the function F has a jump table entry; update this module
-// accordingly. isDefinition describes the type of the jump table entry.
-void LowerTypeTestsModule::importFunction(Function *F, bool isDefinition) {
+// accordingly. isJumpTableCanonical describes the type of the jump table entry.
+void LowerTypeTestsModule::importFunction(
+ Function *F, bool isJumpTableCanonical,
+ std::vector<GlobalAlias *> &AliasesToErase) {
assert(F->getType()->getAddressSpace() == 0);
GlobalValue::VisibilityTypes Visibility = F->getVisibility();
std::string Name = F->getName();
- if (F->isDeclarationForLinker() && isDefinition) {
+ if (F->isDeclarationForLinker() && isJumpTableCanonical) {
// Non-dso_local functions may be overriden at run time,
// don't short curcuit them
if (F->isDSOLocal()) {
@@ -1004,12 +1064,13 @@
}
Function *FDecl;
- if (F->isDeclarationForLinker() && !isDefinition) {
- // Declaration of an external function.
+ if (!isJumpTableCanonical) {
+ // Either a declaration of an external function or a reference to a locally
+ // defined jump table.
FDecl = Function::Create(F->getFunctionType(), GlobalValue::ExternalLinkage,
F->getAddressSpace(), Name + ".cfi_jt", &M);
FDecl->setVisibility(GlobalValue::HiddenVisibility);
- } else if (isDefinition) {
+ } else {
F->setName(Name + ".cfi");
F->setLinkage(GlobalValue::ExternalLinkage);
FDecl = Function::Create(F->getFunctionType(), GlobalValue::ExternalLinkage,
@@ -1018,8 +1079,8 @@
Visibility = GlobalValue::HiddenVisibility;
// Delete aliases pointing to this function, they'll be re-created in the
- // merged output
- SmallVector<GlobalAlias*, 4> ToErase;
+ // merged output. Don't do it yet though because ScopedSaveAliaseesAndUsed
+ // will want to reset the aliasees first.
for (auto &U : F->uses()) {
if (auto *A = dyn_cast<GlobalAlias>(U.getUser())) {
Function *AliasDecl = Function::Create(
@@ -1027,24 +1088,15 @@
F->getAddressSpace(), "", &M);
AliasDecl->takeName(A);
A->replaceAllUsesWith(AliasDecl);
- ToErase.push_back(A);
+ AliasesToErase.push_back(A);
}
}
- for (auto *A : ToErase)
- A->eraseFromParent();
- } else {
- // Function definition without type metadata, where some other translation
- // unit contained a declaration with type metadata. This normally happens
- // during mixed CFI + non-CFI compilation. We do nothing with the function
- // so that it is treated the same way as a function defined outside of the
- // LTO unit.
- return;
}
- if (F->isWeakForLinker())
- replaceWeakDeclarationWithJumpTablePtr(F, FDecl, isDefinition);
+ if (F->hasExternalWeakLinkage())
+ replaceWeakDeclarationWithJumpTablePtr(F, FDecl, isJumpTableCanonical);
else
- replaceCfiUses(F, FDecl, isDefinition);
+ replaceCfiUses(F, FDecl, isJumpTableCanonical);
// Set visibility late because it's used in replaceCfiUses() to determine
// whether uses need to to be replaced.
@@ -1232,7 +1284,7 @@
// Replace all uses of F with (F ? JT : 0).
void LowerTypeTestsModule::replaceWeakDeclarationWithJumpTablePtr(
- Function *F, Constant *JT, bool IsDefinition) {
+ Function *F, Constant *JT, bool IsJumpTableCanonical) {
// The target expression can not appear in a constant initializer on most
// (all?) targets. Switch to a runtime initializer.
SmallSetVector<GlobalVariable *, 8> GlobalVarUsers;
@@ -1246,7 +1298,7 @@
Function::Create(cast<FunctionType>(F->getValueType()),
GlobalValue::ExternalWeakLinkage,
F->getAddressSpace(), "", &M);
- replaceCfiUses(F, PlaceholderFn, IsDefinition);
+ replaceCfiUses(F, PlaceholderFn, IsJumpTableCanonical);
Constant *Target = ConstantExpr::getSelect(
ConstantExpr::getICmp(CmpInst::ICMP_NE, F,
@@ -1283,8 +1335,9 @@
unsigned ArmCount = 0, ThumbCount = 0;
for (const auto GTM : Functions) {
- if (!GTM->isDefinition()) {
+ if (!GTM->isJumpTableCanonical()) {
// PLT stubs are always ARM.
+ // FIXME: This is the wrong heuristic for non-canonical jump tables.
++ArmCount;
continue;
}
@@ -1445,46 +1498,52 @@
lowerTypeTestCalls(TypeIds, JumpTable, GlobalLayout);
- // Build aliases pointing to offsets into the jump table, and replace
- // references to the original functions with references to the aliases.
- for (unsigned I = 0; I != Functions.size(); ++I) {
- Function *F = cast<Function>(Functions[I]->getGlobal());
- bool IsDefinition = Functions[I]->isDefinition();
-
- Constant *CombinedGlobalElemPtr = ConstantExpr::getBitCast(
- ConstantExpr::getInBoundsGetElementPtr(
- JumpTableType, JumpTable,
- ArrayRef<Constant *>{ConstantInt::get(IntPtrTy, 0),
- ConstantInt::get(IntPtrTy, I)}),
- F->getType());
- if (Functions[I]->isExported()) {
- if (IsDefinition) {
- ExportSummary->cfiFunctionDefs().insert(F->getName());
- } else {
- GlobalAlias *JtAlias = GlobalAlias::create(
- F->getValueType(), 0, GlobalValue::ExternalLinkage,
- F->getName() + ".cfi_jt", CombinedGlobalElemPtr, &M);
- JtAlias->setVisibility(GlobalValue::HiddenVisibility);
- ExportSummary->cfiFunctionDecls().insert(F->getName());
+ {
+ ScopedSaveAliaseesAndUsed S(M);
+
+ // Build aliases pointing to offsets into the jump table, and replace
+ // references to the original functions with references to the aliases.
+ for (unsigned I = 0; I != Functions.size(); ++I) {
+ Function *F = cast<Function>(Functions[I]->getGlobal());
+ bool IsJumpTableCanonical = Functions[I]->isJumpTableCanonical();
+
+ Constant *CombinedGlobalElemPtr = ConstantExpr::getBitCast(
+ ConstantExpr::getInBoundsGetElementPtr(
+ JumpTableType, JumpTable,
+ ArrayRef<Constant *>{ConstantInt::get(IntPtrTy, 0),
+ ConstantInt::get(IntPtrTy, I)}),
+ F->getType());
+ if (Functions[I]->isExported()) {
+ if (IsJumpTableCanonical) {
+ ExportSummary->cfiFunctionDefs().insert(F->getName());
+ } else {
+ GlobalAlias *JtAlias = GlobalAlias::create(
+ F->getValueType(), 0, GlobalValue::ExternalLinkage,
+ F->getName() + ".cfi_jt", CombinedGlobalElemPtr, &M);
+ JtAlias->setVisibility(GlobalValue::HiddenVisibility);
+ ExportSummary->cfiFunctionDecls().insert(F->getName());
+ }
}
- }
- if (!IsDefinition) {
- if (F->isWeakForLinker())
- replaceWeakDeclarationWithJumpTablePtr(F, CombinedGlobalElemPtr, IsDefinition);
- else
- replaceCfiUses(F, CombinedGlobalElemPtr, IsDefinition);
- } else {
- assert(F->getType()->getAddressSpace() == 0);
+ if (!IsJumpTableCanonical) {
+ if (F->hasExternalWeakLinkage())
+ replaceWeakDeclarationWithJumpTablePtr(F, CombinedGlobalElemPtr,
+ IsJumpTableCanonical);
+ else
+ replaceCfiUses(F, CombinedGlobalElemPtr, IsJumpTableCanonical);
+ } else {
+ assert(F->getType()->getAddressSpace() == 0);
- GlobalAlias *FAlias = GlobalAlias::create(
- F->getValueType(), 0, F->getLinkage(), "", CombinedGlobalElemPtr, &M);
- FAlias->setVisibility(F->getVisibility());
- FAlias->takeName(F);
- if (FAlias->hasName())
- F->setName(FAlias->getName() + ".cfi");
- replaceCfiUses(F, FAlias, IsDefinition);
- if (!F->hasLocalLinkage())
- F->setVisibility(GlobalVariable::HiddenVisibility);
+ GlobalAlias *FAlias =
+ GlobalAlias::create(F->getValueType(), 0, F->getLinkage(), "",
+ CombinedGlobalElemPtr, &M);
+ FAlias->setVisibility(F->getVisibility());
+ FAlias->takeName(F);
+ if (FAlias->hasName())
+ F->setName(FAlias->getName() + ".cfi");
+ replaceCfiUses(F, FAlias, IsJumpTableCanonical);
+ if (!F->hasLocalLinkage())
+ F->setVisibility(GlobalVariable::HiddenVisibility);
+ }
}
}
@@ -1650,7 +1709,8 @@
return false;
}
-void LowerTypeTestsModule::replaceCfiUses(Function *Old, Value *New, bool IsDefinition) {
+void LowerTypeTestsModule::replaceCfiUses(Function *Old, Value *New,
+ bool IsJumpTableCanonical) {
SmallSetVector<Constant *, 4> Constants;
auto UI = Old->use_begin(), E = Old->use_end();
for (; UI != E;) {
@@ -1662,7 +1722,7 @@
continue;
// Skip direct calls to externally defined or non-dso_local functions
- if (isDirectCall(U) && (Old->isDSOLocal() || !IsDefinition))
+ if (isDirectCall(U) && (Old->isDSOLocal() || !IsJumpTableCanonical))
continue;
// Must handle Constants specially, we cannot call replaceUsesOfWith on a
@@ -1732,10 +1792,16 @@
Decls.push_back(&F);
}
- for (auto F : Defs)
- importFunction(F, /*isDefinition*/ true);
- for (auto F : Decls)
- importFunction(F, /*isDefinition*/ false);
+ std::vector<GlobalAlias *> AliasesToErase;
+ {
+ ScopedSaveAliaseesAndUsed S(M);
+ for (auto F : Defs)
+ importFunction(F, /*isJumpTableCanonical*/ true, AliasesToErase);
+ for (auto F : Decls)
+ importFunction(F, /*isJumpTableCanonical*/ false, AliasesToErase);
+ }
+ for (GlobalAlias *GA : AliasesToErase)
+ GA->eraseFromParent();
return true;
}
@@ -1869,24 +1935,26 @@
Types.clear();
GO.getMetadata(LLVMContext::MD_type, Types);
- bool IsDefinition = !GO.isDeclarationForLinker();
+ bool IsJumpTableCanonical = false;
bool IsExported = false;
if (Function *F = dyn_cast<Function>(&GO)) {
+ IsJumpTableCanonical = isJumpTableCanonical(F);
if (ExportedFunctions.count(F->getName())) {
- IsDefinition |= ExportedFunctions[F->getName()].Linkage == CFL_Definition;
+ IsJumpTableCanonical |=
+ ExportedFunctions[F->getName()].Linkage == CFL_Definition;
IsExported = true;
// TODO: The logic here checks only that the function is address taken,
// not that the address takers are live. This can be updated to check
// their liveness and emit fewer jumptable entries once monolithic LTO
// builds also emit summaries.
} else if (!F->hasAddressTaken()) {
- if (!CrossDsoCfi || !IsDefinition || F->hasLocalLinkage())
+ if (!CrossDsoCfi || !IsJumpTableCanonical || F->hasLocalLinkage())
continue;
}
}
- auto *GTM =
- GlobalTypeMember::create(Alloc, &GO, IsDefinition, IsExported, Types);
+ auto *GTM = GlobalTypeMember::create(Alloc, &GO, IsJumpTableCanonical,
+ IsExported, Types);
GlobalTypeMembers[&GO] = GTM;
for (MDNode *Type : Types) {
verifyTypeMDNode(&GO, Type);
Index: llvm/trunk/lib/Transforms/IPO/CrossDSOCFI.cpp
===================================================================
--- llvm/trunk/lib/Transforms/IPO/CrossDSOCFI.cpp
+++ llvm/trunk/lib/Transforms/IPO/CrossDSOCFI.cpp
@@ -84,13 +84,9 @@
for (GlobalObject &GO : M.global_objects()) {
Types.clear();
GO.getMetadata(LLVMContext::MD_type, Types);
- for (MDNode *Type : Types) {
- // Sanity check. GO must not be a function declaration.
- assert(!isa<Function>(&GO) || !cast<Function>(&GO)->isDeclaration());
-
+ for (MDNode *Type : Types)
if (ConstantInt *TypeId = extractNumericTypeId(Type))
TypeIds.insert(TypeId->getZExtValue());
- }
}
NamedMDNode *CfiFunctionsMD = M.getNamedMetadata("cfi.functions");
Index: llvm/trunk/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
===================================================================
--- llvm/trunk/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
+++ llvm/trunk/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
@@ -24,6 +24,7 @@
#include "llvm/Transforms/IPO.h"
#include "llvm/Transforms/IPO/FunctionAttrs.h"
#include "llvm/Transforms/IPO/FunctionImport.h"
+#include "llvm/Transforms/IPO/LowerTypeTests.h"
#include "llvm/Transforms/Utils/Cloning.h"
#include "llvm/Transforms/Utils/ModuleUtils.h"
using namespace llvm;
@@ -323,9 +324,9 @@
SmallVector<Metadata *, 4> Elts;
Elts.push_back(MDString::get(Ctx, F.getName()));
CfiFunctionLinkage Linkage;
- if (!F.isDeclarationForLinker())
+ if (lowertypetests::isJumpTableCanonical(&F))
Linkage = CFL_Definition;
- else if (F.isWeakForLinker())
+ else if (F.hasExternalWeakLinkage())
Linkage = CFL_WeakDeclaration;
else
Linkage = CFL_Declaration;
Index: llvm/trunk/include/llvm/Transforms/IPO/LowerTypeTests.h
===================================================================
--- llvm/trunk/include/llvm/Transforms/IPO/LowerTypeTests.h
+++ llvm/trunk/include/llvm/Transforms/IPO/LowerTypeTests.h
@@ -193,6 +193,8 @@
uint64_t &AllocByteOffset, uint8_t &AllocMask);
};
+bool isJumpTableCanonical(Function *F);
+
} // end namespace lowertypetests
class LowerTypeTestsPass : public PassInfoMixin<LowerTypeTestsPass> {
Index: llvm/trunk/test/Transforms/ThinLTOBitcodeWriter/cfi-functions-canonical-jump-tables.ll
===================================================================
--- llvm/trunk/test/Transforms/ThinLTOBitcodeWriter/cfi-functions-canonical-jump-tables.ll
+++ llvm/trunk/test/Transforms/ThinLTOBitcodeWriter/cfi-functions-canonical-jump-tables.ll
@@ -0,0 +1,23 @@
+; RUN: opt -thinlto-bc -thinlto-split-lto-unit -o %t %s
+; RUN: llvm-modextract -b -n 1 -o - %t | llvm-dis | FileCheck %s
+
+; Check that cfi.functions metadata has the expected contents.
+
+; CHECK: !"f1", i8 1
+; CHECK: !"f2", i8 1
+; CHECK: !"f3", i8 0
+
+declare !type !1 void @f1()
+
+define void @f2() !type !1 {
+ ret void
+}
+
+define void @f3() "cfi-canonical-jump-table" !type !1 {
+ ret void
+}
+
+!llvm.module.flags = !{!0}
+
+!0 = !{i32 4, !"CFI Canonical Jump Tables", i32 0}
+!1 = !{i32 0, !"typeid1"}
Index: llvm/trunk/test/Transforms/LowerTypeTests/import-icall.ll
===================================================================
--- llvm/trunk/test/Transforms/LowerTypeTests/import-icall.ll
+++ llvm/trunk/test/Transforms/LowerTypeTests/import-icall.ll
@@ -3,6 +3,11 @@
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
+@llvm.used = appending global [1 x i8*] [i8* bitcast (i8* ()* @local_decl to i8*)], section "llvm.metadata"
+@llvm.compiler.used = appending global [1 x i8*] [i8* bitcast (i8* ()* @local_decl to i8*)], section "llvm.metadata"
+
+@local_decl_alias = alias i8* (), i8* ()* @local_decl
+
define i8 @local_a() {
call void @external()
call void @external_weak()
@@ -19,14 +24,16 @@
ret i8 %x
}
-define void @local_decl() {
- call void @local_decl()
- ret void
+define i8* @local_decl() {
+ call i8* @local_decl()
+ ret i8* bitcast (i8* ()* @local_decl to i8*)
}
declare void @external()
declare extern_weak void @external_weak()
+; CHECK: @local_decl_alias = alias i8* (), i8* ()* @local_decl
+
; CHECK: define hidden i8 @local_a.cfi() {
; CHECK-NEXT: call void @external()
; CHECK-NEXT: call void @external_weak()
@@ -37,8 +44,9 @@
; CHECK: define internal i8 @local_b() {
; CHECK-NEXT: call i8 @local_a()
-; CHECK: define void @local_decl()
-; CHECK-NEXT: call void @local_decl()
+; CHECK: define i8* @local_decl()
+; CHECK-NEXT: call i8* @local_decl()
+; CHECK-NEXT: ret i8* bitcast (i8* ()* @local_decl.cfi_jt to i8*)
; CHECK: declare void @external()
; CHECK: declare extern_weak void @external_weak()
Index: cfe/trunk/docs/ControlFlowIntegrity.rst
===================================================================
--- cfe/trunk/docs/ControlFlowIntegrity.rst
+++ cfe/trunk/docs/ControlFlowIntegrity.rst
@@ -235,6 +235,54 @@
``-fsanitize-cfi-icall-generalize-pointers`` is not compatible with
``-fsanitize-cfi-cross-dso``.
+.. _cfi-canonical-jump-tables:
+
+``-fsanitize-cfi-canonical-jump-tables``
+----------------------------------------
+
+The default behavior of Clang's indirect function call checker will replace
+the address of each CFI-checked function in the output file's symbol table
+with the address of a jump table entry which will pass CFI checks. We refer
+to this as making the jump table `canonical`. This property allows code that
+was not compiled with ``-fsanitize=cfi-icall`` to take a CFI-valid address
+of a function, but it comes with a couple of caveats that are especially
+relevant for users of cross-DSO CFI:
+
+- There is a performance and code size overhead associated with each
+ exported function, because each such function must have an associated
+ jump table entry, which must be emitted even in the common case where the
+ function is never address-taken anywhere in the program, and must be used
+ even for direct calls between DSOs, in addition to the PLT overhead.
+
+- There is no good way to take a CFI-valid address of a function written in
+ assembly or a language not supported by Clang. The reason is that the code
+ generator would need to insert a jump table in order to form a CFI-valid
+ address for assembly functions, but there is no way in general for the
+ code generator to determine the language of the function. This may be
+ possible with LTO in the intra-DSO case, but in the cross-DSO case the only
+ information available is the function declaration. One possible solution
+ is to add a C wrapper for each assembly function, but these wrappers can
+ present a significant maintenance burden for heavy users of assembly in
+ addition to adding runtime overhead.
+
+For these reasons, we provide the option of making the jump table non-canonical
+with the flag ``-fno-sanitize-cfi-canonical-jump-tables``. When the jump
+table is made non-canonical, symbol table entries point directly to the
+function body. Any instances of a function's address being taken in C will
+be replaced with a jump table address.
+
+This scheme does have its own caveats, however. It does end up breaking
+function address equality more aggressively than the default behavior,
+especially in cross-DSO mode which normally preserves function address
+equality entirely.
+
+Furthermore, it is occasionally necessary for code not compiled with
+``-fsanitize=cfi-icall`` to take a function address that is valid
+for CFI. For example, this is necessary when a function's address
+is taken by assembly code and then called by CFI-checking C code. The
+``__attribute__((cfi_canonical_jump_table))`` attribute may be used to make
+the jump table entry of a specific function canonical so that the external
+code will end up taking a address for the function that will pass CFI checks.
``-fsanitize=cfi-icall`` and ``-fsanitize=function``
----------------------------------------------------
Index: cfe/trunk/include/clang/Driver/SanitizerArgs.h
===================================================================
--- cfe/trunk/include/clang/Driver/SanitizerArgs.h
+++ cfe/trunk/include/clang/Driver/SanitizerArgs.h
@@ -32,6 +32,7 @@
bool MsanUseAfterDtor = true;
bool CfiCrossDso = false;
bool CfiICallGeneralizePointers = false;
+ bool CfiCanonicalJumpTables = false;
int AsanFieldPadding = 0;
bool SharedRuntime = false;
bool AsanUseAfterScope = true;
Index: cfe/trunk/include/clang/Driver/Options.td
===================================================================
--- cfe/trunk/include/clang/Driver/Options.td
+++ cfe/trunk/include/clang/Driver/Options.td
@@ -1061,6 +1061,13 @@
def fsanitize_cfi_icall_generalize_pointers : Flag<["-"], "fsanitize-cfi-icall-generalize-pointers">,
Group<f_clang_Group>,
HelpText<"Generalize pointers in CFI indirect call type signature checks">;
+def fsanitize_cfi_canonical_jump_tables : Flag<["-"], "fsanitize-cfi-canonical-jump-tables">,
+ Group<f_clang_Group>,
+ HelpText<"Make the jump table addresses canonical in the symbol table">;
+def fno_sanitize_cfi_canonical_jump_tables : Flag<["-"], "fno-sanitize-cfi-canonical-jump-tables">,
+ Group<f_clang_Group>,
+ Flags<[CoreOption, DriverOption]>,
+ HelpText<"Do not make the jump table addresses canonical in the symbol table">;
def fsanitize_stats : Flag<["-"], "fsanitize-stats">,
Group<f_clang_Group>,
HelpText<"Enable sanitizer statistics gathering.">;
Index: cfe/trunk/include/clang/Basic/CodeGenOptions.def
===================================================================
--- cfe/trunk/include/clang/Basic/CodeGenOptions.def
+++ cfe/trunk/include/clang/Basic/CodeGenOptions.def
@@ -195,6 +195,8 @@
///< diagnostics.
CODEGENOPT(SanitizeCfiICallGeneralizePointers, 1, 0) ///< Generalize pointer types in
///< CFI icall function signatures
+CODEGENOPT(SanitizeCfiCanonicalJumpTables, 1, 0) ///< Make jump table symbols canonical
+ ///< instead of creating a local jump table.
CODEGENOPT(SanitizeCoverageType, 2, 0) ///< Type of sanitizer coverage
///< instrumentation.
CODEGENOPT(SanitizeCoverageIndirectCalls, 1, 0) ///< Enable sanitizer coverage
Index: cfe/trunk/include/clang/Basic/AttrDocs.td
===================================================================
--- cfe/trunk/include/clang/Basic/AttrDocs.td
+++ cfe/trunk/include/clang/Basic/AttrDocs.td
@@ -2221,6 +2221,18 @@
}];
}
+def CFICanonicalJumpTableDocs : Documentation {
+ let Category = DocCatFunction;
+ let Heading = "cfi_canonical_jump_table";
+ let Content = [{
+.. _langext-cfi_canonical_jump_table:
+
+Use ``__attribute__((cfi_canonical_jump_table))`` on a function declaration to
+make the function's CFI jump table canonical. See :ref:`the CFI documentation
+<cfi-canonical-jump-tables>` for more details.
+ }];
+}
+
def DocCatTypeSafety : DocumentationCategory<"Type Safety Checking"> {
let Content = [{
Clang supports additional attributes to enable checking type safety properties
Index: cfe/trunk/include/clang/Basic/Attr.td
===================================================================
--- cfe/trunk/include/clang/Basic/Attr.td
+++ cfe/trunk/include/clang/Basic/Attr.td
@@ -2436,6 +2436,12 @@
let ASTNode = 0;
}
+def CFICanonicalJumpTable : InheritableAttr {
+ let Spellings = [Clang<"cfi_canonical_jump_table">];
+ let Subjects = SubjectList<[Function], ErrorDiag>;
+ let Documentation = [CFICanonicalJumpTableDocs];
+}
+
// C/C++ Thread safety attributes (e.g. for deadlock, data race checking)
// Not all of these attributes will be given a [[]] spelling. The attributes
// which require access to function parameter names cannot use the [[]] spelling
Index: cfe/trunk/test/SemaCXX/attr-cfi-canonical-jump-table.cpp
===================================================================
--- cfe/trunk/test/SemaCXX/attr-cfi-canonical-jump-table.cpp
+++ cfe/trunk/test/SemaCXX/attr-cfi-canonical-jump-table.cpp
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsyntax-only -verify %s
+
+__attribute__((cfi_canonical_jump_table)) void fdecl();
+
+__attribute__((cfi_canonical_jump_table)) void f() {}
+
+struct S {
+ __attribute__((cfi_canonical_jump_table)) void f() {}
+};
+
+__attribute__((cfi_canonical_jump_table)) int i; // expected-error {{'cfi_canonical_jump_table' attribute only applies to functions}}
Index: cfe/trunk/test/CodeGen/cfi-icall-cross-dso.c
===================================================================
--- cfe/trunk/test/CodeGen/cfi-icall-cross-dso.c
+++ cfe/trunk/test/CodeGen/cfi-icall-cross-dso.c
@@ -1,27 +1,27 @@
// RUN: %clang_cc1 -triple x86_64-unknown-linux -O1 -fno-experimental-new-pass-manager \
// RUN: -fsanitize=cfi-icall -fsanitize-cfi-cross-dso \
-// RUN: -emit-llvm -o - %s | FileCheck \
+// RUN: -fsanitize-cfi-canonical-jump-tables -emit-llvm -o - %s | FileCheck \
// RUN: --check-prefix=CHECK --check-prefix=CHECK-DIAG \
// RUN: --check-prefix=ITANIUM --check-prefix=ITANIUM-DIAG \
// RUN: %s
// RUN: %clang_cc1 -triple x86_64-unknown-linux -O1 -fno-experimental-new-pass-manager \
// RUN: -fsanitize=cfi-icall -fsanitize-cfi-cross-dso -fsanitize-trap=cfi-icall \
-// RUN: -emit-llvm -o - %s | FileCheck \
+// RUN: -fsanitize-cfi-canonical-jump-tables -emit-llvm -o - %s | FileCheck \
// RUN: --check-prefix=CHECK \
// RUN: --check-prefix=ITANIUM --check-prefix=ITANIUM-TRAP \
// RUN: %s
// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -O1 -fno-experimental-new-pass-manager \
// RUN: -fsanitize=cfi-icall -fsanitize-cfi-cross-dso \
-// RUN: -emit-llvm -o - %s | FileCheck \
+// RUN: -fsanitize-cfi-canonical-jump-tables -emit-llvm -o - %s | FileCheck \
// RUN: --check-prefix=CHECK --check-prefix=CHECK-DIAG \
// RUN: --check-prefix=MS --check-prefix=MS-DIAG \
// RUN: %s
// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -O1 -fno-experimental-new-pass-manager \
// RUN: -fsanitize=cfi-icall -fsanitize-cfi-cross-dso -fsanitize-trap=cfi-icall \
-// RUN: -emit-llvm -o - %s | FileCheck \
+// RUN: -fsanitize-cfi-canonical-jump-tables -emit-llvm -o - %s | FileCheck \
// RUN: --check-prefix=CHECK \
// RUN: --check-prefix=MS --check-prefix=MS-TRAP \
// RUN: %s
Index: cfe/trunk/test/CodeGen/cfi-icall-canonical-jump-tables.c
===================================================================
--- cfe/trunk/test/CodeGen/cfi-icall-canonical-jump-tables.c
+++ cfe/trunk/test/CodeGen/cfi-icall-canonical-jump-tables.c
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-icall -fsanitize-cfi-cross-dso -emit-llvm -o - %s | FileCheck --check-prefixes=CHECK,NOCANON %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-icall -fsanitize-cfi-cross-dso -fsanitize-cfi-canonical-jump-tables -emit-llvm -o - %s | FileCheck --check-prefixes=CHECK,CANON %s
+
+void ext(void);
+
+// CHECK: define void @f({{.*}} [[ATTR1:#[0-9]+]]
+void f() {
+ ext();
+}
+
+// NOCANON: declare !type {{.*}} @ext()
+// CANON: declare void @ext()
+
+// CHECK: define void @g({{.*}} [[ATTR2:#[0-9]+]]
+__attribute__((cfi_canonical_jump_table)) void g() {}
+
+// CHECK: [[ATTR1]] = {
+// CHECK-NOT: "cfi-canonical-jump-table"
+// CHECK: }
+
+// CHECK: [[ATTR2]] = { {{.*}} "cfi-canonical-jump-table" {{.*}} }
+
+// NOCANON: !{i32 4, !"CFI Canonical Jump Tables", i32 0}
+// CANON: !{i32 4, !"CFI Canonical Jump Tables", i32 1}
Index: cfe/trunk/test/Driver/fsanitize.c
===================================================================
--- cfe/trunk/test/Driver/fsanitize.c
+++ cfe/trunk/test/Driver/fsanitize.c
@@ -621,6 +621,12 @@
// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi-icall -fsanitize-cfi-icall-generalize-pointers -fsanitize-cfi-cross-dso -fvisibility=hidden -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-GENERALIZE-AND-CROSS-DSO
// CHECK-CFI-GENERALIZE-AND-CROSS-DSO: error: invalid argument '-fsanitize-cfi-cross-dso' not allowed with '-fsanitize-cfi-icall-generalize-pointers'
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi-icall -fsanitize-cfi-canonical-jump-tables -fvisibility=hidden -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-CANONICAL-JUMP-TABLES
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi-icall -fno-sanitize-cfi-canonical-jump-tables -fvisibility=hidden -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-CFI-CANONICAL-JUMP-TABLES
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi-icall -fvisibility=hidden -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-CANONICAL-JUMP-TABLES
+// CHECK-CFI-CANONICAL-JUMP-TABLES: -fsanitize-cfi-canonical-jump-tables
+// CHECK-NO-CFI-CANONICAL-JUMP-TABLES-NOT: -fsanitize-cfi-canonical-jump-tables
+
// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi -fsanitize-stats -flto -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-STATS
// CHECK-CFI-STATS: -fsanitize-stats
Index: cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
===================================================================
--- cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
+++ cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
@@ -21,6 +21,7 @@
// CHECK-NEXT: Availability ((SubjectMatchRule_record, SubjectMatchRule_enum, SubjectMatchRule_enum_constant, SubjectMatchRule_field, SubjectMatchRule_function, SubjectMatchRule_namespace, SubjectMatchRule_objc_category, SubjectMatchRule_objc_implementation, SubjectMatchRule_objc_interface, SubjectMatchRule_objc_method, SubjectMatchRule_objc_property, SubjectMatchRule_objc_protocol, SubjectMatchRule_record, SubjectMatchRule_type_alias, SubjectMatchRule_variable))
// CHECK-NEXT: CFAuditedTransfer (SubjectMatchRule_function)
// CHECK-NEXT: CFConsumed (SubjectMatchRule_variable_is_parameter)
+// CHECK-NEXT: CFICanonicalJumpTable (SubjectMatchRule_function)
// CHECK-NEXT: CFUnknownTransfer (SubjectMatchRule_function)
// CHECK-NEXT: CPUDispatch (SubjectMatchRule_function)
// CHECK-NEXT: CPUSpecific (SubjectMatchRule_function)
Index: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- cfe/trunk/lib/CodeGen/CodeGenModule.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp
@@ -535,6 +535,12 @@
getModule().addModuleFlag(llvm::Module::Override, "Cross-DSO CFI", 1);
}
+ if (LangOpts.Sanitize.has(SanitizerKind::CFIICall)) {
+ getModule().addModuleFlag(llvm::Module::Override,
+ "CFI Canonical Jump Tables",
+ CodeGenOpts.SanitizeCfiCanonicalJumpTables);
+ }
+
if (CodeGenOpts.CFProtectionReturn &&
Target.checkCFProtectionReturnSupported(getDiags())) {
// Indicate that we want to instrument return control flow protection.
@@ -1605,10 +1611,17 @@
F->setAlignment(2);
}
- // In the cross-dso CFI mode, we want !type attributes on definitions only.
- if (CodeGenOpts.SanitizeCfiCrossDso)
- if (auto *FD = dyn_cast<FunctionDecl>(D))
- CreateFunctionTypeMetadataForIcall(FD, F);
+ // In the cross-dso CFI mode with canonical jump tables, we want !type
+ // attributes on definitions only.
+ if (CodeGenOpts.SanitizeCfiCrossDso &&
+ CodeGenOpts.SanitizeCfiCanonicalJumpTables) {
+ if (auto *FD = dyn_cast<FunctionDecl>(D)) {
+ // Skip available_externally functions. They won't be codegen'ed in the
+ // current module anyway.
+ if (getContext().GetGVALinkageForFunction(FD) != GVA_AvailableExternally)
+ CreateFunctionTypeMetadataForIcall(FD, F);
+ }
+ }
// Emit type metadata on member functions for member function pointer checks.
// These are only ever necessary on definitions; we're guaranteed that the
@@ -1765,14 +1778,6 @@
if (isa<CXXMethodDecl>(FD) && !cast<CXXMethodDecl>(FD)->isStatic())
return;
- // Additionally, if building with cross-DSO support...
- if (CodeGenOpts.SanitizeCfiCrossDso) {
- // Skip available_externally functions. They won't be codegen'ed in the
- // current module anyway.
- if (getContext().GetGVALinkageForFunction(FD) == GVA_AvailableExternally)
- return;
- }
-
llvm::Metadata *MD = CreateMetadataIdentifierForType(FD->getType());
F->addTypeMetadata(0, MD);
F->addTypeMetadata(0, CreateMetadataIdentifierGeneralized(FD->getType()));
@@ -1849,8 +1854,11 @@
F->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
// Don't emit entries for function declarations in the cross-DSO mode. This
- // is handled with better precision by the receiving DSO.
- if (!CodeGenOpts.SanitizeCfiCrossDso)
+ // is handled with better precision by the receiving DSO. But if jump tables
+ // are non-canonical then we need type metadata in order to produce the local
+ // jump table.
+ if (!CodeGenOpts.SanitizeCfiCrossDso ||
+ !CodeGenOpts.SanitizeCfiCanonicalJumpTables)
CreateFunctionTypeMetadataForIcall(FD, F);
if (getLangOpts().OpenMP && FD->hasAttr<OMPDeclareSimdDeclAttr>())
Index: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
+++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
@@ -762,6 +762,9 @@
if (CGM.getCodeGenOpts().ProfileSampleAccurate)
Fn->addFnAttr("profile-sample-accurate");
+ if (D && D->hasAttr<CFICanonicalJumpTableAttr>())
+ Fn->addFnAttr("cfi-canonical-jump-table");
+
if (getLangOpts().OpenCL) {
// Add metadata for a kernel function.
if (const FunctionDecl *FD = dyn_cast_or_null<FunctionDecl>(D))
Index: cfe/trunk/lib/Driver/SanitizerArgs.cpp
===================================================================
--- cfe/trunk/lib/Driver/SanitizerArgs.cpp
+++ cfe/trunk/lib/Driver/SanitizerArgs.cpp
@@ -636,6 +636,10 @@
D.Diag(diag::err_drv_argument_not_allowed_with)
<< "-fsanitize-cfi-cross-dso"
<< "-fsanitize-cfi-icall-generalize-pointers";
+
+ CfiCanonicalJumpTables =
+ Args.hasFlag(options::OPT_fsanitize_cfi_canonical_jump_tables,
+ options::OPT_fno_sanitize_cfi_canonical_jump_tables, true);
}
Stats = Args.hasFlag(options::OPT_fsanitize_stats,
@@ -976,6 +980,9 @@
if (CfiICallGeneralizePointers)
CmdArgs.push_back("-fsanitize-cfi-icall-generalize-pointers");
+ if (CfiCanonicalJumpTables)
+ CmdArgs.push_back("-fsanitize-cfi-canonical-jump-tables");
+
if (Stats)
CmdArgs.push_back("-fsanitize-stats");
Index: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- cfe/trunk/lib/Sema/SemaDeclAttr.cpp
+++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp
@@ -7198,6 +7198,9 @@
// Interacts with -fstack-protector options.
handleSimpleAttribute<NoStackProtectorAttr>(S, D, AL);
break;
+ case ParsedAttr::AT_CFICanonicalJumpTable:
+ handleSimpleAttribute<CFICanonicalJumpTableAttr>(S, D, AL);
+ break;
case ParsedAttr::AT_StdCall:
case ParsedAttr::AT_CDecl:
case ParsedAttr::AT_FastCall:
Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===================================================================
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -1139,6 +1139,8 @@
Opts.SanitizeCfiCrossDso = Args.hasArg(OPT_fsanitize_cfi_cross_dso);
Opts.SanitizeCfiICallGeneralizePointers =
Args.hasArg(OPT_fsanitize_cfi_icall_generalize_pointers);
+ Opts.SanitizeCfiCanonicalJumpTables =
+ Args.hasArg(OPT_fsanitize_cfi_canonical_jump_tables);
Opts.SanitizeStats = Args.hasArg(OPT_fsanitize_stats);
if (Arg *A = Args.getLastArg(
OPT_fsanitize_address_poison_custom_array_cookie,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
