[DebugInfo] Add debug location to stubs generated by CGDeclCXX and mark them as artificial

[aganea]

Summary

Previously, when clang was compiled with -DLLVM_ENABLE_ASSERTIONS=ON, the attached test was yielding:

inlinable function call in a function with debug info must have a !dbg location
  call void @"??1?$c@UB@@@@QEAA@XZ"(%struct.c* @"?f@?1??d@@YAPEAU?$c@UB@@@@XZ@4U2@A")
fatal error: error in backend: Broken module found, compilation aborted!
Stack dump:
0.      Program arguments: <f:\svn\buildninja\bin\clang -cc1 -emit-llvm debug-info-no-location.cpp> -gcodeview -debug-info-kind=limited
1.      <eof> parser at end of file
2.      Per-function optimization

Fixes PR43012

[aprantl]

test/CodeGen/debug-info-no-location.cpp

4 ↗

(On Diff #215526)

Please don't hardcode metadata numbers in the tests, this is bound to break almost immediately.

It seems like this function is compiler-generated and thus should be marked as artificial?

[probinson]

"debug-info-no-location.cpp" is an extremely generic name for a very specific case. "debug-info-atexit-stub.cpp" would be better.

[rnk]

I'd be happy to take this patch, address the comments, and land it, if you don't want to deal with all the nits.

test/CodeGen/debug-info-no-location.cpp

1 ↗

(On Diff #215526)

I think the %clang_cc1 substitution is preferred, it sets the resource directory.

4 ↗

(On Diff #215526)

I think using an artificial location (line 0) is consistent with other calls to StartFunction in CGDeclCXX.

[aprantl]

test/CodeGen/debug-info-no-location.cpp

4 ↗

(On Diff #215526)

I was referring to the DIFlag::Artificial that translates into DISubprogram::isArtificial() here. The line 0 is orthogonal, but also nice to have!

[aganea]

As requested.

[probinson]

test/CodeGenCXX/debug-info-atexit-stub.cpp

14 ↗

(On Diff #216046)

Do these Windows-mangled names work on Linux?

[aganea]

test/CodeGenCXX/debug-info-atexit-stub.cpp

14 ↗

(On Diff #216046)

Ensure Microsoft mangling is always used.

[aganea]

Thank you!

[aganea]

Re-opening this because the previous commit broke Chromium.

• Added a new test debug-info-destroy-helper.cpp to cover the Chromium issue.
• Also added CGDeclCXX.cpp, L775 to create an artificial location for __cxx_global_array_dtor.

Would you possibly please take another look?

[probinson]

I don't see a test for the __cxx_global_array_dtor case?

[aganea]

In D66328#1647062, @probinson wrote:

I don't see a test for the __cxx_global_array_dtor case?

It is actually the arraydestroy.* loop that is covered (generated by CodeGenFunction::emitArrayDestroy), please see debug-info-destroy-helper.cpp below.

[probinson]

In D66328#1647094, @aganea wrote:

In D66328#1647062, @probinson wrote:

I don't see a test for the __cxx_global_array_dtor case?

It is actually the arraydestroy.* loop that is covered (generated by CodeGenFunction::emitArrayDestroy), please see debug-info-destroy-helper.cpp below.

I was expecting a check for the Artificial flag, though.

[aganea]

More tagging functions as artificial.

[rnk]

lib/CodeGen/CGDebugInfo.cpp

3581 ↗

(On Diff #217968)

Is it OK to look up the lexical block stack at this point? The block stack isn't function local, it's part of CGDebugInfo, which is for the whole module, unlike CodeGenFunction. If we start emitting one of these thunks while we're emitting some other function, we could get some strange results. Does anything ensure we've pushed at least one scope by the time we come here?

[aganea]

lib/CodeGen/CGDebugInfo.cpp

3581 ↗

(On Diff #217968)

You're right, it was looking too much like smart code. Replaced by a new flag DynamicInitKind::Global to make explicit the decision to emit an artificial function.

[aganea]

lib/CodeGen/CGDebugInfo.cpp

3581 ↗

(On Diff #217968)

Actually, it is DynamicInitKind::GlobalDestructor, sorry about that.

[rnk]

I'm trying to figure out what exactly the new GlobalDestructor thing is. :) It's not clear to me why we ever emit __cxx_global_array_dtor in the MS ABI. We always call it directly from the ?__F atexit helper stub instead of registering it, so we really don't need it at all. It seems like something you'd only need for Itanium + -fno-use-cxa-atexit. Anyway, there's no reason we can't give it it's own kind, since we need to have it for Itanium anyway.

[rnk]

lgtm with the GlobalArrayDestructor name.

include/clang/AST/GlobalDecl.h

34 ↗

(On Diff #218515)

Maybe we should rename this GlobalArrayDestructor, since it's only used for arrays. Later, we can come back and avoid emitting these when using the MS ABI, since we already have the ?__F finalizer.

cfe/trunk/include/clang/AST/GlobalDecl.h

 #include <cassert>
 
 namespace clang {
 
 enum class DynamicInitKind : unsigned {
   NoStub = 0,
   Initializer,
   AtExit,
+  GlobalArrayDestructor
 };
 
 /// GlobalDecl - represents a global declaration. This can either be a
 /// CXXConstructorDecl and the constructor type (Base, Complete).
 /// a CXXDestructorDecl and the destructor type (Base, Complete) or
 /// a VarDecl, a FunctionDecl or a BlockDecl.
 class GlobalDecl {
   llvm::PointerIntPair<const Decl *, 2> Value;

cfe/trunk/lib/CodeGen/CGDebugInfo.cpp

StringRef CGDebugInfo::getVTableName(const CXXRecordDecl *RD) {
   return internString("_vptr$", RD->getNameAsString());
 }
 
 StringRef CGDebugInfo::getDynamicInitializerName(const VarDecl *VD,
                                                  DynamicInitKind StubKind,
                                                  llvm::Function *InitFn) {
   // If we're not emitting codeview, use the mangled name. For Itanium, this is
   // arbitrary.
-  if (!CGM.getCodeGenOpts().EmitCodeView)
+  if (!CGM.getCodeGenOpts().EmitCodeView ||
+      StubKind == DynamicInitKind::GlobalArrayDestructor)
     return InitFn->getName();
 
   // Print the normal qualified name for the variable, then break off the last
   // NNS, and add the appropriate other text. Clang always prints the global
   // variable name without template arguments, so we can use rsplit("::") and
   // then recombine the pieces.
   SmallString<128> QualifiedGV;
   StringRef Quals;
   StringRef GVName;
   {
     llvm::raw_svector_ostream OS(QualifiedGV);
     VD->printQualifiedName(OS, getPrintingPolicy());
     std::tie(Quals, GVName) = OS.str().rsplit("::");
     if (GVName.empty())
       std::swap(Quals, GVName);
   }
 
   SmallString<128> InitName;
   llvm::raw_svector_ostream OS(InitName);
   if (!Quals.empty())
     OS << Quals << "::";
 
   switch (StubKind) {
   case DynamicInitKind::NoStub:
+  case DynamicInitKind::GlobalArrayDestructor:
     llvm_unreachable("not an initializer");
   case DynamicInitKind::Initializer:
     OS << "`dynamic initializer for '";
     break;
   case DynamicInitKind::AtExit:
     OS << "`dynamic atexit destructor for '";
     break;
   }
void CGDebugInfo::EmitFunctionStart(GlobalDecl GD, SourceLocation Loc,
   } else {
     // Use llvm function name.
     Name = Fn->getName();
     Flags |= llvm::DINode::FlagPrototyped;
   }
   if (Name.startswith("\01"))
     Name = Name.substr(1);
 
-  if (!HasDecl || D->isImplicit() || D->hasAttr<ArtificialAttr>()) {
+  if (!HasDecl || D->isImplicit() || D->hasAttr<ArtificialAttr>() ||
+      (isa<VarDecl>(D) && GD.getDynamicInitKind() != DynamicInitKind::NoStub)) {
     Flags |= llvm::DINode::FlagArtificial;
     // Artificial functions should not silently reuse CurLoc.
     CurLoc = SourceLocation();
   }
 
   if (CurFuncIsThunk)
     Flags |= llvm::DINode::FlagThunk;
 

cfe/trunk/lib/CodeGen/CGDeclCXX.cpp

llvm::Function *CodeGenFunction::createAtExitStub(const VarDecl &VD,
   const CGFunctionInfo &FI = CGM.getTypes().arrangeNullaryFunction();
   llvm::Function *fn = CGM.CreateGlobalInitOrDestructFunction(
       ty, FnName.str(), FI, VD.getLocation());
 
   CodeGenFunction CGF(CGM);
 
   CGF.StartFunction(GlobalDecl(&VD, DynamicInitKind::AtExit),
                     CGM.getContext().VoidTy, fn, FI, FunctionArgList());
+  // Emit an artificial location for this function.
+  auto AL = ApplyDebugLocation::CreateArtificial(CGF);
 
   llvm::CallInst *call = CGF.Builder.CreateCall(dtor, addr);
 
  // Make sure the call and the callee agree on calling convention.
   if (llvm::Function *dtorFn =
           dyn_cast<llvm::Function>(dtor.getCallee()->stripPointerCasts()))
     call->setCallingConv(dtorFn->getCallingConv());
 
void CodeGenFunction::GenerateCXXGlobalVarDeclInitFunc(llvm::Function *Fn,
   // Check if we need to emit debug info for variable initializer.
   if (D->hasAttr<NoDebugAttr>())
     DebugInfo = nullptr; // disable debug info indefinitely for this function
 
   CurEHLocation = D->getBeginLoc();
 
   StartFunction(GlobalDecl(D, DynamicInitKind::Initializer),
                 getContext().VoidTy, Fn, getTypes().arrangeNullaryFunction(),
-                FunctionArgList(), D->getLocation(),
-                D->getInit()->getExprLoc());
+                FunctionArgList());
+  // Emit an artificial location for this function.
+  auto AL = ApplyDebugLocation::CreateArtificial(*this);
 
   // Use guarded initialization if the global variable is weak. This
   // occurs for, e.g., instantiated static data members and
   // definitions explicitly marked weak.
   //
   // Also use guarded initialization for a variable with dynamic TLS and
   // unordered initialization. (If the initialization is ordered, the ABI
   // layer will guard the whole-TU initialization for us.)
llvm::Function *CodeGenFunction::generateDestroyHelper(
   const CGFunctionInfo &FI =
     CGM.getTypes().arrangeBuiltinFunctionDeclaration(getContext().VoidTy, args);
   llvm::FunctionType *FTy = CGM.getTypes().GetFunctionType(FI);
   llvm::Function *fn = CGM.CreateGlobalInitOrDestructFunction(
       FTy, "__cxx_global_array_dtor", FI, VD->getLocation());
 
   CurEHLocation = VD->getBeginLoc();
 
-  StartFunction(VD, getContext().VoidTy, fn, FI, args);
+  StartFunction(GlobalDecl(VD, DynamicInitKind::GlobalArrayDestructor),
+                getContext().VoidTy, fn, FI, args);
+  // Emit an artificial location for this function.
+  auto AL = ApplyDebugLocation::CreateArtificial(*this);
 
   emitDestroy(addr, type, destroyer, useEHCleanupForArray);
 
   FinishFunction();
 
   return fn;
 }

cfe/trunk/test/CodeGenCXX/debug-info-atexit-stub.cpp

+// RUN: %clang_cc1 -emit-llvm %s -triple x86_64-windows-msvc -gcodeview -debug-info-kind=limited -o - | FileCheck %s
+
+struct a {
+  ~a();
+};
+template <typename b> struct c : a {
+  c(void (b::*)());
+};
+struct B {
+  virtual void e();
+};
+c<B> *d() { static c<B> f(&B::e); return &f; }
+
+// CHECK: define internal void @"??__Ff@?1??d@@YAPEAU?$c@UB@@@@XZ@YAXXZ"()
+// CHECK-SAME: !dbg ![[SUBPROGRAM:[0-9]+]] {
+// CHECK: call void @"??1?$c@UB@@@@QEAA@XZ"(%struct.c* @"?f@?1??d@@YAPEAU?$c@UB@@@@XZ@4U2@A"), !dbg ![[LOCATION:[0-9]+]]
+// CHECK-NEXT: ret void, !dbg ![[LOCATION]]
+// CHECK: ![[SUBPROGRAM]] = distinct !DISubprogram(name: "`dynamic atexit destructor for 'f'"
+// CHECK-SAME: flags: DIFlagArtificial
+// CHECK: ![[LOCATION]] = !DILocation(line: 0, scope: ![[SUBPROGRAM]])
 No newline at end of file

cfe/trunk/test/CodeGenCXX/debug-info-destroy-helper.cpp

+// RUN: %clang_cc1 -emit-llvm %s -triple x86_64-windows-msvc -gcodeview -debug-info-kind=limited -o - | FileCheck %s
+
+struct b {
+  b(char *);
+  ~b();
+};
+struct a {
+  ~a();
+};
+struct {
+  b c;
+  const a &d;
+} e[]{nullptr, {}};
+
+// CHECK: define internal void @__cxx_global_array_dtor(i8* %0)
+// CHECK-SAME: !dbg ![[SUBPROGRAM:[0-9]+]] {
+// CHECK: arraydestroy.body
+// CHECK: %arraydestroy.elementPast =
+// CHECK-SAME: !dbg ![[LOCATION:[0-9]+]]
+// CHECK: call void @"??1<unnamed-type-e>@@QEAA@XZ"(%struct.anon* %arraydestroy.element)
+// CHECK-SAME: !dbg ![[LOCATION]]
+// CHECK: ![[SUBPROGRAM]] = distinct !DISubprogram(name: "__cxx_global_array_dtor"
+// CHECK-SAME: flags: DIFlagArtificial
+// CHECK: ![[LOCATION]] = !DILocation(line: 0,

cfe/trunk/test/CodeGenCXX/debug-info-global-ctor-dtor.cpp

struct FooTpl {
   template <typename U>
   static A sdm_tpl;
 };
 template <typename T>
 template <typename U>
 A FooTpl<T>::sdm_tpl(sizeof(U) + sizeof(T));
 template A FooTpl<int>::sdm_tpl<int>;
 
-// CHECK-NOKEXT: !DISubprogram(name: "__cxx_global_var_init",{{.*}} line: 15,{{.*}} DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-NOKEXT: !DISubprogram(name: "__dtor_glob",{{.*}} line: 15,{{.*}} DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-NOKEXT: !DISubprogram(name: "__cxx_global_var_init.1",{{.*}} line: 16,{{.*}} DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-NOKEXT: !DISubprogram(name: "__cxx_global_array_dtor",{{.*}} line: 16,{{.*}} DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-NOKEXT: !DISubprogram(name: "__dtor_array",{{.*}} line: 16,{{.*}} DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-NOKEXT: !DISubprogram(name: "__dtor__ZZ3foovE4stat",{{.*}} line: 19,{{.*}} DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-NOKEXT: !DISubprogram(name: "__cxx_global_var_init",{{.*}} flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-NOKEXT: !DISubprogram(name: "__dtor_glob",{{.*}} flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-NOKEXT: !DISubprogram(name: "__cxx_global_var_init.1",{{.*}} flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-NOKEXT: !DISubprogram(name: "__cxx_global_array_dtor",{{.*}} flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-NOKEXT: !DISubprogram(name: "__dtor_array",{{.*}} flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-NOKEXT: !DISubprogram(name: "__dtor__ZZ3foovE4stat",{{.*}} flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
 // CHECK-NOKEXT: !DISubprogram({{.*}} DISPFlagLocalToUnit | DISPFlagDefinition
 
 // CHECK-KEXT: !DISubprogram({{.*}} DISPFlagLocalToUnit | DISPFlagDefinition
 
-// CHECK-MSVC: !DISubprogram(name: "`dynamic initializer for 'glob'",{{.*}} line: 15,{{.*}}: DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-MSVC: !DISubprogram(name: "`dynamic atexit destructor for 'glob'",{{.*}} line: 15,{{.*}}: DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-MSVC: !DISubprogram(name: "`dynamic initializer for 'array'",{{.*}} line: 16,{{.*}}: DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-MSVC: !DISubprogram(name: "__cxx_global_array_dtor",{{.*}} line: 16,{{.*}}: DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-MSVC: !DISubprogram(name: "`dynamic atexit destructor for 'array'",{{.*}} line: 16,{{.*}}: DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-MSVC: !DISubprogram(name: "`dynamic atexit destructor for 'stat'",{{.*}} line: 19,{{.*}}: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-MSVC: !DISubprogram(name: "`dynamic initializer for 'glob'",{{.*}} flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-MSVC: !DISubprogram(name: "`dynamic atexit destructor for 'glob'",{{.*}} flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-MSVC: !DISubprogram(name: "`dynamic initializer for 'array'",{{.*}} flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-MSVC: !DISubprogram(name: "__cxx_global_array_dtor",{{.*}} flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-MSVC: !DISubprogram(name: "`dynamic atexit destructor for 'array'",{{.*}} flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-MSVC: !DISubprogram(name: "`dynamic atexit destructor for 'stat'",{{.*}} flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
 
 // MSVC does weird stuff when templates are involved, so we don't match exactly,
 // but these names are reasonable.
 // FIXME: These should not be marked DISPFlagLocalToUnit.
-// CHECK-MSVC: !DISubprogram(name: "FooTpl<int>::`dynamic initializer for 'sdm_tpl<int>'",{{.*}} line: 29,{{.*}}: DISPFlagLocalToUnit | DISPFlagDefinition
-// CHECK-MSVC: !DISubprogram(name: "FooTpl<int>::`dynamic atexit destructor for 'sdm_tpl<int>'",{{.*}} line: 29,{{.*}}: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-MSVC: !DISubprogram(name: "FooTpl<int>::`dynamic initializer for 'sdm_tpl<int>'",{{.*}} flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-MSVC: !DISubprogram(name: "FooTpl<int>::`dynamic atexit destructor for 'sdm_tpl<int>'",{{.*}} flags: DIFlagArtificial, spFlags: DISPFlagLocalToUnit | DISPFlagDefinition
+// CHECK-MSVC: !DISubprogram(linkageName: "_GLOBAL__sub_I_debug_info_global_ctor_dtor.cpp",{{.*}} flags: DIFlagArtificial
 No newline at end of file

cfe/trunk/test/CodeGenCXX/debug-info-line.cpp

 // CHECK: [[DBG_F4]] = !DILocation(line: 500,
 // CHECK: [[DBG_F5]] = !DILocation(line: 600,
 // CHECK: [[DBG_F6]] = !DILocation(line: 700,
 // CHECK: [[DBG_F7]] = !DILocation(line: 800,
 // CHECK: [[DBG_F8]] = !DILocation(line: 900,
 // CHECK: [[DBG_F9]] = !DILocation(line: 1000,
 // CHECK: [[DBG_F10_STORE]] = !DILocation(line: 1100,
 // CHECK: [[DBG_GLBL_CTOR_B]] = !DILocation(line: 1200,
-// CHECK: [[DBG_GLBL_DTOR_B]] = !DILocation(line: 1200,
+// CHECK: [[DBG_GLBL_DTOR_B]] = !DILocation(line: 0,
 // CHECK: [[DBG_F11]] = !DILocation(line: 1300,
 // CHECK: [[DBG_F12]] = !DILocation(line: 1400,
 // CHECK: [[DBG_F13]] = !DILocation(line: 1500,
 // CHECK: [[DBG_F14_CTOR_CALL]] = !DILocation(line: 1600,
 // CHECK: [[DBG_F15]] = !DILocation(line: 1700,
 // CHECK: [[DBG_F16]] = !DILocation(line: 1800,
 // CHECK: [[DBG_F17]] = !DILocation(line: 1900,
 // CHECK: [[DBG_F18_1]] = !DILocation(line: 2000,
 // CHECK: [[DBG_F18_2]] = !DILocation(line: 2001,
 // CHECK: [[DBG_F19_1]] = !DILocation(line: 2100,
 // CHECK: [[DBG_F19_2]] = !DILocation(line: 2101,
 // CHECK: [[DBG_F20_1]] = !DILocation(line: 2200,
 // CHECK: [[DBG_F23]] = !DILocation(line: 2500,
 // CHECK: [[DBG_F24]] = !DILocation(line: 2600,