https://github.com/PanTao2 created https://github.com/llvm/llvm-project/pull/225626
clang-cl enables `/GS` (Buffer Security Check) by default, but implements it as `-fstack-protector-strong`. That heuristic protects considerably more functions than MSVC's does — most notably any function that merely takes the address of a local — which costs code size and performance relative to MSVC for no compatibility benefit. This patch adds a new stack protector mode, `LangOptions::SSPMSVC`, which clang-cl now passes via `-stack-protector 4` instead of the "strong" level. It still lowers to the `sspstrong` function attribute, but additionally emits a `"stack-protector-gs-buffer"="true"` function attribute that tells `StackProtector` to select on MSVC's rules: a function is protected only if it takes no variable argument list and allocates a "GS buffer", i.e. - an array larger than 4 bytes with more than two elements whose element type is not a pointer type, - an aggregate larger than 8 bytes that contains no pointers, - an `alloca()` buffer of any size, or - an aggregate containing any of the above. Also matching MSVC, no check is inserted at `-O0`. Stack layout rules are unchanged. `/GS-`, `__declspec(safebuffers)` and `__declspec(strict_gs_check)` keep their existing meanings; `strict_gs_check` continues to select the broader "strong" heuristic and still applies at `-O0`, since it is an explicit per-function opt-in. To keep inlining from weakening protection, `adjustCallerSSPLevel()` drops `"stack-protector-gs-buffer"` from the caller when a callee that was compiled with the stronger heuristic is inlined into it. Because clang-cl no longer defaults to a GCC-compatible level, it no longer predefines `__SSP_STRONG__`; `/clang:-fstack-protector-strong` restores the previous behavior. 🤖 Generated with [Claude Code](https://claude.com/claude-code) >From d4ab2cc089c055101eec0b7bb5b069b464633b46 Mon Sep 17 00:00:00 2001 From: "Pan, Tao" <[email protected]> Date: Wed, 23 Sep 2026 14:52:49 +0800 Subject: [PATCH] [clang-cl] Use MSVC's "GS buffer" heuristic for the default /GS clang-cl enables /GS (Buffer Security Check) by default, but implemented it as -fstack-protector-strong. That heuristic protects considerably more functions than MSVC's does, most notably any function that merely takes the address of a local, which costs code size and performance relative to MSVC for no compatibility benefit. Add a new stack protector mode, LangOptions::SSPMSVC, which clang-cl now passes via -stack-protector 4 instead of the "strong" level. It still lowers to the sspstrong function attribute, but additionally emits a "stack-protector-gs-buffer"="true" function attribute that tells StackProtector to select on MSVC's rules: a function is protected only if it takes no variable argument list and allocates a "GS buffer", i.e. - an array larger than 4 bytes with more than two elements whose element type is not a pointer type, - an aggregate larger than 8 bytes that contains no pointers, - an alloca() buffer of any size, or - an aggregate containing any of the above. Also matching MSVC, no check is inserted at -O0. Stack layout rules are unchanged. /GS-, __declspec(safebuffers) and __declspec(strict_gs_check) keep their existing meanings; strict_gs_check continues to select the broader "strong" heuristic and still applies at -O0, since it is an explicit per-function opt-in. To keep inlining from weakening protection, adjustCallerSSPLevel() drops "stack-protector-gs-buffer" from the caller when a callee that was compiled with the stronger heuristic is inlined into it. Because clang-cl no longer defaults to a GCC-compatible level, it no longer predefines __SSP_STRONG__; /clang:-fstack-protector-strong restores the previous behavior. --- clang/docs/ReleaseNotes.md | 13 ++ clang/include/clang/Basic/LangOptions.def | 2 +- clang/include/clang/Basic/LangOptions.h | 2 +- clang/include/clang/Options/Options.td | 4 +- clang/lib/CodeGen/CodeGenModule.cpp | 18 +- clang/lib/CodeGen/CodeGenModule.h | 2 + clang/lib/Driver/ToolChains/Clang.cpp | 2 +- .../CodeGen/ms-stack-protector-gs-buffer.c | 48 +++++ clang/test/Driver/cl-options.c | 13 +- clang/test/Driver/cl-options.cu | 2 +- llvm/docs/LangRef.md | 24 +++ llvm/lib/CodeGen/StackProtector.cpp | 122 +++++++++++- llvm/lib/IR/Attributes.cpp | 9 + .../CodeGen/X86/stack-protector-gs-buffer.ll | 185 ++++++++++++++++++ .../Transforms/Inline/inline-ssp-gs-buffer.ll | 46 +++++ 15 files changed, 474 insertions(+), 18 deletions(-) create mode 100644 clang/test/CodeGen/ms-stack-protector-gs-buffer.c create mode 100644 llvm/test/CodeGen/X86/stack-protector-gs-buffer.ll create mode 100644 llvm/test/Transforms/Inline/inline-ssp-gs-buffer.ll diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index e5da258b9950a3..24b8ee4f030aaa 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -122,6 +122,19 @@ features cannot lower the translation-unit ABI level; target triples (except 32b arm targets). Can be disabled via `-fno-stack-clash-protection`. +- clang-cl's default `/GS` (Buffer Security Check) now uses MSVC's "GS buffer" + heuristic instead of the GCC-compatible `-fstack-protector-strong` one. A + function is protected if it allocates an array larger than 4 bytes with more + than two elements of non-pointer type, a pointer-free aggregate larger than 8 + bytes, an `_alloca` buffer, or an aggregate containing one of those. Matching + MSVC, a local merely having its address taken no longer protects a function, + varargs functions are never protected, and no protection is inserted when + optimizations are disabled. `/GS-` and `__declspec(safebuffers)` still + disable it, and `__declspec(strict_gs_check)` still selects the broader + heuristic. Since clang-cl no longer defaults to a GCC-compatible level it no + longer predefines `__SSP_STRONG__`; pass `/clang:-fstack-protector-strong` + for the previous behavior. + ### Clang Python Bindings Potentially Breaking Changes - `CompletionChunkKind` instance's `__str__` representation has been adapted to be consistent with other enums in the library. diff --git a/clang/include/clang/Basic/LangOptions.def b/clang/include/clang/Basic/LangOptions.def index d7637f2dfd5079..0e7fc5a30b5c05 100644 --- a/clang/include/clang/Basic/LangOptions.def +++ b/clang/include/clang/Basic/LangOptions.def @@ -369,7 +369,7 @@ ENUM_LANGOPT(ExternDeclNoDLLStorageClassVisibility, VisibilityFromDLLStorageClas LANGOPT(SemanticInterposition , 1, 0, Benign, "semantic interposition") LANGOPT(HalfNoSemanticInterposition, 1, 0, Benign, "Like -fno-semantic-interposition but don't use local aliases") -ENUM_LANGOPT(StackProtector, StackProtectorMode, 2, SSPOff, NotCompatible, +ENUM_LANGOPT(StackProtector, StackProtectorMode, 3, SSPOff, NotCompatible, "stack protector mode") ENUM_LANGOPT(TrivialAutoVarInit, TrivialAutoVarInitKind, 2, TrivialAutoVarInitKind::Uninitialized, Benign, "trivial automatic variable initialization") diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h index 7539e000d03f98..2b80946009070a 100644 --- a/clang/include/clang/Basic/LangOptions.h +++ b/clang/include/clang/Basic/LangOptions.h @@ -96,7 +96,7 @@ class LangOptionsBase { }; enum GCMode { NonGC, GCOnly, HybridGC }; - enum StackProtectorMode { SSPOff, SSPOn, SSPStrong, SSPReq }; + enum StackProtectorMode { SSPOff, SSPOn, SSPStrong, SSPReq, SSPMSVC }; // Automatic variables live on the stack, and when trivial they're usually // uninitialized because it's undefined behavior to use them without diff --git a/clang/include/clang/Options/Options.td b/clang/include/clang/Options/Options.td index d4bf48040029f5..4494a0ca8b15e4 100644 --- a/clang/include/clang/Options/Options.td +++ b/clang/include/clang/Options/Options.td @@ -8984,9 +8984,9 @@ def static_define : Flag<["-"], "static-define">, MarshallingInfoFlag<LangOpts<"Static">>; def stack_protector : Separate<["-"], "stack-protector">, HelpText<"Enable stack protectors">, - Values<"0,1,2,3">, + Values<"0,1,2,3,4">, NormalizedValuesScope<"LangOptions">, - NormalizedValues<["SSPOff", "SSPOn", "SSPStrong", "SSPReq"]>, + NormalizedValues<["SSPOff", "SSPOn", "SSPStrong", "SSPReq", "SSPMSVC"]>, MarshallingInfoEnum<LangOpts<"StackProtector">, "SSPOff">; def stack_protector_buffer_size : Separate<["-"], "stack-protector-buffer-size">, HelpText<"Lower bound for a buffer to be considered for stack protection">, diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index 70445dd65ea2bc..496efdb731fc00 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -1163,12 +1163,24 @@ static bool isStackProtectorOn(const LangOptions &LangOpts, return LangOpts.getStackProtector() == Mode; } +bool CodeGenModule::useMSVCGSBufferHeuristic(const Decl *D) const { + return isStackProtectorOn(LangOpts, getTriple(), LangOptions::SSPMSVC) && + !(D && D->hasAttr<StrictGuardStackCheckAttr>()); +} + std::optional<llvm::Attribute::AttrKind> CodeGenModule::StackProtectorAttribute(const Decl *D) const { + // MSVC does not insert buffer security checks when optimizations are + // disabled. __declspec(strict_gs_check) is an explicit per-function opt-in, + // so it still applies at -O0. + if (useMSVCGSBufferHeuristic(D) && CodeGenOpts.OptimizationLevel == 0) + return std::nullopt; + if (D && D->hasAttr<NoStackProtectorAttr>()) ; // Do nothing. else if (D && D->hasAttr<StrictGuardStackCheckAttr>() && - isStackProtectorOn(LangOpts, getTriple(), LangOptions::SSPOn)) + (isStackProtectorOn(LangOpts, getTriple(), LangOptions::SSPOn) || + isStackProtectorOn(LangOpts, getTriple(), LangOptions::SSPMSVC))) return llvm::Attribute::StackProtectStrong; else if (isStackProtectorOn(LangOpts, getTriple(), LangOptions::SSPOn)) return llvm::Attribute::StackProtect; @@ -1176,6 +1188,8 @@ CodeGenModule::StackProtectorAttribute(const Decl *D) const { return llvm::Attribute::StackProtectStrong; else if (isStackProtectorOn(LangOpts, getTriple(), LangOptions::SSPReq)) return llvm::Attribute::StackProtectReq; + else if (isStackProtectorOn(LangOpts, getTriple(), LangOptions::SSPMSVC)) + return llvm::Attribute::StackProtectStrong; return std::nullopt; } @@ -3170,6 +3184,8 @@ void CodeGenModule::SetLLVMFunctionAttributesForDefinition(const Decl *D, if (std::optional<llvm::Attribute::AttrKind> Attr = StackProtectorAttribute(D)) { B.addAttribute(*Attr); + if (useMSVCGSBufferHeuristic(D)) + B.addAttribute("stack-protector-gs-buffer", "true"); } if (!D) { diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h index 9e3f073c20f4a0..02b5d5d96bdc0a 100644 --- a/clang/lib/CodeGen/CodeGenModule.h +++ b/clang/lib/CodeGen/CodeGenModule.h @@ -1994,6 +1994,8 @@ class CodeGenModule : public CodeGenTypeCache { std::optional<llvm::Attribute::AttrKind> StackProtectorAttribute(const Decl *D) const; + bool useMSVCGSBufferHeuristic(const Decl *D) const; + std::string getPFPFieldName(const FieldDecl *FD); llvm::GlobalValue *getPFPDeactivationSymbol(const FieldDecl *FD); diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp index 6636a5fd6e6551..3511b5630d59b9 100644 --- a/clang/lib/Driver/ToolChains/Clang.cpp +++ b/clang/lib/Driver/ToolChains/Clang.cpp @@ -8894,7 +8894,7 @@ void Clang::AddClangCLArgs(const ArgList &Args, types::ID InputType, if (!isNVPTX && Args.hasFlag(options::OPT__SLASH_GS, options::OPT__SLASH_GS_, /*Default=*/true)) { CmdArgs.push_back("-stack-protector"); - CmdArgs.push_back(Args.MakeArgString(Twine(LangOptions::SSPStrong))); + CmdArgs.push_back(Args.MakeArgString(Twine(LangOptions::SSPMSVC))); } const Driver &D = getToolChain().getDriver(); diff --git a/clang/test/CodeGen/ms-stack-protector-gs-buffer.c b/clang/test/CodeGen/ms-stack-protector-gs-buffer.c new file mode 100644 index 00000000000000..aa536f1108932a --- /dev/null +++ b/clang/test/CodeGen/ms-stack-protector-gs-buffer.c @@ -0,0 +1,48 @@ +// Check the attributes clang-cl's default /GS emits: sspstrong paired with the +// "stack-protector-gs-buffer" marker that selects MSVC's GS-buffer heuristic. +// +// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-extensions -O2 \ +// RUN: -stack-protector 4 -emit-llvm %s -o - | FileCheck %s --check-prefix=GS +// +// MSVC does not insert buffer security checks when optimizations are disabled. +// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-extensions \ +// RUN: -stack-protector 4 -emit-llvm %s -o - | FileCheck %s --check-prefix=NOOPT +// +// An explicit GCC-style level is unaffected by any of this. +// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-extensions -O2 \ +// RUN: -stack-protector 2 -emit-llvm %s -o - | FileCheck %s --check-prefix=STRONG + +void use(void *); + +// GS: define dso_local void @plain() {{.*}}#[[#PLAIN:]] { +// NOOPT: define dso_local void @plain() #[[#PLAIN:]] { +// STRONG: define dso_local void @plain() {{.*}}#[[#SPLAIN:]] { +void plain(void) { char buf[64]; use(buf); } + +// __declspec(safebuffers) opts out entirely, at every level. +// GS: define dso_local void @safe() {{.*}}#[[#SAFE:]] { +// STRONG: define dso_local void @safe() {{.*}}#[[#SSAFE:]] { +__declspec(safebuffers) void safe(void) { char buf[64]; use(buf); } + +// __declspec(strict_gs_check) asks for a cookie in a greater number of +// functions, so it opts back out of the narrower GS-buffer rules and is not +// suppressed at -O0. +// GS: define dso_local void @strict() {{.*}}#[[#STRICT:]] { +// NOOPT: define dso_local void @strict() #[[#STRICT:]] { +__declspec(strict_gs_check) void strict(void) { char buf[64]; use(buf); } + +// The string attribute sorts between "stack-protector-buffer-size" and +// "target-features", so matching those neighbours proves it is absent. +// +// GS: attributes #[[#PLAIN]] = { nounwind sspstrong {{.*}}"stack-protector-buffer-size"="8" "stack-protector-gs-buffer"="true" "target-features" +// GS: attributes #[[#SAFE]] = { nounwind "min-legal-vector-width" +// GS: attributes #[[#STRICT]] = { nounwind sspstrong {{.*}}"stack-protector-buffer-size"="8" "target-features" + +// At -O0 the GS-buffer default produces no stack protector attribute at all, +// so @plain shares an attribute group with @safe. +// NOOPT: attributes #[[#PLAIN]] = { noinline nounwind optnone "min-legal-vector-width" +// NOOPT: attributes #[[#STRICT]] = { noinline nounwind optnone sspstrong {{.*}}"stack-protector-buffer-size"="8" "target-features" + +// -stack-protector 2 keeps the GCC-compatible strong heuristic: no marker. +// STRONG: attributes #[[#SPLAIN]] = { nounwind sspstrong {{.*}}"stack-protector-buffer-size"="8" "target-features" +// STRONG: attributes #[[#SSAFE]] = { nounwind "min-legal-vector-width" diff --git a/clang/test/Driver/cl-options.c b/clang/test/Driver/cl-options.c index 57d82622ef4de6..4f9caa9d022e1e 100644 --- a/clang/test/Driver/cl-options.c +++ b/clang/test/Driver/cl-options.c @@ -119,14 +119,23 @@ // Security Buffer Check is on by default. // RUN: %clang_cl -### -- %s 2>&1 | FileCheck -check-prefix=GS-default %s -// GS-default: "-stack-protector" "2" +// GS-default: "-stack-protector" "4" // RUN: %clang_cl /GS -### -- %s 2>&1 | FileCheck -check-prefix=GS %s -// GS: "-stack-protector" "2" +// GS: "-stack-protector" "4" // RUN: %clang_cl /GS- -### -- %s 2>&1 | FileCheck -check-prefix=GS_ %s // GS_-NOT: -stack-protector +// An explicit GCC-style level is rendered after the /GS default and wins. +// RUN: %clang_cl /clang:-fstack-protector-strong -### -- %s 2>&1 | FileCheck -check-prefix=GS-strong %s +// GS-strong: "-stack-protector" "4" +// GS-strong-SAME: "-stack-protector" "2" + +// RUN: %clang_cl /clang:-fstack-protector-all -### -- %s 2>&1 | FileCheck -check-prefix=GS-all %s +// GS-all: "-stack-protector" "4" +// GS-all-SAME: "-stack-protector" "3" + // RUN: %clang_cl /Gy -### -- %s 2>&1 | FileCheck -check-prefix=Gy %s // Gy: -ffunction-sections diff --git a/clang/test/Driver/cl-options.cu b/clang/test/Driver/cl-options.cu index b241ec6672d851..4c6d4aa9756cbe 100644 --- a/clang/test/Driver/cl-options.cu +++ b/clang/test/Driver/cl-options.cu @@ -8,7 +8,7 @@ // GS-default: "-cc1" "-triple" "nvptx{{(64)?}}-nvidia-cuda" // GS-default-NOT: "-stack-protector" // GS-default: "-cc1" "-triple" -// GS-default: "-stack-protector" "2" +// GS-default: "-stack-protector" "4" // -exceptions should be passed to device-side compilation. // RUN: not %clang_cl /c /GX -### -nocudalib -nocudainc -- %s 2>&1 | FileCheck -check-prefix=GX %s diff --git a/llvm/docs/LangRef.md b/llvm/docs/LangRef.md index a4816d337e02b3..40d9183279a2b5 100644 --- a/llvm/docs/LangRef.md +++ b/llvm/docs/LangRef.md @@ -2836,6 +2836,30 @@ fn -> other_fn -> other_fn ; fn is norecurse function which has an `ssp` or `sspstrong` attribute, the calling function's attribute will be upgraded to `sspreq`. +`"stack-protector-gs-buffer"` +: This attribute replaces the heuristic used by the `ssp` and `sspstrong` + attributes with the one MSVC's `/GS` (Buffer Security Check) option + uses. It has no effect on a function that also has `sspreq`. It takes a + boolean value, and is emitted by clang-cl. + + Under this heuristic a function is protected if it takes no variable + argument list and it allocates a "GS buffer", which is any of: + + - an array that is larger than 4 bytes, has more than two elements, and + has an element type that is not a pointer type; + - a data structure whose size is more than 8 bytes and that contains no + pointers; + - a buffer allocated by `alloca()`, regardless of size; + - any class or structure that contains a GS buffer. + + Unlike `sspstrong`, a local variable merely having its address taken does + not cause a function to be protected. Stack layout rules are unchanged. + + If a function with an `ssp` or `sspstrong` attribute but no + `"stack-protector-gs-buffer"` attribute is inlined into a calling function + that has one, the attribute is dropped from the caller, so that inlining + cannot weaken the callee's protection. + (strictfp)= `strictfp` diff --git a/llvm/lib/CodeGen/StackProtector.cpp b/llvm/lib/CodeGen/StackProtector.cpp index 9ec1d0630952d1..06158545e12150 100644 --- a/llvm/lib/CodeGen/StackProtector.cpp +++ b/llvm/lib/CodeGen/StackProtector.cpp @@ -63,6 +63,17 @@ static cl::opt<bool> EnableSelectionDAGSP("enable-selectiondag-sp", static cl::opt<bool> DisableCheckNoReturn("disable-check-noreturn-call", cl::init(false), cl::Hidden); +/// Function attribute selecting MSVC's /GS (Buffer Security Check) heuristic. +/// Emitted by clang-cl. +static constexpr const char *GSBufferAttr = "stack-protector-gs-buffer"; + +/// An array is only an MSVC "GS buffer" if it is strictly larger than this. +static constexpr uint64_t MinGSArraySize = 4; + +/// A pointer-free aggregate is only an MSVC "GS buffer" if it is strictly +/// larger than this. +static constexpr uint64_t MinGSAggregateSize = 8; + /// InsertStackProtectors - Insert code into the prologue and epilogue of the /// function. /// @@ -275,6 +286,75 @@ static bool ContainsProtectableArray(Type *Ty, Module *M, unsigned SSPBufferSize return NeedsProtector; } +/// Returns true if \p Ty holds a pointer anywhere within it. MSVC's GS-buffer +/// rules treat a pointer-free aggregate as something that may be used as a +/// buffer, so the presence of a pointer is what disqualifies it. +static bool ContainsPointer(Type *Ty) { + if (Ty->isPointerTy()) + return true; + if (auto *AT = dyn_cast<ArrayType>(Ty)) + return ContainsPointer(AT->getElementType()); + if (auto *ST = dyn_cast<StructType>(Ty)) + return any_of(ST->elements(), ContainsPointer); + return false; +} + +/// Returns true if \p Ty is, or contains, a "GS buffer" as defined by MSVC's +/// /GS (Buffer Security Check) documentation: +/// +/// - an array that is larger than 4 bytes, has more than two elements, and +/// has an element type that is not a pointer type; +/// - a data structure whose size is more than 8 bytes and that contains no +/// pointers; +/// - any class or structure that contains a GS buffer. +/// +/// \param [out] IsLarge is set to true if the GS buffer that was found is +/// "large" (>= ssp-buffer-size), so that it is laid out closest to the stack +/// guard. In an aggregate holding several GS buffers this is set if any of +/// them is large. +static bool ContainsGSBuffer(Type *Ty, Module *M, unsigned SSPBufferSize, + bool &IsLarge) { + if (!Ty) + return false; + + const DataLayout &DL = M->getDataLayout(); + auto Found = [&](Type *BufferTy) { + if (DL.getTypeAllocSize(BufferTy).getKnownMinValue() >= SSPBufferSize) + IsLarge = true; + return true; + }; + + if (auto *AT = dyn_cast<ArrayType>(Ty)) { + if (!AT->getElementType()->isPointerTy() && AT->getNumElements() > 2 && + DL.getTypeAllocSize(AT).getKnownMinValue() > MinGSArraySize) + return Found(AT); + + // The array itself is not a GS buffer, but its elements may still be or + // contain one, e.g. an array of two structs that each hold a buffer. + return ContainsGSBuffer(AT->getElementType(), M, SSPBufferSize, IsLarge); + } + + auto *ST = dyn_cast<StructType>(Ty); + if (!ST || ST->isOpaque()) + return false; + + if (DL.getTypeAllocSize(ST).getKnownMinValue() > MinGSAggregateSize && + !ContainsPointer(ST)) + return Found(ST); + + bool NeedsProtector = false; + for (Type *ET : ST->elements()) + if (ContainsGSBuffer(ET, M, SSPBufferSize, IsLarge)) { + // If a member is a large GS buffer then we are done. Otherwise keep + // looking, in case a later member is a large one. + if (IsLarge) + return true; + NeedsProtector = true; + } + + return NeedsProtector; +} + /// Maximum remaining allocation size observed for a phi node, and how often /// the allocation size has already been decreased. We only allow a limited /// number of decreases. @@ -404,7 +484,8 @@ static const CallInst *findStackProtectorIntrinsic(Function &F) { /// Check whether or not this function needs a stack protector based /// upon the stack protector level. /// -/// We use two heuristics: a standard (ssp) and strong (sspstrong). +/// We use three heuristics: a standard (ssp), a strong (sspstrong) and an +/// MSVC-compatible one selected by the "stack-protector-gs-buffer" attribute. /// The standard heuristic which will add a guard variable to functions that /// call alloca with a either a variable size or a size >= SSPBufferSize, /// functions with character buffers larger than SSPBufferSize, and functions @@ -413,12 +494,17 @@ static const CallInst *findStackProtectorIntrinsic(Function &F) { /// regardless of size, functions with any buffer regardless of type and size, /// functions with aggregates that contain any buffer regardless of type and /// size, and functions that contain stack-based variables that have had their -/// address taken. +/// address taken. The MSVC heuristic implements /GS (Buffer Security Check): +/// it adds a guard variable to functions that call alloca regardless of size +/// and to functions holding a "GS buffer" (see ContainsGSBuffer), but it does +/// not consider a variable's address being taken, and it never protects a +/// function that takes a variable argument list. bool SSPLayoutAnalysis::requiresStackProtector(Function *F, SSPLayoutMap *Layout) { Module *M = F->getParent(); bool Strong = false; bool NeedsProtector = false; + bool GSBuffer = false; // The set of PHI nodes visited when determining if a variable's reference has // been taken. This set is maintained to ensure we don't visit the same PHI @@ -447,9 +533,20 @@ bool SSPLayoutAnalysis::requiresStackProtector(Function *F, }); NeedsProtector = true; Strong = true; // Use the same heuristic as strong to determine SSPLayout - } else if (F->hasFnAttribute(Attribute::StackProtectStrong)) - Strong = true; - else if (!F->hasFnAttribute(Attribute::StackProtect)) + } else if (F->hasFnAttribute(Attribute::StackProtectStrong)) { + // clang-cl's default /GS asks for MSVC's GS-buffer rules rather than the + // GCC-compatible strong heuristic. An explicit sspreq (handled above) + // still wins over both. + GSBuffer = F->getFnAttribute(GSBufferAttr).getValueAsBool(); + Strong = !GSBuffer; + } else if (F->hasFnAttribute(Attribute::StackProtect)) { + GSBuffer = F->getFnAttribute(GSBufferAttr).getValueAsBool(); + } else { + return false; + } + + // MSVC's /GS does not protect functions that take a variable argument list. + if (GSBuffer && F->isVarArg()) return false; for (const BasicBlock &BB : *F) { @@ -479,8 +576,10 @@ bool SSPLayoutAnalysis::requiresStackProtector(Function *F, std::make_pair(AI, MachineFrameInfo::SSPLK_LargeArray)); ORE.emit(RemarkBuilder); NeedsProtector = true; - } else if (Strong) { - // Require protectors for all alloca calls in strong mode. + } else if (Strong || GSBuffer) { + // Require protectors for all alloca calls in strong mode. MSVC's + // /GS likewise treats every _alloca buffer as a GS buffer, with + // no size threshold. if (!Layout) return true; Layout->insert( @@ -501,8 +600,13 @@ bool SSPLayoutAnalysis::requiresStackProtector(Function *F, } bool IsLarge = false; - if (ContainsProtectableArray(AI->getAllocatedType(), M, SSPBufferSize, - IsLarge, Strong, false)) { + bool Protect = + GSBuffer ? ContainsGSBuffer(AI->getAllocatedType(), M, + SSPBufferSize, IsLarge) + : ContainsProtectableArray(AI->getAllocatedType(), M, + SSPBufferSize, IsLarge, Strong, + /*InStruct=*/false); + if (Protect) { if (!Layout) return true; Layout->insert(std::make_pair( diff --git a/llvm/lib/IR/Attributes.cpp b/llvm/lib/IR/Attributes.cpp index 7f5f5f34af3899..58c3bcaa41a5e8 100644 --- a/llvm/lib/IR/Attributes.cpp +++ b/llvm/lib/IR/Attributes.cpp @@ -2625,6 +2625,15 @@ static void adjustCallerSSPLevel(Function &Caller, const Function &Callee) { if (!Caller.hasStackProtectorFnAttr()) return; + // The caller may be using MSVC's /GS heuristic, which protects fewer + // functions than "strong" does. If the callee asked for the stronger + // heuristic, drop the /GS marker so that inlining cannot weaken the + // protection the callee was compiled with. + if ((Callee.hasFnAttribute(Attribute::StackProtectStrong) || + Callee.hasFnAttribute(Attribute::StackProtectReq)) && + !Callee.hasFnAttribute("stack-protector-gs-buffer")) + Caller.removeFnAttr("stack-protector-gs-buffer"); + // If upgrading the SSP attribute, clear out the old SSP Attributes first. // Having multiple SSP attributes doesn't actually hurt, but it adds useless // clutter to the IR. diff --git a/llvm/test/CodeGen/X86/stack-protector-gs-buffer.ll b/llvm/test/CodeGen/X86/stack-protector-gs-buffer.ll new file mode 100644 index 00000000000000..a753e1b3bf65d7 --- /dev/null +++ b/llvm/test/CodeGen/X86/stack-protector-gs-buffer.ll @@ -0,0 +1,185 @@ +; Check MSVC's /GS (Buffer Security Check) heuristic, selected by the +; "stack-protector-gs-buffer" function attribute. +; +; A "GS buffer" is an array larger than 4 bytes with more than two elements and +; a non-pointer element type, a pointer-free aggregate larger than 8 bytes, an +; alloca of any size, or any aggregate containing one of those. +; +; RUN: llc -mtriple=x86_64-pc-windows-msvc < %s | FileCheck %s + +declare void @use(ptr) + +;; --- Arrays that are not GS buffers ------------------------------------------ + +; Exactly 4 bytes, so not larger than 4. +; CHECK-LABEL: array_4_bytes: +; CHECK-NOT: __security_cookie +; CHECK: .seh_endproc +define void @array_4_bytes() #0 { + %a = alloca [4 x i8] + call void @use(ptr %a) + ret void +} + +; 8 bytes, but only two elements. +; CHECK-LABEL: array_2_elements: +; CHECK-NOT: __security_cookie +; CHECK: .seh_endproc +define void @array_2_elements() #0 { + %a = alloca [2 x i32] + call void @use(ptr %a) + ret void +} + +; Large and many elements, but the element type is a pointer. +; CHECK-LABEL: array_of_pointers: +; CHECK-NOT: __security_cookie +; CHECK: .seh_endproc +define void @array_of_pointers() #0 { + %a = alloca [8 x ptr] + call void @use(ptr %a) + ret void +} + +;; --- Arrays that are GS buffers ---------------------------------------------- + +; CHECK-LABEL: array_8_bytes: +; CHECK: __security_cookie +define void @array_8_bytes() #0 { + %a = alloca [8 x i8] + call void @use(ptr %a) + ret void +} + +; 6 bytes and three elements: over both thresholds. +; CHECK-LABEL: array_3_shorts: +; CHECK: __security_cookie +define void @array_3_shorts() #0 { + %a = alloca [3 x i16] + call void @use(ptr %a) + ret void +} + +; Two elements, so the array itself is not a GS buffer, but each element is. +; CHECK-LABEL: array_2_buffers: +; CHECK: __security_cookie +define void @array_2_buffers() #0 { + %a = alloca [2 x [8 x i8]] + call void @use(ptr %a) + ret void +} + +;; --- Aggregates -------------------------------------------------------------- + +; 12 bytes with no pointers. +; CHECK-LABEL: struct_pointer_free: +; CHECK: __security_cookie +define void @struct_pointer_free() #0 { + %a = alloca { i32, i32, i32 } + call void @use(ptr %a) + ret void +} + +; 16 bytes, but it holds a pointer, so it is not itself a GS buffer. +; CHECK-LABEL: struct_with_pointer: +; CHECK-NOT: __security_cookie +; CHECK: .seh_endproc +define void @struct_with_pointer() #0 { + %a = alloca { ptr, i32, i32 } + call void @use(ptr %a) + ret void +} + +; Exactly 8 bytes, so not larger than 8. +; CHECK-LABEL: struct_8_bytes: +; CHECK-NOT: __security_cookie +; CHECK: .seh_endproc +define void @struct_8_bytes() #0 { + %a = alloca { i32, i32 } + call void @use(ptr %a) + ret void +} + +; Holds a pointer, but also contains a GS buffer. +; CHECK-LABEL: struct_containing_buffer: +; CHECK: __security_cookie +define void @struct_containing_buffer() #0 { + %a = alloca { ptr, [8 x i8] } + call void @use(ptr %a) + ret void +} + +; The GS buffer is two aggregates down. +; CHECK-LABEL: struct_nested_buffer: +; CHECK: __security_cookie +define void @struct_nested_buffer() #0 { + %a = alloca { ptr, { i32, [8 x i8] } } + call void @use(ptr %a) + ret void +} + +;; --- alloca ------------------------------------------------------------------ + +; CHECK-LABEL: dynamic_alloca: +; CHECK: __security_cookie +define void @dynamic_alloca(i64 %n) #0 { + %a = alloca i8, i64 %n + call void @use(ptr %a) + ret void +} + +; Even a small constant-sized alloca is a GS buffer. +; CHECK-LABEL: small_alloca: +; CHECK: __security_cookie +define void @small_alloca() #0 { + %a = alloca i8, i64 3 + call void @use(ptr %a) + ret void +} + +;; --- Exclusions -------------------------------------------------------------- + +; Unlike sspstrong, merely taking a local's address does not protect a function. +; CHECK-LABEL: address_taken: +; CHECK-NOT: __security_cookie +; CHECK: .seh_endproc +define void @address_taken() #0 { + %a = alloca i32 + call void @use(ptr %a) + ret void +} + +; MSVC never protects a function that takes a variable argument list, even one +; holding an obvious GS buffer. +; CHECK-LABEL: variadic: +; CHECK-NOT: __security_cookie +; CHECK: .seh_endproc +define void @variadic(i32 %n, ...) #0 { + %a = alloca [64 x i8] + call void @use(ptr %a) + ret void +} + +; sspreq overrides the heuristic entirely, so the varargs exclusion and the +; GS-buffer rules do not apply. +; CHECK-LABEL: sspreq_wins: +; CHECK: __security_cookie +define void @sspreq_wins(i32 %n, ...) #1 { + %a = alloca i32 + call void @use(ptr %a) + ret void +} + +; Without the marker, sspstrong keeps the GCC-compatible heuristic and does +; protect a small array. +; CHECK-LABEL: strong_still_protects: +; CHECK: __security_cookie +define void @strong_still_protects() #2 { + %a = alloca [4 x i8] + call void @use(ptr %a) + ret void +} + +attributes #0 = { sspstrong uwtable "stack-protector-gs-buffer"="true" } +attributes #1 = { sspreq uwtable "stack-protector-gs-buffer"="true" } +attributes #2 = { sspstrong uwtable } diff --git a/llvm/test/Transforms/Inline/inline-ssp-gs-buffer.ll b/llvm/test/Transforms/Inline/inline-ssp-gs-buffer.ll new file mode 100644 index 00000000000000..3e7f6b6a80a357 --- /dev/null +++ b/llvm/test/Transforms/Inline/inline-ssp-gs-buffer.ll @@ -0,0 +1,46 @@ +; RUN: opt -passes=inline %s -S | FileCheck %s +; Inlining must not leave a caller applying MSVC's narrower /GS heuristic to +; code that was compiled with the GCC-compatible strong heuristic. + +declare void @sink(ptr) + +define internal void @callee_strong() sspstrong { + %a = alloca [4 x i8] + call void @sink(ptr %a) + ret void +} + +define internal void @callee_gs() sspstrong "stack-protector-gs-buffer"="true" { + %a = alloca [64 x i8] + call void @sink(ptr %a) + ret void +} + +define internal void @callee_nossp() { + ret void +} + +; Inlining a genuinely-strong callee drops the caller's /GS marker. +; CHECK: define void @strong_into_gs() #[[STRONG:[0-9]+]] { +define void @strong_into_gs() sspstrong "stack-protector-gs-buffer"="true" { + call void @callee_strong() + ret void +} + +; Inlining another /GS function leaves the marker in place. +; CHECK: define void @gs_into_gs() #[[GS:[0-9]+]] { +define void @gs_into_gs() sspstrong "stack-protector-gs-buffer"="true" { + call void @callee_gs() + ret void +} + +; A callee with no stack protector attribute at all must not disturb the +; marker, since it does not ask for any protection of its own. +; CHECK: define void @nossp_into_gs() #[[GS]] { +define void @nossp_into_gs() sspstrong "stack-protector-gs-buffer"="true" { + call void @callee_nossp() + ret void +} + +; CHECK: attributes #[[STRONG]] = { sspstrong } +; CHECK: attributes #[[GS]] = { sspstrong "stack-protector-gs-buffer"="true" } _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
