Anonymous namespaces are missing import DW_TAG_imported_module.

[kromanova]

Summary

DW_TAG_imported_module is missing from the compile unit's root scope. Without this tag, anonymous namespace is not imported into the module scope.

Consider the following testcase. Lack of DW_TAG_imported_module prevents our debugger from displaying the value of ‘a’.
It looks like GDB imports the anonymous namespace automatically, so it does not have this problem.
If necessary, I could add an option to control generation of DW_TAG_imported_module for anonymous namespaces.

#include <stdio.h>

namespace
{

int a = 5;

}

int main()
{

printf("%d\n", a);

return 0;

}

[kromanova]

cleaned up the testcase a little.

[ygao]

Dummy comment to make the patch visible on the mailing list.

[dblaikie]

(usually it works better to just trash the code review & create a new one
if you forget to add the mailing list/cc's when creating it - because it'll
never send the intro mail again, but anyway)

I'm not sure if this is really a bug in the debug info - seems like it
might be reasonably treated as a bug in the debugger?

[probinson]

Our debugger guys get pretty pedantic about interpreting the DWARF spec. I know GDB will auto-import info from an anonymous namespace, but:

• the size cost here is really minimal (what, 5 or 6 bytes per anonymous namespace scope);
• they claimed that deciding whether to skip the namespace based on the name would be a problem for them;
• the compiler implementation is obviously trivial;

so I didn't really feel motivated to argue the point.

[kromanova]

David, I will re-post the review to LLVM-dev and abandon this one, if it's OK with you. You are right, it didn't reach LLVM-dev.
BTW, do you know how could I abandon Phabricator review?

[kromanova]

Hi,

Our debugger guys argument is that the debugger just follows the rules laid out by the debug information. If there is no import, the debugger will not import it. There is nothing in the DWARF spec that clearly states an empty name is a magic import.

Personally I don't have a strong opinion about this issue. It was very simple to do in the compiler and it hardly increased size of the DWARF info. Apparently, it was much tougher to fix this in the debugger.

If it is such a nuisance, maybe I could add an option and set it to false to all of the platforms, except PS4?
Or I could follow Fred's suggestion and add DW_AT_artificial attribute?

However I don’t think the patch will break any debugger (although that might be worth testing).

I could manually test GDB.
Fred, which other debuggers do you suggest to test to make sure that this patch didn't break them? Are there any existing debugger tests in LLVM?

Thanks!
Katya

lib/CodeGen/AsmPrinter/DwarfUnit.cpp

   DIE &NDie = createAndAddDIE(dwarf::DW_TAG_namespace, *ContextDIE, NS);
 
   StringRef Name = NS.getName();
-  if (!Name.empty())
+  bool Anonymous = Name.empty();
+  if (!Anonymous)
     addString(NDie, dwarf::DW_AT_name, NS.getName());
   else
     Name = "(anonymous namespace)";
   DD->addAccelNamespace(Name, NDie);
   addGlobalName(Name, NDie, NS.getContext());
+  if (Anonymous) { 
+    // For anonymous namespaces, add a DW_TAG_imported_module tag
+    // containing an DW_AT_import attribute with the reference to this
+    // namespace entry. 
+    DIE &IDie = createAndAddDIE(dwarf::DW_TAG_imported_module, *ContextDIE);
+    addDIEEntry(IDie, dwarf::DW_AT_import, NDie);
+  }
   addSourceLine(NDie, NS);
   return &NDie;
 }

test/DebugInfo/X86/gnu-public-names.ll

 ; in clang.
 ; CHECK:     DW_TAG_imported_module
 ; CHECK-NOT: {{DW_TAG|NULL}}
+; CHECK:     DW_TAG_imported_module
+; CHECK-NOT: {{DW_TAG|NULL}}
 ; CHECK:   NULL
 ; CHECK-NOT: {{DW_TAG|NULL}}
 

 ; CHECK:     DW_AT_name {{.*}} "i"
 ; CHECK-NOT: {{DW_TAG|NULL}}
 ; CHECK:   NULL
