[llvm] r176143 - Debug Info: for static member variables, add AT_MIPS_linkage_name to the
Manman Ren
mren at apple.com
Tue Feb 26 17:13:03 PST 2013
On Feb 26, 2013, at 4:43 PM, Eric Christopher <echristo at gmail.com> wrote:
>
>
>
> On Tue, Feb 26, 2013 at 4:19 PM, Manman Ren <mren at apple.com> wrote:
>
> On Feb 26, 2013, at 4:09 PM, Eric Christopher <echristo at gmail.com> wrote:
>
>>
>> Debug Info: for static member variables, add AT_MIPS_linkage_name to the
>> definition DIE, to make old GDB happy.
>>
>>
>> Again, is this for compatibility or for correctness?
> To me this is to fix a regression. Not sure whether that belongs to compatibility or correctness.
>
>
> Correctness is something that should be so because it's defined to be (or in this case "defined" by the compiler that it started in), compatibility is because another tool has a hard time dealing with the input, but it's not something we control directly.
We are adding redundant information to make old GDB work. I think we can call this a compatibility issue.
>
>>
>> We have a regression for old GDB when Clang uses DW_TAG_member to declare
>> static members inside a class, instead of DW_TAG_variable. This patch will fix
>> this regression.
>>
>>
>> So, did you check the other uses of AT_MIPS_linkage_name?
> Do you mean uses of AT_MIPS_linkage_name by the debuggers?
>
>
> Output of AT_MIPS_linkage_name by both clang and gcc.
I did check uses of AT_MIPS_linkage_name by clang.
>
>>
>>
>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp?rev=176143&r1=176142&r2=176143&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp Tue Feb 26 18:02:32 2013
>> @@ -1348,9 +1348,15 @@ void CompileUnit::createGlobalVariableDI
>> }
>> // Add linkage name.
>> StringRef LinkageName = GV.getLinkageName();
>> - if (!LinkageName.empty() && isGlobalVariable)
>> + if (!LinkageName.empty() && isGlobalVariable) {
>> addString(VariableDIE, dwarf::DW_AT_MIPS_linkage_name,
>> getRealLinkageName(LinkageName));
>> + // To make old GDB happy, for static member variables, we add
>> + // AT_MIPS_linkage_name to the definition DIE as well.
>>
>> Can you explain this more? What's going on? What's the problem?
> I don't quite know how gdb works, but apparently we are getting errors when running gdb (GNU 6.3.50) with a simple testing case:
> class MyClass {
> public:
> static MyClass* A;
> MyClass() {
> }
> };
> MyClass* MyClass::A = 0;
>
>
> OK, this should be your testcase and you should be checking this for the debug information that you expect. Just create some IR, stick the C++ code in as a comment, and use llvm-dwarfdump to verify it.
I thought about adding this as a test case, but X86/debug-info-static-member.ll covers this case (it has static member variables and it checks llvm-dwarfdump), so I updated X86/debug-info-static-member.ll.
>
>
> clang++ test.cpp -O0 -g -c -o test3.o
> with Apple's gdb, run "gdb test3.o"
> Error: gdb does not handle things like classes defined inside functions correctly; debug info for 'test3.o' skipped
>
> This testing case used to work when we use DW_TAG_variable inside a class for static members.
> Adding this extra AT_MIPS_linkage_name to the definition DIE will make old GDB happy.
> It should not break other debugger.
>
> CC' Jim.
>
>
> The difference comes down to whether or not we're emitting something merely for compatibility or if we're more correct to do so. In this case we'll call correct the output of gcc as far as AT_MIPS_linkage_name is concerned, if they're not currently emitting it for that code then stick it under the compatibility option. I've gone ahead and run g++ on the file and the current output of gcc is (sorry, llvm-dwarfdump doesn't support all of dwarf-4 yet so it's not that pretty):
>
> Contents of the .debug_info section:
>
> Compilation Unit @ offset 0x0:
> Length: 0x5d (32-bit)
> Version: 4
> Abbrev Offset: 0x0
> Pointer Size: 8
> <0><b>: Abbrev Number: 1 (DW_TAG_compile_unit)
> <c> DW_AT_producer : (indirect string, offset: 0x0): GNU C++ 4.8.0 20130219 (experimental) -mtune=generic -march=x86-64 -g
> <10> DW_AT_language : 4 (C++)
> <11> DW_AT_name : (indirect string, offset: 0x81): foo.cpp
> <15> DW_AT_comp_dir : (indirect string, offset: 0x55): /usr/local/google/home/echristo/tmp
> <19> DW_AT_stmt_list : 0x0
> <1><1d>: Abbrev Number: 2 (DW_TAG_class_type)
> <1e> DW_AT_name : (indirect string, offset: 0x79): MyClass
> <22> DW_AT_byte_size : 1
> <23> DW_AT_decl_file : 1
> <24> DW_AT_decl_line : 1
> <25> DW_AT_sibling : <0x46>
> <2><29>: Abbrev Number: 3 (DW_TAG_member)
> <2a> DW_AT_name : A
> <2c> DW_AT_decl_file : 1
> <2d> DW_AT_decl_line : 3
> <2e> DW_AT_type : <0x46>
> <32> DW_AT_external : 1
> <32> DW_AT_accessibility: 1 (public)
> <33> DW_AT_declaration : 1
> <2><33>: Abbrev Number: 4 (DW_TAG_subprogram)
> <34> DW_AT_external : 1
> <34> DW_AT_name : (indirect string, offset: 0x79): MyClass
> <38> DW_AT_decl_file : 1
> <39> DW_AT_decl_line : 4
> <3a> DW_AT_accessibility: 1 (public)
> <3b> DW_AT_declaration : 1
> <3b> DW_AT_object_pointer: <0x3f>
> <3><3f>: Abbrev Number: 5 (DW_TAG_formal_parameter)
> <40> DW_AT_type : <0x46>
> <44> DW_AT_artificial : 1
> <1><46>: Abbrev Number: 6 (DW_TAG_pointer_type)
> <47> DW_AT_byte_size : 8
> <48> DW_AT_type : <0x1d>
> <1><4c>: Abbrev Number: 7 (DW_TAG_variable)
> <4d> DW_AT_specification: <0x29>
> <51> DW_AT_decl_line : 7
> <52> DW_AT_linkage_name: (indirect string, offset: 0x46): _ZN7MyClass1AE
> <56> DW_AT_location : 9 byte block: 3 0 0 0 0 0 0 0 0 (DW_OP_addr: 0)
>
>
> which seems to have the linkage name on the TAG_variable die and nowhere else (for this testcase) which partially seems what you're doing (adding it to the TAG_variable).
>
> 0x0000000b: DW_TAG_compile_unit [1] *
> 0x0000000c: DW_AT_producer [DW_FORM_strp] ( .debug_str[0x00000000] = "clang version 3.3 (trunk 176146) (llvm/trunk 176143)")
> 0x00000010: DW_AT_language [DW_FORM_data2] (0x0004)
> 0x00000012: DW_AT_name [DW_FORM_strp] ( .debug_str[0x00000035] = "foo.cpp")
> 0x00000016: DW_AT_low_pc [DW_FORM_addr] (0x0000000000000000)
> 0x0000001e: DW_AT_stmt_list [DW_FORM_data4] (0x00000000)
> 0x00000022: DW_AT_comp_dir [DW_FORM_strp] ( .debug_str[0x0000003d] = "/Users/echristo/tmp")
>
> 0x00000026: DW_TAG_pointer_type [2]
> 0x00000027: DW_AT_type [DW_FORM_ref4] (cu + 0x0030 => {0x00000030})
>
> 0x0000002b: DW_TAG_pointer_type [2]
> 0x0000002c: DW_AT_type [DW_FORM_ref4] (cu + 0x0030 => {0x00000030})
>
> 0x00000030: DW_TAG_class_type [3] *
> 0x00000031: DW_AT_name [DW_FORM_strp] ( .debug_str[0x00000053] = "MyClass")
> 0x00000035: DW_AT_byte_size [DW_FORM_data1] (0x01)
> 0x00000036: DW_AT_decl_file [DW_FORM_data1] (0x01)
> 0x00000037: DW_AT_decl_line [DW_FORM_data1] (0x01)
>
> 0x00000038: DW_TAG_member [4]
> 0x00000039: DW_AT_name [DW_FORM_strp] ( .debug_str[0x00000051] = "A")
> 0x0000003d: DW_AT_type [DW_FORM_ref4] (cu + 0x0026 => {0x00000026})
> 0x00000041: DW_AT_decl_file [DW_FORM_data1] (0x01)
> 0x00000042: DW_AT_decl_line [DW_FORM_data1] (0x03)
> 0x00000043: DW_AT_external [DW_FORM_flag] (0x01)
> 0x00000044: DW_AT_declaration [DW_FORM_flag] (0x01)
> 0x00000045: DW_AT_accessibility [DW_FORM_data1] (0x01)
> 0x00000046: DW_AT_MIPS_linkage_name [DW_FORM_strp] ( .debug_str[0x0000005b] = "_ZN7MyClass1AE")
>
> 0x0000004a: DW_TAG_subprogram [5] *
> 0x0000004b: DW_AT_name [DW_FORM_strp] ( .debug_str[0x00000053] = "MyClass")
> 0x0000004f: DW_AT_decl_file [DW_FORM_data1] (0x01)
> 0x00000050: DW_AT_decl_line [DW_FORM_data1] (0x04)
> 0x00000051: DW_AT_declaration [DW_FORM_flag] (0x01)
> 0x00000052: DW_AT_external [DW_FORM_flag] (0x01)
> 0x00000053: DW_AT_accessibility [DW_FORM_data1] (0x01)
>
> 0x00000054: DW_TAG_formal_parameter [6]
> 0x00000055: DW_AT_type [DW_FORM_ref4] (cu + 0x002b => {0x0000002b})
> 0x00000059: DW_AT_artificial [DW_FORM_flag] (0x01)
>
> 0x0000005a: NULL
>
> 0x0000005b: NULL
>
> 0x0000005c: DW_TAG_variable [7]
> 0x0000005d: DW_AT_specification [DW_FORM_ref4] (cu + 0x0038 => {0x00000038})
> 0x00000061: DW_AT_location [DW_FORM_block1] (<0x09> 03 78 02 00 00 00 00 00 00 )
> 0x0000006b: DW_AT_MIPS_linkage_name [DW_FORM_strp] ( .debug_str[0x0000005b] = "_ZN7MyClass1AE")
>
>
> It doesn't seem to be necessary on the member that we're adding it to above (according to gcc-4.8), but I don't know what the old gdb on darwin systems needs for TAG_member. It should, theoretically, be unnecessary since there's an AT_specification, but if it's a bug in the darwin gdb we should put it under a flag.
Yes gcc 4.7 has MIPS_linkage_name on TAG_variable (the definition DIE), gcc 4.4.3 has it on the declaration DIE according to Paul.
Before my checkins, Clang has MIPS_linkage_name on the declaration DIE and we get error at startup of our old GDB.
The checkin at r176120 tries to move MIPS_linkage_name from the declaration DIE to the definition DIE, but it didn't work out for the old GDB.
It didn't report the error at startup, but when trying to print C::a from static-member.cpp, it says "optimized away" instead of "$1 = 4".
The checkin at r176143 keeps MIPS_linkage_name on the declaration DIE and adds MIPS_linkage_name to the definition DIE as well.
It should not hurt other debuggers, but it increases the dwarf size a little.
The darwin gdb is not up to date, and we can probably say it is a bug there.
Thanks,
Manman
>
> Make sense?
>
> -eric
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130226/02e39b8d/attachment.html>
More information about the llvm-commits
mailing list