rsanthir.quic created this revision.
Herald added subscribers: dexonsmith, dang, hiraditya.
rsanthir.quic requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.
This patch adds support for the -Wstack-usage flag. It also changes
how -Wframe-larger-than reports it's frame size. It now excludes the
space allocated to hold parameters for called functions.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D101964
Files:
clang/include/clang/Basic/DiagnosticFrontendKinds.td
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Driver/Options.td
clang/lib/CodeGen/CodeGenAction.cpp
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Frontend/backend-diagnostic.c
clang/test/Frontend/backend-stack-usage-diagnostic.c
clang/test/Misc/backend-stack-frame-diagnostics-fallback.cpp
llvm/include/llvm/CodeGen/MachineFrameInfo.h
llvm/include/llvm/IR/DiagnosticInfo.h
llvm/lib/CodeGen/PrologEpilogInserter.cpp
Index: llvm/lib/CodeGen/PrologEpilogInserter.cpp
===================================================================
--- llvm/lib/CodeGen/PrologEpilogInserter.cpp
+++ llvm/lib/CodeGen/PrologEpilogInserter.cpp
@@ -139,9 +139,14 @@
char &llvm::PrologEpilogCodeInserterID = PEI::ID;
static cl::opt<unsigned>
-WarnStackSize("warn-stack-size", cl::Hidden, cl::init((unsigned)-1),
- cl::desc("Warn for stack size bigger than the given"
- " number"));
+ WarnFrameSize("warn-frame-size", cl::Hidden, cl::init((unsigned)-1),
+ cl::desc("Warn for frame size bigger than the given"
+ " number"));
+
+static cl::opt<unsigned>
+ WarnStackUsage("warn-stack-usage", cl::Hidden, cl::init((unsigned)-1),
+ cl::desc("Warn for stack size bigger than the given"
+ " number"));
INITIALIZE_PASS_BEGIN(PEI, DEBUG_TYPE, "Prologue/Epilogue Insertion", false,
false)
@@ -275,11 +280,19 @@
if (TRI->requiresRegisterScavenging(MF) && FrameIndexVirtualScavenging)
scavengeFrameVirtualRegs(MF, *RS);
- // Warn on stack size when we exceeds the given limit.
+ // Warn on stack usage size when we exceed the given limit
MachineFrameInfo &MFI = MF.getFrameInfo();
uint64_t StackSize = MFI.getStackSize();
- if (WarnStackSize.getNumOccurrences() > 0 && WarnStackSize < StackSize) {
- DiagnosticInfoStackSize DiagStackSize(F, StackSize);
+ if (WarnStackUsage.getNumOccurrences() > 0 && WarnStackUsage <= StackSize) {
+ DiagnosticInfoStackSize DiagStackUsage(
+ F, StackSize, MFI.hasVarSizedObjects(), true /*Stack Usage*/);
+ F.getContext().diagnose(DiagStackUsage);
+ }
+
+ uint64_t FrameSize = MFI.getFrameSize();
+ // Warn on Frame size when we exceed the given limit.
+ if (WarnFrameSize.getNumOccurrences() > 0 && WarnFrameSize < FrameSize) {
+ DiagnosticInfoStackSize DiagStackSize(F, FrameSize);
F.getContext().diagnose(DiagStackSize);
}
ORE->emit([&]() {
@@ -1048,6 +1061,10 @@
AdjustStackOffset(MFI, SFI, StackGrowsDown, Offset, MaxAlign, Skew);
}
+ int64_t OffsetBeforeMaxCallFrameSize = Offset;
+ int64_t FrameSize = OffsetBeforeMaxCallFrameSize - LocalAreaOffset;
+ MFI.setFrameSize(FrameSize);
+
if (!TFI.targetHandlesStackFrameRounding()) {
// If we have reserved argument space for call sites in the function
// immediately on entry to the current function, count it as part of the
@@ -1073,6 +1090,11 @@
int64_t OffsetBeforeAlignment = Offset;
Offset = alignTo(Offset, StackAlign, Skew);
+ OffsetBeforeMaxCallFrameSize =
+ alignTo(OffsetBeforeMaxCallFrameSize, StackAlign, Skew);
+ int64_t FrameSize = OffsetBeforeMaxCallFrameSize - LocalAreaOffset;
+ MFI.setFrameSize(FrameSize);
+
// If we have increased the offset to fulfill the alignment constrants,
// then the scavenging spill slots may become harder to reach from the
// stack pointer, float them so they stay close.
Index: llvm/include/llvm/IR/DiagnosticInfo.h
===================================================================
--- llvm/include/llvm/IR/DiagnosticInfo.h
+++ llvm/include/llvm/IR/DiagnosticInfo.h
@@ -216,15 +216,24 @@
class DiagnosticInfoStackSize : public DiagnosticInfoResourceLimit {
void anchor() override;
+ bool HasVarSizedObjects;
+ bool PrintStackUsageInfo;
+
public:
DiagnosticInfoStackSize(const Function &Fn, uint64_t StackSize,
+ bool HasVarSizedObjects = false,
+ bool PrintStackUsageInfo = false,
DiagnosticSeverity Severity = DS_Warning,
uint64_t StackLimit = 0)
: DiagnosticInfoResourceLimit(Fn, "stack size", StackSize, Severity,
- DK_StackSize, StackLimit) {}
+ DK_StackSize, StackLimit),
+ HasVarSizedObjects(HasVarSizedObjects),
+ PrintStackUsageInfo(PrintStackUsageInfo) {}
uint64_t getStackSize() const { return getResourceSize(); }
uint64_t getStackLimit() const { return getResourceLimit(); }
+ bool hasVarSizedObjects() const { return HasVarSizedObjects; }
+ bool printStackUsageInfo() const { return PrintStackUsageInfo; }
static bool classof(const DiagnosticInfo *DI) {
return DI->getKind() == DK_StackSize;
Index: llvm/include/llvm/CodeGen/MachineFrameInfo.h
===================================================================
--- llvm/include/llvm/CodeGen/MachineFrameInfo.h
+++ llvm/include/llvm/CodeGen/MachineFrameInfo.h
@@ -243,6 +243,10 @@
/// to be allocated on entry to the function.
uint64_t StackSize = 0;
+ /// The FrameSize is the StackSize above, minus any reserved argument space
+ /// for call sites in the function.
+ uint64_t FrameSize = 0;
+
/// The amount that a frame offset needs to be adjusted to
/// have the actual offset from the stack/frame pointer. The exact usage of
/// this is target-dependent, but it is typically used to adjust between
@@ -568,6 +572,14 @@
/// Estimate and return the size of the stack frame.
uint64_t estimateStackSize(const MachineFunction &MF) const;
+ /// Returns the StackSize without the stack space allocated to hold
+ /// parameters for other functions called from inside the current
+ /// function
+ uint64_t getFrameSize() const { return FrameSize; }
+
+ /// Set the size of the frame.
+ void setFrameSize(uint64_t Size) { FrameSize = Size; }
+
/// Return the correction for frame offsets.
int getOffsetAdjustment() const { return OffsetAdjustment; }
Index: clang/test/Misc/backend-stack-frame-diagnostics-fallback.cpp
===================================================================
--- clang/test/Misc/backend-stack-frame-diagnostics-fallback.cpp
+++ clang/test/Misc/backend-stack-frame-diagnostics-fallback.cpp
@@ -1,5 +1,5 @@
// REQUIRES: x86-registered-target
-// RUN: %clang_cc1 %s -mllvm -warn-stack-size=0 -emit-codegen-only -triple=i386-apple-darwin 2>&1 | FileCheck %s
+// RUN: %clang_cc1 %s -mllvm -warn-frame-size=0 -emit-codegen-only -triple=i386-apple-darwin 2>&1 | FileCheck %s
// TODO: Emit rich diagnostics for thunks and move this into the appropriate test file.
// Until then, test that we fall back and display the LLVM backend diagnostic.
Index: clang/test/Frontend/backend-stack-usage-diagnostic.c
===================================================================
--- /dev/null
+++ clang/test/Frontend/backend-stack-usage-diagnostic.c
@@ -0,0 +1,24 @@
+
+// RUN: %clang -Wstack-usage=0 -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=WARN
+// RUN: %clang -Wstack-usage=0 -Wno-stack-usage= -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=IGNORE --allow-empty
+// RUN: %clang -Wstack-usage=0 -w -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=IGNORE --allow-empty
+// RUN: %clang -Wstack-usage=3 -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=WARN
+// RUN: %clang -Wstack-usage=100 -o /dev/null -c %s 2> %t.err
+// RUN: FileCheck < %t.err %s --check-prefix=IGNORE --allow-empty
+
+// WARN: warning: stack usage is {{[0-9]+}} bytes in function 'test_square'
+// IGNORE-NOT: stack usage is {{[0-9]+}} bytes in function 'test_square'
+int test_square(int num) {
+ return num * num;
+}
+
+// WARN: warning: stack usage might be {{[0-9]+}} bytes in function 'test_unbounded'
+// IGNORE-NOT: stack usage might be {{[0-9]+}} bytes in function 'test_unbounded'
+int test_unbounded(int len) {
+ char a[len];
+ return 1;
+}
Index: clang/test/Frontend/backend-diagnostic.c
===================================================================
--- clang/test/Frontend/backend-diagnostic.c
+++ clang/test/Frontend/backend-diagnostic.c
@@ -4,11 +4,11 @@
// _PROMOTE_: Promote warning to error.
// _IGNORE_: Drop backend warning.
//
-// RUN: not %clang_cc1 %s -mllvm -warn-stack-size=0 -no-integrated-as -S -o - -triple=i386-apple-darwin 2> %t.err
+// RUN: not %clang_cc1 %s -mllvm -warn-frame-size=0 -no-integrated-as -S -o - -triple=i386-apple-darwin 2> %t.err
// RUN: FileCheck < %t.err %s --check-prefix=REGULAR --check-prefix=ASM
-// RUN: not %clang_cc1 %s -mllvm -warn-stack-size=0 -no-integrated-as -S -o - -triple=i386-apple-darwin -Werror=frame-larger-than= 2> %t.err
+// RUN: not %clang_cc1 %s -mllvm -warn-frame-size=0 -no-integrated-as -S -o - -triple=i386-apple-darwin -Werror=frame-larger-than= 2> %t.err
// RUN: FileCheck < %t.err %s --check-prefix=PROMOTE --check-prefix=ASM
-// RUN: not %clang_cc1 %s -mllvm -warn-stack-size=0 -no-integrated-as -S -o - -triple=i386-apple-darwin -Wno-frame-larger-than= 2> %t.err
+// RUN: not %clang_cc1 %s -mllvm -warn-frame-size=0 -no-integrated-as -S -o - -triple=i386-apple-darwin -Wno-frame-larger-than= 2> %t.err
// RUN: FileCheck < %t.err %s --check-prefix=IGNORE --check-prefix=ASM
//
// RUN: %clang_cc1 %s -S -o - -triple=i386-apple-darwin -verify -no-integrated-as
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4777,10 +4777,16 @@
if (Arg *A = Args.getLastArg(options::OPT_Wframe_larger_than_EQ)) {
StringRef v = A->getValue();
CmdArgs.push_back("-mllvm");
- CmdArgs.push_back(Args.MakeArgString("-warn-stack-size=" + v));
+ CmdArgs.push_back(Args.MakeArgString("-warn-frame-size=" + v));
A->claim();
}
+ if (Arg *A = Args.getLastArg(options::OPT_Wstack_usage_EQ)) {
+ StringRef bytes = A->getValue();
+ CmdArgs.push_back("-mllvm");
+ CmdArgs.push_back(Args.MakeArgString("-warn-stack-usage=" + bytes));
+ }
+
if (!Args.hasFlag(options::OPT_fjump_tables, options::OPT_fno_jump_tables,
true))
CmdArgs.push_back("-fno-jump-tables");
Index: clang/lib/CodeGen/CodeGenAction.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenAction.cpp
+++ clang/lib/CodeGen/CodeGenAction.cpp
@@ -567,15 +567,19 @@
// We do not know how to format other severities.
return false;
- if (const Decl *ND = Gen->GetDeclForMangledName(D.getFunction().getName())) {
- // FIXME: Shouldn't need to truncate to uint32_t
- Diags.Report(ND->getASTContext().getFullLoc(ND->getLocation()),
- diag::warn_fe_frame_larger_than)
+ const Decl *ND = Gen->GetDeclForMangledName(D.getFunction().getName());
+ if (!ND)
+ return false;
+
+ unsigned DiagID = diag::warn_fe_frame_larger_than;
+ if (D.printStackUsageInfo())
+ DiagID = D.hasVarSizedObjects() ? diag::warn_fe_stack_usage_dynam
+ : diag::warn_fe_stack_usage;
+ // FIXME: Shouldn't need to truncate to uint32_t
+ Diags.Report(ND->getASTContext().getFullLoc(ND->getLocation()), DiagID)
<< static_cast<uint32_t>(D.getStackSize()) << Decl::castToDeclContext(ND);
- return true;
- }
- return false;
+ return true;
}
const FullSourceLoc BackendConsumer::getBestLocationFromDebugLoc(
@@ -774,7 +778,10 @@
case llvm::DK_StackSize:
if (StackSizeDiagHandler(cast<DiagnosticInfoStackSize>(DI)))
return;
- ComputeDiagID(Severity, backend_frame_larger_than, DiagID);
+ if (cast<DiagnosticInfoStackSize>(DI).printStackUsageInfo())
+ ComputeDiagID(Severity, backend_stack_usage, DiagID);
+ else
+ ComputeDiagID(Severity, backend_frame_larger_than, DiagID);
break;
case DK_Linker:
assert(CurLinkModule);
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -2537,6 +2537,8 @@
def Wlarger_than_ : Joined<["-"], "Wlarger-than-">, Alias<Wlarger_than_EQ>;
def Wframe_larger_than_EQ : Joined<["-"], "Wframe-larger-than=">, Group<f_Group>, Flags<[NoXarchOption]>;
+def Wstack_usage_EQ : Joined<["-"], "Wstack-usage=">, Group<f_Group>, Flags<[NoXarchOption]>;
+
def : Flag<["-"], "fterminated-vtables">, Alias<fapple_kext>;
defm threadsafe_statics : BoolFOption<"threadsafe-statics",
LangOpts<"ThreadsafeStatics">, DefaultTrue,
Index: clang/include/clang/Basic/DiagnosticGroups.td
===================================================================
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1189,6 +1189,7 @@
def BackendInlineAsm : DiagGroup<"inline-asm">;
def BackendSourceMgr : DiagGroup<"source-mgr">;
def BackendFrameLargerThanEQ : DiagGroup<"frame-larger-than=">;
+def BackendStackUsageEQ : DiagGroup<"stack-usage=">;
def BackendPlugin : DiagGroup<"backend-plugin">;
def RemarkBackendPlugin : DiagGroup<"remark-backend-plugin">;
def BackendOptimizationRemark : DiagGroup<"pass">;
Index: clang/include/clang/Basic/DiagnosticFrontendKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticFrontendKinds.td
+++ clang/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -25,6 +25,15 @@
def err_fe_cannot_link_module : Error<"cannot link module '%0': %1">,
DefaultFatal;
+def warn_fe_stack_usage : Warning<"stack usage is %0 bytes in %1">,
+ BackendInfo, InGroup<BackendStackUsageEQ>;
+def warn_fe_stack_usage_dynam : Warning<"stack usage might be %0 bytes in %1">,
+ BackendInfo, InGroup<BackendStackUsageEQ>;
+def warn_fe_backend_stack_usage: Warning<"%0">,
+ BackendInfo, InGroup<BackendStackUsageEQ>;
+def err_fe_backend_stack_usage: Error<"%0">, BackendInfo;
+def note_fe_backend_stack_usage: Note<"%0">, BackendInfo;
+
def warn_fe_frame_larger_than : Warning<"stack frame size of %0 bytes in %q1">,
BackendInfo, InGroup<BackendFrameLargerThanEQ>;
def warn_fe_backend_frame_larger_than: Warning<"%0">,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