+; CHECK:     DW_TAG_imported_module
 ; CHECK-NOT: {{DW_TAG|NULL}}
 
 ; CHECK: [[MEM_FUNC:0x[0-9a-f]+]]: DW_TAG_subprogram

test/DebugInfo/anonymous-namespace.ll

+; RUN: llc -O0 < %s | FileCheck %s
+; If the namespace is anonymous, add a DW_TAG_imported_module tag
+; containing DW_AT_import attribute with the reference to this namespace
+; entry. 
+
+; namespace
+; {
+;   int a = 5;
+; }
+;
+; ModuleID = 'anonymous-namespace.cpp'
+
+@_ZN12_GLOBAL__N_11aE = internal global i32 5, align 4
+
+; Function Attrs: nounwind uwtable
+define i32 @_Z3foov() #0 {
+entry:
+  %0 = load i32* @_ZN12_GLOBAL__N_11aE, align 4, !dbg !15
+  %add = add nsw i32 %0, 2, !dbg !16
+  store i32 %add, i32* @_ZN12_GLOBAL__N_11aE, align 4, !dbg !17
+  %1 = load i32* @_ZN12_GLOBAL__N_11aE, align 4, !dbg !18
+  ret i32 %1, !dbg !19
+}
+
+attributes #0 = { nounwind uwtable "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!12, !13}
+!llvm.ident = !{!14}
+
+!0 = !{!"0x11\004\00clang version 3.7.0 (trunk 230293) (llvm/trunk 230292:230313M)\000\00\000\00\001", !1, !2, !2, !3, !9, !2} ; [ DW_TAG_compile_unit ] [/home/kromanova/anonymous-namespace.cpp] [DW_LANG_C_plus_plus]
+!1 = !{!"anonymous-namespace.cpp", !"/home/kromanova"}
+!2 = !{}
+!3 = !{!4}
+!4 = !{!"0x2e\00foo\00foo\00_Z3foov\005\000\001\000\000\00256\000\006", !1, !5, !6, null, i32 ()* @_Z3foov, null, null, !2} ; [ DW_TAG_subprogram ] [line 5] [def] [scope 6] [foo]
+!5 = !{!"0x29", !1}                               ; [ DW_TAG_file_type ] [/home/kromanova/anonymous-namespace.cpp]
+!6 = !{!"0x15\00\000\000\000\000\000\000", null, null, null, !7, null, null, null} ; [ DW_TAG_subroutine_type ] [line 0, size 0, align 0, offset 0] [from ]
+!7 = !{!8}
+!8 = !{!"0x24\00int\000\0032\0032\000\000\005", null, null} ; [ DW_TAG_base_type ] [int] [line 0, size 32, align 32, offset 0, enc DW_ATE_signed]
+!9 = !{!10}
+!10 = !{!"0x34\00a\00a\00_ZN12_GLOBAL__N_11aE\003\001\001", !11, !5, !8, i32* @_ZN12_GLOBAL__N_11aE, null} ; [ DW_TAG_variable ] [a] [line 3] [local] [def]
+!11 = !{!"0x39\00\002", !1, null}                 ; [ DW_TAG_namespace ] [line 2]
+!12 = !{i32 2, !"Dwarf Version", i32 4}
+!13 = !{i32 2, !"Debug Info Version", i32 2}
+!14 = !{!"clang version 3.7.0 (trunk 230293) (llvm/trunk 230292:230313M)"}
+!15 = !MDLocation(line: 7, column: 7, scope: !4)
+!16 = !MDLocation(line: 7, column: 8, scope: !4)
+!17 = !MDLocation(line: 7, column: 5, scope: !4)
+!18 = !MDLocation(line: 8, column: 10, scope: !4)
+!19 = !MDLocation(line: 8, column: 3, scope: !4)
+
+; int foo()
+; {
+;   a = a+2;
+;   return a;
+; }
+
+;CHECK: DW_TAG_imported_module
+;CHECK-NEXT: DW_AT_import
+
+