diff options
22 files changed, 352 insertions, 113 deletions
diff --git a/clang/docs/ControlFlowIntegrity.rst b/clang/docs/ControlFlowIntegrity.rst index f57bdf5d2cb..03e27d3ede8 100644 --- a/clang/docs/ControlFlowIntegrity.rst +++ b/clang/docs/ControlFlowIntegrity.rst @@ -235,6 +235,54 @@ long as the qualifiers for the type they point to match. For example, ``char*``, ``-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`` ---------------------------------------------------- diff --git a/clang/include/clang/Basic/Attr.td b/clang/include/clang/Basic/Attr.td index 9f9292d4e5f..db13e9163d9 100644 --- a/clang/include/clang/Basic/Attr.td +++ b/clang/include/clang/Basic/Attr.td @@ -2436,6 +2436,12 @@ def NoSanitizeSpecific : InheritableAttr { 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 diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index 0f10d903693..1d6c1ab3fca 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -2221,6 +2221,18 @@ to avoid false positives in other places. }]; } +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 diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def index 400711e7d19..736f201a12b 100644 --- a/clang/include/clang/Basic/CodeGenOptions.def +++ b/clang/include/clang/Basic/CodeGenOptions.def @@ -195,6 +195,8 @@ CODEGENOPT(SanitizeMinimalRuntime, 1, 0) ///< Use "_minimal" sanitizer runtime f ///< 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 diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 19022b4e2cd..0f2c314a337 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -1061,6 +1061,13 @@ def fno_sanitize_cfi_cross_dso : Flag<["-"], "fno-sanitize-cfi-cross-dso">, 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.">; diff --git a/clang/include/clang/Driver/SanitizerArgs.h b/clang/include/clang/Driver/SanitizerArgs.h index e7de509d219..c37499e0f20 100644 --- a/clang/include/clang/Driver/SanitizerArgs.h +++ b/clang/include/clang/Driver/SanitizerArgs.h @@ -32,6 +32,7 @@ class SanitizerArgs { bool MsanUseAfterDtor = true; bool CfiCrossDso = false; bool CfiICallGeneralizePointers = false; + bool CfiCanonicalJumpTables = false; int AsanFieldPadding = 0; bool SharedRuntime = false; bool AsanUseAfterScope = true; diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index eafe2667443..f7d9c8201d9 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -762,6 +762,9 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, 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)) diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index b650526629b..8943b407bdc 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -535,6 +535,12 @@ void CodeGenModule::Release() { 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 @@ void CodeGenModule::SetLLVMFunctionAttributesForDefinition(const Decl *D, 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 @@ void CodeGenModule::CreateFunctionTypeMetadataForIcall(const FunctionDecl *FD, 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 @@ void CodeGenModule::SetFunctionAttributes(GlobalDecl GD, llvm::Function *F, 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>()) diff --git a/clang/lib/Driver/SanitizerArgs.cpp b/clang/lib/Driver/SanitizerArgs.cpp index 8e59e19be35..7d4cc539eaa 100644 --- a/clang/lib/Driver/SanitizerArgs.cpp +++ b/clang/lib/Driver/SanitizerArgs.cpp @@ -636,6 +636,10 @@ SanitizerArgs::SanitizerArgs(const ToolChain &TC, 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 @@ void SanitizerArgs::addArgs(const ToolChain &TC, const llvm::opt::ArgList &Args, 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"); diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp index 7d1b1965f93..fb5f1cc5ac7 100644 --- a/clang/lib/Frontend/CompilerInvocation.cpp +++ b/clang/lib/Frontend/CompilerInvocation.cpp @@ -1139,6 +1139,8 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK, 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, diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp index 950dcac1bbc..2a72f7d5c61 100644 --- a/clang/lib/Sema/SemaDeclAttr.cpp +++ b/clang/lib/Sema/SemaDeclAttr.cpp @@ -7198,6 +7198,9 @@ static void ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, // 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: diff --git a/clang/test/CodeGen/cfi-icall-canonical-jump-tables.c b/clang/test/CodeGen/cfi-icall-canonical-jump-tables.c new file mode 100644 index 00000000000..cf7ca42781e --- /dev/null +++ b/clang/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} diff --git a/clang/test/CodeGen/cfi-icall-cross-dso.c b/clang/test/CodeGen/cfi-icall-cross-dso.c index 67901c4f7d6..fbfe213c583 100644 --- a/clang/test/CodeGen/cfi-icall-cross-dso.c +++ b/clang/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 diff --git a/clang/test/Driver/fsanitize.c b/clang/test/Driver/fsanitize.c index 0caac04637d..6e8e61b2935 100644 --- a/clang/test/Driver/fsanitize.c +++ b/clang/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 diff --git a/clang/test/Misc/pragma-attribute-supported-attributes-list.test b/clang/test/Misc/pragma-attribute-supported-attributes-list.test index cc2d3806018..83b8b9f0b9f 100644 --- a/clang/test/Misc/pragma-attribute-supported-attributes-list.test +++ b/clang/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) diff --git a/clang/test/SemaCXX/attr-cfi-canonical-jump-table.cpp b/clang/test/SemaCXX/attr-cfi-canonical-jump-table.cpp new file mode 100644 index 00000000000..a53fbaebaf5 --- /dev/null +++ b/clang/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}} diff --git a/llvm/include/llvm/Transforms/IPO/LowerTypeTests.h b/llvm/include/llvm/Transforms/IPO/LowerTypeTests.h index 39b23f5957d..3c2bb65b955 100644 --- a/llvm/include/llvm/Transforms/IPO/LowerTypeTests.h +++ b/llvm/include/llvm/Transforms/IPO/LowerTypeTests.h @@ -193,6 +193,8 @@ struct ByteArrayBuilder { uint64_t &AllocByteOffset, uint8_t &AllocMask); }; +bool isJumpTableCanonical(Function *F); + } // end namespace lowertypetests class LowerTypeTestsPass : public PassInfoMixin<LowerTypeTestsPass> { diff --git a/llvm/lib/Transforms/IPO/CrossDSOCFI.cpp b/llvm/lib/Transforms/IPO/CrossDSOCFI.cpp index e30b33aa487..8b2842e18b5 100644 --- a/llvm/lib/Transforms/IPO/CrossDSOCFI.cpp +++ b/llvm/lib/Transforms/IPO/CrossDSOCFI.cpp @@ -84,13 +84,9 @@ void CrossDSOCFI::buildCFICheck(Module &M) { 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"); diff --git a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp index fd1b0cd7ad3..1b83ccb816e 100644 --- a/llvm/lib/Transforms/IPO/LowerTypeTests.cpp +++ b/llvm/lib/Transforms/IPO/LowerTypeTests.cpp @@ -230,6 +230,16 @@ void ByteArrayBuilder::allocate(const std::set<uint64_t> &Bits, 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 @@ class GlobalTypeMember final : TrailingObjects<GlobalTypeMember, MDNode *> { 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 @@ class GlobalTypeMember final : TrailingObjects<GlobalTypeMember, MDNode *> { 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 @@ public: return GO; } - bool isDefinition() const { - return IsDefinition; + bool isJumpTableCanonical() const { + return IsJumpTableCanonical; } bool isExported() const { @@ -320,6 +333,49 @@ private: 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 @@ class LowerTypeTestsModule { 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 @@ class LowerTypeTestsModule { 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 @@ class LowerTypeTestsModule { /// 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 @@ void LowerTypeTestsModule::importTypeTest(CallInst *CI) { } // 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 @@ void LowerTypeTestsModule::importFunction(Function *F, bool isDefinition) { } 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 @@ void LowerTypeTestsModule::importFunction(Function *F, bool isDefinition) { 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 @@ void LowerTypeTestsModule::importFunction(Function *F, bool isDefinition) { 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 @@ void LowerTypeTestsModule::findGlobalVariableUsersOf( // 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 @@ void LowerTypeTestsModule::replaceWeakDeclarationWithJumpTablePtr( 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 @@ selectJumpTableArmEncoding(ArrayRef<GlobalTypeMember *> Functions, 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,47 +1498,53 @@ void LowerTypeTestsModule::buildBitSetsFromFunctionsNative( 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()); + { + 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 (!IsJumpTableCanonical) { + if (F->hasExternalWeakLinkage()) + replaceWeakDeclarationWithJumpTablePtr(F, CombinedGlobalElemPtr, + IsJumpTableCanonical); + else + replaceCfiUses(F, CombinedGlobalElemPtr, IsJumpTableCanonical); } 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()); + 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, IsJumpTableCanonical); + if (!F->hasLocalLinkage()) + F->setVisibility(GlobalVariable::HiddenVisibility); } } - if (!IsDefinition) { - if (F->isWeakForLinker()) - replaceWeakDeclarationWithJumpTablePtr(F, CombinedGlobalElemPtr, IsDefinition); - else - replaceCfiUses(F, CombinedGlobalElemPtr, IsDefinition); - } 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); - } } createJumpTable(JumpTableFn, Functions); @@ -1650,7 +1709,8 @@ static bool isDirectCall(Use& U) { 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 @@ void LowerTypeTestsModule::replaceCfiUses(Function *Old, Value *New, bool IsDefi 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 @@ bool LowerTypeTestsModule::lower() { 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 @@ bool LowerTypeTestsModule::lower() { 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); diff --git a/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp b/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp index f0dea11e1b1..31c731e734f 100644 --- a/llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp +++ b/llvm/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 @@ void splitAndWriteThinLTOBitcode( 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; diff --git a/llvm/test/Transforms/LowerTypeTests/import-icall.ll b/llvm/test/Transforms/LowerTypeTests/import-icall.ll index 0b151db007d..67788a2be4a 100644 --- a/llvm/test/Transforms/LowerTypeTests/import-icall.ll +++ b/llvm/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 @@ define i8 @use_b() { 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 @@ declare extern_weak void @external_weak() ; 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() diff --git a/llvm/test/Transforms/ThinLTOBitcodeWriter/cfi-functions-canonical-jump-tables.ll b/llvm/test/Transforms/ThinLTOBitcodeWriter/cfi-functions-canonical-jump-tables.ll new file mode 100644 index 00000000000..ad775ee12a5 --- /dev/null +++ b/llvm/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"} |
