[X86] [Linux build][Stack Protector] Support for -mstack-protector-guard-symbol

[xiangzhangllvm]

Summary

Add option -mstack-protector-guard-symbol=symbol to
use the given symbol for addressing the stack-protector guard

via https://lore.kernel.org/lkml/[e-mail]/, the Linux kernel is looking to use the flags:
-mstack-protector-guard-symbol=__stack_chk_guard

issues: https://github.com/llvm/llvm-project/issues/48553

[nickdesaulniers]

Tested i386 linux kernel builds+boots with CONFIG_STACKPROTECTOR now selected with clang! :) Thanks for the patch.

Only minor suggestions, feel free to ignore before landing.

Please add an entry about this feature to clang/docs/ReleaseNotes.rst (and maybe llvm/docs/ReleaseNotes.rst for the MD node).

clang/lib/Driver/ToolChains/Clang.cpp

3234–3242

If this is based on other code in clang like somewhere in the lexer, consider adding a comment that this is a similar implementation to that method, we just need it earlier in the driver to verify symbolic command line options.

clang/test/Driver/stack-protector-guard.c

46

The expected one of: part is odd, I wonder if there's a better diag to use here? Maybe diag::err_drv_argument_only_allowed_with and then still use "legal symbol name"?

llvm/lib/Target/X86/X86ISelLowering.cpp

2825–2831

It looks like the test in llvm/test/CodeGen/X86/stack-protector-3.ll tests the true case here, when the GV does not yet exist. Consider adding a test for when it does.

[xiangzhangllvm]

Sorry for later response!
@nickdesaulniers Your suggestions are very make sense.
I'll refine it soon. Thanks so much!

clang/lib/Driver/ToolChains/Clang.cpp

3234–3242

The lexer is checking letter one by one and classify them immediately, not check all the symbol name is legal or not.
For example:

int 3zxzx;

t.c:1:5: error: expected identifier or '('
int 3zxzx;
    ^
1 error generated.

clang/test/Driver/stack-protector-guard.c

46

Good catch and guide, thanks!

llvm/lib/Target/X86/X86ISelLowering.cpp

2825–2831

I think you want to add both existed "__woof" and non-existed case here. Here lack the existed one.
Ok, let me add it.

[xiangzhangllvm]

Address Nick's comments.

[nickdesaulniers]

Thanks again!

clang/docs/ClangCommandLineReference.rst

 .. option:: -mstack-protector-guard-offset=<arg>
 
 Use the given offset for addressing the stack-protector guard
 
 .. option:: -mstack-protector-guard-reg=<arg>
 
 Use the given reg for addressing the stack-protector guard
 
+.. option:: -mstack-protector-guard-symbol=<arg>
+
+Use the given symbol for addressing the stack-protector guard
+
 .. option:: -mstack-protector-guard=<arg>
 
 Use the given guard (global, tls) for addressing the stack-protector guard
 
 .. option:: -mstackrealign, -mno-stackrealign
 
 Force realign the stack at entry to every function
 

clang/include/clang/Basic/CodeGenOptions.h

public:
   std::string StackProtectorGuard;
 
   /// The TLS base register when StackProtectorGuard is "tls", or register used
   /// to store the stack canary for "sysreg".
   /// On x86 this can be "fs" or "gs".
   /// On AArch64 this can only be "sp_el0".
   std::string StackProtectorGuardReg;
 
+  /// Specify a symbol to be the guard value.
+  std::string StackProtectorGuardSymbol;
+
   /// Path to ignorelist file specifying which objects
   /// (files, functions) listed for instrumentation by sanitizer
   /// coverage pass should actually not be instrumented.
   std::vector<std::string> SanitizeCoverageIgnorelistFiles;
 
   /// Name of the stack usage file (i.e., .su file) if user passes
   /// -fstack-usage. If empty, it can be implied that -fstack-usage is not
   /// passed on the command line.

clang/include/clang/Driver/Options.td

def mprefer_vector_width_EQ : Joined<["-"], "mprefer-vector-width=">, Group<m_Group>, Flags<[CC1Option]>,
   HelpText<"Specifies preferred vector width for auto-vectorization. Defaults to 'none' which allows target specific decisions.">,
   MarshallingInfoString<CodeGenOpts<"PreferVectorWidth">>;
 def mstack_protector_guard_EQ : Joined<["-"], "mstack-protector-guard=">, Group<m_Group>, Flags<[CC1Option]>,
   HelpText<"Use the given guard (global, tls) for addressing the stack-protector guard">,
   MarshallingInfoString<CodeGenOpts<"StackProtectorGuard">>;
 def mstack_protector_guard_offset_EQ : Joined<["-"], "mstack-protector-guard-offset=">, Group<m_Group>, Flags<[CC1Option]>,
   HelpText<"Use the given offset for addressing the stack-protector guard">,
   MarshallingInfoInt<CodeGenOpts<"StackProtectorGuardOffset">, "INT_MAX", "int">;
