]> granicus.if.org Git - clang/commitdiff
Implement -Wframe-larger-than backend diagnostic
authorAlp Toker <alp@nuanti.com>
Thu, 5 Jun 2014 22:10:59 +0000 (22:10 +0000)
committerAlp Toker <alp@nuanti.com>
Thu, 5 Jun 2014 22:10:59 +0000 (22:10 +0000)
Add driver and frontend support for the GCC -Wframe-larger-than=bytes warning.
This is the first GCC-compatible backend diagnostic built around LLVM's
reporting feature.

This commit adds infrastructure to perform reverse lookup from mangled names
emitted after LLVM IR generation. We use that to resolve precise locations and
originating AST functions, lambdas or block declarations to produce seamless
codegen-guided diagnostics.

An associated change, StringMap now maintains unique mangled name strings
instead of allocating copies. This is a net memory saving in C++ and a small
hit for C where we no longer reuse IdentifierInfo storage, pending further
optimisation.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@210293 91177308-0d34-0410-b5e6-96231b3b80d8

14 files changed:
include/clang/Basic/DiagnosticFrontendKinds.td
include/clang/CodeGen/ModuleBuilder.h
include/clang/Driver/Options.td
lib/AST/ASTDiagnostic.cpp
lib/CodeGen/CGBlocks.cpp
lib/CodeGen/CGDecl.cpp
lib/CodeGen/CodeGenAction.cpp
lib/CodeGen/CodeGenModule.cpp
lib/CodeGen/CodeGenModule.h
lib/CodeGen/ModuleBuilder.cpp
lib/Driver/Tools.cpp
test/Frontend/backend-diagnostic.c
test/Misc/backend-stack-frame-diagnostics-fallback.cpp [new file with mode: 0644]
test/Misc/backend-stack-frame-diagnostics.cpp [new file with mode: 0644]

index 03ed69116c7cf3a4f45325d69df69544d57018ba..3527e8e3dedfb83dc9e2455f82d1e29f9364229a 100644 (file)
@@ -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;
 
index cda7863445cf938be6abf0b1f73f94535c439c1e..4b7236bfd03b034288e06fb46a24143efad1a3a1 100644 (file)
@@ -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 *GetDeclForMangledName(llvm::StringRef MangledName) = 0;
   };
 
   /// CreateLLVMCodeGen - Create a CodeGenerator instance.
index d2775519645dd0516b69ec8343c46fa91cc7d448..6dc9c23c3703f8bfde83d6196e365dd1204f1994 100644 (file)
@@ -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>;
index bd5c209a9a5e84aa908979273a5f8aac3120eca4..70073ff237805a6252c51d1852e5083619e74ce7 100644 (file)
@@ -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 literal";
+          break;
+        }
+        if (isLambdaCallOperator(DC)) {
+          OS << "lambda expression";
+          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: {
index 71a2447cb5dc471c036f1f0c8a7b108862e9e0e0..7ffebe289101f788ae8038477fb2fe10a067e12b 100644 (file)
@@ -1127,7 +1127,7 @@ CodeGenFunction::GenerateBlockFunction(GlobalDecl GD,
 
   llvm::FunctionType *fnLLVMType = CGM.getTypes().GetFunctionType(fnInfo);
 
-  std::string name = CGM.getBlockMangledName(GD, blockDecl);
+  StringRef name = CGM.getBlockMangledName(GD, blockDecl);
   llvm::Function *fn = llvm::Function::Create(
       fnLLVMType, llvm::GlobalValue::InternalLinkage, name, &CGM.getModule());
   CGM.SetInternalFunctionAttributes(blockDecl, fn, fnInfo);
index 7cf9cb2417b5cec165e71f24fb2d935dfa441128..8d6b4188733e43525a48ee5238fb61bfeddd262f 100644 (file)
@@ -149,12 +149,11 @@ 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();
-  }
 
-  std::string ContextName;
+  if (CGF.getLangOpts().CPlusPlus)
+    return CGM.getMangledName(&D).str();
+
+  StringRef ContextName;
   if (!CGF.CurFuncDecl) {
     // Better be in a block declared in global scope.
     const NamedDecl *ND = cast<NamedDecl>(&D);
@@ -163,15 +162,14 @@ static std::string GetStaticDeclName(CodeGenFunction &CGF, const VarDecl &D,
       ContextName = CGM.getBlockMangledName(GlobalDecl(), BD);
     else
       llvm_unreachable("Unknown context for block static var decl");
-  } else if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(CGF.CurFuncDecl)) {
-    StringRef Name = CGM.getMangledName(FD);
-    ContextName = Name.str();
-  } else if (isa<ObjCMethodDecl>(CGF.CurFuncDecl))
+  } else if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(CGF.CurFuncDecl))
+    ContextName = CGM.getMangledName(FD);
+  else if (isa<ObjCMethodDecl>(CGF.CurFuncDecl))
     ContextName = CGF.CurFn->getName();
   else
     llvm_unreachable("Unknown context for static var decl");
 
