llvmorg-github-actions[bot] wrote:

<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-x86

@llvm/pr-subscribers-clang-driver

Author: Pan Tao (PanTao2)

<details>
<summary>Changes</summary>

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)


---

Patch is 29.51 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/225626.diff


15 Files Affected:

- (modified) clang/docs/ReleaseNotes.md (+13) 
- (modified) clang/include/clang/Basic/LangOptions.def (+1-1) 
- (modified) clang/include/clang/Basic/LangOptions.h (+1-1) 
- (modified) clang/include/clang/Options/Options.td (+2-2) 
- (modified) clang/lib/CodeGen/CodeGenModule.cpp (+17-1) 
- (modified) clang/lib/CodeGen/CodeGenModule.h (+2) 
- (modified) clang/lib/Driver/ToolChains/Clang.cpp (+1-1) 
- (added) clang/test/CodeGen/ms-stack-protector-gs-buffer.c (+48) 
- (modified) clang/test/Driver/cl-options.c (+11-2) 
- (modified) clang/test/Driver/cl-options.cu (+1-1) 
- (modified) llvm/docs/LangRef.md (+24) 
- (modified) llvm/lib/CodeGen/StackProtector.cpp (+113-9) 
- (modified) llvm/lib/IR/Attributes.cpp (+9) 
- (added) llvm/test/CodeGen/X86/stack-protector-gs-buffer.ll (+185) 
- (added) llvm/test/Transforms/Inline/inline-ssp-gs-buffer.ll (+46) 


``````````diff
diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md
index e5da258b9950a..24b8ee4f030aa 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 d7637f2dfd507..0e7fc5a30b5c0 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 7539e000d03f9..2b80946009070 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 d4bf48040029f..4494a0ca8b15e 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 70445dd65ea2b..496efdb731fc0 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 9e3f073c20f4a..02b5d5d96bdc0 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 6636a5fd6e655..3511b5630d59b 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 0000000000000..aa536f1108932
--- /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 57d82622ef4de..4f9caa9d022e1 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 b241ec6672d85..4c6d4aa9756cb 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 a4816d337e02b..40d9183279a2b 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 9ec1d0630952d..06158545e1215 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...
[truncated]

``````````

</details>


https://github.com/llvm/llvm-project/pull/225626
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to