[PATCH] Implement -Wframe-larger-than backend diagnostic
Alp Toker
alp at nuanti.com
Fri May 30 09:23:34 PDT 2014
The attached patch adds driver and frontend support for the GCC
-Wframe-larger-than=bytes warning.
As the first GCC-compatible backend diagnostic built around LLVM's
reporting feature the patch adds infrastructure to perform reverse
lookup from mangled names emitted after LLVM code generation. Using that
we resolve precise locations and originating AST functions, lambdas or
block declarations to provide rich codegen-guided diagnostics.
Supporting changes:
* Get rid of the old MangleBuffer class that was only used for blocks
* Let StringMap maintain unique mangled name strings instead of
allocating copies
The test intentionally conflates driver, frontend and backend facilities
for now, and I expect parts of the approach to evolve as we get a feel
for how it'll play ball with LTO etc.
Alp.
--
http://www.nuanti.com
the browser experts
-------------- next part --------------
diff --git a/include/clang/AST/Mangle.h b/include/clang/AST/Mangle.h
index e740836..a8d1199 100644
--- a/include/clang/AST/Mangle.h
+++ b/include/clang/AST/Mangle.h
@@ -36,33 +36,6 @@ namespace clang {
struct ThunkInfo;
class VarDecl;
-/// MangleBuffer - a convenient class for storing a name which is
-/// either the result of a mangling or is a constant string with
-/// external memory ownership.
-class MangleBuffer {
-public:
- void setString(StringRef Ref) {
- String = Ref;
- }
-
- SmallVectorImpl<char> &getBuffer() {
- return Buffer;
- }
-
- StringRef getString() const {
- if (!String.empty()) return String;
- return Buffer.str();
- }
-
- operator StringRef() const {
- return getString();
- }
-
-private:
- StringRef String;
- SmallString<256> Buffer;
-};
-
/// MangleContext - Context for tracking state which persists across multiple
/// calls to the C++ name mangler.
class MangleContext {
diff --git a/include/clang/Basic/DiagnosticFrontendKinds.td b/include/clang/Basic/DiagnosticFrontendKinds.td
index 03ed691..3527e8e 100644
--- a/include/clang/Basic/DiagnosticFrontendKinds.td
+++ b/include/clang/Basic/DiagnosticFrontendKinds.td
@@ -22,8 +22,9 @@ def note_fe_inline_asm_here : Note<"instantiated into assembly here">;
def err_fe_cannot_link_module : Error<"cannot link module '%0': %1">,
DefaultFatal;
-def warn_fe_backend_frame_larger_than: Warning<"stack size exceeded (%0) in %1">,
+def warn_fe_frame_larger_than : Warning<"stack frame size of %0 bytes in %q1">,
CatBackend, InGroup<BackendFrameLargerThan>;
+def warn_fe_backend_frame_larger_than: Warning<"%0">, CatBackend, InGroup<BackendFrameLargerThan>;
def err_fe_backend_frame_larger_than: Error<"%0">, CatBackend;
def note_fe_backend_frame_larger_than: Note<"%0">, CatBackend;
diff --git a/include/clang/CodeGen/ModuleBuilder.h b/include/clang/CodeGen/ModuleBuilder.h
index cda7863..9499374 100644
--- a/include/clang/CodeGen/ModuleBuilder.h
+++ b/include/clang/CodeGen/ModuleBuilder.h
@@ -27,12 +27,14 @@ namespace clang {
class LangOptions;
class CodeGenOptions;
class TargetOptions;
+ class Decl;
class CodeGenerator : public ASTConsumer {
virtual void anchor();
public:
virtual llvm::Module* GetModule() = 0;
virtual llvm::Module* ReleaseModule() = 0;
+ virtual const Decl *Demangle(llvm::StringRef MangledName) = 0;
};
/// CreateLLVMCodeGen - Create a CodeGenerator instance.
diff --git a/include/clang/Driver/Options.td b/include/clang/Driver/Options.td
index d277551..6dc9c23 100644
--- a/include/clang/Driver/Options.td
+++ b/include/clang/Driver/Options.td
@@ -854,12 +854,12 @@ def Wlarge_by_value_copy_def : Flag<["-"], "Wlarge-by-value-copy">,
"in bytes than a given value">, Flags<[HelpHidden]>;
def Wlarge_by_value_copy_EQ : Joined<["-"], "Wlarge-by-value-copy=">, Flags<[CC1Option]>;
-// Just silence warnings about -Wlarger-than, -Wframe-larger-than for now.
+// Just silence warnings about -Wlarger-than for now.
def Wlarger_than : Separate<["-"], "Wlarger-than">, Group<clang_ignored_f_Group>;
def Wlarger_than_EQ : Joined<["-"], "Wlarger-than=">, Alias<Wlarger_than>;
def Wlarger_than_ : Joined<["-"], "Wlarger-than-">, Alias<Wlarger_than>;
-def Wframe_larger_than : Separate<["-"], "Wframe-larger-than">, Group<clang_ignored_f_Group>;
-def Wframe_larger_than_EQ : Joined<["-"], "Wframe-larger-than=">, Alias<Wframe_larger_than>;
+def Wframe_larger_than_EQ : Joined<["-"], "Wframe-larger-than=">, Flags<[DriverOption]>;
+def Wstack_usage_EQ : Joined<["-"], "Wstack-usage=">, Group<clang_ignored_f_Group>;
def : Flag<["-"], "fterminated-vtables">, Alias<fapple_kext>;
def fthreadsafe_statics : Flag<["-"], "fthreadsafe-statics">, Group<f_Group>;
diff --git a/lib/AST/ASTDiagnostic.cpp b/lib/AST/ASTDiagnostic.cpp
index bd5c209..71ce9d9 100644
--- a/lib/AST/ASTDiagnostic.cpp
+++ b/lib/AST/ASTDiagnostic.cpp
@@ -12,6 +12,7 @@
//===----------------------------------------------------------------------===//
#include "clang/AST/ASTDiagnostic.h"
#include "clang/AST/ASTContext.h"
+#include "clang/AST/ASTLambda.h"
#include "clang/AST/Attr.h"
#include "clang/AST/DeclObjC.h"
#include "clang/AST/DeclTemplate.h"
@@ -345,7 +346,8 @@ void clang::FormatASTNodeDiagnosticArgument(
case DiagnosticsEngine::ak_declcontext: {
DeclContext *DC = reinterpret_cast<DeclContext *> (Val);
assert(DC && "Should never have a null declaration context");
-
+ NeedQuotes = false;
+
if (DC->isTranslationUnit()) {
// FIXME: Get these strings from some localized place
if (Context.getLangOpts().CPlusPlus)
@@ -359,6 +361,14 @@ void clang::FormatASTNodeDiagnosticArgument(
QualTypeVals);
} else {
// FIXME: Get these strings from some localized place
+ if (isa<BlockDecl>(DC)) {
+ OS << "block function";
+ break;
+ }
+ if (isLambdaCallOperator(DC)) {
+ OS << "lambda function";
+ break;
+ }
NamedDecl *ND = cast<NamedDecl>(DC);
if (isa<NamespaceDecl>(ND))
OS << "namespace ";
@@ -371,7 +381,6 @@ void clang::FormatASTNodeDiagnosticArgument(
ND->getNameForDiagnostic(OS, Context.getPrintingPolicy(), true);
OS << '\'';
}
- NeedQuotes = false;
break;
}
case DiagnosticsEngine::ak_attr: {
diff --git a/lib/CodeGen/CGBlocks.cpp b/lib/CodeGen/CGBlocks.cpp
index 21896da..7ffebe2 100644
--- a/lib/CodeGen/CGBlocks.cpp
+++ b/lib/CodeGen/CGBlocks.cpp
@@ -1127,11 +1127,9 @@ CodeGenFunction::GenerateBlockFunction(GlobalDecl GD,
llvm::FunctionType *fnLLVMType = CGM.getTypes().GetFunctionType(fnInfo);
- MangleBuffer name;
- CGM.getBlockMangledName(GD, name, blockDecl);
- llvm::Function *fn =
- llvm::Function::Create(fnLLVMType, llvm::GlobalValue::InternalLinkage,
- name.getString(), &CGM.getModule());
+ StringRef name = CGM.getBlockMangledName(GD, blockDecl);
+ llvm::Function *fn = llvm::Function::Create(
+ fnLLVMType, llvm::GlobalValue::InternalLinkage, name, &CGM.getModule());
CGM.SetInternalFunctionAttributes(blockDecl, fn, fnInfo);
// Begin generating the function.
diff --git a/lib/CodeGen/CGDecl.cpp b/lib/CodeGen/CGDecl.cpp
index 1869c1c..91f0b5d 100644
--- a/lib/CodeGen/CGDecl.cpp
+++ b/lib/CodeGen/CGDecl.cpp
@@ -149,21 +149,17 @@ void CodeGenFunction::EmitVarDecl(const VarDecl &D) {
static std::string GetStaticDeclName(CodeGenFunction &CGF, const VarDecl &D,
const char *Separator) {
CodeGenModule &CGM = CGF.CGM;
- if (CGF.getLangOpts().CPlusPlus) {
- StringRef Name = CGM.getMangledName(&D);
- return Name.str();
- }
+
+ if (CGF.getLangOpts().CPlusPlus)
+ return CGM.getMangledName(&D).str();
std::string ContextName;
if (!CGF.CurFuncDecl) {
// Better be in a block declared in global scope.
const NamedDecl *ND = cast<NamedDecl>(&D);
const DeclContext *DC = ND->getDeclContext();
- if (const BlockDecl *BD = dyn_cast<BlockDecl>(DC)) {
- MangleBuffer Name;
- CGM.getBlockMangledName(GlobalDecl(), Name, BD);
- ContextName = Name.getString();
- }
+ if (const BlockDecl *BD = dyn_cast<BlockDecl>(DC))
+ ContextName = CGM.getBlockMangledName(GlobalDecl(), BD).str();
else
llvm_unreachable("Unknown context for block static var decl");
} else if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(CGF.CurFuncDecl)) {
diff --git a/lib/CodeGen/CodeGenAction.cpp b/lib/CodeGen/CodeGenAction.cpp
index d337fa7..b3a7c78 100644
--- a/lib/CodeGen/CodeGenAction.cpp
+++ b/lib/CodeGen/CodeGenAction.cpp
@@ -393,12 +393,17 @@ BackendConsumer::StackSizeDiagHandler(const llvm::DiagnosticInfoStackSize &D) {
// We do not know how to format other severities.
return false;
- // FIXME: We should demangle the function name.
- // FIXME: Is there a way to get a location for that function?
- FullSourceLoc Loc;
- Diags.Report(Loc, diag::warn_fe_backend_frame_larger_than)
- << D.getStackSize() << D.getFunction().getName();
- return true;
+ if (const Decl *ND = Gen->Demangle(D.getFunction().getName())) {
+ if (auto FD = dyn_cast<FunctionDecl>(ND))
+ if (FD->hasBody(FD))
+ ND = FD;
+ Diags.Report(ND->getASTContext().getFullLoc(ND->getLocation()),
+ diag::warn_fe_frame_larger_than)
+ << D.getStackSize() << Decl::castToDeclContext(ND);
+ return true;
+ }
+
+ return false;
}
void BackendConsumer::EmitOptimizationRemark(
diff --git a/lib/CodeGen/CodeGenModule.cpp b/lib/CodeGen/CodeGenModule.cpp
index f42e67d..00ae8a8 100644
--- a/lib/CodeGen/CodeGenModule.cpp
+++ b/lib/CodeGen/CodeGenModule.cpp
@@ -465,8 +465,9 @@ StringRef CodeGenModule::getMangledName(GlobalDecl GD) {
IdentifierInfo *II = ND->getIdentifier();
assert(II && "Attempt to mangle unnamed decl.");
- Str = II->getName();
- return Str;
+ auto &Mangled = Manglings.GetOrCreateValue(II->getName());
+ Mangled.second = GD;
+ return Str = Mangled.first();
}
SmallString<256> Buffer;
@@ -478,22 +479,18 @@ StringRef CodeGenModule::getMangledName(GlobalDecl GD) {
else
getCXXABI().getMangleContext().mangleName(ND, Out);
- // Allocate space for the mangled name.
- Out.flush();
- size_t Length = Buffer.size();
- char *Name = MangledNamesAllocator.Allocate<char>(Length);
- std::copy(Buffer.begin(), Buffer.end(), Name);
-
- Str = StringRef(Name, Length);
-
- return Str;
+ auto &Mangled = Manglings.GetOrCreateValue(Out.str());
+ Mangled.second = GD;
+ return Str = Mangled.first();
}
-void CodeGenModule::getBlockMangledName(GlobalDecl GD, MangleBuffer &Buffer,
- const BlockDecl *BD) {
+StringRef CodeGenModule::getBlockMangledName(GlobalDecl GD,
+ const BlockDecl *BD) {
MangleContext &MangleCtx = getCXXABI().getMangleContext();
const Decl *D = GD.getDecl();
- llvm::raw_svector_ostream Out(Buffer.getBuffer());
+
+ SmallString<256> Buffer;
+ llvm::raw_svector_ostream Out(Buffer);
if (!D)
MangleCtx.mangleGlobalBlock(BD,
dyn_cast_or_null<VarDecl>(initializedGlobalDecl.getDecl()), Out);
@@ -503,6 +500,10 @@ void CodeGenModule::getBlockMangledName(GlobalDecl GD, MangleBuffer &Buffer,
MangleCtx.mangleDtorBlock(DD, GD.getDtorType(), BD, Out);
else
MangleCtx.mangleBlock(cast<DeclContext>(D), BD, Out);
+
+ auto &Mangled = Manglings.GetOrCreateValue(Out.str());
+ Mangled.second = BD;
+ return Mangled.first();
}
llvm::GlobalValue *CodeGenModule::GetGlobalValue(StringRef Name) {
@@ -1415,7 +1416,7 @@ CodeGenModule::GetOrCreateLLVMFunction(StringRef MangledName,
// This is the first use or definition of a mangled name. If there is a
// deferred decl with this name, remember that we need to emit it at the end
// of the file.
- llvm::StringMap<GlobalDecl>::iterator DDI = DeferredDecls.find(MangledName);
+ auto DDI = DeferredDecls.find(MangledName);
if (DDI != DeferredDecls.end()) {
// Move the potentially referenced deferred decl to the
// DeferredDeclsToEmit list, and remove it from DeferredDecls (since we
@@ -1575,7 +1576,7 @@ CodeGenModule::GetOrCreateLLVMGlobal(StringRef MangledName,
// This is the first use or definition of a mangled name. If there is a
// deferred decl with this name, remember that we need to emit it at the end
// of the file.
- llvm::StringMap<GlobalDecl>::iterator DDI = DeferredDecls.find(MangledName);
+ auto DDI = DeferredDecls.find(MangledName);
if (DDI != DeferredDecls.end()) {
// Move the potentially referenced deferred decl to the DeferredDeclsToEmit
// list, and remove it from DeferredDecls (since we don't need it anymore).
@@ -3209,6 +3210,15 @@ void CodeGenModule::EmitStaticExternCAliases() {
}
}
+bool CodeGenModule::LookupRepresentativeDecl(StringRef MangledName,
+ GlobalDecl &Result) {
+ auto Res = Manglings.find(MangledName);
+ if (Res == Manglings.end())
+ return false;
+ Result = Res->getValue();
+ return true;
+}
+
/// Emits metadata nodes associating all the global values in the
/// current module with the Decls they came from. This is useful for
/// projects using IR gen as a subroutine.
diff --git a/lib/CodeGen/CodeGenModule.h b/lib/CodeGen/CodeGenModule.h
index c54f6de..2f0ce16 100644
--- a/lib/CodeGen/CodeGenModule.h
+++ b/lib/CodeGen/CodeGenModule.h
@@ -71,7 +71,6 @@ class CodeGenOptions;
class DiagnosticsEngine;
class AnnotateAttr;
class CXXDestructorDecl;
-class MangleBuffer;
class Module;
namespace CodeGen {
@@ -328,8 +327,8 @@ class CodeGenModule : public CodeGenTypeCache {
/// A map of canonical GlobalDecls to their mangled names.
llvm::DenseMap<GlobalDecl, StringRef> MangledDeclNames;
- llvm::BumpPtrAllocator MangledNamesAllocator;
-
+ llvm::StringMap<GlobalDecl> Manglings;
+
/// Global annotations.
std::vector<llvm::Constant*> Annotations;
@@ -523,6 +522,8 @@ public:
StaticLocalDeclGuardMap[D] = C;
}
+ bool LookupRepresentativeDecl(StringRef MangledName, GlobalDecl &Result);
+
llvm::Constant *getAtomicSetterHelperFnMap(QualType Ty) {
return AtomicSetterHelperFnMap[Ty];
}
@@ -937,8 +938,7 @@ public:
bool AttrOnCallSite);
StringRef getMangledName(GlobalDecl GD);
- void getBlockMangledName(GlobalDecl GD, MangleBuffer &Buffer,
- const BlockDecl *BD);
+ StringRef getBlockMangledName(GlobalDecl GD, const BlockDecl *BD);
void EmitTentativeDefinition(const VarDecl *D);
diff --git a/lib/CodeGen/ModuleBuilder.cpp b/lib/CodeGen/ModuleBuilder.cpp
index 78cb82d..206a049 100644
--- a/lib/CodeGen/ModuleBuilder.cpp
+++ b/lib/CodeGen/ModuleBuilder.cpp
@@ -49,6 +49,13 @@ namespace {
return M.get();
}
+ const Decl *Demangle(StringRef MangledName) override {
+ GlobalDecl Result;
+ if (!Builder->LookupRepresentativeDecl(MangledName, Result))
+ return nullptr;
+ return Result.getCanonicalDecl().getDecl();
+ }
+
llvm::Module *ReleaseModule() override { return M.release(); }
void Initialize(ASTContext &Context) override {
diff --git a/lib/Driver/Tools.cpp b/lib/Driver/Tools.cpp
index ee3b7ff..b36d70d 100644
--- a/lib/Driver/Tools.cpp
+++ b/lib/Driver/Tools.cpp
@@ -2511,6 +2511,13 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
// LLVM Code Generator Options.
+ 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));
+ A->claim();
+ }
+
if (Arg *A = Args.getLastArg(options::OPT_mregparm_EQ)) {
CmdArgs.push_back("-mregparm");
CmdArgs.push_back(A->getValue());
diff --git a/test/Frontend/backend-diagnostic.c b/test/Frontend/backend-diagnostic.c
index 8b61c03..7bd2bf8 100644
--- a/test/Frontend/backend-diagnostic.c
+++ b/test/Frontend/backend-diagnostic.c
@@ -18,9 +18,9 @@
extern void doIt(char *);
-// REGULAR: warning: stack size exceeded ({{[0-9]+}}) in stackSizeWarning
-// PROMOTE: error: stack size exceeded ({{[0-9]+}}) in stackSizeWarning
-// IGNORE-NOT: stack size exceeded ({{[0-9]+}}) in stackSizeWarning
+// REGULAR: warning: stack frame size of {{[0-9]+}} bytes in function 'stackSizeWarning'
+// PROMOTE: error: stack frame size of {{[0-9]+}} bytes in function 'stackSizeWarning'
+// IGNORE-NOT: stack frame size of {{[0-9]+}} bytes in function 'stackSizeWarning'
void stackSizeWarning() {
char buffer[80];
doIt(buffer);
diff --git a/test/Misc/backend-stack-frame-diagnostics.cpp b/test/Misc/backend-stack-frame-diagnostics.cpp
new file mode 100644
index 0000000..4d4e0af
--- /dev/null
+++ b/test/Misc/backend-stack-frame-diagnostics.cpp
@@ -0,0 +1,51 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang -target i386-apple-darwin -std=c++11 -fblocks -Wframe-larger-than=70 -Xclang -verify -o /dev/null -c %s
+
+// Test that:
+// * The driver passes the option through to the backend.
+// * The frontend diagnostic handler 'demangles' and resolves the correct function definition.
+
+// TODO: Support rich backend diagnostics for Objective-C methods.
+
+extern void doIt(char *);
+
+void frameSizeWarning(int, int) {}
+
+void frameSizeWarning();
+
+void frameSizeWarning() { // expected-warning-re {{stack frame size of {{[0-9]+}} bytes in function 'frameSizeWarning'}}
+ char buffer[80];
+ doIt(buffer);
+}
+
+void frameSizeWarning();
+
+void frameSizeWarning(int) {}
+
+void frameSizeLocalClassWarning() {
+ struct S {
+ S() { // expected-warning-re {{stack frame size of {{[0-9]+}} bytes in function 'frameSizeLocalClassWarning()::S::S'}}
+ char buffer[80];
+ doIt(buffer);
+ }
+ };
+ S();
+}
+
+void frameSizeLambdaWarning() {
+ auto fn =
+ []() { // expected-warning-re {{stack frame size of {{[0-9]+}} bytes in lambda function}}
+ char buffer[80];
+ doIt(buffer);
+ };
+ fn();
+}
+
+void frameSizeBlocksWarning() {
+ auto fn =
+ ^() { // expected-warning-re {{stack frame size of {{[0-9]+}} bytes in block function}}
+ char buffer[80];
+ doIt(buffer);
+ };
+ fn();
+}
More information about the cfe-commits
mailing list