[DebugInfo][dsymutil] Keep locations for function-local globals

[ellis]

Summary

In debug info, we expect variables to have location info if they are used and we don't want location info for functions that are not used. However, if an unused function is inlined, we could have the scenario where a function is not in the final binary but its static variable is in the final binary. Ensure that variables in the final binary have location debug info even if their scope was inlined.

Also add --implicit-check-not to a test for clarity.

[ellis]

Fix test

[ellis]

Change comment.

[ellis]

Add entry to debug map.

[ellis]

Add comments to test.

[JDevlieghere]

Small readability nit, but otherwise this LGTM.

llvm/lib/DWARFLinker/DWARFLinker.cpp

1426–1432

Just for symmetry with the clause above, I would structure this slightly differently.

[ellis]

Restructure logic.

llvm/lib/DWARFLinker/DWARFLinker.cpp

DIE *DWARFLinker::DIECloner::cloneDIE(const DWARFDie &InputDIE,
   if (Die->getTag() == dwarf::DW_TAG_subprogram)
     PCOffset = Info.AddrAdjust;
   AttrInfo.PCOffset = PCOffset;
 
   if (Abbrev->getTag() == dwarf::DW_TAG_subprogram) {
     Flags |= TF_InFunctionScope;
     if (!Info.InDebugMap && LLVM_LIKELY(!Update))
       Flags |= TF_SkipPC;
+  } else if (Abbrev->getTag() == dwarf::DW_TAG_variable) {
+    // Function-local globals could be in the debug map even when the function
+    // is not, e.g., inlined functions.
+    if ((Flags & TF_InFunctionScope) && Info.InDebugMap)
+      Flags &= ~TF_SkipPC;
   }
 
   for (const auto &AttrSpec : Abbrev->attributes()) {
     if (LLVM_LIKELY(!Update) &&
         shouldSkipAttribute(AttrSpec, Die->getTag(), Info.InDebugMap,
                             Flags & TF_SkipPC, Flags & TF_InFunctionScope)) {
       DWARFFormValue::skipValue(AttrSpec.Form, Data, &Offset,
                                 U.getFormParams());
       continue;
     }

llvm/test/tools/dsymutil/X86/dead-stripped.cpp

-// RUN: dsymutil -f -y %p/dummy-debug-map.map -oso-prepend-path %p/../Inputs/dead-stripped -o - | llvm-dwarfdump - --debug-info | FileCheck %s
+// RUN: dsymutil -f -y %p/dummy-debug-map.map -oso-prepend-path %p/../Inputs/dead-stripped -o - | llvm-dwarfdump - --debug-info | FileCheck %s --implicit-check-not "{{DW_AT_low_pc|DW_AT_high_pc|DW_AT_location|DW_TAG|NULL}}"
 
 // The test was compiled with:
 // clang++ -O2 -g -c dead-strip.cpp -o 1.o
 
 // The goal of the test is to exercise dsymutil's behavior in presence of
 // functions/variables that have been dead-stripped by the linker but
 // that are still present in the linked debug info (in this case because
 // they have been DW_TAG_import'd in another namespace).
 
-// Everything in the N namespace bellow doesn't have a debug map entry, and
-// thus is considered dead (::foo() has a debug map entry, otherwse dsymutil
+// Everything in the N namespace below doesn't have a debug map entry, and
+// thus is considered dead (::foo() has a debug map entry, otherwise dsymutil
 // would just drop the CU altogether).
 
+// CHECK: DW_TAG_compile_unit
+// CHECK:   DW_AT_low_pc
+// CHECK:   DW_AT_high_pc
+// CHECK:   DW_TAG_namespace
 namespace N {
 int blah = 42;
 // This is actually a dsymutil-classic bug that we reproduced
 // CHECK: DW_TAG_variable
-// CHECK-NOT: DW_TAG
 // CHECK: DW_AT_location
 
 __attribute__((always_inline)) int foo() { return blah; }
 // CHECK: DW_TAG_subprogram
-// CHECK-NOT: DW_AT_low_pc
-// CHECK-NOT: DW_AT_high_pc
-// CHECK: DW_AT_frame_base
+// CHECK:   DW_AT_frame_base
 
 // CHECK: DW_TAG_subprogram
 
 int bar(unsigned i) {
   int val = foo();
   if (i)
     return val + bar(i-1);
   return foo();
 }
 // CHECK: DW_TAG_subprogram
-// CHECK-NOT: DW_AT_low_pc
-// CHECK-NOT: DW_AT_high_pc
-// CHECK: DW_AT_frame_base
-// CHECK-NOT: DW_AT_location
-// CHECK-NOT: DW_AT_low_pc
-// CHECK-NOT: DW_AT_high_pc
-
+// CHECK:   DW_AT_frame_base
+// CHECK:   DW_TAG_formal_parameter
+// CHECK:   DW_TAG_variable
+// CHECK:   DW_TAG_inlined_subroutine
+// CHECK:   NULL
+// CHECK: NULL
 }
-// CHECK: TAG_imported_module
+// CHECK: DW_TAG_base_type
+// CHECK: DW_TAG_imported_module
+// CHECK: DW_TAG_subprogram
+// CHECK:   DW_AT_low_pc
+// CHECK:   DW_AT_high_pc
+// CHECK: DW_TAG_base_type
+// CHECK: NULL
+
 using namespace N;
 
 void foo() {}

llvm/test/tools/dsymutil/X86/dummy-debug-map.map

symbols:
       - { sym: __Z3foov, objAddr: 0x0, binAddr: 0x10000, size: 0x10 }
   - filename: 2.o
     symbols:
       - { sym: __Z3foov, objAddr: 0x0, binAddr: 0x20000, size: 0x10 }
   - filename: 3.o
     symbols:
       - { sym: __Z3foov, objAddr: 0x0, binAddr: 0x30000, size: 0x10 }
       - { sym: __ZN1S3bazIiEEvT_, objAddr: 0x0, binAddr: 0x30010, size: 0x10 }
+  - filename: 4.o
+    symbols:
+      - { sym: __Z3foov, objAddr: 0x0, binAddr: 0x40000, size: 0x10 }
+      - { sym: __ZZ11not_removedvE1b, objAddr: 0x0, binAddr: 0x40010, size: 0x10 }
+      - { sym: __ZZ7removedvE1a, objAddr: 0x0, binAddr: 0x40020, size: 0x10 }
 ...
 

llvm/test/tools/dsymutil/X86/inlined-static-variable.cpp

+// RUN: dsymutil -f -y %p/dummy-debug-map.map -oso-prepend-path %p/../Inputs/inlined-static-variable -o - | llvm-dwarfdump - | FileCheck %s --implicit-check-not "{{DW_AT_low_pc|DW_AT_high_pc|DW_AT_location|DW_TAG|NULL}}"
+
+// clang -g -c inlined-static-variable.cpp -o 4.o
+
+// The functions removed and not_removed are not in the debug map and are
+// considered dead, but they are also inlined into the function foo which is
+// in the debug map. Those function-local globals are alive and thus should
+// have locations in the debug info even if their functions do not.
+
+inline __attribute__((always_inline)) int removed() {
+  static int a = 0;
+  return ++a;
+}
+
+__attribute__((always_inline)) int not_removed() {
+  static int b = 0;
+  return ++b;
+}
+
+int unused() {
+  static int c = 0;
+  return ++c;
+}
+
+int foo() {
+  return removed() + not_removed();
+}
+
+// CHECK: DW_TAG_compile_unit
+// CHECK:   DW_AT_low_pc
+// CHECK:   DW_AT_high_pc
+
+// CHECK:   DW_TAG_subprogram
+// CHECK:     DW_AT_name  ("removed")
+// CHECK:     DW_TAG_variable
+// CHECK:       DW_AT_name  ("a")
+// CHECK:       DW_AT_location
+// CHECK:     NULL
+
+// CHECK:   DW_TAG_base_type
+// CHECK:   DW_TAG_subprogram
+// CHECK:     DW_AT_name  ("not_removed")
+// CHECK:     DW_TAG_variable
+// CHECK:       DW_AT_name  ("b")
+// CHECK:       DW_AT_location
+// CHECK:     NULL
+
+// CHECK:   DW_TAG_subprogram
+// CHECK:     DW_AT_low_pc
+// CHECK:     DW_AT_high_pc
+// CHECK:     DW_AT_name  ("foo")
+// CHECK:     DW_TAG_inlined_subroutine
+// CHECK:       DW_AT_low_pc
+// CHECK:       DW_AT_high_pc
+// CHECK:     DW_TAG_inlined_subroutine
+// CHECK:       DW_AT_low_pc
+// CHECK:       DW_AT_high_pc
+// CHECK:     NULL
+// CHECK:   NULL