r205999 - Debug info: (Bugfix) Make sure artificial functions like _GLOBAL__I_a

Adrian Prantl aprantl at apple.com
Thu Apr 10 16:53:21 PDT 2014


On Apr 10, 2014, at 4:38 PM, David Blaikie <dblaikie at gmail.com> wrote:

> On Thu, Apr 10, 2014 at 4:21 PM, Adrian Prantl <aprantl at apple.com> wrote:
>> Author: adrian
>> Date: Thu Apr 10 18:21:53 2014
>> New Revision: 205999
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=205999&view=rev
>> Log:
>> Debug info: (Bugfix) Make sure artificial functions like _GLOBAL__I_a
>> are not associated with any source lines.
>> 
>> Previously, if the Location of a Decl was empty, EmitFunctionStart would
>> just keep using CurLoc, which would sometimes be correct (e.g., thunks)
>> but in other cases would just point to a hilariously random location.
>> 
>> This patch fixes this by completely eliminating all uses of CurLoc from
>> EmitFunctionStart and rather have clients explicitly pass in a
>> SourceLocation for the function header and the function body.
>> 
>> rdar://problem/14985269
>> 
>> Added:
>>    cfe/trunk/test/CodeGenCXX/globalinit-loc.cpp
>> Modified:
>>    cfe/trunk/lib/CodeGen/CGBlocks.cpp
>>    cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>>    cfe/trunk/lib/CodeGen/CGDebugInfo.h
>>    cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
>>    cfe/trunk/lib/CodeGen/CGObjC.cpp
>>    cfe/trunk/lib/CodeGen/CGStmt.cpp
>>    cfe/trunk/lib/CodeGen/CGVTables.cpp
>>    cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
>>    cfe/trunk/lib/CodeGen/CodeGenFunction.h
>> 
>> Modified: cfe/trunk/lib/CodeGen/CGBlocks.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBlocks.cpp?rev=205999&r1=205998&r2=205999&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CGBlocks.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGBlocks.cpp Thu Apr 10 18:21:53 2014
>> @@ -1135,6 +1135,7 @@ CodeGenFunction::GenerateBlockFunction(G
>> 
>>   // Begin generating the function.
>>   StartFunction(blockDecl, fnType->getReturnType(), fn, fnInfo, args,
>> +                blockDecl->getLocation(),
>>                 blockInfo.getBlockExpr()->getBody()->getLocStart());
>> 
>>   // Okay.  Undo some of what StartFunction did.
>> @@ -1306,7 +1307,7 @@ CodeGenFunction::GenerateCopyHelperFunct
>>                                           false);
>>   // Create a scope with an artificial location for the body of this function.
>>   ArtificialLocation AL(*this, Builder);
>> -  StartFunction(FD, C.VoidTy, Fn, FI, args, SourceLocation());
>> +  StartFunction(FD, C.VoidTy, Fn, FI, args, SourceLocation(), SourceLocation());
>>   AL.Emit();
>> 
>>   llvm::Type *structPtrTy = blockInfo.StructureType->getPointerTo();
>> @@ -1476,7 +1477,7 @@ CodeGenFunction::GenerateDestroyHelperFu
>>                                           false, false);
>>   // Create a scope with an artificial location for the body of this function.
>>   ArtificialLocation AL(*this, Builder);
>> -  StartFunction(FD, C.VoidTy, Fn, FI, args, SourceLocation());
>> +  StartFunction(FD, C.VoidTy, Fn, FI, args, SourceLocation(), SourceLocation());
>>   AL.Emit();
>> 
>>   llvm::Type *structPtrTy = blockInfo.StructureType->getPointerTo();
>> @@ -1765,7 +1766,7 @@ generateByrefCopyHelper(CodeGenFunction
>>                                           SC_Static,
>>                                           false, false);
>> 
>> -  CGF.StartFunction(FD, R, Fn, FI, args, SourceLocation());
>> +  CGF.StartFunction(FD, R, Fn, FI, args, SourceLocation(), SourceLocation());
>> 
>>   if (byrefInfo.needsCopy()) {
>>     llvm::Type *byrefPtrType = byrefType.getPointerTo(0);
>> @@ -1834,7 +1835,7 @@ generateByrefDisposeHelper(CodeGenFuncti
>>                                           SourceLocation(), II, R, 0,
>>                                           SC_Static,
>>                                           false, false);
>> -  CGF.StartFunction(FD, R, Fn, FI, args, SourceLocation());
>> +  CGF.StartFunction(FD, R, Fn, FI, args, SourceLocation(), SourceLocation());
>> 
>>   if (byrefInfo.needsDispose()) {
>>     llvm::Value *V = CGF.GetAddrOfLocalVar(&src);
>> 
>> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=205999&r1=205998&r2=205999&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Thu Apr 10 18:21:53 2014
>> @@ -2504,7 +2504,10 @@ llvm::DICompositeType CGDebugInfo::getOr
>> }
>> 
>> /// EmitFunctionStart - Constructs the debug code for entering a function.
>> -void CGDebugInfo::EmitFunctionStart(GlobalDecl GD, QualType FnType,
>> +void CGDebugInfo::EmitFunctionStart(GlobalDecl GD,
>> +                                    SourceLocation Loc,
>> +                                    SourceLocation ScopeLoc,
>> +                                    QualType FnType,
>>                                     llvm::Function *Fn,
>>                                     CGBuilderTy &Builder) {
>> 
>> @@ -2514,24 +2517,7 @@ void CGDebugInfo::EmitFunctionStart(Glob
>>   FnBeginRegionCount.push_back(LexicalBlockStack.size());
>> 
>>   const Decl *D = GD.getDecl();
>> -
>> -  // Use the location of the start of the function to determine where
>> -  // the function definition is located. By default use the location
>> -  // of the declaration as the location for the subprogram. A function
>> -  // may lack a declaration in the source code if it is created by code
>> -  // gen. (examples: _GLOBAL__I_a, __cxx_global_array_dtor, thunk).
>>   bool HasDecl = (D != 0);
>> -  SourceLocation Loc;
>> -  if (HasDecl) {
>> -    Loc = D->getLocation();
>> -
>> -    // If this is a function specialization then use the pattern body
>> -    // as the location for the function.
>> -    if (const FunctionDecl *FD = dyn_cast<FunctionDecl>(D))
>> -      if (const FunctionDecl *SpecDecl = FD->getTemplateInstantiationPattern())
>> -        if (SpecDecl->hasBody(SpecDecl))
>> -          Loc = SpecDecl->getLocation();
>> -  }
>> 
>>   unsigned Flags = 0;
>>   llvm::DIFile Unit = getOrCreateFile(Loc);
>> @@ -2591,9 +2577,14 @@ void CGDebugInfo::EmitFunctionStart(Glob
>>   if (!Name.empty() && Name[0] == '\01')
>>     Name = Name.substr(1);
>> 
>> -  unsigned LineNo = getLineNumber(Loc);
>> -  if (!HasDecl || D->isImplicit())
>> +  if (!HasDecl || D->isImplicit()) {
>>     Flags |= llvm::DIDescriptor::FlagArtificial;
>> +    // Artificial functions without a location should not silently reuse CurLoc.
>> +    if (Loc.isInvalid())
>> +      CurLoc = SourceLocation();
>> +  }
>> +  unsigned LineNo = getLineNumber(Loc);
>> +  unsigned ScopeLine = getLineNumber(ScopeLoc);
>> 
>>   // FIXME: The function declaration we're constructing here is mostly reusing
>>   // declarations from CXXMethodDecl and not constructing new ones for arbitrary
>> @@ -2604,7 +2595,7 @@ void CGDebugInfo::EmitFunctionStart(Glob
>>       DBuilder.createFunction(FDContext, Name, LinkageName, Unit, LineNo,
>>                               getOrCreateFunctionType(D, FnType, Unit),
>>                               Fn->hasInternalLinkage(), true /*definition*/,
>> -                              getLineNumber(CurLoc), Flags,
>> +                              ScopeLine, Flags,
>>                               CGM.getLangOpts().Optimize, Fn, TParamsArray,
>>                               getFunctionDeclaration(D));
>>   if (HasDecl)
>> 
>> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.h?rev=205999&r1=205998&r2=205999&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CGDebugInfo.h (original)
>> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.h Thu Apr 10 18:21:53 2014
>> @@ -220,8 +220,12 @@ public:
>> 
>>   /// EmitFunctionStart - Emit a call to llvm.dbg.function.start to indicate
>>   /// start of a new function.
>> -  void EmitFunctionStart(GlobalDecl GD, QualType FnType,
>> -                         llvm::Function *Fn, CGBuilderTy &Builder);
>> +  /// \param Loc       The location of the function header.
>> +  /// \param ScopeLoc  The location of the function body.
>> +  void EmitFunctionStart(GlobalDecl GD,
>> +                         SourceLocation Loc, SourceLocation ScopeLoc,
>> +                         QualType FnType, llvm::Function *Fn,
>> +                         CGBuilderTy &Builder);
>> 
>>   /// EmitFunctionEnd - Constructs the debug code for exiting a function.
>>   void EmitFunctionEnd(CGBuilderTy &Builder);
>> 
>> Modified: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDeclCXX.cpp?rev=205999&r1=205998&r2=205999&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CGDeclCXX.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGDeclCXX.cpp Thu Apr 10 18:21:53 2014
>> @@ -179,7 +179,7 @@ static llvm::Constant *createAtExitStub(
>> 
>>   CGF.StartFunction(&VD, CGM.getContext().VoidTy, fn,
>>                     CGM.getTypes().arrangeNullaryFunction(), FunctionArgList(),
>> -                    SourceLocation());
>> +                    SourceLocation(), SourceLocation());
>> 
>>   llvm::CallInst *call = CGF.Builder.CreateCall(dtor, addr);
>> 
>> @@ -412,7 +412,8 @@ void CodeGenFunction::GenerateCXXGlobalV
>> 
>>   StartFunction(GlobalDecl(D), getContext().VoidTy, Fn,
>>                 getTypes().arrangeNullaryFunction(),
>> -                FunctionArgList(), D->getInit()->getExprLoc());
>> +                FunctionArgList(), D->getLocation(),
>> +                D->getInit()->getExprLoc());
>> 
>>   // Use guarded initialization if the global variable is weak. This
>>   // occurs for, e.g., instantiated static data members and
>> @@ -433,7 +434,7 @@ CodeGenFunction::GenerateCXXGlobalInitFu
>>                                            llvm::GlobalVariable *Guard) {
>>   StartFunction(GlobalDecl(), getContext().VoidTy, Fn,
>>                 getTypes().arrangeNullaryFunction(),
>> -                FunctionArgList(), SourceLocation());
>> +                FunctionArgList(), SourceLocation(), SourceLocation());
>> 
>>   llvm::BasicBlock *ExitBlock = 0;
>>   if (Guard) {
>> @@ -479,7 +480,7 @@ void CodeGenFunction::GenerateCXXGlobalD
>>                                                 &DtorsAndObjects) {
>>   StartFunction(GlobalDecl(), getContext().VoidTy, Fn,
>>                 getTypes().arrangeNullaryFunction(),
>> -                FunctionArgList(), SourceLocation());
>> +                FunctionArgList(), SourceLocation(), SourceLocation());
>> 
>>   // Emit the dtors, in reverse order from construction.
>>   for (unsigned i = 0, e = DtorsAndObjects.size(); i != e; ++i) {
>> @@ -509,7 +510,8 @@ llvm::Function *CodeGenFunction::generat
>>   llvm::Function *fn =
>>     CreateGlobalInitOrDestructFunction(CGM, FTy, "__cxx_global_array_dtor");
>> 
>> -  StartFunction(VD, getContext().VoidTy, fn, FI, args, SourceLocation());
>> +  StartFunction(VD, getContext().VoidTy, fn, FI, args,
>> +                SourceLocation(), SourceLocation());
>> 
>>   emitDestroy(addr, type, destroyer, useEHCleanupForArray);
>> 
>> 
>> Modified: cfe/trunk/lib/CodeGen/CGObjC.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjC.cpp?rev=205999&r1=205998&r2=205999&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CGObjC.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGObjC.cpp Thu Apr 10 18:21:53 2014
>> @@ -481,7 +481,8 @@ void CodeGenFunction::StartObjCMethod(co
>> 
>>   CurGD = OMD;
>> 
>> -  StartFunction(OMD, OMD->getReturnType(), Fn, FI, args, StartLoc);
>> +  StartFunction(OMD, OMD->getReturnType(), Fn, FI, args,
>> +                OMD->getLocation(), StartLoc);
>> 
>>   // In ARC, certain methods get an extra cleanup.
>>   if (CGM.getLangOpts().ObjCAutoRefCount &&
>> @@ -2915,7 +2916,7 @@ CodeGenFunction::GenerateObjCAtomicSette
>>                            "__assign_helper_atomic_property_",
>>                            &CGM.getModule());
>> 
>> -  StartFunction(FD, C.VoidTy, Fn, FI, args, SourceLocation());
>> +  StartFunction(FD, C.VoidTy, Fn, FI, args, SourceLocation(), SourceLocation());
>> 
>>   DeclRefExpr DstExpr(&dstDecl, false, DestTy,
>>                       VK_RValue, SourceLocation());
>> @@ -2993,7 +2994,7 @@ CodeGenFunction::GenerateObjCAtomicGette
>>   llvm::Function::Create(LTy, llvm::GlobalValue::InternalLinkage,
>>                          "__copy_helper_atomic_property_", &CGM.getModule());
>> 
>> -  StartFunction(FD, C.VoidTy, Fn, FI, args, SourceLocation());
>> +  StartFunction(FD, C.VoidTy, Fn, FI, args, SourceLocation(), SourceLocation());
>> 
>>   DeclRefExpr SrcExpr(&srcDecl, false, SrcTy,
>>                       VK_RValue, SourceLocation());
>> 
>> Modified: cfe/trunk/lib/CodeGen/CGStmt.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmt.cpp?rev=205999&r1=205998&r2=205999&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CGStmt.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGStmt.cpp Thu Apr 10 18:21:53 2014
>> @@ -1944,7 +1944,9 @@ CodeGenFunction::GenerateCapturedStmtFun
>>   CGM.SetInternalFunctionAttributes(CD, F, FuncInfo);
>> 
>>   // Generate the function.
>> -  StartFunction(CD, Ctx.VoidTy, F, FuncInfo, Args, CD->getBody()->getLocStart());
>> +  StartFunction(CD, Ctx.VoidTy, F, FuncInfo, Args,
>> +                CD->getLocation(),
>> +                CD->getBody()->getLocStart());
>> 
>>   // Set the context parameter in CapturedStmtInfo.
>>   llvm::Value *DeclPtr = LocalDeclMap[CD->getContextParam()];
>> 
>> Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=205999&r1=205998&r2=205999&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CGVTables.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGVTables.cpp Thu Apr 10 18:21:53 2014
>> @@ -217,7 +217,7 @@ void CodeGenFunction::StartThunk(llvm::F
>> 
>>   // Start defining the function.
>>   StartFunction(GlobalDecl(), ResultType, Fn, FnInfo, FunctionArgs,
>> -                SourceLocation());
>> +                MD->getLocation(), SourceLocation());
>> 
>>   // Since we didn't pass a GlobalDecl to StartFunction, do this ourselves.
>>   CGM.getCXXABI().EmitInstanceFunctionProlog(*this);
>> 
>> Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=205999&r1=205998&r2=205999&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Thu Apr 10 18:21:53 2014
>> @@ -503,6 +503,7 @@ void CodeGenFunction::StartFunction(Glob
>>                                     llvm::Function *Fn,
>>                                     const CGFunctionInfo &FnInfo,
>>                                     const FunctionArgList &Args,
>> +                                    SourceLocation Loc,
>>                                     SourceLocation StartLoc) {
>>   const Decl *D = GD.getDecl();
>> 
>> @@ -580,9 +581,7 @@ void CodeGenFunction::StartFunction(Glob
>>     QualType FnType =
>>       getContext().getFunctionType(RetTy, ArgTypes,
>>                                    FunctionProtoType::ExtProtoInfo());
>> -
>> -    DI->setLocation(StartLoc);
>> -    DI->EmitFunctionStart(GD, FnType, CurFn, Builder);
>> +    DI->EmitFunctionStart(GD, Loc, StartLoc, FnType, CurFn, Builder);
>>   }
>> 
>>   if (ShouldInstrumentFunction())
>> @@ -759,8 +758,24 @@ void CodeGenFunction::GenerateCode(Globa
>>   if (Stmt *Body = FD->getBody()) BodyRange = Body->getSourceRange();
>>   CurEHLocation = BodyRange.getEnd();
>> 
>> +  // Use the location of the start of the function to determine where
>> +  // the function definition is located. By default use the location
>> +  // of the declaration as the location for the subprogram. A function
>> +  // may lack a declaration in the source code if it is created by code
>> +  // gen. (examples: _GLOBAL__I_a, __cxx_global_array_dtor, thunk).
>> +  SourceLocation Loc;
>> +  if (FD) {
>> +    Loc = FD->getLocation();
>> +
>> +    // If this is a function specialization then use the pattern body
>> +    // as the location for the function.
>> +    if (const FunctionDecl *SpecDecl = FD->getTemplateInstantiationPattern())
>> +      if (SpecDecl->hasBody(SpecDecl))
>> +        Loc = SpecDecl->getLocation();
>> +  }
> 
> I'm not sure it's  ideal to have pushed this logic up out of
> CGDebugInfo into CodeGenFunction. Perhaps it's OK, but generally we
> try to keep the debug info stuff inside CGDebugInfo - perhaps we could
> have a specific wrapper of StartFunction in CGDebugInfo that does this
> stuff then calls into the current/main implementation?

If we want to have the possibility to override the location (thunks!) this needs to be where the location is extracted from the Decl. Also, there is already lots of similar code at all the call sites of StartFunction to extract StartLoc already.


> 
>> +
>>   // Emit the standard function prologue.
>> -  StartFunction(GD, ResTy, Fn, FnInfo, Args, BodyRange.getBegin());
>> +  StartFunction(GD, ResTy, Fn, FnInfo, Args, Loc, BodyRange.getBegin());
>> 
>>   // Generate the body of the function.
>>   PGO.assignRegionCounters(GD.getDecl(), CurFn);
>> 
>> Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.h?rev=205999&r1=205998&r2=205999&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CodeGenFunction.h (original)
>> +++ cfe/trunk/lib/CodeGen/CodeGenFunction.h Thu Apr 10 18:21:53 2014
>> @@ -1140,11 +1140,15 @@ public:
>> 
>>   void GenerateCode(GlobalDecl GD, llvm::Function *Fn,
>>                     const CGFunctionInfo &FnInfo);
>> +  /// \brief Emit code for the start of a function.
>> +  /// \param Loc       The location to be associated with the function.
>> +  /// \param StartLoc  The location of the function body.
>>   void StartFunction(GlobalDecl GD,
>>                      QualType RetTy,
>>                      llvm::Function *Fn,
>>                      const CGFunctionInfo &FnInfo,
>>                      const FunctionArgList &Args,
>> +                     SourceLocation Loc,
>>                      SourceLocation StartLoc);
> 
> It might be worth adding "= SourceLocation()" for both of these if
> we're allowing them to be unspecified. Save having lots of ",
> SourceLocation()" stuff in the call sites.

Now that I am sure that I updated all call sites, of course :-)
> 
>>   void EmitConstructorBody(FunctionArgList &Args);
>> 
>> Added: cfe/trunk/test/CodeGenCXX/globalinit-loc.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/globalinit-loc.cpp?rev=205999&view=auto
>> ==============================================================================
>> --- cfe/trunk/test/CodeGenCXX/globalinit-loc.cpp (added)
>> +++ cfe/trunk/test/CodeGenCXX/globalinit-loc.cpp Thu Apr 10 18:21:53 2014
>> @@ -0,0 +1,22 @@
>> +// RUN: %clang_cc1 -emit-llvm -g %s -o - | FileCheck %s
>> +// rdar://problem/14985269.
>> +//
>> +// Verify that the global init helper function does not get associated
>> +// with any source location.
>> +//
>> +// CHECK: define internal void @_GLOBAL__I_a
>> +// CHECK-NOT: !dbg
>> +// CHECK: "_GLOBAL__I_a", i32 0, {{.*}}, i32 0} ; [ DW_TAG_subprogram ] [line 0] [local] [def]
>> +# 99 "someheader.h"
>> +class A {
>> +public:
>> +  A();
>> +  int foo() { return 0; }
>> +};
>> +# 5 "main.cpp"
>> +A a;
>> +
>> +int f() {
>> +  return a.foo();
>> +}
>> +
>> 
>> 
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits




More information about the cfe-commits mailing list