This is an archive of the discontinued LLVM Phabricator instance.

Improve detection of global vs local variables
ClosedPublic

Authored by sammccall on Nov 21 2016, 3:30 AM.

Details

Summary

Improve detection of global vs local variables.

Currently when a global variable is optimized out or otherwise has an unknown
location (DW_AT_location is empty) it gets reported as local.

I added two new heuristics:

  • if a mangled name is present, the variable is global (or static)
  • if DW_AT_location is present but invalid, the variable is global (or static)

Diff Detail

Repository
rL LLVM

Event Timeline

sammccall updated this revision to Diff 78700.Nov 21 2016, 3:30 AM
sammccall retitled this revision from to Improve detection of global vs local variables.
sammccall updated this object.
sammccall added a subscriber: lldb-commits.

Please add @clayborg as reviewer. Also paste the dwarf generated for this case from both gcc and clang.

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
3911 ↗(On Diff #78700)

Why not put the checks in the same line. Something like...
bool is_static_lifetime = has_explicit_mangled || (has_explicit_location && !location.IsValid());

You probably need logical operator and not the bitwise.

3925 ↗(On Diff #78700)

similar issue here

sammccall updated this revision to Diff 78711.Nov 21 2016, 5:58 AM
sammccall marked 2 inline comments as done.

Address review comments.

@abidh: thanks for the pointer, still getting the hang of how reviews work here :-)

Also reordered the heuristics in the comment to match the code.

Test case:

struct Obj{};
namespace N { extern Obj& o; }
using N::o;
int main(){}

clang++ -g -O0 -o test.clang test.cc; readelf --debug-dump=info test.clang

 Compilation Unit @ offset 0x0:
  Length:        0x71 (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): clang version 4.0.0 	
   <10>   DW_AT_language    : 4	(C++)
   <12>   DW_AT_name        : (indirect string, offset: 0x15): test.cc	
   <16>   DW_AT_stmt_list   : 0x0	
   <1a>   DW_AT_comp_dir    : (indirect string, offset: 0x1d): /usr/local/google/home/sammccall/src/debugexample	
   <1e>   DW_AT_low_pc      : 0x400590	
   <26>   DW_AT_high_pc     : 0x8	
<1><2a>: Abbrev Number: 2 (DW_TAG_namespace)
   <2b>   DW_AT_name        : (indirect string, offset: 0x4f): N	
   <2f>   DW_AT_decl_file   : 1	
   <30>   DW_AT_decl_line   : 2	
<2><31>: Abbrev Number: 3 (DW_TAG_variable)
   <32>   DW_AT_name        : (indirect string, offset: 0x51): o	
   <36>   DW_AT_type        : <0x42>	
   <3a>   DW_AT_external    : 1	
   <3a>   DW_AT_decl_file   : 1	
   <3b>   DW_AT_decl_line   : 2	
   <3c>   DW_AT_declaration : 1	
   <3c>   DW_AT_location    : 0 byte block: 	()
   <3d>   DW_AT_linkage_name: (indirect string, offset: 0x57): _ZN1N1oE	
<2><41>: Abbrev Number: 0
<1><42>: Abbrev Number: 4 (DW_TAG_reference_type)
   <43>   DW_AT_type        : <0x47>	
<1><47>: Abbrev Number: 5 (DW_TAG_structure_type)
   <48>   DW_AT_name        : (indirect string, offset: 0x53): Obj	
   <4c>   DW_AT_byte_size   : 1	
   <4d>   DW_AT_declaration : 1	
<1><4d>: Abbrev Number: 6 (DW_TAG_imported_declaration)
   <4e>   DW_AT_decl_file   : 1	
   <4f>   DW_AT_decl_line   : 3	
   <50>   DW_AT_import      : <0x31>	[Abbrev Number: 3 (DW_TAG_variable)]
<1><54>: Abbrev Number: 7 (DW_TAG_subprogram)
   <55>   DW_AT_low_pc      : 0x400590	
   <5d>   DW_AT_high_pc     : 0x8	
   <61>   DW_AT_frame_base  : 1 byte block: 56 	(DW_OP_reg6 (rbp))
   <63>   DW_AT_name        : (indirect string, offset: 0x60): main	
   <67>   DW_AT_decl_file   : 1	
   <68>   DW_AT_decl_line   : 4	
   <69>   DW_AT_type        : <0x6d>	
   <6d>   DW_AT_external    : 1	
<1><6d>: Abbrev Number: 8 (DW_TAG_base_type)
   <6e>   DW_AT_name        : (indirect string, offset: 0x65): int	
   <72>   DW_AT_encoding    : 5	(signed)
   <73>   DW_AT_byte_size   : 4	
<1><74>: Abbrev Number: 0

g++ -g -O0 -o test.gcc test.cc; readelf --debug-dump=info test.gcc

 Compilation Unit @ offset 0x0:
  Length:        0x7f (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: 0x32): GNU C++ 4.8.4 -mtune=generic -march=x86-64 -g -O0 -fstack-protector	
   <10>   DW_AT_language    : 4	(C++)
   <11>   DW_AT_name        : (indirect string, offset: 0x7f): test.cc	
   <15>   DW_AT_comp_dir    : (indirect string, offset: 0x0): /usr/local/google/home/sammccall/src/debugexample	
   <19>   DW_AT_low_pc      : 0x40057d	
   <21>   DW_AT_high_pc     : 0xb	
   <29>   DW_AT_stmt_list   : 0x0	
<1><2d>: Abbrev Number: 2 (DW_TAG_structure_type)
   <2e>   DW_AT_name        : Obj	
   <32>   DW_AT_byte_size   : 1	
   <33>   DW_AT_decl_file   : 1	
   <34>   DW_AT_decl_line   : 1	
<1><35>: Abbrev Number: 3 (DW_TAG_namespace)
   <36>   DW_AT_name        : N	
   <38>   DW_AT_decl_file   : 1	
   <39>   DW_AT_decl_line   : 2	
   <3a>   DW_AT_sibling     : <0x4c>	
<2><3e>: Abbrev Number: 4 (DW_TAG_variable)
   <3f>   DW_AT_name        : o	
   <41>   DW_AT_decl_file   : 1	
   <42>   DW_AT_decl_line   : 2	
   <43>   DW_AT_linkage_name: (indirect string, offset: 0x76): _ZN1N1oE	
   <47>   DW_AT_type        : <0x4c>	
   <4b>   DW_AT_external    : 1	
   <4b>   DW_AT_declaration : 1	
<2><4b>: Abbrev Number: 0
<1><4c>: Abbrev Number: 5 (DW_TAG_const_type)
   <4d>   DW_AT_type        : <0x51>	
<1><51>: Abbrev Number: 6 (DW_TAG_reference_type)
   <52>   DW_AT_byte_size   : 8	
   <53>   DW_AT_type        : <0x2d>	
<1><57>: Abbrev Number: 7 (DW_TAG_imported_declaration)
   <58>   DW_AT_decl_file   : 1	
   <59>   DW_AT_decl_line   : 3	
   <5a>   DW_AT_import      : <0x3e>	[Abbrev Number: 4 (DW_TAG_variable)]
<1><5e>: Abbrev Number: 8 (DW_TAG_subprogram)
   <5f>   DW_AT_external    : 1	
   <5f>   DW_AT_name        : (indirect string, offset: 0x87): main	
   <63>   DW_AT_decl_file   : 1	
   <64>   DW_AT_decl_line   : 4	
   <65>   DW_AT_type        : <0x7b>	
   <69>   DW_AT_low_pc      : 0x40057d	
   <71>   DW_AT_high_pc     : 0xb	
   <79>   DW_AT_frame_base  : 1 byte block: 9c 	(DW_OP_call_frame_cfa)
   <7b>   DW_AT_GNU_all_call_sites: 1	
<1><7b>: Abbrev Number: 9 (DW_TAG_base_type)
   <7c>   DW_AT_byte_size   : 4	
   <7d>   DW_AT_encoding    : 5	(signed)
   <7e>   DW_AT_name        : int	
<1><82>: Abbrev Number: 0

Happy to add an automated test, LMK if it should be lit or follow unittests/SymbolFile/PDB, etc.

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
3925 ↗(On Diff #78700)

(I believe |= is fine in c++, because bools are 1 or 0. But an explicit test is clearer)

clayborg edited edge metadata.Nov 21 2016, 2:20 PM

Looks fine.

clayborg accepted this revision.Nov 21 2016, 2:20 PM
clayborg edited edge metadata.
This revision is now accepted and ready to land.Nov 21 2016, 2:20 PM
This revision was automatically updated to reflect the committed changes.