[clang] [llvm] [clang-cl] Use MSVC's "GS buffer" heuristic for the default /GS (PR #225626)
Pan Tao via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 16 02:27:01 PDT 2026
https://github.com/PanTao2 updated https://github.com/llvm/llvm-project/pull/225626
>From d4ab2cc089c055101eec0b7bb5b069b464633b46 Mon Sep 17 00:00:00 2001
From: "Pan, Tao" <tao.pan at intel.com>
Date: Wed, 23 Sep 2026 14:52:49 +0800
Subject: [PATCH 1/8] [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" }
>From 3d8d0e8ba5e48dd81ce24ab4c4631530d31b01e7 Mon Sep 17 00:00:00 2001
From: "Pan, Tao" <tao.pan at intel.com>
Date: Thu, 8 Oct 2026 10:09:30 +0800
Subject: [PATCH 2/8] Add "stack-protector" metadata on the alloca
---
clang/docs/ReleaseNotes.md | 21 +-
clang/lib/CodeGen/CGBuiltin.cpp | 17 +
clang/lib/CodeGen/CGCall.cpp | 8 +-
clang/lib/CodeGen/CGDecl.cpp | 338 +++++++++++++++++-
clang/lib/CodeGen/CGExpr.cpp | 20 +-
clang/lib/CodeGen/CodeGenFunction.cpp | 5 +
clang/lib/CodeGen/CodeGenFunction.h | 54 ++-
llvm/docs/LangRef.md | 48 ++-
llvm/lib/CodeGen/StackProtector.cpp | 225 +++++++-----
.../InstCombineLoadStoreAlloca.cpp | 3 +
.../CodeGen/X86/stack-protector-gs-buffer.ll | 206 +++++++----
llvm/test/Transforms/InstCombine/alloca.ll | 18 +
12 files changed, 760 insertions(+), 203 deletions(-)
diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md
index 24b8ee4f030aaa..0ebeea4af28e73 100644
--- a/clang/docs/ReleaseNotes.md
+++ b/clang/docs/ReleaseNotes.md
@@ -122,19 +122,6 @@ 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.
@@ -855,6 +842,14 @@ features cannot lower the translation-unit ABI level;
subsequent base; Clang now does the same.
([#210174](https://github.com/llvm/llvm-project/issues/210174))
+- `clang-cl`'s `/GS` now uses MSVC's buffer heuristic instead of
+ `-fstack-protector-strong`. MSVC decides from the declared type of each stack
+ object, so it protects functions Clang previously left alone — an array of
+ pointers, or a struct whose pointer-free member array is only two bytes wide —
+ and leaves alone functions Clang previously protected, such as one whose only
+ address-taken local is a scalar. `/GS-`, and the GCC-style
+ `-fstack-protector*` options when passed through `/clang:`, are unaffected.
+
#### LoongArch Support
- `loongarch32-*-none-elf` and `loongarch64-*-none-elf` targets now use the
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index f5001625327777..5d83675a111277 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -177,6 +177,21 @@ static void initializeAlloca(CodeGenFunction &CGF, AllocaInst *AI, Value *Size,
I->addAnnotationMetadata("auto-init");
}
+/// MSVC's /GS treats a buffer allocated by _alloca as a GS buffer whatever its
+/// size, so mark \p AI as one. The element count that would let the backend
+/// recognise it on its own does not survive InstCombine folding a
+/// constant-sized alloca to an array type, so record it here.
+static void markAllocaAsGSBuffer(CodeGenFunction &CGF, AllocaInst *AI,
+ Value *Size) {
+ unsigned SSPBufferSize = CGF.CGM.getCodeGenOpts().SSPBufferSize;
+ const auto *CI = dyn_cast<llvm::ConstantInt>(Size);
+ // Lay it out as the backend would if the element count were still visible:
+ // a variable size, or one of at least ssp-buffer-size bytes, goes closest to
+ // the stack guard.
+ CGF.MarkGSBufferAlloca(AI, !CI || CI->getLimitedValue(SSPBufferSize) >=
+ SSPBufferSize);
+}
+
/// getBuiltinLibFunction - Given a builtin id for a function like
/// "__builtin_fabsf", return a Function* for "fabsf".
llvm::Constant *CodeGenModule::getBuiltinLibFunction(const FunctionDecl *FD,
@@ -4813,6 +4828,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
.getAsAlign();
AllocaInst *AI = Builder.CreateAlloca(Builder.getInt8Ty(), Size);
AI->setAlignment(SuitableAlignmentInBytes);
+ markAllocaAsGSBuffer(*this, AI, Size);
if (BuiltinID != Builtin::BI__builtin_alloca_uninitialized)
initializeAlloca(*this, AI, Size, SuitableAlignmentInBytes);
if (AI->getAddressSpace() !=
@@ -4834,6 +4850,7 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
CGM.getContext().toCharUnitsFromBits(AlignmentInBits).getAsAlign();
AllocaInst *AI = Builder.CreateAlloca(Builder.getInt8Ty(), Size);
AI->setAlignment(AlignmentInBytes);
+ markAllocaAsGSBuffer(*this, AI, Size);
if (BuiltinID != Builtin::BI__builtin_alloca_with_align_uninitialized)
initializeAlloca(*this, AI, Size, AlignmentInBytes);
if (AI->getAddressSpace() !=
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 2193a89203f293..4acbbdeef9cbcf 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -5293,9 +5293,11 @@ void CodeGenFunction::EmitCallArg(CallArgList &args, const Expr *E,
type->castAsRecordDecl()->isParamDestroyedInCallee()) {
// If we're using inalloca, use the argument memory. Otherwise, use a
// temporary.
- AggValueSlot Slot = args.isUsingInAlloca()
- ? createPlaceholderSlot(*this, type)
- : CreateAggTemp(type, "agg.tmp");
+ AggValueSlot Slot =
+ args.isUsingInAlloca()
+ ? createPlaceholderSlot(*this, type)
+ : CreateAggTemp(type, "agg.tmp", /*Alloca=*/nullptr,
+ GSObjectKind::ArgumentCopy);
bool DestroyedInCallee = true, NeedsCleanup = true;
if (const auto *RD = type->getAsCXXRecordDecl())
diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index e1ed66ae71243e..2eed9fd2cee751 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -1483,6 +1483,326 @@ static bool shouldExtendLifetime(const ASTContext &Context,
return true;
}
+/// An array is only an MSVC "GS buffer" if it is strictly larger than this.
+static constexpr int64_t MinGSArraySize = 4;
+
+/// A pointer-free aggregate is only an MSVC "GS buffer" if it is strictly
+/// larger than this.
+static constexpr int64_t MinGSAggregateSize = 8;
+
+/// Returns true if \p Ty is a pointer as far as MSVC's GS-buffer rules are
+/// concerned, i.e. something that holds an address rather than data.
+static bool isGSPointerType(QualType Ty) {
+ return Ty->isAnyPointerType() || Ty->isReferenceType() ||
+ Ty->isBlockPointerType() || Ty->isMemberFunctionPointerType() ||
+ Ty->isNullPtrType();
+}
+
+/// 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.
+///
+/// A pointer a class inherits does not count, at any depth: cl.exe sees
+/// `struct B { void *p; }; struct D : B { long long a; };` as a GS buffer, and
+/// likewise an aggregate with a `D` member, while the same shapes built out of
+/// `B` members are not. The base classes get their own chance to make the
+/// derived class a buffer, in isGSBuffer.
+static bool containsGSPointer(const ASTContext &Ctx, QualType Ty,
+ llvm::SmallPtrSetImpl<const RecordDecl *> &Seen) {
+ if (isGSPointerType(Ty))
+ return true;
+ if (const ArrayType *AT = Ctx.getAsArrayType(Ty))
+ return containsGSPointer(Ctx, AT->getElementType(), Seen);
+ if (const auto *AT = Ty->getAs<AtomicType>())
+ return containsGSPointer(Ctx, AT->getValueType(), Seen);
+
+ const RecordDecl *RD = Ty->getAsRecordDecl();
+ if (!RD || !RD->isCompleteDefinition() || !Seen.insert(RD).second)
+ return false;
+
+ // A polymorphic class stores a vptr, and a virtual base is reached through a
+ // pointer, neither of which shows up as a field. Both are properties of the
+ // class itself rather than of a base, even when the vptr physically lives in
+ // a base subobject.
+ if (const auto *CXXRD = dyn_cast<CXXRecordDecl>(RD))
+ if (CXXRD->isDynamicClass() || CXXRD->getNumVBases() != 0)
+ return true;
+
+ return llvm::any_of(RD->fields(), [&](const FieldDecl *FD) {
+ return containsGSPointer(Ctx, FD->getType(), Seen);
+ });
+}
+
+static bool containsGSPointer(const ASTContext &Ctx, QualType Ty) {
+ llvm::SmallPtrSet<const RecordDecl *, 4> Seen;
+ return containsGSPointer(Ctx, Ty, Seen);
+}
+
+/// Returns true if \p RD is laid out as plain, publicly accessible data, which
+/// is what MSVC's GS-buffer rules require of an aggregate before its size alone
+/// makes it a buffer. MSDN does not mention this condition, but cl.exe applies
+/// it: `struct S { long long a, b; };` is a GS buffer while the otherwise
+/// identical `class C { long long a, b; };` is not.
+///
+/// No standard type trait captures this; __is_pod, __is_trivial,
+/// __is_standard_layout and __is_aggregate each disagree with cl.exe on at
+/// least one case, so it is computed here.
+static bool isGSPlainData(const ASTContext &Ctx, const RecordDecl *RD,
+ llvm::DenseMap<const RecordDecl *, bool> &Cache) {
+ if (!RD || !RD->isCompleteDefinition())
+ return false;
+
+ auto [It, Inserted] = Cache.try_emplace(RD, true);
+ if (!Inserted)
+ return It->second;
+
+ auto Reject = [&] {
+ Cache[RD] = false;
+ return false;
+ };
+
+ if (const auto *CXXRD = dyn_cast<CXXRecordDecl>(RD)) {
+ // Anything that needs a constructor, a destructor or a non-trivial copy is
+ // not a raw data buffer as far as cl.exe is concerned. This covers the base
+ // classes too, since a class with a non-trivial base is not trivial.
+ if (!CXXRD->isTrivial() || CXXRD->isDynamicClass() ||
+ CXXRD->getNumVBases() != 0)
+ return Reject();
+ }
+
+ // Access control only exists in C++; a C struct has no AS_public on its
+ // fields to find. Only the class's own fields are checked: a base class is
+ // not, and neither is the access specifier it is inherited with, so
+ // `struct D : private B {}` is plain data even when `B` is not.
+ bool CheckAccess = isa<CXXRecordDecl>(RD);
+ for (const FieldDecl *FD : RD->fields()) {
+ if (CheckAccess && FD->getAccess() != AS_public)
+ return Reject();
+ // A member that is too small to be a buffer in its own right is just
+ // bytes, so cl.exe does not care whether it is plain data. A larger one
+ // disqualifies the enclosing aggregate. An array member is exempt: cl.exe
+ // treats `struct { Opaque m[1]; }` as plain data but `struct { Opaque m; }`
+ // not.
+ QualType FieldTy = FD->getType();
+ const RecordDecl *FieldRD = FieldTy->getAsRecordDecl();
+ if (FieldRD && !FieldTy->isArrayType() &&
+ Ctx.getTypeSizeInChars(FieldTy).getQuantity() > MinGSAggregateSize &&
+ !isGSPlainData(Ctx, FieldRD, Cache))
+ return Reject();
+ }
+ return true;
+}
+
+static bool isGSPlainData(const ASTContext &Ctx, const RecordDecl *RD) {
+ llvm::DenseMap<const RecordDecl *, bool> Cache;
+ return isGSPlainData(Ctx, RD, Cache);
+}
+
+/// Where in a type a candidate GS buffer sits. cl.exe applies a different array
+/// rule below the top level, dropping the element-count condition and adding
+/// one on the element type, so `struct { int m[1]; }` is a GS buffer while
+/// `struct { void *m[4]; }` is not -- the other way round from how the two
+/// types fare as locals.
+enum class GSTypePosition {
+ /// The type of the stack object itself.
+ TopLevel,
+ /// The type of a non-static data member, or of a nested array's elements.
+ Nested,
+};
+
+/// Returns true if \p Ty is an array of more than two elements, counting a
+/// multidimensional array as the flat array it is laid out as: cl.exe sees
+/// `void *a[2][2]` as four elements, not two.
+static bool hasMoreThanTwoGSElements(const ASTContext &Ctx, QualType Ty) {
+ uint64_t N = 1;
+ while (const ConstantArrayType *CAT = Ctx.getAsConstantArrayType(Ty)) {
+ // N is at most two here, or the previous round would have returned, so
+ // this cannot overflow.
+ N *= CAT->getSize().getLimitedValue(3);
+ if (N > 2)
+ return true;
+ Ty = CAT->getElementType();
+ }
+ return false;
+}
+
+/// Returns true if \p Ty is, or contains, a "GS buffer" as MSVC's /GS (Buffer
+/// Security Check) defines one. MSDN describes it as:
+///
+/// - 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.
+///
+/// cl.exe differs from that description in three ways, each confirmed against
+/// cl.exe 19.44 and each reproduced here:
+///
+/// - the two array conditions do not both apply in both positions. For the
+/// type of the stack object itself only the element count matters, so
+/// `void *a[8]` is a GS buffer; for the type of a member only the element
+/// type matters, so `struct { int m[1]; }` is one but
+/// `struct { void *m[4]; }` is not (see GSTypePosition);
+/// - the element count of a multidimensional array is the flattened one, so
+/// `void *a[2][2]` is a GS buffer;
+/// - the size rule for an aggregate additionally requires it to be plain
+/// data (see isGSPlainData).
+///
+/// (A buffer allocated by _alloca is also a GS buffer, but that one is visible
+/// in the IR and so is left to the backend.)
+///
+/// \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 isGSBuffer(const ASTContext &Ctx, QualType Ty,
+ unsigned SSPBufferSize, bool &IsLarge,
+ GSTypePosition Position = GSTypePosition::TopLevel) {
+ if (Ty.isNull() || Ty->isDependentType() || Ty->isIncompleteType())
+ return false;
+
+ auto Found = [&](QualType BufferTy) {
+ if (Ctx.getTypeSizeInChars(BufferTy).getQuantity() >=
+ static_cast<int64_t>(SSPBufferSize))
+ IsLarge = true;
+ return true;
+ };
+
+ if (const ConstantArrayType *CAT = Ctx.getAsConstantArrayType(Ty)) {
+ QualType ElemTy = CAT->getElementType();
+ if (Ctx.getTypeSizeInChars(Ty).getQuantity() > MinGSArraySize &&
+ (Position == GSTypePosition::TopLevel ? hasMoreThanTwoGSElements(Ctx, Ty)
+ : !isGSPointerType(ElemTy)))
+ return Found(Ty);
+
+ // 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 isGSBuffer(Ctx, ElemTy, SSPBufferSize, IsLarge,
+ GSTypePosition::Nested);
+ }
+
+ const RecordDecl *RD = Ty->getAsRecordDecl();
+ if (!RD || !RD->isCompleteDefinition())
+ return false;
+
+ if (Ctx.getTypeSizeInChars(Ty).getQuantity() > MinGSAggregateSize &&
+ !containsGSPointer(Ctx, Ty) && isGSPlainData(Ctx, RD))
+ return Found(Ty);
+
+ // If a member is a large GS buffer then we are done. Otherwise keep looking,
+ // in case a later member is a large one.
+ bool NeedsProtector = false;
+ auto Check = [&](QualType MemberTy) {
+ if (isGSBuffer(Ctx, MemberTy, SSPBufferSize, IsLarge,
+ GSTypePosition::Nested))
+ NeedsProtector = true;
+ return IsLarge;
+ };
+ if (const auto *CXXRD = dyn_cast<CXXRecordDecl>(RD))
+ for (const CXXBaseSpecifier &Base : CXXRD->bases())
+ if (Check(Base.getType()))
+ return true;
+ for (const FieldDecl *FD : RD->fields()) {
+ // cl.exe does not look inside an anonymous struct or union, so
+ // `struct { union { char b[16]; }; void *p; }` is not a GS buffer while
+ // the same thing with a named union member is. Its size still counts
+ // towards the enclosing aggregate, and a pointer in it still disqualifies
+ // the aggregate.
+ if (FD->isAnonymousStructOrUnion())
+ continue;
+ if (Check(FD->getType()))
+ return true;
+ }
+
+ return NeedsProtector;
+}
+
+void CodeGenFunction::MarkGSBufferAlloca(llvm::AllocaInst *AI, bool IsLarge,
+ bool IsTrivial) {
+ if (!MarkGSBuffers || !AI)
+ return;
+
+ // An explicit opt-out (see StackProtectorIgnoreAttr) has the last word.
+ if (AI->getMetadata("stack-protector"))
+ return;
+
+ // 2 is a large buffer and 1 a small one. The second operand says the object is
+ // trivial, and is left out when it is not, which is the common case; see the
+ // "stack-protector" metadata in LLVM's LangRef.
+ llvm::LLVMContext &Ctx = Builder.getContext();
+ SmallVector<llvm::Metadata *, 2> Ops = {
+ llvm::ConstantAsMetadata::get(Builder.getInt32(IsLarge ? 2 : 1))};
+ if (IsTrivial)
+ Ops.push_back(llvm::ConstantAsMetadata::get(Builder.getTrue()));
+ AI->setMetadata("stack-protector", llvm::MDNode::get(Ctx, Ops));
+}
+
+void CodeGenFunction::EmitGSBufferStackProtectorMD(llvm::Value *AllocaPtr,
+ QualType Ty,
+ GSObjectKind Kind) {
+ if (!MarkGSBuffers)
+ return;
+
+ auto *AI = dyn_cast_or_null<llvm::AllocaInst>(AllocaPtr);
+ if (!AI)
+ return;
+
+ // An ABI slot holding a trivial object is anonymous storage, which cl.exe
+ // does not analyse; it protects the callee instead (see
+ // EmitGSBufferParamCopy). Once the type needs a constructor or a destructor
+ // the temporary becomes a real object, and cl.exe covers it.
+ bool IsTrivial = Ty.isTrivialType(getContext());
+ if (Kind != GSObjectKind::Named && IsTrivial)
+ return;
+
+ // For an outgoing by-value argument that is a real object, cl.exe only gets
+ // to put it inside the guarded region if it can then copy it into the
+ // argument area bitwise. Otherwise it has to construct it there directly,
+ // where the guard does not reach. The destructor plays no part: an object
+ // with a trivial copy constructor and a user-provided destructor is copied,
+ // and the destructor runs on the original.
+ if (Kind == GSObjectKind::ArgumentCopy) {
+ const auto *RD = Ty->getAsCXXRecordDecl();
+ if (RD && !(RD->hasSimpleCopyConstructor() &&
+ RD->hasTrivialCopyConstructor()))
+ return;
+ }
+
+ bool IsLarge = false;
+ if (!isGSBuffer(getContext(), Ty, CGM.getCodeGenOpts().SSPBufferSize,
+ IsLarge))
+ return;
+
+ MarkGSBufferAlloca(AI, IsLarge, IsTrivial);
+}
+
+Address CodeGenFunction::EmitGSBufferParamCopy(const VarDecl &D,
+ Address ParamAddr) {
+ QualType Ty = D.getType();
+ bool IsLarge = false;
+ if (!MarkGSBuffers ||
+ !isGSBuffer(getContext(), Ty, CGM.getCodeGenOpts().SSPBufferSize,
+ IsLarge))
+ return ParamAddr;
+
+ // Only copy what can be copied by memcpy and needs no destructor, so that
+ // the copy is indistinguishable from the original. cl.exe likewise leaves
+ // parameters of non-trivial type in the caller's storage.
+ if (!Ty.isTriviallyCopyableType(getContext()) || Ty.isDestructedType())
+ return ParamAddr;
+
+ RawAddress GSCopy = CreateMemTempWithoutCast(
+ Ty, getContext().getDeclAlign(&D), D.getName() + "$GSCopy$");
+ EmitAggregateCopy(MakeAddrLValue(GSCopy, Ty), MakeAddrLValue(ParamAddr, Ty),
+ Ty, AggValueSlot::DoesNotOverlap);
+ // This copy exists only so that the object sits inside the guarded region,
+ // so it is never a candidate for the unguarded temporary area.
+ MarkGSBufferAlloca(
+ dyn_cast<llvm::AllocaInst>(GSCopy.getPointer()->stripPointerCasts()),
+ IsLarge, /*IsTrivial=*/false);
+ return GSCopy;
+}
+
/// EmitAutoVarAlloca - Emit the alloca and debug information for a
/// local variable. Does not emit initialization or destruction.
CodeGenFunction::AutoVarEmission
@@ -1720,6 +2040,8 @@ CodeGenFunction::EmitAutoVarAlloca(const VarDecl &D) {
EmitAndRegisterVariableArrayDimensions(DI, D, EmitDebugInfo);
}
+ EmitGSBufferStackProtectorMD(address.getBasePointer(), Ty);
+
setAddrOfLocalVar(&D, address);
emission.Addr = address;
emission.AllocaAddr = AllocaAddr;
@@ -2721,12 +3043,23 @@ void CodeGenFunction::EmitParmDecl(const VarDecl &D, ParamValue Arg,
if (Arg.isIndirect()) {
DeclPtr = Arg.getIndirectAddress();
DeclPtr = DeclPtr.withElementType(ConvertTypeForMem(Ty));
+ ABIArgInfo ArgInfo = CurFnInfo->arguments()[ArgNo - 1].info;
+
+ // An indirectly passed parameter lives in storage the caller allocated, so
+ // this frame's stack guard does not cover it. When it is a GS buffer,
+ // relocate it the way cl.exe does. Only `byval` and plain `indirect` own
+ // their storage; for `byref` the callee's writes are visible to the caller,
+ // so a copy would change behaviour.
+ if (MarkGSBuffers && !CurFuncIsThunk && isa<ParmVarDecl>(&D) &&
+ ArgInfo.isIndirect() &&
+ CurFnInfo->getExtParameterInfo(ArgNo - 1).getABI() ==
+ ParameterABI::Ordinary)
+ DeclPtr = EmitGSBufferParamCopy(D, DeclPtr);
auto *V = DeclPtr.emitRawPointer(*this);
AllocaPtr = RawAddress(V, DeclPtr.getElementType(), DeclPtr.getAlignment());
// For truly ABI indirect arguments -- those that are not `byval` -- store
// the address of the argument on the stack to preserve debug information.
- ABIArgInfo ArgInfo = CurFnInfo->arguments()[ArgNo - 1].info;
if (ArgInfo.isIndirect())
UseIndirectDebugAddress = !ArgInfo.getIndirectByVal();
if (UseIndirectDebugAddress) {
@@ -2773,6 +3106,9 @@ void CodeGenFunction::EmitParmDecl(const VarDecl &D, ParamValue Arg,
// Otherwise, create a casted temporary to hold the value.
DeclPtr = CreateMemTemp(Ty, getContext().getDeclAlign(&D),
D.getName() + ".addr", &AllocaPtr);
+ // This alloca holds a parameter, not an anonymous ABI slot, and it is on
+ // this frame, so cl.exe's GS analysis covers it.
+ EmitGSBufferStackProtectorMD(AllocaPtr.getPointer(), Ty);
}
DoStore = true;
}
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index aca0415d7f5449..76eae97ada2f0f 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -203,11 +203,15 @@ RawAddress CodeGenFunction::CreateMemTemp(QualType Ty, const Twine &Name,
RawAddress CodeGenFunction::CreateMemTemp(QualType Ty, CharUnits Align,
const Twine &Name,
- RawAddress *Alloca) {
+ RawAddress *Alloca,
+ GSObjectKind GSKind) {
RawAddress Result =
CreateTempAlloca(ConvertTypeForMem(Ty), Ty.getAddressSpace(), Align, Name,
/*ArraySize=*/nullptr, Alloca);
+ EmitGSBufferStackProtectorMD(
+ Alloca ? Alloca->getPointer() : Result.getPointer(), Ty, GSKind);
+
if (Ty->isConstantMatrixType()) {
auto *ArrayTy = cast<llvm::ArrayType>(Result.getElementType());
auto *ArrayElementTy = ArrayTy->getElementType();
@@ -228,7 +232,11 @@ RawAddress CodeGenFunction::CreateMemTemp(QualType Ty, CharUnits Align,
RawAddress CodeGenFunction::CreateMemTempWithoutCast(QualType Ty,
CharUnits Align,
const Twine &Name) {
- return CreateTempAllocaWithoutCast(ConvertTypeForMem(Ty), Align, Name);
+ RawAddress Result =
+ CreateTempAllocaWithoutCast(ConvertTypeForMem(Ty), Align, Name);
+ EmitGSBufferStackProtectorMD(Result.getPointer(), Ty,
+ GSObjectKind::Temporary);
+ return Result;
}
RawAddress CodeGenFunction::CreateMemTempWithoutCast(QualType Ty,
@@ -504,7 +512,13 @@ static RawAddress createReferenceTemporary(CodeGenFunction &CGF,
// FIXME: Should we put the new global into a COMDAT?
return RawAddress(C, GV->getValueType(), alignment);
}
- return CGF.CreateMemTemp(Ty, "ref.tmp", Alloca);
+ RawAddress Object = CGF.CreateMemTemp(Ty, "ref.tmp", Alloca);
+ // A temporary bound to a reference outlives the full-expression and has an
+ // address the program can pass around, so cl.exe treats it as a real local
+ // object rather than an anonymous ABI slot.
+ CGF.EmitGSBufferStackProtectorMD(
+ Alloca ? Alloca->getPointer() : Object.getPointer(), Ty);
+ return Object;
}
case SD_Thread:
case SD_Static:
diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp
index 745423945ca838..81685f8c3c8838 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -756,6 +756,11 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, QualType RetTy,
if (FD && FD->usesSEHTry())
CurSEHParent = GD;
CurFuncDecl = (D ? D->getNonClosureContext() : nullptr);
+ // Under MSVC's /GS heuristic the frontend decides which allocas are buffers,
+ // so mark them as they are created. Only worth doing if the function ends up
+ // with a stack protector attribute at all.
+ MarkGSBuffers = CGM.useMSVCGSBufferHeuristic(D) &&
+ CGM.StackProtectorAttribute(D).has_value();
FnRetTy = RetTy;
CurFn = Fn;
CurFnInfo = &FnInfo;
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 653e883012229b..404296cb9c262c 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -1572,6 +1572,11 @@ class CodeGenFunction : public CodeGenTypeCache {
/// calling llvm.stacksave for multiple VLAs in the same scope.
bool DidCallStackSave = false;
+ /// Whether this function is compiled with MSVC's /GS heuristic, so that its
+ /// allocas have to be classified here rather than in the backend. See
+ /// EmitGSBufferStackProtectorMD.
+ bool MarkGSBuffers = false;
+
/// IndirectBranch - The first time an indirect goto is seen we create a block
/// with an indirect branch. Every time we see the address of a label taken,
/// we add the label to the indirect goto. Every subsequent indirect goto is
@@ -2992,6 +2997,23 @@ class CodeGenFunction : public CodeGenTypeCache {
/// ConvertType instead of ConvertTypeForMem.
RawAddress CreateIRTempWithoutCast(QualType T, const Twine &Name = "tmp");
+ /// What kind of stack object an alloca holds, for MSVC's /GS heuristic.
+ enum class GSObjectKind {
+ /// An object the source declares: a local variable, a by-value parameter,
+ /// or a temporary bound to a reference. Always classified.
+ Named,
+ /// A slot clang materialises to satisfy the ABI, such as an sret return
+ /// slot. cl.exe keeps these out of its GS analysis unless the type needs a
+ /// constructor or destructor to run, in which case it becomes a real local
+ /// object.
+ Temporary,
+ /// The copy of an outgoing by-value argument. Like Temporary, except that
+ /// cl.exe also leaves it out when the object cannot be copied bitwise:
+ /// then it has to be constructed in the outgoing argument area directly,
+ /// which lies outside the guarded region.
+ ArgumentCopy,
+ };
+
/// CreateMemTemp - Create a temporary memory object of the given type, with
/// appropriate alignmen and cast it to the default address space. Returns
/// the original alloca instruction by \p Alloca if it is not nullptr.
@@ -2999,7 +3021,8 @@ class CodeGenFunction : public CodeGenTypeCache {
RawAddress *Alloca = nullptr);
RawAddress CreateMemTemp(QualType T, CharUnits Align,
const Twine &Name = "tmp",
- RawAddress *Alloca = nullptr);
+ RawAddress *Alloca = nullptr,
+ GSObjectKind GSKind = GSObjectKind::Temporary);
/// CreateMemTemp - Create a temporary memory object of the given type, with
/// appropriate alignmen without casting it to the default address space.
@@ -3010,8 +3033,10 @@ class CodeGenFunction : public CodeGenTypeCache {
/// CreateAggTemp - Create a temporary memory object for the given
/// aggregate type.
AggValueSlot CreateAggTemp(QualType T, const Twine &Name = "tmp",
- RawAddress *Alloca = nullptr) {
- RawAddress Addr = CreateMemTemp(T, Name, Alloca);
+ RawAddress *Alloca = nullptr,
+ GSObjectKind GSKind = GSObjectKind::Temporary) {
+ RawAddress Addr = CreateMemTemp(T, getContext().getTypeAlignInChars(T),
+ Name, Alloca, GSKind);
return AggValueSlot::forAddr(
Addr, T.getQualifiers(), AggValueSlot::IsNotDestructed,
AggValueSlot::DoesNotNeedGCBarriers, AggValueSlot::IsNotAliased,
@@ -3587,6 +3612,29 @@ class CodeGenFunction : public CodeGenTypeCache {
}
};
AutoVarEmission EmitAutoVarAlloca(const VarDecl &var);
+
+ /// If this function uses MSVC's /GS (Buffer Security Check) heuristic,
+ /// attach "stack-protector" metadata to \p AllocaPtr when \p Ty is a "GS
+ /// buffer". Whether it is depends on source-level type information that
+ /// lowering to IR does not preserve, so the backend cannot work it out.
+ void EmitGSBufferStackProtectorMD(llvm::Value *AllocaPtr, QualType Ty,
+ GSObjectKind Kind = GSObjectKind::Named);
+
+ /// If this function uses MSVC's /GS (Buffer Security Check) heuristic and
+ /// \p Ty is a "GS buffer", copy the by-value parameter at \p ParamAddr into
+ /// a fresh local so that it is covered by this frame's stack guard, and
+ /// return the copy. cl.exe does the same, naming the copy "<param>$GSCopy$".
+ Address EmitGSBufferParamCopy(const VarDecl &D, Address ParamAddr);
+
+ /// If this function uses MSVC's /GS (Buffer Security Check) heuristic, mark
+ /// \p AI as a "GS buffer", so that the function requires a stack protector.
+ /// \p IsLarge says the buffer should be laid out closest to the guard.
+ /// \p IsTrivial says the allocation holds an object of trivial type, which
+ /// cl.exe is willing to leave in the unguarded temporary area; see the
+ /// "stack-protector" metadata in LLVM's LangRef.
+ void MarkGSBufferAlloca(llvm::AllocaInst *AI, bool IsLarge,
+ bool IsTrivial = false);
+
void EmitAutoVarInit(const AutoVarEmission &emission);
void EmitAutoVarCleanups(const AutoVarEmission &emission);
void emitAutoVarTypeCleanup(const AutoVarEmission &emission,
diff --git a/llvm/docs/LangRef.md b/llvm/docs/LangRef.md
index 40d9183279a2b5..99bc6e1c492487 100644
--- a/llvm/docs/LangRef.md
+++ b/llvm/docs/LangRef.md
@@ -2843,17 +2843,16 @@ fn -> other_fn -> other_fn ; fn is norecurse
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.
+ argument list and it allocates a "GS buffer", which is either an `alloca`
+ with a non-constant or non-one element count, or an alloca that the
+ frontend marked with non-zero `stack-protector` metadata. Whether a given
+ variable is a GS buffer depends on its source-level type, which lowering
+ to IR does not preserve, so classifying the fixed-size allocations is the
+ frontend's job.
Unlike `sspstrong`, a local variable merely having its address taken does
- not cause a function to be protected. Stack layout rules are unchanged.
+ not cause a function to be protected, and no alloca is classified from its
+ IR type. 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
@@ -9211,18 +9210,37 @@ call ptr @malloc(i64 64), !alloc_token !0
#### '`stack-protector`' Metadata
-The `stack-protector` metadata may be attached to alloca instructions. An
-alloca instruction with this metadata and value `i32 0` will be skipped when
-deciding whether a given function requires a stack protector. The function
-may still use a stack protector, if other criteria determine it needs one.
+The `stack-protector` metadata may be attached to alloca instructions. It
+overrides, for that one allocation, the heuristic that the `ssp`, `sspstrong`
+and `"stack-protector-gs-buffer"` function attributes would otherwise apply.
+The function may still use a stack protector, if other criteria determine it
+needs one.
+
+The metadata contains an integer, and optionally a second, boolean operand:
+
+- `0` opts the given alloca out of requiring a stack protector.
+- `1` marks the given alloca as a buffer that requires a stack protector, and
+ that is laid out like a small (`< ssp-buffer-size`) array.
+- `2` marks the given alloca as a buffer that requires a stack protector, and
+ that is laid out like a large (`>= ssp-buffer-size`) array.
+
+The non-zero values let a frontend classify an allocation itself, which is
+necessary when the decision depends on source-level information that lowering
+to IR does not preserve.
-The metadata contains an integer, where a 0 value opts the given alloca out
-of requiring a stack protector.
+A second operand of `i1 true` says the allocation holds an object of trivial
+type, one the backend is therefore free to relocate. Only the
+`"stack-protector-gs-buffer"` heuristic reads it, and only to leave out an
+allocation whose address is used for nothing but receiving the indirect return
+value of a call: MSVC gives such an object a slot outside the region the stack
+cookie guards. The operand may be omitted, and is then taken as `false`.
```
%a = alloca [1000 x i8], align 1, !stack-protector !0
+ %b = alloca [1000 x i8], align 1, !stack-protector !1
!0 = !{i32 0}
+!1 = !{i32 2, i1 true}
```
#### '`implicit.ref`' Metadata
diff --git a/llvm/lib/CodeGen/StackProtector.cpp b/llvm/lib/CodeGen/StackProtector.cpp
index 06158545e12150..05e6fa85fdd0ef 100644
--- a/llvm/lib/CodeGen/StackProtector.cpp
+++ b/llvm/lib/CodeGen/StackProtector.cpp
@@ -67,12 +67,100 @@ static cl::opt<bool> DisableCheckNoReturn("disable-check-noreturn-call",
/// 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;
+/// Values of the "stack-protector" metadata that may be attached to an alloca
+/// to override the heuristic for that allocation. See LangRef.
+enum SSPAllocaMD : uint32_t {
+ /// Ignore this alloca when deciding whether the function needs a protector.
+ SSPAMD_Ignore = 0,
+ /// This alloca is a buffer that requires a protector, and is laid out like a
+ /// small (< ssp-buffer-size) array.
+ SSPAMD_SmallBuffer = 1,
+ /// This alloca is a buffer that requires a protector, and is laid out like a
+ /// large (>= ssp-buffer-size) array.
+ SSPAMD_LargeBuffer = 2,
+};
+
+/// Read the "stack-protector" metadata of \p AI, if it has any. \p IsTrivial is
+/// set from the optional second operand, which says the allocation holds an
+/// object of trivial type.
+static std::optional<uint32_t> getSSPAllocaMD(const AllocaInst &AI,
+ bool &IsTrivial) {
+ IsTrivial = false;
+ const MDNode *MD = AI.getMetadata("stack-protector");
+ if (!MD || MD->getNumOperands() < 1 || MD->getNumOperands() > 2)
+ return std::nullopt;
+ const auto *CI = mdconst::dyn_extract<ConstantInt>(MD->getOperand(0));
+ if (!CI)
+ return std::nullopt;
+ if (MD->getNumOperands() == 2)
+ if (const auto *Flag = mdconst::dyn_extract<ConstantInt>(MD->getOperand(1)))
+ IsTrivial = !Flag->isZero();
+ // Clamp, so that a value from a newer producer errs towards more protection
+ // rather than less.
+ return CI->getLimitedValue(SSPAMD_LargeBuffer);
+}
-/// A pointer-free aggregate is only an MSVC "GS buffer" if it is strictly
-/// larger than this.
-static constexpr uint64_t MinGSAggregateSize = 8;
+/// Returns true if the only thing \p AI's address is ever used for, besides
+/// being read and written here, is receiving the indirect return value of a
+/// call.
+///
+/// cl.exe gives such an allocation a slot in the frame's temporary area, which
+/// the cookie does not cover, and leaves it out of the GS analysis: the only
+/// code that writes it from outside is a callee, and a callee writes exactly one
+/// object of the type it returns. As soon as the address reaches anywhere else,
+/// cl.exe relocates the object into the protected region instead, so look for
+/// that here rather than in the frontend, which cannot see the uses.
+///
+/// Only an object cl.exe is free to relocate can end up in that area, which is
+/// why the caller also requires the allocation to hold a trivial object.
+static bool isIndirectReturnSlot(const AllocaInst &AI) {
+ bool FoundSRet = false;
+ SmallVector<const Instruction *, 8> Worklist = {&AI};
+ SmallPtrSet<const Instruction *, 8> Visited = {&AI};
+
+ while (!Worklist.empty()) {
+ for (const Use &U : Worklist.pop_back_val()->uses()) {
+ const auto *User = cast<Instruction>(U.getUser());
+
+ if (const auto *CB = dyn_cast<CallBase>(User)) {
+ const auto *II = dyn_cast<IntrinsicInst>(CB);
+ if (II && (II->isLifetimeStartOrEnd() || II->isAssumeLikeIntrinsic()))
+ continue;
+ // A memory intrinsic reads or writes through the pointer, just as a
+ // load or a store does, and does not let it escape.
+ if (isa_and_nonnull<MemIntrinsic>(II))
+ continue;
+ if (!CB->isArgOperand(&U) ||
+ !CB->paramHasAttr(CB->getArgOperandNo(&U), Attribute::StructRet))
+ return false;
+ FoundSRet = true;
+ continue;
+ }
+
+ switch (User->getOpcode()) {
+ case Instruction::Load:
+ continue;
+ case Instruction::Store:
+ // Writing through the pointer is fine; writing the pointer itself
+ // somewhere else lets it escape.
+ if (U.getOperandNo() != StoreInst::getPointerOperandIndex())
+ return false;
+ continue;
+ case Instruction::BitCast:
+ case Instruction::AddrSpaceCast:
+ case Instruction::GetElementPtr:
+ case Instruction::PHI:
+ case Instruction::Select:
+ if (Visited.insert(User).second)
+ Worklist.push_back(User);
+ continue;
+ default:
+ return false;
+ }
+ }
+ }
+ return FoundSRet;
+}
/// InsertStackProtectors - Insert code into the prologue and epilogue of the
/// function.
@@ -286,75 +374,6 @@ 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.
@@ -496,9 +515,15 @@ static const CallInst *findStackProtectorIntrinsic(Function &F) {
/// size, and functions that contain stack-based variables that have had their
/// 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.
+/// and to functions with an alloca that the frontend marked as a "GS buffer"
+/// using "stack-protector" metadata, but it does not consider a variable's
+/// address being taken, and it never protects a function that takes a variable
+/// argument list. Deciding what a GS buffer is needs source-level type
+/// information that lowering to IR does not preserve, which is why that part
+/// of the heuristic lives in the frontend.
+///
+/// In every mode, "stack-protector" metadata on an alloca overrides the
+/// heuristic for that allocation.
bool SSPLayoutAnalysis::requiresStackProtector(Function *F,
SSPLayoutMap *Layout) {
Module *M = F->getParent();
@@ -552,11 +577,37 @@ bool SSPLayoutAnalysis::requiresStackProtector(Function *F,
for (const BasicBlock &BB : *F) {
for (const Instruction &I : BB) {
if (const AllocaInst *AI = dyn_cast<AllocaInst>(&I)) {
- if (const MDNode *MD = AI->getMetadata("stack-protector")) {
- const auto *CI = mdconst::dyn_extract<ConstantInt>(MD->getOperand(0));
- if (CI->isZero())
+ bool IsTrivial = false;
+ if (std::optional<uint32_t> MD = getSSPAllocaMD(*AI, IsTrivial)) {
+ if (*MD == SSPAMD_Ignore)
continue;
+
+ // MSVC leaves a trivial object that is only an indirect-return
+ // destination out of its GS analysis.
+ if (GSBuffer && IsTrivial && isIndirectReturnSlot(*AI))
+ continue;
+
+ // The frontend classified this allocation as a buffer, which is
+ // final: it knows things about the allocation that the IR no longer
+ // records. This is how clang-cl communicates MSVC's "GS buffer"
+ // analysis.
+ if (!Layout)
+ return true;
+ Layout->insert(
+ std::make_pair(AI, *MD == SSPAMD_LargeBuffer
+ ? MachineFrameInfo::SSPLK_LargeArray
+ : MachineFrameInfo::SSPLK_SmallArray));
+ ORE.emit([&]() {
+ return OptimizationRemark(DEBUG_TYPE, "StackProtectorBuffer", &I)
+ << "Stack protection applied to function "
+ << ore::NV("Function", F)
+ << " due to a stack allocated buffer or struct containing a "
+ "buffer";
+ });
+ NeedsProtector = true;
+ continue;
}
+
if (AI->isArrayAllocation()) {
auto RemarkBuilder = [&]() {
return OptimizationRemark(DEBUG_TYPE, "StackProtectorAllocaOrArray",
@@ -599,14 +650,14 @@ bool SSPLayoutAnalysis::requiresStackProtector(Function *F,
continue;
}
+ // Under MSVC's /GS the frontend has already picked out the buffers, so
+ // an unmarked alloca never requires a protector.
+ if (GSBuffer)
+ continue;
+
bool IsLarge = false;
- bool Protect =
- GSBuffer ? ContainsGSBuffer(AI->getAllocatedType(), M,
- SSPBufferSize, IsLarge)
- : ContainsProtectableArray(AI->getAllocatedType(), M,
- SSPBufferSize, IsLarge, Strong,
- /*InStruct=*/false);
- if (Protect) {
+ if (ContainsProtectableArray(AI->getAllocatedType(), M, SSPBufferSize,
+ IsLarge, Strong, /*InStruct=*/false)) {
if (!Layout)
return true;
Layout->insert(std::make_pair(
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
index 74322d599bd286..3d4090b6f0ae16 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -199,6 +199,9 @@ static Instruction *simplifyAllocaArraySize(InstCombinerImpl &IC,
nullptr, AI.getName());
New->setAlignment(AI.getAlign());
New->setUsedWithInAlloca(AI.isUsedWithInAlloca());
+ // It is the same allocation, so it keeps what was said about it, e.g.
+ // "stack-protector".
+ New->copyMetadata(AI);
replaceAllDbgUsesWith(AI, *New, *New, DT);
return IC.replaceInstUsesWith(AI, New);
diff --git a/llvm/test/CodeGen/X86/stack-protector-gs-buffer.ll b/llvm/test/CodeGen/X86/stack-protector-gs-buffer.ll
index a753e1b3bf65d7..c87a14b2186b26 100644
--- a/llvm/test/CodeGen/X86/stack-protector-gs-buffer.ll
+++ b/llvm/test/CodeGen/X86/stack-protector-gs-buffer.ll
@@ -1,167 +1,202 @@
; 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.
+; Under this heuristic the only allocas that require a protector are calls to
+; alloca and the ones the frontend marked with "stack-protector" metadata;
+; nothing is inferred from the alloca's type.
;
; RUN: llc -mtriple=x86_64-pc-windows-msvc < %s | FileCheck %s
declare void @use(ptr)
+declare void @make(ptr sret([64 x i8]))
+declare void @llvm.memcpy.p0.p0.i64(ptr, ptr, i64, i1)
-;; --- Arrays that are not GS buffers ------------------------------------------
+;; --- Unmarked allocas --------------------------------------------------------
-; Exactly 4 bytes, so not larger than 4.
-; CHECK-LABEL: array_4_bytes:
+; An obvious buffer is not protected unless the frontend says so, because the
+; IR type is not what the heuristic is defined in terms of.
+; CHECK-LABEL: unmarked_array:
; CHECK-NOT: __security_cookie
; CHECK: .seh_endproc
-define void @array_4_bytes() #0 {
- %a = alloca [4 x i8]
+define void @unmarked_array() #0 {
+ %a = alloca [64 x i8]
call void @use(ptr %a)
ret void
}
-; 8 bytes, but only two elements.
-; CHECK-LABEL: array_2_elements:
+; 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 @array_2_elements() #0 {
- %a = alloca [2 x i32]
+define void @address_taken() #0 {
+ %a = alloca 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]
+;; --- Marked allocas ----------------------------------------------------------
+
+; CHECK-LABEL: marked_small:
+; CHECK: __security_cookie
+define void @marked_small() #0 {
+ %a = alloca [6 x i8], !stack-protector !1
call void @use(ptr %a)
ret void
}
-;; --- Arrays that are GS buffers ----------------------------------------------
-
-; CHECK-LABEL: array_8_bytes:
+; CHECK-LABEL: marked_large:
; CHECK: __security_cookie
-define void @array_8_bytes() #0 {
- %a = alloca [8 x i8]
+define void @marked_large() #0 {
+ %a = alloca [64 x i8], !stack-protector !2
call void @use(ptr %a)
ret void
}
-; 6 bytes and three elements: over both thresholds.
-; CHECK-LABEL: array_3_shorts:
+; The mark is what matters, not the type: an i32 carrying it is protected.
+; CHECK-LABEL: marked_scalar:
; CHECK: __security_cookie
-define void @array_3_shorts() #0 {
- %a = alloca [3 x i16]
+define void @marked_scalar() #0 {
+ %a = alloca i32, !stack-protector !1
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]]
+; A zero mark opts out, as it does in every other mode.
+; CHECK-LABEL: marked_ignore:
+; CHECK-NOT: __security_cookie
+; CHECK: .seh_endproc
+define void @marked_ignore() #0 {
+ %a = alloca [64 x i8], !stack-protector !0
call void @use(ptr %a)
ret void
}
-;; --- Aggregates --------------------------------------------------------------
+;; --- alloca ------------------------------------------------------------------
-; 12 bytes with no pointers.
-; CHECK-LABEL: struct_pointer_free:
+; A call to alloca is a GS buffer, and is visible as such in the IR, so it
+; needs no mark.
+; CHECK-LABEL: dynamic_alloca:
; CHECK: __security_cookie
-define void @struct_pointer_free() #0 {
- %a = alloca { i32, i32, i32 }
+define void @dynamic_alloca(i64 %n) #0 {
+ %a = alloca i8, i64 %n
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:
+; 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
+}
+
+;; --- Indirect return slots ---------------------------------------------------
+
+; MSVC gives an object it is free to relocate and that only ever receives a
+; call's indirect return value a frame slot the cookie does not guard. The
+; frontend cannot see that, so the second metadata operand tells the backend the
+; object is trivial and the backend checks the uses.
+; CHECK-LABEL: sret_only:
; CHECK-NOT: __security_cookie
; CHECK: .seh_endproc
-define void @struct_with_pointer() #0 {
- %a = alloca { ptr, i32, i32 }
- call void @use(ptr %a)
+define void @sret_only() #0 {
+ %a = alloca [64 x i8], !stack-protector !3
+ call void @make(ptr sret([64 x i8]) %a)
+ %v = load i8, ptr %a
+ store i8 %v, ptr %a
ret void
}
-; Exactly 8 bytes, so not larger than 8.
-; CHECK-LABEL: struct_8_bytes:
+; Reading and writing through the pointer, including with a memcpy, is not an
+; escape, and several returns into the same slot are still just returns.
+; CHECK-LABEL: sret_twice_and_memcpy:
; CHECK-NOT: __security_cookie
; CHECK: .seh_endproc
-define void @struct_8_bytes() #0 {
- %a = alloca { i32, i32 }
- call void @use(ptr %a)
+define void @sret_twice_and_memcpy(ptr %p) #0 {
+ %a = alloca [64 x i8], !stack-protector !3
+ call void @make(ptr sret([64 x i8]) %a)
+ call void @make(ptr sret([64 x i8]) %a)
+ call void @llvm.memcpy.p0.p0.i64(ptr %p, ptr %a, i64 64, i1 false)
ret void
}
-; Holds a pointer, but also contains a GS buffer.
-; CHECK-LABEL: struct_containing_buffer:
+; As soon as the address reaches anywhere else, MSVC moves the object into the
+; guarded region instead.
+; CHECK-LABEL: sret_escapes:
; CHECK: __security_cookie
-define void @struct_containing_buffer() #0 {
- %a = alloca { ptr, [8 x i8] }
+define void @sret_escapes() #0 {
+ %a = alloca [64 x i8], !stack-protector !3
+ call void @make(ptr sret([64 x i8]) %a)
call void @use(ptr %a)
ret void
}
-; The GS buffer is two aggregates down.
-; CHECK-LABEL: struct_nested_buffer:
+; A derived pointer escaping counts just the same.
+; CHECK-LABEL: sret_gep_escapes:
; CHECK: __security_cookie
-define void @struct_nested_buffer() #0 {
- %a = alloca { ptr, { i32, [8 x i8] } }
- call void @use(ptr %a)
+define void @sret_gep_escapes() #0 {
+ %a = alloca [64 x i8], !stack-protector !3
+ call void @make(ptr sret([64 x i8]) %a)
+ %g = getelementptr [64 x i8], ptr %a, i64 0, i64 8
+ call void @use(ptr %g)
ret void
}
-;; --- alloca ------------------------------------------------------------------
-
-; CHECK-LABEL: dynamic_alloca:
+; Storing the pointer itself is an escape; storing through it is not.
+; CHECK-LABEL: sret_pointer_stored:
; CHECK: __security_cookie
-define void @dynamic_alloca(i64 %n) #0 {
- %a = alloca i8, i64 %n
- call void @use(ptr %a)
+define void @sret_pointer_stored(ptr %p) #0 {
+ %a = alloca [64 x i8], !stack-protector !3
+ call void @make(ptr sret([64 x i8]) %a)
+ store ptr %a, ptr %p
ret void
}
-; Even a small constant-sized alloca is a GS buffer.
-; CHECK-LABEL: small_alloca:
+; Without an indirect return there is nothing to exempt.
+; CHECK-LABEL: trivial_without_sret:
; CHECK: __security_cookie
-define void @small_alloca() #0 {
- %a = alloca i8, i64 3
- call void @use(ptr %a)
+define void @trivial_without_sret() #0 {
+ %a = alloca [64 x i8], !stack-protector !3
+ store i8 0, ptr %a
ret void
}
-;; --- Exclusions --------------------------------------------------------------
+; The exemption needs the object to be one MSVC may relocate, which is what the
+; second operand says. Without it the slot is protected.
+; CHECK-LABEL: nontrivial_sret:
+; CHECK: __security_cookie
+define void @nontrivial_sret() #0 {
+ %a = alloca [64 x i8], !stack-protector !2
+ call void @make(ptr sret([64 x i8]) %a)
+ ret void
+}
-; 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)
+; The exemption is specific to the /GS heuristic.
+; CHECK-LABEL: strong_sret:
+; CHECK: __security_cookie
+define void @strong_sret() #2 {
+ %a = alloca [64 x i8], !stack-protector !3
+ call void @make(ptr sret([64 x i8]) %a)
ret void
}
+;; --- Exclusions --------------------------------------------------------------
+
; 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]
+ %a = alloca [64 x i8], !stack-protector !2
call void @use(ptr %a)
ret void
}
-; sspreq overrides the heuristic entirely, so the varargs exclusion and the
-; GS-buffer rules do not apply.
+; sspreq overrides the heuristic entirely, so the varargs exclusion does not
+; apply.
; CHECK-LABEL: sspreq_wins:
; CHECK: __security_cookie
define void @sspreq_wins(i32 %n, ...) #1 {
@@ -180,6 +215,21 @@ define void @strong_still_protects() #2 {
ret void
}
+; The marker is honoured under the other heuristics too: sspstrong would not
+; protect this function on its own.
+; CHECK-LABEL: strong_honours_mark:
+; CHECK: __security_cookie
+define void @strong_honours_mark() #2 {
+ %a = alloca i32, !stack-protector !1
+ store i32 0, 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 }
+
+!0 = !{i32 0}
+!1 = !{i32 1}
+!2 = !{i32 2}
+!3 = !{i32 2, i1 true}
diff --git a/llvm/test/Transforms/InstCombine/alloca.ll b/llvm/test/Transforms/InstCombine/alloca.ll
index 15b43e780f1497..e2113fa648509b 100644
--- a/llvm/test/Transforms/InstCombine/alloca.ll
+++ b/llvm/test/Transforms/InstCombine/alloca.ll
@@ -259,3 +259,21 @@ define void @test_inalloca_with_element_count(ptr %a) {
call void @test9_aux(ptr inalloca(%struct_type) %alloca)
ret void
}
+
+; Folding the element count into the allocated type must not lose what was
+; said about the allocation.
+define void @test_metadata_is_preserved() {
+; ALL-LABEL: @test_metadata_is_preserved(
+; ALL-NEXT: [[ALLOCA1:%.*]] = alloca [10 x i8], align 1, !stack-protector [[META0:![0-9]+]]
+; ALL-NEXT: call void (...) @use(ptr nonnull [[ALLOCA1]])
+; ALL-NEXT: ret void
+;
+ %alloca = alloca i8, i32 10, !stack-protector !0
+ call void (...) @use(ptr %alloca)
+ ret void
+}
+
+!0 = !{i32 2}
+;.
+; ALL: [[META0]] = !{i32 2}
+;.
>From 8132b84928161a6142c0e6f4b6a3a1b99a539294 Mon Sep 17 00:00:00 2001
From: "Pan, Tao" <tao.pan at intel.com>
Date: Thu, 8 Oct 2026 10:19:09 +0800
Subject: [PATCH 3/8] Add test case
---
.../ms-stack-protector-gs-buffer-alloca.cpp | 269 ++++++++++++++++++
.../ms-stack-protector-gs-buffer-temp.cpp | 95 +++++++
2 files changed, 364 insertions(+)
create mode 100644 clang/test/CodeGen/ms-stack-protector-gs-buffer-alloca.cpp
create mode 100644 clang/test/CodeGen/ms-stack-protector-gs-buffer-temp.cpp
diff --git a/clang/test/CodeGen/ms-stack-protector-gs-buffer-alloca.cpp b/clang/test/CodeGen/ms-stack-protector-gs-buffer-alloca.cpp
new file mode 100644
index 00000000000000..167d4650d3b3ea
--- /dev/null
+++ b/clang/test/CodeGen/ms-stack-protector-gs-buffer-alloca.cpp
@@ -0,0 +1,269 @@
+// Check the "GS buffer" classification clang-cl's default /GS applies to each
+// stack object. The result is recorded as "stack-protector" metadata on the
+// alloca, because it depends on source-level type information that LLVM IR does
+// not preserve.
+//
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-extensions -O2 \
+// RUN: -stack-protector 4 -emit-llvm %s -o - | FileCheck %s
+//
+// The classification is only emitted for the /GS heuristic: the GCC-compatible
+// levels infer it from the IR type instead. The one exception is an explicit
+// opt-out, which is not a classification.
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-extensions -O2 \
+// RUN: -stack-protector 2 -emit-llvm %s -o - | FileCheck %s --check-prefix=STRONG
+//
+// STRONG-NOT: = !{i32 1}
+// STRONG-NOT: = !{i32 2}
+// STRONG-NOT: = !{i32 1, i1
+// STRONG-NOT: = !{i32 2, i1
+
+extern "C" void *_alloca(unsigned long long);
+
+void use(void *);
+
+//--- Arrays -------------------------------------------------------------------
+
+// An array has to be larger than 4 bytes...
+// CHECK-LABEL: @"?array_4@@YAXXZ"
+// CHECK: alloca [4 x i8], align 1{{$}}
+void array_4() { char a[4]; use(a); }
+
+// ...and have more than two elements.
+// CHECK-LABEL: @"?array_2_elements@@YAXXZ"
+// CHECK: alloca [2 x i32], align 4{{$}}
+void array_2_elements() { int a[2]; use(a); }
+
+// For the type of the stack object itself those are the only two conditions, so
+// unlike for a member (see struct_array_of_pointers) the element type does not
+// matter. MSDN says otherwise, but cl.exe protects this.
+// CHECK-LABEL: @"?array_of_pointers@@YAXXZ"
+// CHECK: alloca [8 x ptr], align 8, !stack-protector ![[LARGE:[0-9]+]]
+void array_of_pointers() { void *a[8]; use(a); }
+
+// A multidimensional array is counted as the flat array it is laid out as, so
+// this has four elements rather than two.
+// CHECK-LABEL: @"?array_2_by_2_pointers@@YAXXZ"
+// CHECK: alloca [2 x [2 x ptr]], align 8, !stack-protector ![[LARGE]]
+void array_2_by_2_pointers() { void *a[2][2]; use(a); }
+
+// A GS buffer of at least ssp-buffer-size bytes is laid out closest to the
+// guard, so it is marked 2 rather than 1.
+// CHECK-LABEL: @"?array_8_bytes@@YAXXZ"
+// CHECK: alloca [8 x i8], align 1, !stack-protector ![[LARGE]]
+void array_8_bytes() { char a[8]; use(a); }
+
+// 6 bytes and three elements: over both thresholds, but under ssp-buffer-size.
+// CHECK-LABEL: @"?array_3_shorts@@YAXXZ"
+// CHECK: alloca [3 x i16], align 2, !stack-protector ![[SMALL:[0-9]+]]
+void array_3_shorts() { short a[3]; use(a); }
+
+// Two elements of two bytes each: too small either way.
+// CHECK-LABEL: @"?array_2_chars_nested@@YAXXZ"
+// CHECK: alloca [2 x [2 x i8]], align 1{{$}}
+void array_2_chars_nested() { char a[2][2]; use(a); }
+
+//--- Aggregates ---------------------------------------------------------------
+
+struct PointerFree { int a, b, c; };
+struct WithPointer { void *p; int a, b; };
+struct EightBytes { int a, b; };
+struct ContainsBuffer { void *p; char buf[8]; };
+struct NestedBuffer { void *p; ContainsBuffer c; };
+struct ArrayOfPointers { void *m[4]; };
+struct TwoIntArray { int m[2]; };
+
+// More than 8 bytes with no pointers.
+// CHECK-LABEL: @"?struct_pointer_free@@YAXXZ"
+// CHECK: alloca %struct.PointerFree, align 4, !stack-protector ![[LARGE]]
+void struct_pointer_free() { PointerFree s; use(&s); }
+
+// CHECK-LABEL: @"?struct_with_pointer@@YAXXZ"
+// CHECK: alloca %struct.WithPointer, align 8{{$}}
+void struct_with_pointer() { WithPointer s; use(&s); }
+
+// Exactly 8 bytes, so not larger than 8.
+// CHECK-LABEL: @"?struct_8_bytes@@YAXXZ"
+// CHECK: alloca %struct.EightBytes, align 4{{$}}
+void struct_8_bytes() { EightBytes s; use(&s); }
+
+// Holds a pointer, but also contains a GS buffer.
+// CHECK-LABEL: @"?struct_containing_buffer@@YAXXZ"
+// CHECK: alloca %struct.ContainsBuffer, align 8, !stack-protector ![[LARGE]]
+void struct_containing_buffer() { ContainsBuffer s; use(&s); }
+
+// CHECK-LABEL: @"?struct_nested_buffer@@YAXXZ"
+// CHECK: alloca %struct.NestedBuffer, align 8, !stack-protector ![[LARGE]]
+void struct_nested_buffer() { NestedBuffer s; use(&s); }
+
+// Below the top level the two array conditions swap over: the element count
+// stops mattering and the element type starts to, which is the opposite of how
+// the same two types fare as locals (see array_of_pointers and
+// array_2_elements).
+// CHECK-LABEL: @"?struct_array_of_pointers@@YAXXZ"
+// CHECK: alloca %struct.ArrayOfPointers, align 8{{$}}
+void struct_array_of_pointers() { ArrayOfPointers s; use(&s); }
+
+// CHECK-LABEL: @"?struct_two_int_array@@YAXXZ"
+// CHECK: alloca %struct.TwoIntArray, align 4, !stack-protector ![[LARGE]]
+void struct_two_int_array() { TwoIntArray s; use(&s); }
+
+//--- Anonymous members --------------------------------------------------------
+
+// Each of these is 8 bytes, so only the member rule can make it a buffer, not
+// the size rule for the aggregate as a whole.
+struct AnonUnionBuffer { union { char b[8]; }; };
+struct AnonStructBuffer { struct { char b[8]; }; };
+struct NamedUnionBuffer { union U { char b[8]; } u; };
+struct AnonUnionPointer { union { char b[16]; void *p; }; int n; };
+
+// cl.exe does not look inside an anonymous struct or union for a buffer...
+// CHECK-LABEL: @"?anon_union_buffer@@YAXXZ"
+// CHECK: alloca %struct.AnonUnionBuffer, align 1{{$}}
+void anon_union_buffer() { AnonUnionBuffer s; use(&s); }
+
+// CHECK-LABEL: @"?anon_struct_buffer@@YAXXZ"
+// CHECK: alloca %struct.AnonStructBuffer, align 1{{$}}
+void anon_struct_buffer() { AnonStructBuffer s; use(&s); }
+
+// ...though the same union as a named member is found.
+// CHECK-LABEL: @"?named_union_buffer@@YAXXZ"
+// CHECK: alloca %struct.NamedUnionBuffer, align 1, !stack-protector ![[LARGE]]
+void named_union_buffer() { NamedUnionBuffer s; use(&s); }
+
+// A pointer inside one still disqualifies the enclosing aggregate.
+// CHECK-LABEL: @"?anon_union_pointer@@YAXXZ"
+// CHECK: alloca %struct.AnonUnionPointer, align 8{{$}}
+void anon_union_pointer() { AnonUnionPointer s; use(&s); }
+
+//--- Access control -----------------------------------------------------------
+
+class PrivateData { long long a, b; };
+struct ProtectedData { protected: long long a, b; };
+struct PrivateArray { private: char b[16]; };
+
+// An aggregate only becomes a GS buffer by size if it is plain, publicly
+// accessible data. MSDN does not mention this, but cl.exe applies it.
+// CHECK-LABEL: @"?private_data@@YAXXZ"
+// CHECK: alloca %class.PrivateData, align 8{{$}}
+void private_data() { PrivateData s; use(&s); }
+
+// CHECK-LABEL: @"?protected_data@@YAXXZ"
+// CHECK: alloca %struct.ProtectedData, align 8{{$}}
+void protected_data() { ProtectedData s; use(&s); }
+
+// The array rule is not subject to it.
+// CHECK-LABEL: @"?private_array@@YAXXZ"
+// CHECK: alloca %struct.PrivateArray, align 1, !stack-protector ![[LARGE]]
+void private_array() { PrivateArray s; use(&s); }
+
+//--- Base classes -------------------------------------------------------------
+
+struct BasePointer { void *p; };
+struct BaseBuffer { char b[16]; };
+struct DerivedPointer : BasePointer { long long a, b; };
+struct DerivedBuffer : BaseBuffer {};
+struct DerivedPrivate : private PrivateData {};
+struct HoldsDerivedPointer { DerivedPointer m; };
+
+// Neither the pointer veto nor the access-control rule looks into a base class,
+// so this is a GS buffer even though it inherits a pointer...
+// CHECK-LABEL: @"?derived_pointer@@YAXXZ"
+// CHECK: alloca %struct.DerivedPointer, align 8, !stack-protector ![[LARGE]]
+void derived_pointer() { DerivedPointer s; use(&s); }
+
+// ...and so is an aggregate holding one as a member.
+// CHECK-LABEL: @"?holds_derived_pointer@@YAXXZ"
+// CHECK: alloca %struct.HoldsDerivedPointer, align 8, !stack-protector ![[LARGE]]
+void holds_derived_pointer() { HoldsDerivedPointer s; use(&s); }
+
+// A private base with private members does not disqualify the derived class.
+// CHECK-LABEL: @"?derived_private@@YAXXZ"
+// CHECK: alloca %struct.DerivedPrivate, align 8, !stack-protector ![[LARGE]]
+void derived_private() { DerivedPrivate s; use(&s); }
+
+// The search for a buffer does reach into a base class.
+// CHECK-LABEL: @"?derived_buffer@@YAXXZ"
+// CHECK: alloca %struct.DerivedBuffer, align 1, !stack-protector ![[LARGE]]
+void derived_buffer() { DerivedBuffer s; use(&s); }
+
+//--- C++ specifics ------------------------------------------------------------
+
+struct Polymorphic { virtual void f(); int a, b, c; };
+struct Derived : PointerFree { int d; };
+struct HoldsReference { int &r; int a, b; HoldsReference(int &x) : r(x) {} };
+
+// The vptr is a pointer even though it is not a field, and it counts for a
+// derived class too, where it physically lives in the base subobject.
+// CHECK-LABEL: @"?polymorphic@@YAXXZ"
+// CHECK: alloca %struct.Polymorphic, align 8{{$}}
+void polymorphic() { Polymorphic s; use(&s); }
+
+// A base class is searched for a buffer just like a member.
+// CHECK-LABEL: @"?derived@@YAXXZ"
+// CHECK: alloca %struct.Derived, align 4, !stack-protector ![[LARGE]]
+void derived() { Derived s; use(&s); }
+
+// A reference holds an address, so it disqualifies the aggregate just like a
+// pointer does.
+// CHECK-LABEL: @"?holds_reference@@YAXAEAH at Z"
+// CHECK: alloca %struct.HoldsReference, align 8{{$}}
+void holds_reference(int &x) { HoldsReference s(x); use(&s); }
+
+//--- _alloca ------------------------------------------------------------------
+
+// A buffer allocated by _alloca is a GS buffer whatever its size. The element
+// count that lets the backend recognise one does not survive a constant-sized
+// alloca being folded to an array type, so it is marked here too.
+// CHECK-LABEL: @"?alloca_large@@YAXXZ"
+// CHECK: alloca [64 x i8], align 16, !stack-protector ![[LARGE_NT:[0-9]+]]
+void alloca_large() { use(_alloca(64)); }
+
+// Below ssp-buffer-size, but still a GS buffer: _alloca has no threshold.
+// CHECK-LABEL: @"?alloca_small@@YAXXZ"
+// CHECK: alloca [3 x i8], align 16, !stack-protector ![[SMALL_NT:[0-9]+]]
+void alloca_small() { use(_alloca(3)); }
+
+// CHECK-LABEL: @"?alloca_dynamic@@YAXH at Z"
+// CHECK: alloca i8, i64 %{{.*}}, align 16, !stack-protector ![[LARGE_NT]]
+void alloca_dynamic(int n) { use(_alloca(n)); }
+
+// CHECK-LABEL: @"?alloca_with_align@@YAXXZ"
+// CHECK: alloca [64 x i8], align 16, !stack-protector ![[LARGE_NT]]
+void alloca_with_align() { use(__builtin_alloca_with_align(64, 128)); }
+
+//--- Left to the backend ------------------------------------------------------
+
+// A variable-length array lowers to an alloca with an element count, which the
+// backend recognises as a GS buffer on its own.
+// CHECK-LABEL: @"?vla@@YAXH at Z"
+// CHECK: alloca i8, i64 %{{[0-9]+}}, align 1{{$}}
+void vla(int n) { char a[n]; use(a); }
+
+//--- Opting out ---------------------------------------------------------------
+
+// An explicit opt-out wins over the classification.
+// CHECK-LABEL: @"?ignored@@YAXXZ"
+// CHECK: alloca [64 x i8], align 1, !stack-protector ![[IGNORE:[0-9]+]]
+void ignored() {
+ __attribute__((stack_protector_ignore)) char a[64];
+ use(a);
+}
+
+// __declspec(safebuffers) turns the check off, so there is nothing to record.
+// CHECK-LABEL: @"?safe@@YAXXZ"
+// CHECK: alloca [64 x i8], align 1{{$}}
+__declspec(safebuffers) void safe() { char a[64]; use(a); }
+
+// __declspec(strict_gs_check) asks for the broader "strong" heuristic, which
+// the backend applies to the IR type itself.
+// CHECK-LABEL: @"?strict@@YAXXZ"
+// CHECK: alloca [64 x i8], align 1{{$}}
+__declspec(strict_gs_check) void strict() { char a[64]; use(a); }
+
+// The second operand says the object is trivial, and so is one the backend may
+// leave in the unguarded temporary area; see ms-stack-protector-gs-buffer-temp.
+// CHECK-DAG: ![[LARGE]] = !{i32 2, i1 true}
+// CHECK-DAG: ![[SMALL]] = !{i32 1, i1 true}
+// CHECK-DAG: ![[LARGE_NT]] = !{i32 2}
+// CHECK-DAG: ![[SMALL_NT]] = !{i32 1}
+// CHECK-DAG: ![[IGNORE]] = !{i32 0}
diff --git a/clang/test/CodeGen/ms-stack-protector-gs-buffer-temp.cpp b/clang/test/CodeGen/ms-stack-protector-gs-buffer-temp.cpp
new file mode 100644
index 00000000000000..007eda37de881a
--- /dev/null
+++ b/clang/test/CodeGen/ms-stack-protector-gs-buffer-temp.cpp
@@ -0,0 +1,95 @@
+// Check which of the stack objects clang materialises itself clang-cl's default
+// /GS classifies. cl.exe analyses the objects the source declares, but gives an
+// anonymous ABI slot holding a trivial object a frame slot outside the guarded
+// region, and so leaves it out.
+//
+// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-extensions -O1 \
+// RUN: -disable-llvm-passes -stack-protector 4 -emit-llvm %s -o - \
+// RUN: | FileCheck %s
+
+struct Buf { char b[16]; };
+struct Ctor { char b[16]; Ctor(); };
+struct Dtor { char b[16]; ~Dtor(); };
+struct CopyCtor { char b[16]; CopyCtor(); CopyCtor(const CopyCtor &); };
+struct NoCopy { char b[16]; NoCopy(); NoCopy(NoCopy &&); };
+
+Buf makeBuf();
+Ctor makeCtor();
+Dtor makeDtor();
+void takeBuf(Buf);
+void takeCtor(Ctor);
+void takeCopyCtor(CopyCtor);
+void takeNoCopy(NoCopy);
+void use(const void *);
+
+//--- Indirect return slots ----------------------------------------------------
+
+// The slot for a discarded trivial return value is anonymous storage, so it is
+// not classified at all.
+// CHECK-LABEL: @"?discard_trivial@@YAXXZ"
+// CHECK: alloca %struct.Buf, align 1{{$}}
+void discard_trivial() { makeBuf(); }
+
+// Once the type needs a constructor or a destructor to run, the slot holds a
+// real object and cl.exe covers it.
+// CHECK-LABEL: @"?discard_dtor@@YAXXZ"
+// CHECK: alloca %struct.Dtor, align 1, !stack-protector ![[LARGE:[0-9]+]]
+void discard_dtor() { makeDtor(); }
+
+// A temporary the source binds to a reference is a declared object, so it is
+// classified even when trivial. The second operand lets the backend drop it
+// again if its address turns out to be used for nothing but the return value;
+// see llvm/test/CodeGen/X86/stack-protector-gs-buffer.ll.
+// CHECK-LABEL: @"?bind_trivial@@YAXXZ"
+// CHECK: alloca %struct.Buf, align 1, !stack-protector ![[LARGE_TRIV:[0-9]+]]
+void bind_trivial() { const Buf &r = makeBuf(); use(&r); }
+
+// Likewise a declared local, which clang gives to the callee directly.
+// CHECK-LABEL: @"?local_from_call@@YAXXZ"
+// CHECK: alloca %struct.Buf, align 1, !stack-protector ![[LARGE_TRIV]]
+void local_from_call() { Buf b = makeBuf(); use(&b); }
+
+//--- Outgoing by-value arguments ----------------------------------------------
+
+// cl.exe builds the copy in the outgoing argument area, which the cookie does
+// not reach, so a trivial argument is not classified.
+// CHECK-LABEL: @"?pass_trivial@@YAXXZ"
+// CHECK: alloca %struct.Buf, align 1{{$}}
+void pass_trivial() { Buf b; takeBuf(b); }
+
+// With a non-trivial type it builds a real object first. It can only then copy
+// that into the argument area, and so only then benefits from the guard, if the
+// copy is bitwise.
+// CHECK-LABEL: @"?pass_ctor@@YAXXZ"
+// CHECK: alloca %struct.Ctor, align 1, !stack-protector ![[LARGE]]
+void pass_ctor() { takeCtor(Ctor()); }
+
+// A user-provided copy constructor has to run on the argument area itself.
+// CHECK-LABEL: @"?pass_copy_ctor@@YAXXZ"
+// CHECK: alloca %struct.CopyCtor, align 1{{$}}
+void pass_copy_ctor() { takeCopyCtor(CopyCtor()); }
+
+// A deleted copy constructor is no better.
+// CHECK-LABEL: @"?pass_no_copy@@YAXXZ"
+// CHECK: alloca %struct.NoCopy, align 1{{$}}
+void pass_no_copy() { takeNoCopy(NoCopy()); }
+
+//--- By-value parameters ------------------------------------------------------
+
+// A by-value parameter lives in the caller's frame, where the guard does not
+// reach, so clang relocates a vulnerable one into a copy of its own. This
+// mirrors the `<name>$GSCopy$` slot cl.exe emits.
+// CHECK-LABEL: @"?param@@YAXUBuf@@@Z"
+// CHECK: %[[COPY:["a-zA-Z0-9$]+]] = alloca %struct.Buf, align 1, !stack-protector ![[LARGE]]
+// CHECK: call void @llvm.memcpy{{.*}}(ptr {{[^,]*}}%[[COPY]],
+// CHECK: call void @"?use@@YAXPEBX at Z"(ptr {{[^,]*}}%[[COPY]])
+void param(Buf b) { use(&b); }
+
+// A parameter that is not a GS buffer is left where it is.
+// CHECK-LABEL: @"?param_int@@YAXH at Z"
+// CHECK-NOT: !stack-protector
+// CHECK: }
+void param_int(int n) { use(&n); }
+
+// CHECK-DAG: ![[LARGE]] = !{i32 2}
+// CHECK-DAG: ![[LARGE_TRIV]] = !{i32 2, i1 true}
>From c367544c45a8388a3e94811c17f94461568fcf08 Mon Sep 17 00:00:00 2001
From: "Pan, Tao" <tao.pan at intel.com>
Date: Thu, 8 Oct 2026 10:45:15 +0800
Subject: [PATCH 4/8] Fix code format failure
---
clang/lib/CodeGen/CGCall.cpp | 42 ++++++++++++++++++------------------
1 file changed, 21 insertions(+), 21 deletions(-)
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 4acbbdeef9cbcf..31309846cfa57b 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -705,10 +705,10 @@ arrangeFreeFunctionLikeCall(CodeGenTypes &CGT, CodeGenModule &CGM,
addExtParameterInfosForCall(paramInfos, proto, numExtraRequiredArgs,
args.size());
- // If we don't have a prototype at all, but we're supposed to
- // explicitly use the variadic convention for unprototyped calls,
- // treat all of the arguments as required but preserve the nominal
- // possibility of variadics.
+ // If we don't have a prototype at all, but we're supposed to
+ // explicitly use the variadic convention for unprototyped calls,
+ // treat all of the arguments as required but preserve the nominal
+ // possibility of variadics.
} else if (CGM.getTargetCodeGenInfo().isNoProtoCallVariadic(
args, cast<FunctionNoProtoType>(fnType))) {
required = RequiredArgs(args.size());
@@ -3985,7 +3985,8 @@ static llvm::Value *tryEmitFusedAutoreleaseOfResult(CodeGenFunction &CGF,
// Look for:
// %generator = call i8* @objc_retain(i8* %originalResult)
// or
- // %generator = call i8* @objc_retainAutoreleasedReturnValue(i8* %originalResult)
+ // %generator = call i8* @objc_retainAutoreleasedReturnValue(i8*
+ // %originalResult)
llvm::CallInst *call = dyn_cast<llvm::CallInst>(generator);
if (!call)
return nullptr;
@@ -4465,7 +4466,7 @@ void CodeGenFunction::EmitFunctionEpilog(
RV = SI->getValueOperand();
SI->eraseFromParent();
- // Otherwise, we have to do a simple load.
+ // Otherwise, we have to do a simple load.
} else {
RV = Builder.CreateLoad(ReturnValue);
}
@@ -4533,7 +4534,7 @@ void CodeGenFunction::EmitFunctionEpilog(
if (results.size() == 1) {
RV = results[0];
- // Otherwise, we need to make a first-class aggregate.
+ // Otherwise, we need to make a first-class aggregate.
} else {
// Construct a return type that lacks padding elements.
llvm::Type *returnType = RetAI.getUnpaddedCoerceAndExpandType();
@@ -4683,11 +4684,11 @@ void CodeGenFunction::EmitDelegateCallArg(CallArgList &args,
if (type->isReferenceType()) {
args.add(RValue::get(Builder.CreateLoad(local)), type);
- // In ARC, move out of consumed arguments so that the release cleanup
- // entered by StartFunction doesn't cause an over-release. This isn't
- // optimal -O0 code generation, but it should get cleaned up when
- // optimization is enabled. This also assumes that delegate calls are
- // performed exactly once for a set of arguments, but that should be safe.
+ // In ARC, move out of consumed arguments so that the release cleanup
+ // entered by StartFunction doesn't cause an over-release. This isn't
+ // optimal -O0 code generation, but it should get cleaned up when
+ // optimization is enabled. This also assumes that delegate calls are
+ // performed exactly once for a set of arguments, but that should be safe.
} else if (getLangOpts().ObjCAutoRefCount &&
param->hasAttr<NSConsumedAttr>() && type->isObjCRetainableType()) {
llvm::Value *ptr = Builder.CreateLoad(local);
@@ -4696,8 +4697,8 @@ void CodeGenFunction::EmitDelegateCallArg(CallArgList &args,
Builder.CreateStore(null, local);
args.add(RValue::get(ptr), type);
- // For the most part, we just need to load the alloca, except that
- // aggregate r-values are actually pointers to temporaries.
+ // For the most part, we just need to load the alloca, except that
+ // aggregate r-values are actually pointers to temporaries.
} else {
args.add(convertTempToRValue(local, type, loc), type);
}
@@ -4786,7 +4787,7 @@ static void emitWriteback(CodeGenFunction &CGF,
// Release the old value.
CGF.EmitARCRelease(oldValue, srcLV.isARCPreciseLifetime());
- // Otherwise, we can just do a normal lvalue store.
+ // Otherwise, we can just do a normal lvalue store.
} else {
CGF.EmitStoreThroughLValue(RValue::get(value), srcLV);
}
@@ -4827,7 +4828,7 @@ static void emitWritebackArg(CodeGenFunction &CGF, CallArgList &args,
if (const Expr *lvExpr = maybeGetUnaryAddrOfOperand(CRE->getSubExpr())) {
srcLV = CGF.EmitLValue(lvExpr);
- // Otherwise, just emit it as a scalar.
+ // Otherwise, just emit it as a scalar.
} else {
Address srcAddr = CGF.EmitPointerWithAlignment(CRE->getSubExpr());
@@ -5293,11 +5294,10 @@ void CodeGenFunction::EmitCallArg(CallArgList &args, const Expr *E,
type->castAsRecordDecl()->isParamDestroyedInCallee()) {
// If we're using inalloca, use the argument memory. Otherwise, use a
// temporary.
- AggValueSlot Slot =
- args.isUsingInAlloca()
- ? createPlaceholderSlot(*this, type)
- : CreateAggTemp(type, "agg.tmp", /*Alloca=*/nullptr,
- GSObjectKind::ArgumentCopy);
+ AggValueSlot Slot = args.isUsingInAlloca()
+ ? createPlaceholderSlot(*this, type)
+ : CreateAggTemp(type, "agg.tmp", /*Alloca=*/nullptr,
+ GSObjectKind::ArgumentCopy);
bool DestroyedInCallee = true, NeedsCleanup = true;
if (const auto *RD = type->getAsCXXRecordDecl())
>From ccd246b15a75447aac7cd96a4c110ce96c0f6e45 Mon Sep 17 00:00:00 2001
From: "Pan, Tao" <tao.pan at intel.com>
Date: Thu, 8 Oct 2026 14:40:15 +0800
Subject: [PATCH 5/8] Fix code format failure
---
clang/lib/CodeGen/CGCall.cpp | 42 ++++++++++++++++++------------------
1 file changed, 21 insertions(+), 21 deletions(-)
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 31309846cfa57b..4acbbdeef9cbcf 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -705,10 +705,10 @@ arrangeFreeFunctionLikeCall(CodeGenTypes &CGT, CodeGenModule &CGM,
addExtParameterInfosForCall(paramInfos, proto, numExtraRequiredArgs,
args.size());
- // If we don't have a prototype at all, but we're supposed to
- // explicitly use the variadic convention for unprototyped calls,
- // treat all of the arguments as required but preserve the nominal
- // possibility of variadics.
+ // If we don't have a prototype at all, but we're supposed to
+ // explicitly use the variadic convention for unprototyped calls,
+ // treat all of the arguments as required but preserve the nominal
+ // possibility of variadics.
} else if (CGM.getTargetCodeGenInfo().isNoProtoCallVariadic(
args, cast<FunctionNoProtoType>(fnType))) {
required = RequiredArgs(args.size());
@@ -3985,8 +3985,7 @@ static llvm::Value *tryEmitFusedAutoreleaseOfResult(CodeGenFunction &CGF,
// Look for:
// %generator = call i8* @objc_retain(i8* %originalResult)
// or
- // %generator = call i8* @objc_retainAutoreleasedReturnValue(i8*
- // %originalResult)
+ // %generator = call i8* @objc_retainAutoreleasedReturnValue(i8* %originalResult)
llvm::CallInst *call = dyn_cast<llvm::CallInst>(generator);
if (!call)
return nullptr;
@@ -4466,7 +4465,7 @@ void CodeGenFunction::EmitFunctionEpilog(
RV = SI->getValueOperand();
SI->eraseFromParent();
- // Otherwise, we have to do a simple load.
+ // Otherwise, we have to do a simple load.
} else {
RV = Builder.CreateLoad(ReturnValue);
}
@@ -4534,7 +4533,7 @@ void CodeGenFunction::EmitFunctionEpilog(
if (results.size() == 1) {
RV = results[0];
- // Otherwise, we need to make a first-class aggregate.
+ // Otherwise, we need to make a first-class aggregate.
} else {
// Construct a return type that lacks padding elements.
llvm::Type *returnType = RetAI.getUnpaddedCoerceAndExpandType();
@@ -4684,11 +4683,11 @@ void CodeGenFunction::EmitDelegateCallArg(CallArgList &args,
if (type->isReferenceType()) {
args.add(RValue::get(Builder.CreateLoad(local)), type);
- // In ARC, move out of consumed arguments so that the release cleanup
- // entered by StartFunction doesn't cause an over-release. This isn't
- // optimal -O0 code generation, but it should get cleaned up when
- // optimization is enabled. This also assumes that delegate calls are
- // performed exactly once for a set of arguments, but that should be safe.
+ // In ARC, move out of consumed arguments so that the release cleanup
+ // entered by StartFunction doesn't cause an over-release. This isn't
+ // optimal -O0 code generation, but it should get cleaned up when
+ // optimization is enabled. This also assumes that delegate calls are
+ // performed exactly once for a set of arguments, but that should be safe.
} else if (getLangOpts().ObjCAutoRefCount &&
param->hasAttr<NSConsumedAttr>() && type->isObjCRetainableType()) {
llvm::Value *ptr = Builder.CreateLoad(local);
@@ -4697,8 +4696,8 @@ void CodeGenFunction::EmitDelegateCallArg(CallArgList &args,
Builder.CreateStore(null, local);
args.add(RValue::get(ptr), type);
- // For the most part, we just need to load the alloca, except that
- // aggregate r-values are actually pointers to temporaries.
+ // For the most part, we just need to load the alloca, except that
+ // aggregate r-values are actually pointers to temporaries.
} else {
args.add(convertTempToRValue(local, type, loc), type);
}
@@ -4787,7 +4786,7 @@ static void emitWriteback(CodeGenFunction &CGF,
// Release the old value.
CGF.EmitARCRelease(oldValue, srcLV.isARCPreciseLifetime());
- // Otherwise, we can just do a normal lvalue store.
+ // Otherwise, we can just do a normal lvalue store.
} else {
CGF.EmitStoreThroughLValue(RValue::get(value), srcLV);
}
@@ -4828,7 +4827,7 @@ static void emitWritebackArg(CodeGenFunction &CGF, CallArgList &args,
if (const Expr *lvExpr = maybeGetUnaryAddrOfOperand(CRE->getSubExpr())) {
srcLV = CGF.EmitLValue(lvExpr);
- // Otherwise, just emit it as a scalar.
+ // Otherwise, just emit it as a scalar.
} else {
Address srcAddr = CGF.EmitPointerWithAlignment(CRE->getSubExpr());
@@ -5294,10 +5293,11 @@ void CodeGenFunction::EmitCallArg(CallArgList &args, const Expr *E,
type->castAsRecordDecl()->isParamDestroyedInCallee()) {
// If we're using inalloca, use the argument memory. Otherwise, use a
// temporary.
- AggValueSlot Slot = args.isUsingInAlloca()
- ? createPlaceholderSlot(*this, type)
- : CreateAggTemp(type, "agg.tmp", /*Alloca=*/nullptr,
- GSObjectKind::ArgumentCopy);
+ AggValueSlot Slot =
+ args.isUsingInAlloca()
+ ? createPlaceholderSlot(*this, type)
+ : CreateAggTemp(type, "agg.tmp", /*Alloca=*/nullptr,
+ GSObjectKind::ArgumentCopy);
bool DestroyedInCallee = true, NeedsCleanup = true;
if (const auto *RD = type->getAsCXXRecordDecl())
>From dc8d42aa0c814afb0d789085d037fdd99214e95f Mon Sep 17 00:00:00 2001
From: "Pan, Tao" <tao.pan at intel.com>
Date: Thu, 8 Oct 2026 15:00:19 +0800
Subject: [PATCH 6/8] Fix code format failure
---
clang/lib/CodeGen/CGCall.cpp | 9 ++++-----
clang/lib/CodeGen/CGDecl.cpp | 15 ++++++++-------
clang/lib/CodeGen/CGExpr.cpp | 3 +--
llvm/lib/CodeGen/StackProtector.cpp | 8 ++++----
4 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/clang/lib/CodeGen/CGCall.cpp b/clang/lib/CodeGen/CGCall.cpp
index 4acbbdeef9cbcf..066f1923cdf292 100644
--- a/clang/lib/CodeGen/CGCall.cpp
+++ b/clang/lib/CodeGen/CGCall.cpp
@@ -5293,11 +5293,10 @@ void CodeGenFunction::EmitCallArg(CallArgList &args, const Expr *E,
type->castAsRecordDecl()->isParamDestroyedInCallee()) {
// If we're using inalloca, use the argument memory. Otherwise, use a
// temporary.
- AggValueSlot Slot =
- args.isUsingInAlloca()
- ? createPlaceholderSlot(*this, type)
- : CreateAggTemp(type, "agg.tmp", /*Alloca=*/nullptr,
- GSObjectKind::ArgumentCopy);
+ AggValueSlot Slot = args.isUsingInAlloca()
+ ? createPlaceholderSlot(*this, type)
+ : CreateAggTemp(type, "agg.tmp", /*Alloca=*/nullptr,
+ GSObjectKind::ArgumentCopy);
bool DestroyedInCallee = true, NeedsCleanup = true;
if (const auto *RD = type->getAsCXXRecordDecl())
diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index 2eed9fd2cee751..1259a8d92327d2 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -1671,8 +1671,9 @@ static bool isGSBuffer(const ASTContext &Ctx, QualType Ty,
if (const ConstantArrayType *CAT = Ctx.getAsConstantArrayType(Ty)) {
QualType ElemTy = CAT->getElementType();
if (Ctx.getTypeSizeInChars(Ty).getQuantity() > MinGSArraySize &&
- (Position == GSTypePosition::TopLevel ? hasMoreThanTwoGSElements(Ctx, Ty)
- : !isGSPointerType(ElemTy)))
+ (Position == GSTypePosition::TopLevel
+ ? hasMoreThanTwoGSElements(Ctx, Ty)
+ : !isGSPointerType(ElemTy)))
return Found(Ty);
// The array itself is not a GS buffer, but its elements may still be or
@@ -1726,9 +1727,9 @@ void CodeGenFunction::MarkGSBufferAlloca(llvm::AllocaInst *AI, bool IsLarge,
if (AI->getMetadata("stack-protector"))
return;
- // 2 is a large buffer and 1 a small one. The second operand says the object is
- // trivial, and is left out when it is not, which is the common case; see the
- // "stack-protector" metadata in LLVM's LangRef.
+ // 2 is a large buffer and 1 a small one. The second operand says the object
+ // is trivial, and is left out when it is not, which is the common case; see
+ // the "stack-protector" metadata in LLVM's LangRef.
llvm::LLVMContext &Ctx = Builder.getContext();
SmallVector<llvm::Metadata *, 2> Ops = {
llvm::ConstantAsMetadata::get(Builder.getInt32(IsLarge ? 2 : 1))};
@@ -1763,8 +1764,8 @@ void CodeGenFunction::EmitGSBufferStackProtectorMD(llvm::Value *AllocaPtr,
// and the destructor runs on the original.
if (Kind == GSObjectKind::ArgumentCopy) {
const auto *RD = Ty->getAsCXXRecordDecl();
- if (RD && !(RD->hasSimpleCopyConstructor() &&
- RD->hasTrivialCopyConstructor()))
+ if (RD &&
+ !(RD->hasSimpleCopyConstructor() && RD->hasTrivialCopyConstructor()))
return;
}
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 76eae97ada2f0f..a68e658eb835f8 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -202,8 +202,7 @@ RawAddress CodeGenFunction::CreateMemTemp(QualType Ty, const Twine &Name,
}
RawAddress CodeGenFunction::CreateMemTemp(QualType Ty, CharUnits Align,
- const Twine &Name,
- RawAddress *Alloca,
+ const Twine &Name, RawAddress *Alloca,
GSObjectKind GSKind) {
RawAddress Result =
CreateTempAlloca(ConvertTypeForMem(Ty), Ty.getAddressSpace(), Align, Name,
diff --git a/llvm/lib/CodeGen/StackProtector.cpp b/llvm/lib/CodeGen/StackProtector.cpp
index 05e6fa85fdd0ef..b8476f347f4cd5 100644
--- a/llvm/lib/CodeGen/StackProtector.cpp
+++ b/llvm/lib/CodeGen/StackProtector.cpp
@@ -106,10 +106,10 @@ static std::optional<uint32_t> getSSPAllocaMD(const AllocaInst &AI,
///
/// cl.exe gives such an allocation a slot in the frame's temporary area, which
/// the cookie does not cover, and leaves it out of the GS analysis: the only
-/// code that writes it from outside is a callee, and a callee writes exactly one
-/// object of the type it returns. As soon as the address reaches anywhere else,
-/// cl.exe relocates the object into the protected region instead, so look for
-/// that here rather than in the frontend, which cannot see the uses.
+/// code that writes it from outside is a callee, and a callee writes exactly
+/// one object of the type it returns. As soon as the address reaches anywhere
+/// else, cl.exe relocates the object into the protected region instead, so look
+/// for that here rather than in the frontend, which cannot see the uses.
///
/// Only an object cl.exe is free to relocate can end up in that area, which is
/// why the caller also requires the allocation to hold a trivial object.
>From 3dd1e3c17ddad2e01cce41b21cc82fc42569a004 Mon Sep 17 00:00:00 2001
From: "Pan, Tao" <tao.pan at intel.com>
Date: Mon, 12 Oct 2026 11:04:34 +0800
Subject: [PATCH 7/8] Handle efriedma-quic's review comments
---
clang/lib/CodeGen/CGDecl.cpp | 31 +++++++++++++------
clang/lib/CodeGen/CodeGenModule.cpp | 6 ----
.../ms-stack-protector-gs-buffer-alloca.cpp | 19 +++++++-----
.../ms-stack-protector-gs-buffer-temp.cpp | 10 ++++++
.../CodeGen/ms-stack-protector-gs-buffer.c | 15 +++++----
llvm/lib/CodeGen/StackProtector.cpp | 7 +----
.../InstCombineLoadStoreAlloca.cpp | 3 --
.../CodeGen/X86/stack-protector-gs-buffer.ll | 25 ++++++++++-----
llvm/test/Transforms/InstCombine/alloca.ll | 18 -----------
9 files changed, 71 insertions(+), 63 deletions(-)
diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index 1259a8d92327d2..dfebd65f637845 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -3046,16 +3046,29 @@ void CodeGenFunction::EmitParmDecl(const VarDecl &D, ParamValue Arg,
DeclPtr = DeclPtr.withElementType(ConvertTypeForMem(Ty));
ABIArgInfo ArgInfo = CurFnInfo->arguments()[ArgNo - 1].info;
- // An indirectly passed parameter lives in storage the caller allocated, so
- // this frame's stack guard does not cover it. When it is a GS buffer,
- // relocate it the way cl.exe does. Only `byval` and plain `indirect` own
- // their storage; for `byref` the callee's writes are visible to the caller,
- // so a copy would change behaviour.
- if (MarkGSBuffers && !CurFuncIsThunk && isa<ParmVarDecl>(&D) &&
- ArgInfo.isIndirect() &&
- CurFnInfo->getExtParameterInfo(ArgNo - 1).getABI() ==
+ // A by-value parameter is an object the source declared, so cl.exe
+ // classifies it. Where it lives decides what has to happen for this
+ // frame's guard to cover it.
+ if (MarkGSBuffers && !CurFuncIsThunk && isa<ParmVarDecl>(&D)) {
+ if (ArgInfo.isIndirect()) {
+ // The caller allocated the storage, so the guard does not reach it.
+ // When it is a GS buffer, relocate it the way cl.exe does. Only
+ // `byval` and plain `indirect` own their storage; for `byref` the
+ // callee's writes are visible to the caller, so a copy would change
+ // behaviour.
+ if (CurFnInfo->getExtParameterInfo(ArgNo - 1).getABI() ==
ParameterABI::Ordinary)
- DeclPtr = EmitGSBufferParamCopy(D, DeclPtr);
+ DeclPtr = EmitGSBufferParamCopy(D, DeclPtr);
+ } else if (isa<llvm::AllocaInst>(
+ DeclPtr.getBasePointer()->stripPointerCasts())) {
+ // The argument arrived in registers and the prologue spilled it to a
+ // slot of this frame's own, which the guard already covers. Nothing
+ // needs moving, but it still has to be classified: without this an
+ // aggregate small enough to be passed in registers would be missed.
+ EmitGSBufferStackProtectorMD(
+ DeclPtr.getBasePointer()->stripPointerCasts(), Ty);
+ }
+ }
auto *V = DeclPtr.emitRawPointer(*this);
AllocaPtr = RawAddress(V, DeclPtr.getElementType(), DeclPtr.getAlignment());
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp
index 496efdb731fc00..00ccabfc290456 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -1170,12 +1170,6 @@ bool CodeGenModule::useMSVCGSBufferHeuristic(const Decl *D) const {
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>() &&
diff --git a/clang/test/CodeGen/ms-stack-protector-gs-buffer-alloca.cpp b/clang/test/CodeGen/ms-stack-protector-gs-buffer-alloca.cpp
index 167d4650d3b3ea..7c8ceebc5a205c 100644
--- a/clang/test/CodeGen/ms-stack-protector-gs-buffer-alloca.cpp
+++ b/clang/test/CodeGen/ms-stack-protector-gs-buffer-alloca.cpp
@@ -4,13 +4,15 @@
// not preserve.
//
// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-extensions -O2 \
-// RUN: -stack-protector 4 -emit-llvm %s -o - | FileCheck %s
+// RUN: -disable-llvm-passes -stack-protector 4 -emit-llvm %s -o - \
+// RUN: | FileCheck %s
//
// The classification is only emitted for the /GS heuristic: the GCC-compatible
// levels infer it from the IR type instead. The one exception is an explicit
// opt-out, which is not a classification.
// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-extensions -O2 \
-// RUN: -stack-protector 2 -emit-llvm %s -o - | FileCheck %s --check-prefix=STRONG
+// RUN: -disable-llvm-passes -stack-protector 2 -emit-llvm %s -o - \
+// RUN: | FileCheck %s --check-prefix=STRONG
//
// STRONG-NOT: = !{i32 1}
// STRONG-NOT: = !{i32 2}
@@ -211,16 +213,17 @@ void holds_reference(int &x) { HoldsReference s(x); use(&s); }
//--- _alloca ------------------------------------------------------------------
-// A buffer allocated by _alloca is a GS buffer whatever its size. The element
-// count that lets the backend recognise one does not survive a constant-sized
-// alloca being folded to an array type, so it is marked here too.
+// A buffer allocated by _alloca is a GS buffer whatever its size. An alloca
+// with an element count is one the backend recognises on its own, but a
+// constant count does not stay one for long -- InstCombine folds it into the
+// allocated type -- so the classification is recorded here as well.
// CHECK-LABEL: @"?alloca_large@@YAXXZ"
-// CHECK: alloca [64 x i8], align 16, !stack-protector ![[LARGE_NT:[0-9]+]]
+// CHECK: alloca i8, i64 64, align 16, !stack-protector ![[LARGE_NT:[0-9]+]]
void alloca_large() { use(_alloca(64)); }
// Below ssp-buffer-size, but still a GS buffer: _alloca has no threshold.
// CHECK-LABEL: @"?alloca_small@@YAXXZ"
-// CHECK: alloca [3 x i8], align 16, !stack-protector ![[SMALL_NT:[0-9]+]]
+// CHECK: alloca i8, i64 3, align 16, !stack-protector ![[SMALL_NT:[0-9]+]]
void alloca_small() { use(_alloca(3)); }
// CHECK-LABEL: @"?alloca_dynamic@@YAXH at Z"
@@ -228,7 +231,7 @@ void alloca_small() { use(_alloca(3)); }
void alloca_dynamic(int n) { use(_alloca(n)); }
// CHECK-LABEL: @"?alloca_with_align@@YAXXZ"
-// CHECK: alloca [64 x i8], align 16, !stack-protector ![[LARGE_NT]]
+// CHECK: alloca i8, i64 64, align 16, !stack-protector ![[LARGE_NT]]
void alloca_with_align() { use(__builtin_alloca_with_align(64, 128)); }
//--- Left to the backend ------------------------------------------------------
diff --git a/clang/test/CodeGen/ms-stack-protector-gs-buffer-temp.cpp b/clang/test/CodeGen/ms-stack-protector-gs-buffer-temp.cpp
index 007eda37de881a..384b03879ba75e 100644
--- a/clang/test/CodeGen/ms-stack-protector-gs-buffer-temp.cpp
+++ b/clang/test/CodeGen/ms-stack-protector-gs-buffer-temp.cpp
@@ -8,6 +8,7 @@
// RUN: | FileCheck %s
struct Buf { char b[16]; };
+struct SmallBuf { char b[8]; }; // passed in a register, unlike Buf
struct Ctor { char b[16]; Ctor(); };
struct Dtor { char b[16]; ~Dtor(); };
struct CopyCtor { char b[16]; CopyCtor(); CopyCtor(const CopyCtor &); };
@@ -85,6 +86,15 @@ void pass_no_copy() { takeNoCopy(NoCopy()); }
// CHECK: call void @"?use@@YAXPEBX at Z"(ptr {{[^,]*}}%[[COPY]])
void param(Buf b) { use(&b); }
+// A parameter small enough to arrive in registers is spilled by the prologue to
+// a slot of this frame's own, which the guard already covers. Nothing has to be
+// relocated, but it is still a declared object and still gets classified.
+// CHECK-LABEL: @"?param_in_register@@YAXUSmallBuf@@@Z"
+// CHECK: alloca %struct.SmallBuf, align 1, !stack-protector ![[LARGE_TRIV]]
+// CHECK-NOT: call void @llvm.memcpy
+// CHECK: }
+void param_in_register(SmallBuf b) { use(&b); }
+
// A parameter that is not a GS buffer is left where it is.
// CHECK-LABEL: @"?param_int@@YAXH at Z"
// CHECK-NOT: !stack-protector
diff --git a/clang/test/CodeGen/ms-stack-protector-gs-buffer.c b/clang/test/CodeGen/ms-stack-protector-gs-buffer.c
index aa536f1108932a..ac09cfd819c0e0 100644
--- a/clang/test/CodeGen/ms-stack-protector-gs-buffer.c
+++ b/clang/test/CodeGen/ms-stack-protector-gs-buffer.c
@@ -4,7 +4,8 @@
// 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.
+// Disabling optimizations does not disable the checks: cl.exe emits them at
+// /Od just as it does at /O1 and /O2.
// RUN: %clang_cc1 -triple x86_64-windows-msvc -fms-extensions \
// RUN: -stack-protector 4 -emit-llvm %s -o - | FileCheck %s --check-prefix=NOOPT
//
@@ -24,9 +25,11 @@ void plain(void) { char buf[64]; use(buf); }
// STRONG: define dso_local void @safe() {{.*}}#[[#SSAFE:]] {
__declspec(safebuffers) void safe(void) { char buf[64]; use(buf); }
+// __declspec(safebuffers) opts out at -O0 too.
+// NOOPT: define dso_local void @safe() #[[#SAFE:]] {
+
// __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.
+// functions, so it opts back out of the narrower GS-buffer rules.
// 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); }
@@ -38,9 +41,9 @@ __declspec(strict_gs_check) void strict(void) { char buf[64]; use(buf); }
// 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"
+// -O0 gets the same attributes, marker included.
+// NOOPT: attributes #[[#PLAIN]] = { noinline nounwind optnone sspstrong {{.*}}"stack-protector-buffer-size"="8" "stack-protector-gs-buffer"="true" "target-features"
+// NOOPT: attributes #[[#SAFE]] = { 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.
diff --git a/llvm/lib/CodeGen/StackProtector.cpp b/llvm/lib/CodeGen/StackProtector.cpp
index b8476f347f4cd5..6d0c7375b2ab70 100644
--- a/llvm/lib/CodeGen/StackProtector.cpp
+++ b/llvm/lib/CodeGen/StackProtector.cpp
@@ -517,8 +517,7 @@ static const CallInst *findStackProtectorIntrinsic(Function &F) {
/// it adds a guard variable to functions that call alloca regardless of size
/// and to functions with an alloca that the frontend marked as a "GS buffer"
/// using "stack-protector" metadata, but it does not consider a variable's
-/// address being taken, and it never protects a function that takes a variable
-/// argument list. Deciding what a GS buffer is needs source-level type
+/// address being taken. Deciding what a GS buffer is needs source-level type
/// information that lowering to IR does not preserve, which is why that part
/// of the heuristic lives in the frontend.
///
@@ -570,10 +569,6 @@ bool SSPLayoutAnalysis::requiresStackProtector(Function *F,
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) {
for (const Instruction &I : BB) {
if (const AllocaInst *AI = dyn_cast<AllocaInst>(&I)) {
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
index 3d4090b6f0ae16..74322d599bd286 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
@@ -199,9 +199,6 @@ static Instruction *simplifyAllocaArraySize(InstCombinerImpl &IC,
nullptr, AI.getName());
New->setAlignment(AI.getAlign());
New->setUsedWithInAlloca(AI.isUsedWithInAlloca());
- // It is the same allocation, so it keeps what was said about it, e.g.
- // "stack-protector".
- New->copyMetadata(AI);
replaceAllDbgUsesWith(AI, *New, *New, DT);
return IC.replaceInstUsesWith(AI, New);
diff --git a/llvm/test/CodeGen/X86/stack-protector-gs-buffer.ll b/llvm/test/CodeGen/X86/stack-protector-gs-buffer.ll
index c87a14b2186b26..f7549f731600fb 100644
--- a/llvm/test/CodeGen/X86/stack-protector-gs-buffer.ll
+++ b/llvm/test/CodeGen/X86/stack-protector-gs-buffer.ll
@@ -182,21 +182,32 @@ define void @strong_sret() #2 {
ret void
}
-;; --- Exclusions --------------------------------------------------------------
+;; --- Varargs -----------------------------------------------------------------
-; MSVC never protects a function that takes a variable argument list, even one
-; holding an obvious GS buffer.
+; A function that takes a variable argument list is protected like any other;
+; cl.exe does the same.
; CHECK-LABEL: variadic:
-; CHECK-NOT: __security_cookie
-; CHECK: .seh_endproc
+; CHECK: __security_cookie
define void @variadic(i32 %n, ...) #0 {
%a = alloca [64 x i8], !stack-protector !2
call void @use(ptr %a)
ret void
}
-; sspreq overrides the heuristic entirely, so the varargs exclusion does not
-; apply.
+; Taking a variable argument list is not on its own a reason to protect, so an
+; unmarked alloca in one still does not require a protector.
+; CHECK-LABEL: variadic_unmarked:
+; CHECK-NOT: __security_cookie
+; CHECK: .seh_endproc
+define void @variadic_unmarked(i32 %n, ...) #0 {
+ %a = alloca [64 x i8]
+ call void @use(ptr %a)
+ ret void
+}
+
+;; --- Other modes -------------------------------------------------------------
+
+; sspreq overrides the heuristic entirely.
; CHECK-LABEL: sspreq_wins:
; CHECK: __security_cookie
define void @sspreq_wins(i32 %n, ...) #1 {
diff --git a/llvm/test/Transforms/InstCombine/alloca.ll b/llvm/test/Transforms/InstCombine/alloca.ll
index e2113fa648509b..15b43e780f1497 100644
--- a/llvm/test/Transforms/InstCombine/alloca.ll
+++ b/llvm/test/Transforms/InstCombine/alloca.ll
@@ -259,21 +259,3 @@ define void @test_inalloca_with_element_count(ptr %a) {
call void @test9_aux(ptr inalloca(%struct_type) %alloca)
ret void
}
-
-; Folding the element count into the allocated type must not lose what was
-; said about the allocation.
-define void @test_metadata_is_preserved() {
-; ALL-LABEL: @test_metadata_is_preserved(
-; ALL-NEXT: [[ALLOCA1:%.*]] = alloca [10 x i8], align 1, !stack-protector [[META0:![0-9]+]]
-; ALL-NEXT: call void (...) @use(ptr nonnull [[ALLOCA1]])
-; ALL-NEXT: ret void
-;
- %alloca = alloca i8, i32 10, !stack-protector !0
- call void (...) @use(ptr %alloca)
- ret void
-}
-
-!0 = !{i32 2}
-;.
-; ALL: [[META0]] = !{i32 2}
-;.
>From a9f8ea3de60a5d6d0997de688db46b6eae0ff5cd Mon Sep 17 00:00:00 2001
From: "Pan, Tao" <tao.pan at intel.com>
Date: Fri, 16 Oct 2026 16:39:10 +0800
Subject: [PATCH 8/8] Restore Attribute::StructRet related logic
---
clang/lib/CodeGen/CGDecl.cpp | 25 ++----
clang/lib/CodeGen/CodeGenFunction.h | 6 +-
.../ms-stack-protector-gs-buffer-alloca.cpp | 16 ++--
.../ms-stack-protector-gs-buffer-temp.cpp | 11 +--
llvm/docs/LangRef.md | 11 +--
llvm/lib/CodeGen/StackProtector.cpp | 83 +----------------
.../CodeGen/X86/stack-protector-gs-buffer.ll | 90 ++-----------------
7 files changed, 32 insertions(+), 210 deletions(-)
diff --git a/clang/lib/CodeGen/CGDecl.cpp b/clang/lib/CodeGen/CGDecl.cpp
index dfebd65f637845..607550c94be9df 100644
--- a/clang/lib/CodeGen/CGDecl.cpp
+++ b/clang/lib/CodeGen/CGDecl.cpp
@@ -1718,8 +1718,7 @@ static bool isGSBuffer(const ASTContext &Ctx, QualType Ty,
return NeedsProtector;
}
-void CodeGenFunction::MarkGSBufferAlloca(llvm::AllocaInst *AI, bool IsLarge,
- bool IsTrivial) {
+void CodeGenFunction::MarkGSBufferAlloca(llvm::AllocaInst *AI, bool IsLarge) {
if (!MarkGSBuffers || !AI)
return;
@@ -1727,15 +1726,12 @@ void CodeGenFunction::MarkGSBufferAlloca(llvm::AllocaInst *AI, bool IsLarge,
if (AI->getMetadata("stack-protector"))
return;
- // 2 is a large buffer and 1 a small one. The second operand says the object
- // is trivial, and is left out when it is not, which is the common case; see
- // the "stack-protector" metadata in LLVM's LangRef.
+ // 2 is a large buffer and 1 a small one; see the "stack-protector" metadata
+ // in LLVM's LangRef.
llvm::LLVMContext &Ctx = Builder.getContext();
- SmallVector<llvm::Metadata *, 2> Ops = {
- llvm::ConstantAsMetadata::get(Builder.getInt32(IsLarge ? 2 : 1))};
- if (IsTrivial)
- Ops.push_back(llvm::ConstantAsMetadata::get(Builder.getTrue()));
- AI->setMetadata("stack-protector", llvm::MDNode::get(Ctx, Ops));
+ auto *Operand =
+ llvm::ConstantAsMetadata::get(Builder.getInt32(IsLarge ? 2 : 1));
+ AI->setMetadata("stack-protector", llvm::MDNode::get(Ctx, {Operand}));
}
void CodeGenFunction::EmitGSBufferStackProtectorMD(llvm::Value *AllocaPtr,
@@ -1752,8 +1748,7 @@ void CodeGenFunction::EmitGSBufferStackProtectorMD(llvm::Value *AllocaPtr,
// does not analyse; it protects the callee instead (see
// EmitGSBufferParamCopy). Once the type needs a constructor or a destructor
// the temporary becomes a real object, and cl.exe covers it.
- bool IsTrivial = Ty.isTrivialType(getContext());
- if (Kind != GSObjectKind::Named && IsTrivial)
+ if (Kind != GSObjectKind::Named && Ty.isTrivialType(getContext()))
return;
// For an outgoing by-value argument that is a real object, cl.exe only gets
@@ -1774,7 +1769,7 @@ void CodeGenFunction::EmitGSBufferStackProtectorMD(llvm::Value *AllocaPtr,
IsLarge))
return;
- MarkGSBufferAlloca(AI, IsLarge, IsTrivial);
+ MarkGSBufferAlloca(AI, IsLarge);
}
Address CodeGenFunction::EmitGSBufferParamCopy(const VarDecl &D,
@@ -1796,11 +1791,9 @@ Address CodeGenFunction::EmitGSBufferParamCopy(const VarDecl &D,
Ty, getContext().getDeclAlign(&D), D.getName() + "$GSCopy$");
EmitAggregateCopy(MakeAddrLValue(GSCopy, Ty), MakeAddrLValue(ParamAddr, Ty),
Ty, AggValueSlot::DoesNotOverlap);
- // This copy exists only so that the object sits inside the guarded region,
- // so it is never a candidate for the unguarded temporary area.
MarkGSBufferAlloca(
dyn_cast<llvm::AllocaInst>(GSCopy.getPointer()->stripPointerCasts()),
- IsLarge, /*IsTrivial=*/false);
+ IsLarge);
return GSCopy;
}
diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h
index 404296cb9c262c..e2fd49338629b4 100644
--- a/clang/lib/CodeGen/CodeGenFunction.h
+++ b/clang/lib/CodeGen/CodeGenFunction.h
@@ -3629,11 +3629,7 @@ class CodeGenFunction : public CodeGenTypeCache {
/// If this function uses MSVC's /GS (Buffer Security Check) heuristic, mark
/// \p AI as a "GS buffer", so that the function requires a stack protector.
/// \p IsLarge says the buffer should be laid out closest to the guard.
- /// \p IsTrivial says the allocation holds an object of trivial type, which
- /// cl.exe is willing to leave in the unguarded temporary area; see the
- /// "stack-protector" metadata in LLVM's LangRef.
- void MarkGSBufferAlloca(llvm::AllocaInst *AI, bool IsLarge,
- bool IsTrivial = false);
+ void MarkGSBufferAlloca(llvm::AllocaInst *AI, bool IsLarge);
void EmitAutoVarInit(const AutoVarEmission &emission);
void EmitAutoVarCleanups(const AutoVarEmission &emission);
diff --git a/clang/test/CodeGen/ms-stack-protector-gs-buffer-alloca.cpp b/clang/test/CodeGen/ms-stack-protector-gs-buffer-alloca.cpp
index 7c8ceebc5a205c..843e60ee2bccac 100644
--- a/clang/test/CodeGen/ms-stack-protector-gs-buffer-alloca.cpp
+++ b/clang/test/CodeGen/ms-stack-protector-gs-buffer-alloca.cpp
@@ -218,20 +218,20 @@ void holds_reference(int &x) { HoldsReference s(x); use(&s); }
// constant count does not stay one for long -- InstCombine folds it into the
// allocated type -- so the classification is recorded here as well.
// CHECK-LABEL: @"?alloca_large@@YAXXZ"
-// CHECK: alloca i8, i64 64, align 16, !stack-protector ![[LARGE_NT:[0-9]+]]
+// CHECK: alloca i8, i64 64, align 16, !stack-protector ![[LARGE]]
void alloca_large() { use(_alloca(64)); }
// Below ssp-buffer-size, but still a GS buffer: _alloca has no threshold.
// CHECK-LABEL: @"?alloca_small@@YAXXZ"
-// CHECK: alloca i8, i64 3, align 16, !stack-protector ![[SMALL_NT:[0-9]+]]
+// CHECK: alloca i8, i64 3, align 16, !stack-protector ![[SMALL]]
void alloca_small() { use(_alloca(3)); }
// CHECK-LABEL: @"?alloca_dynamic@@YAXH at Z"
-// CHECK: alloca i8, i64 %{{.*}}, align 16, !stack-protector ![[LARGE_NT]]
+// CHECK: alloca i8, i64 %{{.*}}, align 16, !stack-protector ![[LARGE]]
void alloca_dynamic(int n) { use(_alloca(n)); }
// CHECK-LABEL: @"?alloca_with_align@@YAXXZ"
-// CHECK: alloca i8, i64 64, align 16, !stack-protector ![[LARGE_NT]]
+// CHECK: alloca i8, i64 64, align 16, !stack-protector ![[LARGE]]
void alloca_with_align() { use(__builtin_alloca_with_align(64, 128)); }
//--- Left to the backend ------------------------------------------------------
@@ -263,10 +263,6 @@ __declspec(safebuffers) void safe() { char a[64]; use(a); }
// CHECK: alloca [64 x i8], align 1{{$}}
__declspec(strict_gs_check) void strict() { char a[64]; use(a); }
-// The second operand says the object is trivial, and so is one the backend may
-// leave in the unguarded temporary area; see ms-stack-protector-gs-buffer-temp.
-// CHECK-DAG: ![[LARGE]] = !{i32 2, i1 true}
-// CHECK-DAG: ![[SMALL]] = !{i32 1, i1 true}
-// CHECK-DAG: ![[LARGE_NT]] = !{i32 2}
-// CHECK-DAG: ![[SMALL_NT]] = !{i32 1}
+// CHECK-DAG: ![[LARGE]] = !{i32 2}
+// CHECK-DAG: ![[SMALL]] = !{i32 1}
// CHECK-DAG: ![[IGNORE]] = !{i32 0}
diff --git a/clang/test/CodeGen/ms-stack-protector-gs-buffer-temp.cpp b/clang/test/CodeGen/ms-stack-protector-gs-buffer-temp.cpp
index 384b03879ba75e..11c723e3b01441 100644
--- a/clang/test/CodeGen/ms-stack-protector-gs-buffer-temp.cpp
+++ b/clang/test/CodeGen/ms-stack-protector-gs-buffer-temp.cpp
@@ -38,16 +38,14 @@ void discard_trivial() { makeBuf(); }
void discard_dtor() { makeDtor(); }
// A temporary the source binds to a reference is a declared object, so it is
-// classified even when trivial. The second operand lets the backend drop it
-// again if its address turns out to be used for nothing but the return value;
-// see llvm/test/CodeGen/X86/stack-protector-gs-buffer.ll.
+// classified even when trivial.
// CHECK-LABEL: @"?bind_trivial@@YAXXZ"
-// CHECK: alloca %struct.Buf, align 1, !stack-protector ![[LARGE_TRIV:[0-9]+]]
+// CHECK: alloca %struct.Buf, align 1, !stack-protector ![[LARGE]]
void bind_trivial() { const Buf &r = makeBuf(); use(&r); }
// Likewise a declared local, which clang gives to the callee directly.
// CHECK-LABEL: @"?local_from_call@@YAXXZ"
-// CHECK: alloca %struct.Buf, align 1, !stack-protector ![[LARGE_TRIV]]
+// CHECK: alloca %struct.Buf, align 1, !stack-protector ![[LARGE]]
void local_from_call() { Buf b = makeBuf(); use(&b); }
//--- Outgoing by-value arguments ----------------------------------------------
@@ -90,7 +88,7 @@ void param(Buf b) { use(&b); }
// a slot of this frame's own, which the guard already covers. Nothing has to be
// relocated, but it is still a declared object and still gets classified.
// CHECK-LABEL: @"?param_in_register@@YAXUSmallBuf@@@Z"
-// CHECK: alloca %struct.SmallBuf, align 1, !stack-protector ![[LARGE_TRIV]]
+// CHECK: alloca %struct.SmallBuf, align 1, !stack-protector ![[LARGE]]
// CHECK-NOT: call void @llvm.memcpy
// CHECK: }
void param_in_register(SmallBuf b) { use(&b); }
@@ -102,4 +100,3 @@ void param_in_register(SmallBuf b) { use(&b); }
void param_int(int n) { use(&n); }
// CHECK-DAG: ![[LARGE]] = !{i32 2}
-// CHECK-DAG: ![[LARGE_TRIV]] = !{i32 2, i1 true}
diff --git a/llvm/docs/LangRef.md b/llvm/docs/LangRef.md
index 99bc6e1c492487..c4005892dbdaff 100644
--- a/llvm/docs/LangRef.md
+++ b/llvm/docs/LangRef.md
@@ -9216,7 +9216,7 @@ and `"stack-protector-gs-buffer"` function attributes would otherwise apply.
The function may still use a stack protector, if other criteria determine it
needs one.
-The metadata contains an integer, and optionally a second, boolean operand:
+The metadata contains a single integer:
- `0` opts the given alloca out of requiring a stack protector.
- `1` marks the given alloca as a buffer that requires a stack protector, and
@@ -9228,19 +9228,12 @@ The non-zero values let a frontend classify an allocation itself, which is
necessary when the decision depends on source-level information that lowering
to IR does not preserve.
-A second operand of `i1 true` says the allocation holds an object of trivial
-type, one the backend is therefore free to relocate. Only the
-`"stack-protector-gs-buffer"` heuristic reads it, and only to leave out an
-allocation whose address is used for nothing but receiving the indirect return
-value of a call: MSVC gives such an object a slot outside the region the stack
-cookie guards. The operand may be omitted, and is then taken as `false`.
-
```
%a = alloca [1000 x i8], align 1, !stack-protector !0
%b = alloca [1000 x i8], align 1, !stack-protector !1
!0 = !{i32 0}
-!1 = !{i32 2, i1 true}
+!1 = !{i32 2}
```
#### '`implicit.ref`' Metadata
diff --git a/llvm/lib/CodeGen/StackProtector.cpp b/llvm/lib/CodeGen/StackProtector.cpp
index 6d0c7375b2ab70..a264d16986ac3c 100644
--- a/llvm/lib/CodeGen/StackProtector.cpp
+++ b/llvm/lib/CodeGen/StackProtector.cpp
@@ -80,88 +80,19 @@ enum SSPAllocaMD : uint32_t {
SSPAMD_LargeBuffer = 2,
};
-/// Read the "stack-protector" metadata of \p AI, if it has any. \p IsTrivial is
-/// set from the optional second operand, which says the allocation holds an
-/// object of trivial type.
-static std::optional<uint32_t> getSSPAllocaMD(const AllocaInst &AI,
- bool &IsTrivial) {
- IsTrivial = false;
+/// Read the "stack-protector" metadata of \p AI, if it has any.
+static std::optional<uint32_t> getSSPAllocaMD(const AllocaInst &AI) {
const MDNode *MD = AI.getMetadata("stack-protector");
- if (!MD || MD->getNumOperands() < 1 || MD->getNumOperands() > 2)
+ if (!MD || MD->getNumOperands() != 1)
return std::nullopt;
const auto *CI = mdconst::dyn_extract<ConstantInt>(MD->getOperand(0));
if (!CI)
return std::nullopt;
- if (MD->getNumOperands() == 2)
- if (const auto *Flag = mdconst::dyn_extract<ConstantInt>(MD->getOperand(1)))
- IsTrivial = !Flag->isZero();
// Clamp, so that a value from a newer producer errs towards more protection
// rather than less.
return CI->getLimitedValue(SSPAMD_LargeBuffer);
}
-/// Returns true if the only thing \p AI's address is ever used for, besides
-/// being read and written here, is receiving the indirect return value of a
-/// call.
-///
-/// cl.exe gives such an allocation a slot in the frame's temporary area, which
-/// the cookie does not cover, and leaves it out of the GS analysis: the only
-/// code that writes it from outside is a callee, and a callee writes exactly
-/// one object of the type it returns. As soon as the address reaches anywhere
-/// else, cl.exe relocates the object into the protected region instead, so look
-/// for that here rather than in the frontend, which cannot see the uses.
-///
-/// Only an object cl.exe is free to relocate can end up in that area, which is
-/// why the caller also requires the allocation to hold a trivial object.
-static bool isIndirectReturnSlot(const AllocaInst &AI) {
- bool FoundSRet = false;
- SmallVector<const Instruction *, 8> Worklist = {&AI};
- SmallPtrSet<const Instruction *, 8> Visited = {&AI};
-
- while (!Worklist.empty()) {
- for (const Use &U : Worklist.pop_back_val()->uses()) {
- const auto *User = cast<Instruction>(U.getUser());
-
- if (const auto *CB = dyn_cast<CallBase>(User)) {
- const auto *II = dyn_cast<IntrinsicInst>(CB);
- if (II && (II->isLifetimeStartOrEnd() || II->isAssumeLikeIntrinsic()))
- continue;
- // A memory intrinsic reads or writes through the pointer, just as a
- // load or a store does, and does not let it escape.
- if (isa_and_nonnull<MemIntrinsic>(II))
- continue;
- if (!CB->isArgOperand(&U) ||
- !CB->paramHasAttr(CB->getArgOperandNo(&U), Attribute::StructRet))
- return false;
- FoundSRet = true;
- continue;
- }
-
- switch (User->getOpcode()) {
- case Instruction::Load:
- continue;
- case Instruction::Store:
- // Writing through the pointer is fine; writing the pointer itself
- // somewhere else lets it escape.
- if (U.getOperandNo() != StoreInst::getPointerOperandIndex())
- return false;
- continue;
- case Instruction::BitCast:
- case Instruction::AddrSpaceCast:
- case Instruction::GetElementPtr:
- case Instruction::PHI:
- case Instruction::Select:
- if (Visited.insert(User).second)
- Worklist.push_back(User);
- continue;
- default:
- return false;
- }
- }
- }
- return FoundSRet;
-}
-
/// InsertStackProtectors - Insert code into the prologue and epilogue of the
/// function.
///
@@ -572,16 +503,10 @@ bool SSPLayoutAnalysis::requiresStackProtector(Function *F,
for (const BasicBlock &BB : *F) {
for (const Instruction &I : BB) {
if (const AllocaInst *AI = dyn_cast<AllocaInst>(&I)) {
- bool IsTrivial = false;
- if (std::optional<uint32_t> MD = getSSPAllocaMD(*AI, IsTrivial)) {
+ if (std::optional<uint32_t> MD = getSSPAllocaMD(*AI)) {
if (*MD == SSPAMD_Ignore)
continue;
- // MSVC leaves a trivial object that is only an indirect-return
- // destination out of its GS analysis.
- if (GSBuffer && IsTrivial && isIndirectReturnSlot(*AI))
- continue;
-
// The frontend classified this allocation as a buffer, which is
// final: it knows things about the allocation that the IR no longer
// records. This is how clang-cl communicates MSVC's "GS buffer"
diff --git a/llvm/test/CodeGen/X86/stack-protector-gs-buffer.ll b/llvm/test/CodeGen/X86/stack-protector-gs-buffer.ll
index f7549f731600fb..de32177e13e334 100644
--- a/llvm/test/CodeGen/X86/stack-protector-gs-buffer.ll
+++ b/llvm/test/CodeGen/X86/stack-protector-gs-buffer.ll
@@ -9,7 +9,6 @@
declare void @use(ptr)
declare void @make(ptr sret([64 x i8]))
-declare void @llvm.memcpy.p0.p0.i64(ptr, ptr, i64, i1)
;; --- Unmarked allocas --------------------------------------------------------
@@ -94,94 +93,18 @@ define void @small_alloca() #0 {
;; --- Indirect return slots ---------------------------------------------------
-; MSVC gives an object it is free to relocate and that only ever receives a
-; call's indirect return value a frame slot the cookie does not guard. The
-; frontend cannot see that, so the second metadata operand tells the backend the
-; object is trivial and the backend checks the uses.
-; CHECK-LABEL: sret_only:
-; CHECK-NOT: __security_cookie
-; CHECK: .seh_endproc
-define void @sret_only() #0 {
- %a = alloca [64 x i8], !stack-protector !3
- call void @make(ptr sret([64 x i8]) %a)
- %v = load i8, ptr %a
- store i8 %v, ptr %a
- ret void
-}
-
-; Reading and writing through the pointer, including with a memcpy, is not an
-; escape, and several returns into the same slot are still just returns.
-; CHECK-LABEL: sret_twice_and_memcpy:
-; CHECK-NOT: __security_cookie
-; CHECK: .seh_endproc
-define void @sret_twice_and_memcpy(ptr %p) #0 {
- %a = alloca [64 x i8], !stack-protector !3
- call void @make(ptr sret([64 x i8]) %a)
- call void @make(ptr sret([64 x i8]) %a)
- call void @llvm.memcpy.p0.p0.i64(ptr %p, ptr %a, i64 64, i1 false)
- ret void
-}
-
-; As soon as the address reaches anywhere else, MSVC moves the object into the
-; guarded region instead.
-; CHECK-LABEL: sret_escapes:
-; CHECK: __security_cookie
-define void @sret_escapes() #0 {
- %a = alloca [64 x i8], !stack-protector !3
- call void @make(ptr sret([64 x i8]) %a)
- call void @use(ptr %a)
- ret void
-}
-
-; A derived pointer escaping counts just the same.
-; CHECK-LABEL: sret_gep_escapes:
+; A marked alloca is protected even when all it does is receive a call's
+; indirect return value. cl.exe leaves such a slot alone, but telling that apart
+; from a buffer that is merely returned into needs more than the backend can
+; see, so it is protected like any other marked alloca.
+; CHECK-LABEL: sret_slot:
; CHECK: __security_cookie
-define void @sret_gep_escapes() #0 {
- %a = alloca [64 x i8], !stack-protector !3
- call void @make(ptr sret([64 x i8]) %a)
- %g = getelementptr [64 x i8], ptr %a, i64 0, i64 8
- call void @use(ptr %g)
- ret void
-}
-
-; Storing the pointer itself is an escape; storing through it is not.
-; CHECK-LABEL: sret_pointer_stored:
-; CHECK: __security_cookie
-define void @sret_pointer_stored(ptr %p) #0 {
- %a = alloca [64 x i8], !stack-protector !3
- call void @make(ptr sret([64 x i8]) %a)
- store ptr %a, ptr %p
- ret void
-}
-
-; Without an indirect return there is nothing to exempt.
-; CHECK-LABEL: trivial_without_sret:
-; CHECK: __security_cookie
-define void @trivial_without_sret() #0 {
- %a = alloca [64 x i8], !stack-protector !3
- store i8 0, ptr %a
- ret void
-}
-
-; The exemption needs the object to be one MSVC may relocate, which is what the
-; second operand says. Without it the slot is protected.
-; CHECK-LABEL: nontrivial_sret:
-; CHECK: __security_cookie
-define void @nontrivial_sret() #0 {
+define void @sret_slot() #0 {
%a = alloca [64 x i8], !stack-protector !2
call void @make(ptr sret([64 x i8]) %a)
ret void
}
-; The exemption is specific to the /GS heuristic.
-; CHECK-LABEL: strong_sret:
-; CHECK: __security_cookie
-define void @strong_sret() #2 {
- %a = alloca [64 x i8], !stack-protector !3
- call void @make(ptr sret([64 x i8]) %a)
- ret void
-}
-
;; --- Varargs -----------------------------------------------------------------
; A function that takes a variable argument list is protected like any other;
@@ -243,4 +166,3 @@ attributes #2 = { sspstrong uwtable }
!0 = !{i32 0}
!1 = !{i32 1}
!2 = !{i32 2}
-!3 = !{i32 2, i1 true}
More information about the cfe-commits
mailing list