samitolvanen created this revision.
Herald added subscribers: ormris, jeroen.dobbelaere, dexonsmith, dang,
steven_wu, hiraditya.
samitolvanen requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.
Inline assembly refererences to static functions with ThinLTO+CFI were
fixed in D104058 <https://reviews.llvm.org/D104058> by creating aliases for
promoted functions. Creating
the aliases unconditionally resulted in an unexpected size increase in
a Chrome helper binary:
https://bugs.chromium.org/p/chromium/issues/detail?id=1261715
As promotion aliases are only required for very specific use cases,
such as compiling the Linux kernel with CFI, add a command line flag
to allow users to create these aliases only when needed.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D112761
Files:
clang/docs/ClangCommandLineReference.rst
clang/docs/ControlFlowIntegrity.rst
clang/docs/UsersManual.rst
clang/include/clang/Basic/CodeGenOptions.def
clang/include/clang/Driver/Options.td
clang/include/clang/Driver/SanitizerArgs.h
clang/lib/CodeGen/CodeGenModule.cpp
clang/lib/Driver/SanitizerArgs.cpp
clang/test/CodeGen/cfi-promotion-aliases.c
clang/test/Driver/fsanitize.c
llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
llvm/test/ThinLTO/X86/devirt2.ll
llvm/test/Transforms/ThinLTOBitcodeWriter/cfi-icall-static-inline-asm.ll
Index: llvm/test/Transforms/ThinLTOBitcodeWriter/cfi-icall-static-inline-asm.ll
===================================================================
--- llvm/test/Transforms/ThinLTOBitcodeWriter/cfi-icall-static-inline-asm.ll
+++ llvm/test/Transforms/ThinLTOBitcodeWriter/cfi-icall-static-inline-asm.ll
@@ -19,4 +19,7 @@
ret void
}
+!llvm.module.flags = !{!1}
+
!0 = !{i64 0, !"typeid1"}
+!1 = !{i32 4, !"CFI Promotion Aliases", i32 1}
Index: llvm/test/ThinLTO/X86/devirt2.ll
===================================================================
--- llvm/test/ThinLTO/X86/devirt2.ll
+++ llvm/test/ThinLTO/X86/devirt2.ll
@@ -131,12 +131,10 @@
; RUN: -r=%t1.o,_ZN1D1mEi, \
; RUN: -r=%t1.o,test2, \
; RUN: -r=%t2.o,_ZN1A1nEi,p \
-; RUN: -r=%t2.o,_ZN1A1nEi, \
; RUN: -r=%t2.o,_ZN1B1fEi,p \
; RUN: -r=%t2.o,_ZN1C1fEi,p \
; RUN: -r=%t2.o,_ZN1D1mEi,p \
; RUN: -r=%t2.o,_ZN1E1mEi,p \
-; RUN: -r=%t2.o,_ZN1E1mEi, \
; RUN: -r=%t2.o,_ZTV1B, \
; RUN: -r=%t2.o,_ZTV1C, \
; RUN: -r=%t2.o,_ZTV1D, \
@@ -169,12 +167,10 @@
; RUN: -r=%t1.o,_ZN1D1mEi, \
; RUN: -r=%t1.o,test2, \
; RUN: -r=%t2.o,_ZN1A1nEi,p \
-; RUN: -r=%t2.o,_ZN1A1nEi, \
; RUN: -r=%t2.o,_ZN1B1fEi,p \
; RUN: -r=%t2.o,_ZN1C1fEi,p \
; RUN: -r=%t2.o,_ZN1D1mEi,p \
; RUN: -r=%t2.o,_ZN1E1mEi,p \
-; RUN: -r=%t2.o,_ZN1E1mEi, \
; RUN: -r=%t2.o,_ZTV1B, \
; RUN: -r=%t2.o,_ZTV1C, \
; RUN: -r=%t2.o,_ZTV1D, \
Index: llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
===================================================================
--- llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
+++ llvm/lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp
@@ -33,6 +33,13 @@
namespace {
+// Determine if promotion aliases should be created for a module.
+static bool createPromotionAliases(Module &M) {
+ auto *CI = mdconst::extract_or_null<ConstantInt>(
+ M.getModuleFlag("CFI Promotion Aliases"));
+ return (CI && CI->getZExtValue() != 0);
+}
+
// Determine if a promotion alias should be created for a symbol name.
static bool allowPromotionAlias(const std::string &Name) {
// Promotion aliases are used only in inline assembly. It's safe to
@@ -51,6 +58,8 @@
void promoteInternals(Module &ExportM, Module &ImportM, StringRef ModuleId,
SetVector<GlobalValue *> &PromoteExtra) {
DenseMap<const Comdat *, Comdat *> RenamedComdats;
+ bool CreateAliases = createPromotionAliases(ExportM);
+
for (auto &ExportGV : ExportM.global_values()) {
if (!ExportGV.hasLocalLinkage())
continue;
@@ -84,7 +93,8 @@
ImportGV->setVisibility(GlobalValue::HiddenVisibility);
}
- if (isa<Function>(&ExportGV) && allowPromotionAlias(OldName)) {
+ if (CreateAliases && isa<Function>(&ExportGV) &&
+ allowPromotionAlias(OldName)) {
// Create a local alias with the original name to avoid breaking
// references from inline assembly.
std::string Alias = ".set " + OldName + "," + NewName + "\n";
Index: clang/test/Driver/fsanitize.c
===================================================================
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -654,6 +654,11 @@
// CHECK-CFI-GENERALIZE-POINTERS: -fsanitize-cfi-icall-generalize-pointers
// CHECK-NO-CFI-GENERALIZE-POINTERS-NOT: -fsanitize-cfi-icall-generalize-pointers
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi-icall -fsanitize-cfi-promotion-aliases -fvisibility=hidden -flto=thin -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-CFI-PROMOTION-ALIASES
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=cfi-icall -fvisibility=hidden -flto=thin -c %s -### 2>&1 | FileCheck %s --check-prefix=CHECK-NO-CFI-PROMOTION-ALIASES
+// CHECK-CFI-PROMOTION-ALIASES: -fsanitize-cfi-promotion-aliases
+// CHECK-NO-CFI-PROMOTION-ALIASES-NOT: -fsanitize-cfi-promotion-aliases
+
// 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'
Index: clang/test/CodeGen/cfi-promotion-aliases.c
===================================================================
--- /dev/null
+++ clang/test/CodeGen/cfi-promotion-aliases.c
@@ -0,0 +1,7 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-icall -emit-llvm -o - %s | FileCheck --check-prefixes=NOALIASES %s
+// RUN: %clang_cc1 -triple x86_64-unknown-linux -fsanitize=cfi-icall -fsanitize-cfi-promotion-aliases -emit-llvm -o - %s | FileCheck --check-prefixes=ALIASES %s
+
+void a(void) {}
+
+// NOALIASES: !{i32 4, !"CFI Promotion Aliases", i32 0}
+// ALIASES: !{i32 4, !"CFI Promotion Aliases", i32 1}
Index: clang/lib/Driver/SanitizerArgs.cpp
===================================================================
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -651,6 +651,9 @@
CfiCanonicalJumpTables =
Args.hasFlag(options::OPT_fsanitize_cfi_canonical_jump_tables,
options::OPT_fno_sanitize_cfi_canonical_jump_tables, true);
+
+ CfiPromotionAliases =
+ Args.hasArg(options::OPT_fsanitize_cfi_promotion_aliases);
}
Stats = Args.hasFlag(options::OPT_fsanitize_stats,
@@ -1089,6 +1092,9 @@
if (CfiICallGeneralizePointers)
CmdArgs.push_back("-fsanitize-cfi-icall-generalize-pointers");
+ if (CfiPromotionAliases)
+ CmdArgs.push_back("-fsanitize-cfi-promotion-aliases");
+
if (CfiCanonicalJumpTables)
CmdArgs.push_back("-fsanitize-cfi-canonical-jump-tables");
Index: clang/lib/CodeGen/CodeGenModule.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -672,6 +672,9 @@
getModule().addModuleFlag(llvm::Module::Override,
"CFI Canonical Jump Tables",
CodeGenOpts.SanitizeCfiCanonicalJumpTables);
+ getModule().addModuleFlag(llvm::Module::Override,
+ "CFI Promotion Aliases",
+ CodeGenOpts.SanitizeCfiPromotionAliases);
}
if (CodeGenOpts.CFProtectionReturn &&
Index: clang/include/clang/Driver/SanitizerArgs.h
===================================================================
--- clang/include/clang/Driver/SanitizerArgs.h
+++ clang/include/clang/Driver/SanitizerArgs.h
@@ -36,6 +36,7 @@
bool CfiCrossDso = false;
bool CfiICallGeneralizePointers = false;
bool CfiCanonicalJumpTables = false;
+ bool CfiPromotionAliases = false;
int AsanFieldPadding = 0;
bool SharedRuntime = false;
bool AsanUseAfterScope = true;
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1714,6 +1714,10 @@
Group<f_clang_Group>,
HelpText<"Generalize pointers in CFI indirect call type signature checks">,
MarshallingInfoFlag<CodeGenOpts<"SanitizeCfiICallGeneralizePointers">>;
+def fsanitize_cfi_promotion_aliases : Flag<["-"], "fsanitize-cfi-promotion-aliases">,
+ Group<f_clang_Group>,
+ HelpText<"Generate aliases for promoted static functions for inline assembly">,
+ MarshallingInfoFlag<CodeGenOpts<"SanitizeCfiPromotionAliases">>;
defm sanitize_cfi_canonical_jump_tables : BoolOption<"f", "sanitize-cfi-canonical-jump-tables",
CodeGenOpts<"SanitizeCfiCanonicalJumpTables">, DefaultFalse,
PosFlag<SetTrue, [], "Make">, NegFlag<SetFalse, [CoreOption, NoXarchOption], "Do not make">,
Index: clang/include/clang/Basic/CodeGenOptions.def
===================================================================
--- clang/include/clang/Basic/CodeGenOptions.def
+++ clang/include/clang/Basic/CodeGenOptions.def
@@ -236,6 +236,8 @@
///< diagnostics.
CODEGENOPT(SanitizeCfiICallGeneralizePointers, 1, 0) ///< Generalize pointer types in
///< CFI icall function signatures
+CODEGENOPT(SanitizeCfiPromotionAliases, 1, 0) ///< Create aliases for promoted local functions
+ ///< for inline assembly
CODEGENOPT(SanitizeCfiCanonicalJumpTables, 1, 0) ///< Make jump table symbols canonical
///< instead of creating a local jump table.
CODEGENOPT(SanitizeCoverageType, 2, 0) ///< Type of sanitizer coverage
Index: clang/docs/UsersManual.rst
===================================================================
--- clang/docs/UsersManual.rst
+++ clang/docs/UsersManual.rst
@@ -1689,6 +1689,12 @@
checked by Control Flow Integrity indirect call checking. See
:doc:`ControlFlowIntegrity` for more details.
+.. option:: -fsanitize-cfi-promotion-aliases
+
+ Create aliases with the original name for local functions that are renamed
+ when they are promoted to global visibility, so they can be referenced from
+ inline assembly.
+
.. option:: -fstrict-vtable-pointers
Enable optimizations based on the strict rules for overwriting polymorphic
Index: clang/docs/ControlFlowIntegrity.rst
===================================================================
--- clang/docs/ControlFlowIntegrity.rst
+++ clang/docs/ControlFlowIntegrity.rst
@@ -285,6 +285,15 @@
the jump table entry of a specific function canonical so that the external
code will end up taking an address for the function that will pass CFI checks.
+``-fsanitize-cfi-promotion-aliases``
+--------------------------------------------
+
+With ``-flto=thin`` and CFI, static functions are renamed when they are promoted
+to global visibility, which breaks references to them from inline assembly. With
+``-fsanitize-cfi-promotion-aliases`` the compiler creates local aliases with the
+original name to allow static functions to be referenced from inline assembly.
+
+
``-fsanitize=cfi-icall`` and ``-fsanitize=function``
----------------------------------------------------
Index: clang/docs/ClangCommandLineReference.rst
===================================================================
--- clang/docs/ClangCommandLineReference.rst
+++ clang/docs/ClangCommandLineReference.rst
@@ -929,6 +929,10 @@
Generalize pointers in CFI indirect call type signature checks
+.. option:: -fsanitize-cfi-promotion-aliases
+
+Create aliases for promoted static functions to allow them to be referenced from inline assembly
+
.. option:: -fsanitize-coverage-allowlist=<arg>, -fsanitize-coverage-whitelist=<arg>
Restrict sanitizer coverage instrumentation exclusively to modules and functions that match the provided special case list, except the blocked ones
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