+def mstack_protector_guard_symbol_EQ : Joined<["-"], "mstack-protector-guard-symbol=">, Group<m_Group>, Flags<[CC1Option]>,
+  HelpText<"Use the given symbol for addressing the stack-protector guard">,
+  MarshallingInfoString<CodeGenOpts<"StackProtectorGuardSymbol">>;
 def mstack_protector_guard_reg_EQ : Joined<["-"], "mstack-protector-guard-reg=">, Group<m_Group>, Flags<[CC1Option]>,
   HelpText<"Use the given reg for addressing the stack-protector guard">,
   MarshallingInfoString<CodeGenOpts<"StackProtectorGuardReg">>;
 def mfentry : Flag<["-"], "mfentry">, HelpText<"Insert calls to fentry at function entry (x86/SystemZ only)">,
   Flags<[CC1Option]>, Group<m_Group>,
   MarshallingInfoFlag<CodeGenOpts<"CallFEntry">>;
 def mnop_mcount : Flag<["-"], "mnop-mcount">, HelpText<"Generate mcount/__fentry__ calls as nops. To activate they need to be patched in.">,
   Flags<[CC1Option]>, Group<m_Group>,

clang/lib/CodeGen/CodeGenModule.cpp

void CodeGenModule::Release() {
   if (!getCodeGenOpts().RecordCommandLine.empty())
     EmitCommandLineMetadata();
 
   if (!getCodeGenOpts().StackProtectorGuard.empty())
     getModule().setStackProtectorGuard(getCodeGenOpts().StackProtectorGuard);
   if (!getCodeGenOpts().StackProtectorGuardReg.empty())
     getModule().setStackProtectorGuardReg(
         getCodeGenOpts().StackProtectorGuardReg);
+  if (!getCodeGenOpts().StackProtectorGuardSymbol.empty())
+    getModule().setStackProtectorGuardSymbol(
+        getCodeGenOpts().StackProtectorGuardSymbol);
   if (getCodeGenOpts().StackProtectorGuardOffset != INT_MAX)
     getModule().setStackProtectorGuardOffset(
         getCodeGenOpts().StackProtectorGuardOffset);
   if (getCodeGenOpts().StackAlignment)
     getModule().setOverrideStackAlignment(getCodeGenOpts().StackAlignment);
   if (getCodeGenOpts().SkipRaxSetup)
     getModule().addModuleFlag(llvm::Module::Override, "SkipRaxSetup", 1);
 

clang/lib/Driver/ToolChains/Clang.cpp

static void RenderAnalyzerOptions(const ArgList &Args, ArgStringList &CmdArgs,
   // --analyze.  We only want to show static analyzer diagnostics or frontend
   // errors.
   CmdArgs.push_back("-w");
 
   // Add -Xanalyzer arguments when running as analyzer.
   Args.AddAllArgValues(CmdArgs, options::OPT_Xanalyzer);
 }
 
+static bool isValidSymbolName(StringRef S) {
+  if (S.empty())
+    return false;
+
+  if (std::isdigit(S[0]))
+    return false;
+
+  return llvm::all_of(S, [](char C) { return std::isalnum(C) || C == '_'; });
+}
+
 static void RenderSSPOptions(const Driver &D, const ToolChain &TC,
                              const ArgList &Args, ArgStringList &CmdArgs,
                              bool KernelOrKext) {
   const llvm::Triple &EffectiveTriple = TC.getEffectiveTriple();
 
   // NVPTX doesn't support stack protectors; from the compiler's perspective, it
   // doesn't even have a stack!
   if (EffectiveTriple.isNVPTX())
if (EffectiveTriple.isX86() && (Value != "fs" && Value != "gs")) {
       return;
     }
     if (EffectiveTriple.isAArch64() && Value != "sp_el0") {
       D.Diag(diag::err_drv_invalid_value) << A->getOption().getName() << Value;
       return;
     }
     A->render(Args, CmdArgs);
   }
+
+  if (Arg *A = Args.getLastArg(options::OPT_mstack_protector_guard_symbol_EQ)) {
+    StringRef Value = A->getValue();
+    if (!isValidSymbolName(Value)) {
+      D.Diag(diag::err_drv_invalid_value_with_suggestion)
+          << A->getOption().getName() << Value << "legal symbol name";
+      return;
+    }
+    A->render(Args, CmdArgs);
+  }
 }
 
 static void RenderSCPOptions(const ToolChain &TC, const ArgList &Args,
                              ArgStringList &CmdArgs) {
   const llvm::Triple &EffectiveTriple = TC.getEffectiveTriple();
 
   if (!EffectiveTriple.isOSFreeBSD() && !EffectiveTriple.isOSLinux())
     return;

clang/test/Driver/stack-protector-guard.c

 // RUN: %clang -### -target x86_64-unknown-unknown -mstack-protector-guard=tls %s 2>&1 | \
 // RUN:   FileCheck -check-prefix=CHECK-TLS %s
 // RUN: %clang -### -target x86_64-unknown-unknown -mstack-protector-guard=global %s 2>&1 | \
 // RUN:   FileCheck -check-prefix=CHECK-GLOBAL %s
 // RUN: %clang -### -target x86_64-unknown-unknown -mstack-protector-guard=local %s 2>&1 | \
 // RUN:   FileCheck -check-prefix=INVALID-VALUE %s
 
 // CHECK-TLS: "-cc1" {{.*}}"-mstack-protector-guard=tls"
 // CHECK-GLOBAL: "-cc1" {{.*}}"-mstack-protector-guard=global"
 // INVALID-VALUE: error: invalid value 'local' in 'mstack-protector-guard=', expected one of: tls global
 
 // RUN: %clang -### -target x86_64-unknown-unknown -mstack-protector-guard-reg=fs %s 2>&1 | \
 // RUN:   FileCheck -check-prefix=CHECK-FS %s
 // RUN: %clang -### -target x86_64-unknown-unknown -mstack-protector-guard-reg=gs %s 2>&1 | \
 // RUN:   FileCheck -check-prefix=CHECK-GS %s
+// RUN: %clang -### -target x86_64-unknown-unknown -mstack-protector-guard-symbol=sym %s 2>&1 | \
+// RUN:   FileCheck -check-prefix=CHECK-SYM %s
 
 // Invalid arch
 // RUN: not %clang -target powerpc64le-linux-gnu -mstack-protector-guard=tls %s 2>&1 | \
 // RUN:   FileCheck -check-prefix=INVALID-ARCH %s
 // INVALID-ARCH: unsupported option '-mstack-protector-guard=tls' for target
 
 // RUN: not %clang -target powerpc64le-linux-gnu -mstack-protector-guard-reg=fs %s 2>&1 | \
 // RUN:   FileCheck -check-prefix=INVALID-ARCH2 %s
 // INVALID-ARCH2: unsupported option '-mstack-protector-guard-reg=fs' for target
 
 // RUN: not %clang -target powerpc64le-linux-gnu -mstack-protector-guard-offset=10 %s 2>&1 | \
 // RUN:   FileCheck -check-prefix=INVALID-ARCH3 %s
 // INVALID-ARCH3: unsupported option '-mstack-protector-guard-offset=10' for target
 
 // Invalid option value
 // RUN: not %clang -target x86_64-unknown-unknown -c -mstack-protector-guard-reg=cs %s 2>&1 | \
 // RUN:   FileCheck -check-prefix=INVALID-REG %s
 // RUN: not %clang -target x86_64-unknown-unknown -c -mstack-protector-guard-reg=ds %s 2>&1 | \
 // RUN:   FileCheck -check-prefix=INVALID-REG %s
+// RUN: not %clang -target x86_64-unknown-unknown -c -mstack-protector-guard-symbol=2s %s 2>&1 | \
+// RUN:   FileCheck -check-prefix=INVALID-SYM %s
+// RUN: not %clang -target x86_64-unknown-unknown -c -mstack-protector-guard-symbol= %s 2>&1 | \
+// RUN:   FileCheck -check-prefix=INVALID-SYM %s
 
 // CHECK-FS: "-cc1" {{.*}}"-mstack-protector-guard-reg=fs"
 // CHECK-GS: "-cc1" {{.*}}"-mstack-protector-guard-reg=gs"
 // INVALID-REG: error: invalid value {{.*}} in 'mstack-protector-guard-reg=', expected one of: fs gs
+// CHECK-SYM: "-cc1" {{.*}}"-mstack-protector-guard-symbol=sym"
+// INVALID-SYM: error: invalid value {{.*}} in 'mstack-protector-guard-symbol=', expected one of: legal symbol name
 
 // RUN: not %clang -target arm-eabi-c -mstack-protector-guard=tls %s 2>&1 | \
 // RUN:   FileCheck -check-prefix=MISSING-OFFSET %s
 // MISSING-OFFSET: error: '-mstack-protector-guard=tls' is used without '-mstack-protector-guard-offset', and there is no default
 
 // RUN: not %clang -target arm-eabi-c -mstack-protector-guard-offset=1048576 %s 2>&1 | \
 // RUN:   FileCheck -check-prefix=INVALID-OFFSET %s
 // INVALID-OFFSET: invalid integral value '1048576' in 'mstack-protector-guard-offset='

llvm/include/llvm/IR/Module.h

/// @}
   void setStackProtectorGuard(StringRef Kind);
 
   /// Get/set which register to use as the stack protector guard register. The
   /// empty string is equivalent to "global". Other values may be "tls" or
   /// "sysreg".
   StringRef getStackProtectorGuardReg() const;
   void setStackProtectorGuardReg(StringRef Reg);
 
+  /// Get/set a symbol to use as the stack protector guard.
+  StringRef getStackProtectorGuardSymbol() const;
+  void setStackProtectorGuardSymbol(StringRef Symbol);
+
   /// Get/set what offset from the stack protector to use.
   int getStackProtectorGuardOffset() const;
   void setStackProtectorGuardOffset(int Offset);
 
   /// Get/set the stack alignment overridden from the default.
   unsigned getOverrideStackAlignment() const;
   void setOverrideStackAlignment(unsigned Align);
 

llvm/lib/IR/Module.cpp

StringRef Module::getStackProtectorGuardReg() const {
   return {};
 }
 
 void Module::setStackProtectorGuardReg(StringRef Reg) {
   MDString *ID = MDString::get(getContext(), Reg);
   addModuleFlag(ModFlagBehavior::Error, "stack-protector-guard-reg", ID);
 }
 
+StringRef Module::getStackProtectorGuardSymbol() const {
+  Metadata *MD = getModuleFlag("stack-protector-guard-symbol");
+  if (auto *MDS = dyn_cast_or_null<MDString>(MD))
+    return MDS->getString();
+  return {};
+}
+
+void Module::setStackProtectorGuardSymbol(StringRef Symbol) {
+  MDString *ID = MDString::get(getContext(), Symbol);
+  addModuleFlag(ModFlagBehavior::Error, "stack-protector-guard-symbol", ID);
+}
+
 int Module::getStackProtectorGuardOffset() const {
   Metadata *MD = getModuleFlag("stack-protector-guard-offset");
   if (auto *CI = mdconst::dyn_extract_or_null<ConstantInt>(MD))
     return CI->getSExtValue();
   return INT_MAX;
 }
 
 void Module::setStackProtectorGuardOffset(int Offset) {

llvm/lib/Target/X86/X86ISelLowering.cpp

if (Subtarget.isTargetFuchsia()) {
       if (Offset == INT_MAX)
         Offset = (Subtarget.is64Bit()) ? 0x28 : 0x14;
 
       StringRef GuardReg = M->getStackProtectorGuardReg();
       if (GuardReg == "fs")
         AddressSpace = X86AS::FS;
       else if (GuardReg == "gs")
         AddressSpace = X86AS::GS;
+
+      // Use symbol guard if user specify.
+      StringRef GuardSymb = M->getStackProtectorGuardSymbol();
+      if (!GuardSymb.empty()) {
+        GlobalVariable *GV = M->getGlobalVariable(GuardSymb);
+        if (!GV) {
+          Type *Ty = Subtarget.is64Bit() ? Type::getInt64Ty(M->getContext())
+                                         : Type::getInt32Ty(M->getContext());
+          GV = new GlobalVariable(*M, Ty, false, GlobalValue::ExternalLinkage,
+                                  nullptr, GuardSymb, nullptr,
+                                  GlobalValue::NotThreadLocal, AddressSpace);
+        }
+        return GV;
+      }
+
       return SegmentOffset(IRB, Offset, AddressSpace);
     }
   }
   return TargetLowering::getIRStackGuard(IRB);
 }
 
 void X86TargetLowering::insertSSPDeclarations(Module &M) const {
   // MSVC CRT provides functionalities for stack protection.

llvm/test/CodeGen/X86/stack-protector-3.ll

 ; RUN: split-file %s %t
 ; RUN: cat %t/main.ll %t/a.ll > %t/a2.ll
 ; RUN: cat %t/main.ll %t/b.ll > %t/b2.ll
 ; RUN: cat %t/main.ll %t/c.ll > %t/c2.ll
 ; RUN: cat %t/main.ll %t/d.ll > %t/d2.ll
 ; RUN: cat %t/main.ll %t/e.ll > %t/e2.ll
 ; RUN: cat %t/main.ll %t/f.ll > %t/f2.ll
 ; RUN: cat %t/main.ll %t/g.ll > %t/g2.ll
+; RUN: cat %t/main.ll %t/h.ll > %t/h2.ll
 ; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %t/a2.ll | FileCheck --check-prefix=CHECK-TLS-FS-40 %s
 ; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %t/b2.ll | FileCheck --check-prefix=CHECK-TLS-FS-40 %s
 ; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %t/c2.ll | FileCheck --check-prefix=CHECK-GLOBAL %s
 ; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %t/d2.ll | FileCheck --check-prefix=CHECK-TLS-FS-40 %s
 ; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %t/e2.ll | FileCheck --check-prefix=CHECK-GS %s
 ; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %t/f2.ll | FileCheck --check-prefix=CHECK-OFFSET %s
 ; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %t/g2.ll | FileCheck --check-prefix=CHECK-NEGATIVE-OFFSET %s
+; RUN: llc -mtriple=x86_64-pc-linux-gnu -o - < %t/h2.ll | FileCheck --check-prefix=CHECK-SYM %s
 
 ;--- main.ll
 
 ; CHECK-TLS-FS-40:       movq    %fs:40, %rax
 ; CHECK-TLS-FS-40:       movq    %fs:40, %rax
 ; CHECK-TLS-FS-40-NEXT:  cmpq    16(%rsp), %rax
 ; CHECK-TLS-FS-40-NEXT:  jne     .LBB0_2
 ; CHECK-TLS-FS-40:       .LBB0_2:

 ; CHECK-GLOBAL:       movq    __stack_chk_guard(%rip), %rax
 ; CHECK-GLOBAL:       movq    __stack_chk_guard(%rip), %rax
 ; CHECK-GLOBAL-NEXT:  cmpq    16(%rsp), %rax
 ; CHECK-GLOBAL-NEXT:  jne     .LBB0_2
 ; CHECK-GLOBAL:       .LBB0_2:
 ; CHECK-GLOBAL-NEXT:  .cfi_def_cfa_offset 32
 ; CHECK-GLOBAL-NEXT:  callq   __stack_chk_fail
 
+; CHECK-SYM:         movq    __woof@GOTPCREL(%rip), %rax
+; CHECK-SYM-NEXT:    movq    %fs:(%rax), %rcx
+; CHECK-SYM-NEXT:    movq    %rcx, 16(%rsp)
+; CHECK-SYM:         movq    %fs:(%rax), %rax
+; CHECK-SYM-NEXT:    cmpq    16(%rsp), %rax
+; CHECK-SYM-NEXT:    jne     .LBB0_2
+; CHECK-SYM:         .LBB0_2:
+; CHECK-SYM-NEXT:    .cfi_def_cfa_offset 32
+; CHECK-SYM-NEXT:    callq   __stack_chk_fail@PLT
+
 ; ModuleID = 't.c'
 @.str = private unnamed_addr constant [14 x i8] c"stackoverflow\00", align 1
 @a = dso_local local_unnamed_addr global ptr null, align 8
 
 ; Function Attrs: nounwind sspreq uwtable writeonly
 define dso_local i32 @main() local_unnamed_addr #0 {
 entry:
   %array = alloca [5 x i8], align 1

 !llvm.module.flags = !{!1}
 !1 = !{i32 2, !"stack-protector-guard-reg", !"gs"}
 ;--- f.ll
 !llvm.module.flags = !{!1}
 !1 = !{i32 2, !"stack-protector-guard-offset", i32 20}
 ;--- g.ll
 !llvm.module.flags = !{!1}
 !1 = !{i32 2, !"stack-protector-guard-offset", i32 -20}
+;--- h.ll
+!llvm.module.flags = !{!1}
+!1 = !{i32 2, !"stack-protector-guard-symbol", !"__woof"}