[DebugInfo] Fix debug info for imported declarations of inlined functions

[ellis]

Summary

When searching for a subprogram DIE in getOrCreateSubprogramDIE(), also search through the abstract origin DIEs by default. This fixes imported declarations that reference inlined functions which are also removed.

The exception is in updateSubprogramScopeDIE() where we do not want to add pc attributes to abstract origin SPs.

Special thanks to @krisb for finding this bug in https://reviews.llvm.org/D109703#2998350.

Fixes https://bugs.llvm.org/show_bug.cgi?id=52159

[krisb]

Thank you for the patch! I thought about exactly the same change :)
But I also started thinking that we may have similar problems for other entities that have inlined function as a scope.
For example, a type defined within a function will also have an empty parent subprogram:

inline __attribute__((always_inline))
int foo() {
  struct S { int a; int b; };
  S s({4,5});
  return s.a;
}

int main() {
  return foo();
}

It would be great if we could fix all the issues with inlined functions here.

[dblaikie]

There have been a few different patches going around related to mis-scoped things - imported entities and types, maybe other things too? (static variables? Or has that already been fixed now) Perhaps some summary of the state of all these patches sent to llvm-dev/linked from the various reviews would be helpful? It'd be good to know if/how all these patches are hopefully aiming for some unified solution.

[ellis]

Make the solution more general

[dblaikie]

I don't think this is the right approach - because it depends on the abstract SP being created before the query - and would produce a different result if the abstract SP is created after the query. Abstract SPs are created only once an inlined subroutine is discovered, I believe - so the DWARF would be different depending on function ordering - in some cases having things attached to the abstract DIE, in some cases to the concrete DIE. That's not ideal.

Looking at the callers, for instance - constructCallSiteEntryDIE looks like it could have that issue with the new getOrCreateSubprogramDIE behavior - depending on whether the call site is before or after the first inlining of the caller, the DWARF would point to the abstract origin or the (possibly incorrect/invalid/weird-empty) concrete subprogram DIE, perhaps?

[ellis]

In D110294#3040455, @dblaikie wrote:

I don't think this is the right approach - because it depends on the abstract SP being created before the query - and would produce a different result if the abstract SP is created after the query. Abstract SPs are created only once an inlined subroutine is discovered, I believe - so the DWARF would be different depending on function ordering - in some cases having things attached to the abstract DIE, in some cases to the concrete DIE. That's not ideal.

Looking at the callers, for instance - constructCallSiteEntryDIE looks like it could have that issue with the new getOrCreateSubprogramDIE behavior - depending on whether the call site is before or after the first inlining of the caller, the DWARF would point to the abstract origin or the (possibly incorrect/invalid/weird-empty) concrete subprogram DIE, perhaps?

I found that constructCallSiteEntryDIE() is actually called in endFunctionImpl() after constructAbstractSubprogramScopeDIE() is called for each scope in the function, so those inlined SPs will have abstract origins.

But regardless, I kind of agree with you since there are several call sites and I'm not sure I've considered all cases. For now, I'm going to update D108492 so that it doesn't depend on this and I'll also send an email to the llvm-dev explaining what I know about this. Thanks!

[ellis]

Moved to https://reviews.llvm.org/D112337

llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp

 //===- llvm/CodeGen/DwarfCompileUnit.cpp - Dwarf Compile Units ------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
