Remove incorrectly implemented -mibt-seal

[MaskRay]

Summary

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).

[samitolvanen]

I agree, it's probably best to temporarily revert this until Joao has time to address the issues you mentioned. The Linux kernel doesn't use -mibt-seal yet, so dropping the feature for now shouldn't be a problem.

[joaomoreira]

FWIIW, agreed on removing this until we figure out how to make it work properly. Thanks for the patch @MaskRay.

[MaskRay]

In D140363#4014131, @joaomoreira wrote:

FWIIW, agreed on removing this until we figure out how to make it work properly. Thanks for the patch @MaskRay.

Thanks for accepting the removal:)

clang/include/clang/Basic/CodeGenOptions.def

 CODEGENOPT(InstrumentFunctionsAfterInlining , 1, 0) ///< Set when
                           ///< -finstrument-functions-after-inlining is enabled.
 CODEGENOPT(InstrumentFunctionEntryBare , 1, 0) ///< Set when
                                ///< -finstrument-function-entry-bare is enabled.
 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.
 
 CODEGENOPT(XRayInstrumentFunctions , 1, 0) ///< Set when -fxray-instrument is
                                            ///< enabled.
 CODEGENOPT(StackSizeSection  , 1, 0) ///< Set when -fstack-size-section is enabled.
 CODEGENOPT(ForceDwarfFrameSection , 1, 0) ///< Set when -fforce-dwarf-frame is

clang/include/clang/Driver/Options.td

 def finstrument_function_entry_bare : Flag<["-"], "finstrument-function-entry-bare">, Group<f_Group>, Flags<[CC1Option]>,
   HelpText<"Instrument function entry only, after inlining, without arguments to the instrumentation call">,
   MarshallingInfoFlag<CodeGenOpts<"InstrumentFunctionEntryBare">>;
 def fcf_protection_EQ : Joined<["-"], "fcf-protection=">, Flags<[CoreOption, CC1Option]>, Group<f_Group>,
   HelpText<"Instrument control-flow architecture protection">, Values<"return,branch,full,none">;
 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)">,
   Values<"keep,thunk-extern">,
   NormalizedValues<["Keep", "Extern"]>,
   NormalizedValuesScope<"llvm::FunctionReturnThunksKind">,
   MarshallingInfoEnum<CodeGenOpts<"FunctionReturnThunks">, "Keep">;
 def mindirect_branch_cs_prefix : Flag<["-"], "mindirect-branch-cs-prefix">,

clang/lib/CodeGen/CodeGenModule.cpp

void CodeGenModule::Release() {
 
   if (CodeGenOpts.CFProtectionBranch &&
       Target.checkCFProtectionBranchSupported(getDiags())) {
     // Indicate that we want to instrument branch control flow protection.
     getModule().addModuleFlag(llvm::Module::Min, "cf-protection-branch",
                               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);
 
   if (CodeGenOpts.IndirectBranchCSPrefix)
     getModule().addModuleFlag(llvm::Module::Override, "indirect_branch_cs_prefix", 1);
 
   // Add module metadata for return address signing (ignoring
   // non-leaf/all) and stack tagging. These are actually turned on by function

clang/lib/Driver/ToolChains/Clang.cpp

   if (Args.hasArg(options::OPT_nogpulib))
     CmdArgs.push_back("-nogpulib");
 
   if (Arg *A = Args.getLastArg(options::OPT_fcf_protection_EQ)) {
     CmdArgs.push_back(
         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()));
 
   Args.AddLastArg(CmdArgs, options::OPT_mindirect_branch_cs_prefix);
 
   // Forward -f options with positive and negative forms; we translate these by
   // hand.  Do not propagate PGO options to the GPU-side compilations as the

clang/lib/Frontend/CompilerInvocation.cpp

#undef CODEGEN_OPTION_WITH_MARSHALLING
 
   if (Opts.CFProtectionReturn && Opts.CFProtectionBranch)
     GenerateArg(Args, OPT_fcf_protection_EQ, "full", SA);
   else if (Opts.CFProtectionReturn)
     GenerateArg(Args, OPT_fcf_protection_EQ, "return", SA);
   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);
 
   for (const auto &F : Opts.LinkBitcodeFiles) {
     bool Builtint = F.LinkFlags == llvm::Linker::Flags::LinkOnlyNeeded &&
                     F.PropagateAttrs && F.Internalize;
     GenerateArg(Args,
                 Builtint ? OPT_mlink_builtin_bitcode : OPT_mlink_bitcode_file,
else if (Val == llvm::FunctionReturnThunksKind::Extern &&
              Args.getLastArgValue(OPT_mcmodel_EQ).equals("large"))
       Diags.Report(diag::err_drv_argument_not_allowed_with)
           << A->getAsString(Args)
           << Args.getLastArg(OPT_mcmodel_EQ)->getAsString(Args);
     else
       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;
     F.Filename = A->getValue();
     if (A->getOption().matches(OPT_mlink_builtin_bitcode)) {
       F.LinkFlags = llvm::Linker::Flags::LinkOnlyNeeded;
       // When linking CUDA bitcode, propagate function attributes so that
       // e.g. libdevice gets fast-math attrs if we're building with fast-math.

clang/test/CodeGen/X86/x86-cf-protection.c

 // 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
 
 void foo() {}

llvm/lib/Target/X86/X86IndirectBranchTracking.cpp

 
 // Checks if function should have an ENDBR in its prologue
 static bool needsPrologueENDBR(MachineFunction &MF, const Module *M) {
   Function &F = MF.getFunction();
 
   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());
   }
 }
 
 bool X86IndirectBranchTrackingPass::runOnMachineFunction(MachineFunction &MF) {
   const X86Subtarget &SubTarget = MF.getSubtarget<X86Subtarget>();

llvm/test/CodeGen/X86/ibtseal-kernel.ll

-; 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}

llvm/test/CodeGen/X86/ibtseal-large.ll

-; 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}

llvm/test/CodeGen/X86/ibtseal-small.ll

-; 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}