aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFangrui Song <i@maskray.me>2022-12-22 12:32:59 -0800
committerFangrui Song <i@maskray.me>2022-12-22 12:32:59 -0800
commit69243cdb926b1057c54522df305ffc195b2863ec (patch)
treeee02576f1b5456ed6999fe10db315a78b2ebfc0e
parent23bc8f730d99960c93946286f03d97d2fc54e8fe (diff)
downloadllvm-69243cdb926b1057c54522df305ffc195b2863ec.zip
llvm-69243cdb926b1057c54522df305ffc195b2863ec.tar.gz
llvm-69243cdb926b1057c54522df305ffc195b2863ec.tar.bz2
Remove incorrectly implemented -mibt-seal
The option from D116070 does not work as intended and will not be needed when hidden visibility is used. A function needs ENDBR if it may be reached indirectly. If we make ThinLTO combine the address-taken property (close to `!GV.use_empty() && !GV.hasAtLeastLocalUnnamedAddr()`), then the condition can be expressed with: `AddressTaken || (!F.hasLocalLinkage() && (VisibleToRegularObj || !F.hasHiddenVisibility()))` The current `F.hasAddressTaken()` condition does not take into acount of address-significance in another bitcode file or ELF relocatable file. For the Linux kernel, it uses relocatable linking. lld/ELF uses a conservative approach by setting all `VisibleToRegularObj` to true. Using the non-relocatable semantics may under-estimate `VisibleToRegularObj`. As @pcc mentioned on https://github.com/ClangBuiltLinux/linux/issues/1737#issuecomment-1343414686 , we probably need a symbol list to supply additional `VisibleToRegularObj` symbols (not part of the relocatable LTO link). Reviewed By: samitolvanen Differential Revision: https://reviews.llvm.org/D140363
-rw-r--r--clang/include/clang/Basic/CodeGenOptions.def1
-rw-r--r--clang/include/clang/Driver/Options.td2
-rw-r--r--clang/lib/CodeGen/CodeGenModule.cpp3
-rw-r--r--clang/lib/Driver/ToolChains/Clang.cpp3
-rw-r--r--clang/lib/Frontend/CompilerInvocation.cpp6
-rw-r--r--clang/test/CodeGen/X86/x86-cf-protection.c5
-rw-r--r--llvm/lib/Target/X86/X86IndirectBranchTracking.cpp15
-rw-r--r--llvm/test/CodeGen/X86/ibtseal-kernel.ll19
-rw-r--r--llvm/test/CodeGen/X86/ibtseal-large.ll19
-rw-r--r--llvm/test/CodeGen/X86/ibtseal-small.ll19
10 files changed, 1 insertions, 91 deletions
diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def
index 81d5ccd..0545a4d 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -105,7 +105,6 @@ CODEGENOPT(CFProtectionReturn , 1, 0) ///< if -fcf-protection is
///< set to full or return.
CODEGENOPT(CFProtectionBranch , 1, 0) ///< if -fcf-protection is
///< set to full or branch.
-CODEGENOPT(IBTSeal, 1, 0) ///< set to optimize CFProtectionBranch.
CODEGENOPT(FunctionReturnThunks, 1, 0) ///< -mfunction-return={keep|thunk-extern}
CODEGENOPT(IndirectBranchCSPrefix, 1, 0) ///< if -mindirect-branch-cs-prefix
///< is set.
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index f1fd45d..e7765db 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -2047,8 +2047,6 @@ def fcf_protection_EQ : Joined<["-"], "fcf-protection=">, Flags<[CoreOption, CC1
def fcf_protection : Flag<["-"], "fcf-protection">, Group<f_Group>, Flags<[CoreOption, CC1Option]>,
Alias<fcf_protection_EQ>, AliasArgs<["full"]>,
HelpText<"Enable cf-protection in 'full' mode">;
-def mibt_seal : Flag<["-"], "mibt-seal">, Group<m_Group>, Flags<[CoreOption, CC1Option]>,
- HelpText<"Optimize fcf-protection=branch/full (requires LTO).">;
def mfunction_return_EQ : Joined<["-"], "mfunction-return=">,
Group<m_Group>, Flags<[CoreOption, CC1Option]>,
HelpText<"Replace returns with jumps to ``__x86_return_thunk`` (x86 only, error otherwise)">,
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index fcb60e3..1d93c76 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -775,9 +775,6 @@ void CodeGenModule::Release() {
1);
}
- if (CodeGenOpts.IBTSeal)
- getModule().addModuleFlag(llvm::Module::Min, "ibt-seal", 1);
-
if (CodeGenOpts.FunctionReturnThunks)
getModule().addModuleFlag(llvm::Module::Override, "function_return_thunk_extern", 1);
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 61294a8..4aee050 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -6361,9 +6361,6 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
Args.MakeArgString(Twine("-fcf-protection=") + A->getValue()));
}
- if (IsUsingLTO)
- Args.AddLastArg(CmdArgs, options::OPT_mibt_seal);
-
if (Arg *A = Args.getLastArg(options::OPT_mfunction_return_EQ))
CmdArgs.push_back(
Args.MakeArgString(Twine("-mfunction-return=") + A->getValue()));
diff --git a/clang/lib/Frontend/CompilerInvocation.cpp b/clang/lib/Frontend/CompilerInvocation.cpp
index d89a80e..1cabf28 100644
--- a/clang/lib/Frontend/CompilerInvocation.cpp
+++ b/clang/lib/Frontend/CompilerInvocation.cpp
@@ -1495,9 +1495,6 @@ void CompilerInvocation::GenerateCodeGenArgs(
else if (Opts.CFProtectionBranch)
GenerateArg(Args, OPT_fcf_protection_EQ, "branch", SA);
- if (Opts.IBTSeal)
- GenerateArg(Args, OPT_mibt_seal, SA);
-
if (Opts.FunctionReturnThunks)
GenerateArg(Args, OPT_mfunction_return_EQ, "thunk-extern", SA);
@@ -1857,9 +1854,6 @@ bool CompilerInvocation::ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args,
Opts.FunctionReturnThunks = static_cast<unsigned>(Val);
}
- if (Opts.PrepareForLTO && Args.hasArg(OPT_mibt_seal))
- Opts.IBTSeal = 1;
-
for (auto *A :
Args.filtered(OPT_mlink_bitcode_file, OPT_mlink_builtin_bitcode)) {
CodeGenOptions::BitcodeFileToLink F;
diff --git a/clang/test/CodeGen/X86/x86-cf-protection.c b/clang/test/CodeGen/X86/x86-cf-protection.c
index 359bad7..ba63b9e 100644
--- a/clang/test/CodeGen/X86/x86-cf-protection.c
+++ b/clang/test/CodeGen/X86/x86-cf-protection.c
@@ -1,17 +1,12 @@
// RUN: %clang_cc1 -E -triple i386 -dM -o - -fcf-protection=return %s | FileCheck %s --check-prefix=RETURN
// RUN: %clang_cc1 -E -triple i386 -dM -o - -fcf-protection=branch %s | FileCheck %s --check-prefix=BRANCH
// RUN: %clang_cc1 -E -triple i386 -dM -o - -fcf-protection=full %s | FileCheck %s --check-prefix=FULL
-// RUN: %clang_cc1 -emit-llvm -triple i386 -o - -fcf-protection=branch -mibt-seal -flto %s | FileCheck %s --check-prefixes=CFPROT,IBTSEAL
-// RUN: %clang_cc1 -emit-llvm -triple i386 -o - -fcf-protection=branch -flto %s | FileCheck %s --check-prefixes=CFPROT,NOIBTSEAL
-// RUN: %clang_cc1 -emit-llvm -triple i386 -o - -fcf-protection=branch -mibt-seal %s | FileCheck %s --check-prefixes=CFPROT,NOIBTSEAL
// RUN: not %clang_cc1 -emit-llvm-only -triple i386 -target-cpu pentium-mmx -fcf-protection=branch %s 2>&1 | FileCheck %s --check-prefix=NOCFPROT
// RETURN: #define __CET__ 2
// BRANCH: #define __CET__ 1
// FULL: #define __CET__ 3
// CFPROT: !{i32 8, !"cf-protection-branch", i32 1}
-// IBTSEAL: !{i32 8, !"ibt-seal", i32 1}
-// NOIBTSEAL-NOT: "ibt-seal", i32 1
// NOCFPROT: error: option 'cf-protection=branch' cannot be specified on this target
diff --git a/llvm/lib/Target/X86/X86IndirectBranchTracking.cpp b/llvm/lib/Target/X86/X86IndirectBranchTracking.cpp
index 6f8b87d..3baf733 100644
--- a/llvm/lib/Target/X86/X86IndirectBranchTracking.cpp
+++ b/llvm/lib/Target/X86/X86IndirectBranchTracking.cpp
@@ -102,23 +102,10 @@ static bool needsPrologueENDBR(MachineFunction &MF, const Module *M) {
if (F.doesNoCfCheck())
return false;
- const X86TargetMachine *TM =
- static_cast<const X86TargetMachine *>(&MF.getTarget());
- Metadata *IBTSeal = M->getModuleFlag("ibt-seal");
-
- switch (TM->getCodeModel()) {
+ switch (MF.getTarget().getCodeModel()) {
// Large code model functions always reachable through indirect calls.
case CodeModel::Large:
return true;
- // Only address taken functions in LTO'ed kernel are reachable indirectly.
- // IBTSeal implies LTO, thus only check if function is address taken.
- case CodeModel::Kernel:
- // Check if ibt-seal was enabled (implies LTO is being used).
- if (IBTSeal) {
- return F.hasAddressTaken();
- }
- // if !IBTSeal, fall into default case.
- [[fallthrough]];
// Address taken or externally linked functions may be reachable.
default:
return (F.hasAddressTaken() || !F.hasLocalLinkage());
diff --git a/llvm/test/CodeGen/X86/ibtseal-kernel.ll b/llvm/test/CodeGen/X86/ibtseal-kernel.ll
deleted file mode 100644
index e8c0cd0..0000000
--- a/llvm/test/CodeGen/X86/ibtseal-kernel.ll
+++ /dev/null
@@ -1,19 +0,0 @@
-; RUN: llc < %s -O2 -mtriple=x86_64-unknown-linux-gnu -x86-indirect-branch-tracking --code-model=kernel | FileCheck %s --check-prefix=CHECK-KERNEL-IBTSEAL
-
-; CHECK-KERNEL-IBTSEAL: foo:
-; CHECK-KERNEL-IBTSEAL: endbr
-; CHECK-KERNEL-IBTSEAL: bar:
-; CHECK-KERNEL-IBTSEAL-NOT: endbr
-
-target triple = "x86_64-unknown-linux-gnu"
-
-define dso_local void @foo() {
- ret void
-}
-
-define dso_local ptr @bar() {
- ret ptr @foo
-}
-
-!llvm.module.flags = !{!1}
-!1 = !{i32 4, !"ibt-seal", i32 1}
diff --git a/llvm/test/CodeGen/X86/ibtseal-large.ll b/llvm/test/CodeGen/X86/ibtseal-large.ll
deleted file mode 100644
index cefd3e6..0000000
--- a/llvm/test/CodeGen/X86/ibtseal-large.ll
+++ /dev/null
@@ -1,19 +0,0 @@
-; RUN: llc < %s -O2 -mtriple=x86_64-unknown-linux-gnu -x86-indirect-branch-tracking --code-model=large | FileCheck %s --check-prefix=CHECK-LARGE-IBTSEAL
-
-; CHECK-LARGE-IBTSEAL: foo:
-; CHECK-LARGE-IBTSEAL: endbr
-; CHECK-LARGE-IBTSEAL: bar:
-; CHECK-LARGE-IBTSEAL: endbr
-
-target triple = "x86_64-unknown-linux-gnu"
-
-define dso_local void @foo() {
- ret void
-}
-
-define dso_local ptr @bar() {
- ret ptr @foo
-}
-
-!llvm.module.flags = !{!1}
-!1 = !{i32 4, !"ibt-seal", i32 1}
diff --git a/llvm/test/CodeGen/X86/ibtseal-small.ll b/llvm/test/CodeGen/X86/ibtseal-small.ll
deleted file mode 100644
index 6fe21de..0000000
--- a/llvm/test/CodeGen/X86/ibtseal-small.ll
+++ /dev/null
@@ -1,19 +0,0 @@
-; RUN: llc < %s -O2 -mtriple=x86_64-unknown-linux-gnu -x86-indirect-branch-tracking --code-model=small | FileCheck %s --check-prefix=CHECK-SMALL-IBTSEAL
-
-; CHECK-SMALL-IBTSEAL: foo:
-; CHECK-SMALL-IBTSEAL: endbr
-; CHECK-SMALL-IBTSEAL: bar:
-; CHECK-SMALL-IBTSEAL: endbr
-
-target triple = "x86_64-unknown-linux-gnu"
-
-define dso_local void @foo() {
- ret void
-}
-
-define dso_local ptr @bar() {
- ret ptr @foo
-}
-
-!llvm.module.flags = !{!1}
-!1 = !{i32 4, !"ibt-seal", i32 1}