void DwarfCompileUnit::attachLowHighPC(DIE &D, const MCSymbol *Begin,
   else
     addLabelDelta(D, dwarf::DW_AT_high_pc, End, Begin);
 }
 
 // Find DIE for the given subprogram and attach appropriate DW_AT_low_pc
 // and DW_AT_high_pc attributes. If there are global variables in this
 // scope then create and insert DIEs for these variables.
 DIE &DwarfCompileUnit::updateSubprogramScopeDIE(const DISubprogram *SP) {
-  DIE *SPDie = getOrCreateSubprogramDIE(SP, includeMinimalInlineScopes());
+  // Do not consider abstract SP DIEs since they should not have pc attributes.
+  DIE *SPDie = getOrCreateSubprogramDIE(SP, includeMinimalInlineScopes(),
+                                        /*AllowAbstract=*/false);
 
   SmallVector<RangeSpan, 2> BB_List;
   // If basic block sections are on, ranges for each basic block section has
   // to be emitted separately.
   for (const auto &R : Asm->MBBSectionRanges)
     BB_List.push_back({R.second.BeginLabel, R.second.EndLabel});
 
   attachRangesOrLowHighPC(*SPDie, BB_List);

llvm/lib/CodeGen/AsmPrinter/DwarfUnit.h

 //===-- llvm/CodeGen/DwarfUnit.h - Dwarf Compile Unit ---*- C++ -*--===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
public:
   ///
   /// This takes and attribute parameter because DW_AT_friend attributes are
   /// also type references.
   void addType(DIE &Entity, const DIType *Ty,
                dwarf::Attribute Attribute = dwarf::DW_AT_type);
 
   DIE *getOrCreateNameSpace(const DINamespace *NS);
   DIE *getOrCreateModule(const DIModule *M);
-  DIE *getOrCreateSubprogramDIE(const DISubprogram *SP, bool Minimal = false);
+  DIE *getOrCreateSubprogramDIE(const DISubprogram *SP, bool Minimal = false,
+                                bool AllowAbstract = true);
 
   void applySubprogramAttributes(const DISubprogram *SP, DIE &SPDie,
                                  bool SkipSPAttributes = false);
 
   /// Creates type DIE with specific context.
   DIE *createTypeDIE(const DIScope *Context, DIE &ContextDIE, const DIType *Ty);
 
   /// Find existing DIE or create new DIE for the given type.

llvm/lib/CodeGen/AsmPrinter/DwarfUnit.cpp

 //===-- llvm/CodeGen/DwarfUnit.cpp - Dwarf Type and Compile Units ---------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
DIE *DwarfUnit::getOrCreateModule(const DIModule *M) {
   if (M->getLineNo())
     addUInt(MDie, dwarf::DW_AT_decl_line, None, M->getLineNo());
   if (M->getIsDecl())
     addFlag(MDie, dwarf::DW_AT_declaration);
 
   return &MDie;
 }
 
-DIE *DwarfUnit::getOrCreateSubprogramDIE(const DISubprogram *SP, bool Minimal) {
+DIE *DwarfUnit::getOrCreateSubprogramDIE(const DISubprogram *SP, bool Minimal,
+                                         bool AllowAbstract) {
   // Construct the context before querying for the existence of the DIE in case
   // such construction creates the DIE (as is the case for member function
   // declarations).
   DIE *ContextDIE =
       Minimal ? &getUnitDie() : getOrCreateContextDIE(SP->getScope());
 
   if (DIE *SPDie = getDIE(SP))
     return SPDie;
 
   if (auto *SPDecl = SP->getDeclaration()) {
     if (!Minimal) {
       // Add subprogram definitions to the CU die directly.
       ContextDIE = &getUnitDie();
       // Build the decl now to ensure it precedes the definition.
       getOrCreateSubprogramDIE(SPDecl);
     }
   }
 
+  if (AllowAbstract)
+    if (auto *SPDie = DU->getAbstractSPDies().lookup(SP))
+      return SPDie;
+
   // DW_TAG_inlined_subroutine may refer to this DIE.
   DIE &SPDie = createAndAddDIE(dwarf::DW_TAG_subprogram, *ContextDIE, SP);
 
   // Stop here and fill this in later, depending on whether or not this
   // subprogram turns out to have inlined instances or not.
   if (SP->isDefinition())
     return &SPDie;
 

llvm/test/DebugInfo/Generic/import-inlined-declaration.ll

+; RUN: %llc_dwarf -O0 -filetype=obj < %s | llvm-dwarfdump - | FileCheck --implicit-check-not "{{DW_TAG|NULL}}" %s
+
+; namespace ns {
+; inline __attribute__((always_inline))
+; void foo() { int a = 4; }
+; }
+;
+; void goo() {
+;   using ns::foo;
+;   foo();
+; }
+
+; Ensure that imported declarations reference the correct subprograms even if
+; those subprograms are inlined.
+
+; CHECK: DW_TAG_compile_unit
+; CHECK:   DW_TAG_namespace
+; CHECK:     DW_AT_name     ("ns")
+; CHECK: [[FOO:0x.*]]:     DW_TAG_subprogram
+; CHECK:       DW_AT_name   ("foo")
+; CHECK:       DW_TAG_variable
+; CHECK:       NULL
+; CHECK:     NULL
+; CHECK:   DW_TAG_base_type
+; CHECK:   DW_TAG_subprogram
+; CHECK:     DW_AT_name     ("goo")
+; CHECK:     DW_TAG_imported_declaration
+; CHECK:       DW_AT_import ([[FOO]])
+; CHECK:     DW_TAG_inlined_subroutine
+; CHECK:       DW_TAG_variable
+; CHECK:       NULL
+; CHECK:     NULL
+; CHECK:   NULL
+
+; Function Attrs: mustprogress noinline optnone uwtable
+define dso_local void @_Z3goov() !dbg !4 {
+entry:
+  %a.i = alloca i32, align 4
+  call void @llvm.dbg.declare(metadata i32* %a.i, metadata !16, metadata !DIExpression()), !dbg !18
+  store i32 4, i32* %a.i, align 4, !dbg !18
+  ret void, !dbg !20
+}
+
+; Function Attrs: nofree nosync nounwind readnone speculatable willreturn
+declare void @llvm.dbg.declare(metadata, metadata, metadata)
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!10, !11, !12, !13, !14}
+!llvm.ident = !{!15}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C_plus_plus_14, file: !1, producer: "clang version 14.0.0", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, imports: !2, splitDebugInlining: false, nameTableKind: None)
+!1 = !DIFile(filename: "imported-inlined-declaration.cpp", directory: "")
+!2 = !{!3}
+!3 = !DIImportedEntity(tag: DW_TAG_imported_declaration, scope: !4, entity: !8, file: !1, line: 7)
+!4 = distinct !DISubprogram(name: "goo", linkageName: "_Z3goov", scope: !1, file: !1, line: 6, type: !5, scopeLine: 6, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !7)
+!5 = !DISubroutineType(types: !6)
+!6 = !{null}
+!7 = !{}
+!8 = distinct !DISubprogram(name: "foo", linkageName: "_ZN2ns3fooEv", scope: !9, file: !1, line: 3, type: !5, scopeLine: 3, flags: DIFlagPrototyped, spFlags: DISPFlagDefinition, unit: !0, retainedNodes: !7)
+!9 = !DINamespace(name: "ns", scope: null)
+!10 = !{i32 7, !"Dwarf Version", i32 4}
+!11 = !{i32 2, !"Debug Info Version", i32 3}
+!12 = !{i32 1, !"wchar_size", i32 4}
+!13 = !{i32 7, !"uwtable", i32 1}
+!14 = !{i32 7, !"frame-pointer", i32 2}
+!15 = !{!"clang version 14.0.0"}
+!16 = !DILocalVariable(name: "a", scope: !8, file: !1, line: 3, type: !17)
+!17 = !DIBasicType(name: "int", size: 32, encoding: DW_ATE_signed)
+!18 = !DILocation(line: 3, column: 18, scope: !8, inlinedAt: !19)
+!19 = distinct !DILocation(line: 8, column: 2, scope: !4)
+!20 = !DILocation(line: 9, column: 1, scope: !4)