-  return ContextName + Separator + D.getNameAsString();
+  return ContextName.str() + Separator + D.getNameAsString();
 }
 
 llvm::Constant *
index f593ccf431d2771cb46d5e22de5a5b5c2d8dfd0c..455299f2df7e9b3f32f0843d95c85c1e76b46fb0 100644 (file)
@@ -404,12 +404,14 @@ 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->GetDeclForMangledName(D.getFunction().getName())) {
+    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(
index ac89ce699a608eb8b3ea035d0c30feff713c2221..9cb84f890afbdc170a3f8a91a7ba923b0d22fdde 100644 (file)
@@ -498,47 +498,40 @@ void CodeGenModule::setTLSMode(llvm::GlobalVariable *GV,
 }
 
 StringRef CodeGenModule::getMangledName(GlobalDecl GD) {
-  const auto *ND = cast<NamedDecl>(GD.getDecl());
-
-  StringRef &Str = MangledDeclNames[GD.getCanonicalDecl()];
-  if (!Str.empty())
-    return Str;
+  StringRef &FoundStr = MangledDeclNames[GD.getCanonicalDecl()];
+  if (!FoundStr.empty())
+    return FoundStr;
 
-  if (!getCXXABI().getMangleContext().shouldMangleDeclName(ND)) {
+  const auto *ND = cast<NamedDecl>(GD.getDecl());
+  SmallString<256> Buffer;
+  StringRef Str;
+  if (getCXXABI().getMangleContext().shouldMangleDeclName(ND)) {
+    llvm::raw_svector_ostream Out(Buffer);
+    if (const auto *D = dyn_cast<CXXConstructorDecl>(ND))
+      getCXXABI().getMangleContext().mangleCXXCtor(D, GD.getCtorType(), Out);
+    else if (const auto *D = dyn_cast<CXXDestructorDecl>(ND))
+      getCXXABI().getMangleContext().mangleCXXDtor(D, GD.getDtorType(), Out);
+    else
+      getCXXABI().getMangleContext().mangleName(ND, Out);
+    Str = Out.str();
+  } else {
     IdentifierInfo *II = ND->getIdentifier();
     assert(II && "Attempt to mangle unnamed decl.");
-
     Str = II->getName();
-    return Str;
   }
-  
-  SmallString<256> Buffer;
-  llvm::raw_svector_ostream Out(Buffer);
-  if (const auto *D = dyn_cast<CXXConstructorDecl>(ND))
-    getCXXABI().getMangleContext().mangleCXXCtor(D, GD.getCtorType(), Out);
-  else if (const auto *D = dyn_cast<CXXDestructorDecl>(ND))
-    getCXXABI().getMangleContext().mangleCXXDtor(D, GD.getDtorType(), Out);
-  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(Str);
+  Mangled.second = GD;
+  return FoundStr = Mangled.first();
 }
 
-std::string CodeGenModule::getBlockMangledName(GlobalDecl GD,
-                                               const BlockDecl *BD) {
+StringRef CodeGenModule::getBlockMangledName(GlobalDecl GD,
+                                             const BlockDecl *BD) {
   MangleContext &MangleCtx = getCXXABI().getMangleContext();
   const Decl *D = GD.getDecl();
 
-  std::string Buffer;
-  llvm::raw_string_ostream Out(Buffer);
+  SmallString<256> Buffer;
+  llvm::raw_svector_ostream Out(Buffer);
   if (!D)
     MangleCtx.mangleGlobalBlock(BD, 
       dyn_cast_or_null<VarDecl>(initializedGlobalDecl.getDecl()), Out);
@@ -549,7 +542,9 @@ std::string CodeGenModule::getBlockMangledName(GlobalDecl GD,
   else
     MangleCtx.mangleBlock(cast<DeclContext>(D), BD, Out);
 
-  return Out.str();
+  auto &Mangled = Manglings.GetOrCreateValue(Out.str());
+  Mangled.second = BD;
+  return Mangled.first();
 }
 
 llvm::GlobalValue *CodeGenModule::GetGlobalValue(StringRef Name) {
@@ -1462,7 +1457,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
@@ -1622,7 +1617,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).
@@ -3205,6 +3200,15 @@ void CodeGenModule::EmitStaticExternCAliases() {
   }
 }
 
+bool CodeGenModule::lookupRepresentativeDecl(StringRef MangledName,
+                                             GlobalDecl &Result) const {
+  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.
index f157d20e6cf46bd56e918156ffda671058fb7449..6cb4fc17c5e3df1dc8daf830dcb75f8eb8ab2644 100644 (file)
@@ -287,7 +287,7 @@ class CodeGenModule : public CodeGenTypeCache {
   /// for emission and therefore should only be output if they are actually
   /// used. If a decl is in this, then it is known to have not been referenced
   /// yet.
-  llvm::StringMap<GlobalDecl> DeferredDecls;
+  std::map<StringRef, GlobalDecl> DeferredDecls;
 
   /// This is a list of deferred decls which we have seen that *are* actually
   /// referenced. These get code generated when the module is done.
@@ -327,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, llvm::BumpPtrAllocator> Manglings;
+
   /// Global annotations.
   std::vector<llvm::Constant*> Annotations;
 
@@ -522,6 +522,9 @@ public:
     StaticLocalDeclGuardMap[D] = C;
   }
 
+  bool lookupRepresentativeDecl(StringRef MangledName,
+                                GlobalDecl &Result) const;
+
   llvm::Constant *getAtomicSetterHelperFnMap(QualType Ty) {
     return AtomicSetterHelperFnMap[Ty];
   }
@@ -922,7 +925,7 @@ public:
                               bool AttrOnCallSite);
 
   StringRef getMangledName(GlobalDecl GD);
-  std::string getBlockMangledName(GlobalDecl GD, const BlockDecl *BD);
+  StringRef getBlockMangledName(GlobalDecl GD, const BlockDecl *BD);
 
   void EmitTentativeDefinition(const VarDecl *D);
 
index 78cb82dc558cc45a65f4a3a45f420a951b097079..e5bdae93fe7ed8da452c53237a1283ead60713a4 100644 (file)
@@ -49,6 +49,21 @@ namespace {
       return M.get();
     }
 
+    const Decl *GetDeclForMangledName(StringRef MangledName) override {
+      GlobalDecl Result;
+      if (!Builder->lookupRepresentativeDecl(MangledName, Result))
+        return nullptr;
+      const Decl *D = Result.getCanonicalDecl().getDecl();
+      if (auto FD = dyn_cast<FunctionDecl>(D)) {
+        if (FD->hasBody(FD))
+          return FD;
+      } else if (auto TD = dyn_cast<TagDecl>(D)) {
+        if (auto Def = TD->getDefinition())
+          return Def;
+      }
+      return D;
+    }
+
     llvm::Module *ReleaseModule() override { return M.release(); }
 
     void Initialize(ASTContext &Context) override {
index 37df7eea9b9b166289326ba52ab6abadc0e77966..0316357bd162b5f050b03ace4e5c227bf7d9b94a 100644 (file)
@@ -2517,6 +2517,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());
index 8b61c0350eca2e45cfd39fefb5a8ce2ec7316b89..d6fe48bc9c568589bbcf6d889960d82e1c764e76 100644 (file)
 // 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: FileCheck < %t.err %s --check-prefix=IGNORE --check-prefix=ASM
 //
-// Currently the stack size reporting cannot be checked with -verify because
-//  no source location is attached to the diagnostic. Therefore do not emit
-// them for the -verify test for now.
 // RUN: %clang_cc1 %s -S -o - -triple=i386-apple-darwin -verify -no-integrated-as
 
 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-fallback.cpp b/test/Misc/backend-stack-frame-diagnostics-fallback.cpp
new file mode 100644 (file)
index 0000000..8ae8c55
--- /dev/null
@@ -0,0 +1,18 @@
+// REQUIRES: x86-registered-target
+// RUN: %clang_cc1 %s -mllvm -warn-stack-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.
+namespace frameSizeThunkWarning {
+  struct A {
+    virtual void f();
+  };
+
+  struct B : virtual A {
+    virtual void f();
+  };
+
+  // CHECK: warning: stack frame size of {{[0-9]+}} bytes in function 'frameSizeThunkWarning::B::f'
+  // CHECK: warning: stack size limit exceeded ({{[0-9]+}}) in {{[^ ]+}}
+  void B::f() { }
+}
diff --git a/test/Misc/backend-stack-frame-diagnostics.cpp b/test/Misc/backend-stack-frame-diagnostics.cpp
new file mode 100644 (file)
index 0000000..1930359
--- /dev/null
@@ -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 expression}}
+    char buffer[80];
+    doIt(buffer);
+  };
+  fn();
+}
+
+void frameSizeBlocksWarning() {
+  auto fn =
+      ^() { // expected-warning-re {{stack frame size of {{[0-9]+}} bytes in block literal}}
+    char buffer[80];
+    doIt(buffer);
+  };
+  fn();
+}