[DWARF] Assign the correct scope to constant variables

[davide]

Summary

SymbolFileDWARF::ParseVariableDIE consider all constant variables as "static".
This is incorrect, and causes frame var to fail when printing them.

e.g.

volatile int a;	
   main() {
     	  {
     	    int b = 3;
   	    a;
   	  }
   	}

const_value variables are more common at -O1/-O2/.., so this wasn't noticed by many.

Fixes rdar://problem/61402307

I also need to craft a testcase. Ideas on how to do this are appreciated

[davide]

See also https://bugs.llvm.org/show_bug.cgi?id=45471

[labath]

I also need to craft a testcase. Ideas on how to do this are appreciated

You could check in the assembly generated by clang for this test case and then run llvm-mc && lldb-test symbols over it. Currently I get something like Variable{0x7fffffff0000007f}, name = "b", type = {7fffffff00000044} 0x0000564FADF7F9F0 (int), scope = ??? (2) when running that. After this patch that should presumably say scope = local. That won't actually check that the "frame variable" command succeeds, but maybe that's enough for this patch?

Using llvm ir instead of assembly might work too .. the relevant ir (call void @llvm.dbg.value(metadata i32 3, metadata !16, metadata !DIExpression()), !dbg !18) generates DW_AT_const_value even without any further optimizations, and it looks like it could be reasonably expected to keep doing that in the future. Adding @aprantl for IR insights.

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp

1028–1032

This comment does not make sense in the new setting.

[davide]

Added test, updated comment.

[jingham]

The code looks good to me. I'll defer to Adrian as to whether the test is robust, though it also seems reasonable to deal with a failure caused by llvm changes when it happens.

[davide]

commit d51b38f1b3a34c2a8e1869af6434ebd743ce7a5e (HEAD -> master, origin/master, origin/HEAD)
Author: Davide Italiano <[e-mail]>
Date: Wed Apr 8 11:06:00 2020 -0700

[DWARF] Not all the constant variables are "static".

Fixes rdar://problem/61402307

Differential Revision: https://reviews.llvm.org/D77698

[aprantl]

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp

1028

[Sorry for showing up late.]

This seems unintuitive. How is

void f() {
static int g_i = 0;
}

represented in DWARF?

1029

To save everyone the trouble:

0x0000002a:   DW_TAG_subprogram
                DW_AT_low_pc	(0x0000000000000000)
                DW_AT_high_pc	(0x0000000000000006)
                DW_AT_frame_base	(DW_OP_reg6 RBP)
                DW_AT_name	("f")
                DW_AT_decl_file	("/tmp/t.c")
                DW_AT_decl_line	(1)
                DW_AT_external	(true)

0x0000003f:     DW_TAG_variable
                  DW_AT_name	("g_i")
                  DW_AT_type	(0x00000055 "int")
                  DW_AT_decl_file	("/tmp/t.c")
                  DW_AT_decl_line	(2)
                  DW_AT_location	(DW_OP_addr 0x280)

0x00000054:     NULL

[davide]

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp

1029

hmm, so, what's the DWARF'y way to find out whether a variable is static?
This code is just moved from another place which did this.

[aprantl]

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp

1029

Looks like the function's name is just confusing. We should add a doxgygen comment for it,

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h

   void SetSiblingIndex(uint32_t idx) { m_sibling_idx = idx; }
   void SetParentIndex(uint32_t idx) { m_parent_idx = idx; }
 
+  bool IsGlobalOrStaticVariable() const;
+
 protected:
   static DWARFDeclContext
   GetDWARFDeclContextStatic(const DWARFDebugInfoEntry *die, DWARFUnit *cu);

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp

   return nullptr;
 }
 
+bool DWARFDebugInfoEntry::IsGlobalOrStaticVariable() const {
+  if (Tag() != DW_TAG_variable)
+    return false;
+  const DWARFDebugInfoEntry *parent_die = GetParent();
+  while (parent_die != nullptr) {
+    switch (parent_die->Tag()) {
+    case DW_TAG_subprogram:
+    case DW_TAG_lexical_block:
+    case DW_TAG_inlined_subroutine:
+      return false;
+
+    case DW_TAG_compile_unit:
+    case DW_TAG_partial_unit:
+      return true;
+
+    default:
+      break;
+    }
+    parent_die = parent_die->GetParent();
+  }
+  return false;
+}
+
 bool DWARFDebugInfoEntry::operator==(const DWARFDebugInfoEntry &rhs) const {
   return m_offset == rhs.m_offset && m_parent_idx == rhs.m_parent_idx &&
          m_sibling_idx == rhs.m_sibling_idx &&

lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp

         case DW_AT_location:
         case DW_AT_const_value:
           has_location_or_const_value = true;
-          if (tag == DW_TAG_variable) {
-            const DWARFDebugInfoEntry *parent_die = die.GetParent();
-            while (parent_die != nullptr) {
-              switch (parent_die->Tag()) {
-              case DW_TAG_subprogram:
-              case DW_TAG_lexical_block:
-              case DW_TAG_inlined_subroutine:
-                // Even if this is a function level static, we don't add it. We
-                // could theoretically add these if we wanted to by
-                // introspecting into the DW_AT_location and seeing if the
-                // location describes a hard coded address, but we don't want
-                // the performance penalty of that right now.
-                is_global_or_static_variable = false;
-                parent_die = nullptr; // Terminate the while loop.
-                break;
-
-              case DW_TAG_compile_unit:
-              case DW_TAG_partial_unit:
-                is_global_or_static_variable = true;
-                parent_die = nullptr; // Terminate the while loop.
-                break;
-
-              default:
-                parent_die =
-                    parent_die->GetParent(); // Keep going in the while loop.
-                break;
-              }
-            }
-          }
+          is_global_or_static_variable = die.IsGlobalOrStaticVariable();
+
           break;
 
         case DW_AT_specification:

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

             }
           }
         } else {
-          if (location_is_const_value_data)
+          if (location_is_const_value_data && die.GetDIE()->IsGlobalOrStaticVariable())
             scope = eValueTypeVariableStatic;
           else {
             scope = eValueTypeVariableLocal;

lldb/test/Shell/SymbolFile/DWARF/static_scope.s

+# Test that the DWARF parser assigns the right scope to the
+# variable `b`, which is `local` and not `static`.
+
+# REQUIRES: x86
+# UNSUPPORTED: lldb-repro
+
+# RUN: llvm-mc -triple=x86_64-apple-macosx10.15.0 -filetype=obj %s > %t.o
+# RUN: lldb-test symbols %t.o | FileCheck %s
+
+# CHECK: Variable{{.*}}, name = "b", type = {{.*}} (int), scope = local
+
+  .section  __TEXT,__text,regular,pure_instructions
+  .macosx_version_min 10, 15
+  .file 1 "/Users/davide/work/build/bin" "a.c"
+  .globl  _main                   ## -- Begin function main
+  .p2align  4, 0x90
+_main:                                  ## @main
+Lfunc_begin0:
+  .loc  1 2 0                   ## a.c:2:0
+  .cfi_startproc
+## %bb.0:
+  pushq %rbp
+  .cfi_def_cfa_offset 16
+  .cfi_offset %rbp, -16
+  movq  %rsp, %rbp
+  .cfi_def_cfa_register %rbp
+Ltmp0:
+  ##DEBUG_VALUE: b <- 3
+  .loc  1 5 9 prologue_end      ## a.c:5:9
+  movl  _a(%rip), %eax
+Ltmp1:
+  .loc  1 7 5                   ## a.c:7:5
+  xorl  %eax, %eax
+  popq  %rbp
+  retq
+Ltmp2:
+Lfunc_end0:
+  .cfi_endproc
+                                        ## -- End function
+  .globl  _a                      ## @a
+.zerofill __DATA,__common,_a,4,2
+  .section  __DWARF,__debug_abbrev,regular,debug
+Lsection_abbrev:
+  .byte 1                       ## Abbreviation Code
+  .byte 17                      ## DW_TAG_compile_unit
+  .byte 1                       ## DW_CHILDREN_yes
+  .byte 37                      ## DW_AT_producer
+  .byte 14                      ## DW_FORM_strp
+  .byte 19                      ## DW_AT_language
+  .byte 5                       ## DW_FORM_data2
+  .byte 3                       ## DW_AT_name
+  .byte 14                      ## DW_FORM_strp
+  .ascii  "\202|"                 ## DW_AT_LLVM_sysroot
+  .byte 14                      ## DW_FORM_strp
+  .byte 16                      ## DW_AT_stmt_list
+  .byte 23                      ## DW_FORM_sec_offset
+  .byte 27                      ## DW_AT_comp_dir
+  .byte 14                      ## DW_FORM_strp
+  .ascii  "\341\177"              ## DW_AT_APPLE_optimized
+  .byte 25                      ## DW_FORM_flag_present
+  .byte 17                      ## DW_AT_low_pc
+  .byte 1                       ## DW_FORM_addr
+  .byte 18                      ## DW_AT_high_pc
+  .byte 6                       ## DW_FORM_data4
+  .byte 0                       ## EOM(1)
+  .byte 0                       ## EOM(2)
+  .byte 2                       ## Abbreviation Code
+  .byte 52                      ## DW_TAG_variable
+  .byte 0                       ## DW_CHILDREN_no
+  .byte 3                       ## DW_AT_name
+  .byte 14                      ## DW_FORM_strp
+  .byte 73                      ## DW_AT_type
+  .byte 19                      ## DW_FORM_ref4
+  .byte 63                      ## DW_AT_external
+  .byte 25                      ## DW_FORM_flag_present
+  .byte 58                      ## DW_AT_decl_file
+  .byte 11                      ## DW_FORM_data1
+  .byte 59                      ## DW_AT_decl_line
+  .byte 11                      ## DW_FORM_data1
+  .byte 2                       ## DW_AT_location
+  .byte 24                      ## DW_FORM_exprloc
+  .byte 0                       ## EOM(1)
+  .byte 0                       ## EOM(2)
+  .byte 3                       ## Abbreviation Code
+  .byte 53                      ## DW_TAG_volatile_type
+  .byte 0                       ## DW_CHILDREN_no
+  .byte 73                      ## DW_AT_type
+  .byte 19                      ## DW_FORM_ref4
+  .byte 0                       ## EOM(1)
+  .byte 0                       ## EOM(2)
+  .byte 4                       ## Abbreviation Code
+  .byte 36                      ## DW_TAG_base_type
+  .byte 0                       ## DW_CHILDREN_no
+  .byte 3                       ## DW_AT_name
+  .byte 14                      ## DW_FORM_strp
+  .byte 62                      ## DW_AT_encoding
+  .byte 11                      ## DW_FORM_data1
+  .byte 11                      ## DW_AT_byte_size
+  .byte 11                      ## DW_FORM_data1
+  .byte 0                       ## EOM(1)
+  .byte 0                       ## EOM(2)
+  .byte 5                       ## Abbreviation Code
+  .byte 46                      ## DW_TAG_subprogram
+  .byte 1                       ## DW_CHILDREN_yes
+  .byte 17                      ## DW_AT_low_pc
+  .byte 1                       ## DW_FORM_addr
+  .byte 18                      ## DW_AT_high_pc
+  .byte 6                       ## DW_FORM_data4
+  .byte 64                      ## DW_AT_frame_base
+  .byte 24                      ## DW_FORM_exprloc
+  .byte 122                     ## DW_AT_call_all_calls
+  .byte 25                      ## DW_FORM_flag_present
+  .byte 3                       ## DW_AT_name
+  .byte 14                      ## DW_FORM_strp
+  .byte 58                      ## DW_AT_decl_file
+  .byte 11                      ## DW_FORM_data1
+  .byte 59                      ## DW_AT_decl_line
+  .byte 11                      ## DW_FORM_data1
+  .byte 73                      ## DW_AT_type
+  .byte 19                      ## DW_FORM_ref4
+  .byte 63                      ## DW_AT_external
+  .byte 25                      ## DW_FORM_flag_present
+  .ascii  "\341\177"              ## DW_AT_APPLE_optimized
+  .byte 25                      ## DW_FORM_flag_present
+  .byte 0                       ## EOM(1)
+  .byte 0                       ## EOM(2)
+  .byte 6                       ## Abbreviation Code
+  .byte 11                      ## DW_TAG_lexical_block
+  .byte 1                       ## DW_CHILDREN_yes
+  .byte 17                      ## DW_AT_low_pc
+  .byte 1                       ## DW_FORM_addr
+  .byte 18                      ## DW_AT_high_pc
+  .byte 6                       ## DW_FORM_data4
+  .byte 0                       ## EOM(1)
+  .byte 0                       ## EOM(2)
+  .byte 7                       ## Abbreviation Code
+  .byte 52                      ## DW_TAG_variable
+  .byte 0                       ## DW_CHILDREN_no
+  .byte 28                      ## DW_AT_const_value
+  .byte 13                      ## DW_FORM_sdata
+  .byte 3                       ## DW_AT_name
+  .byte 14                      ## DW_FORM_strp
+  .byte 58                      ## DW_AT_decl_file
+  .byte 11                      ## DW_FORM_data1
+  .byte 59                      ## DW_AT_decl_line
+  .byte 11                      ## DW_FORM_data1
+  .byte 73                      ## DW_AT_type
+  .byte 19                      ## DW_FORM_ref4
+  .byte 0                       ## EOM(1)
+  .byte 0                       ## EOM(2)
+  .byte 0                       ## EOM(3)
+  .section  __DWARF,__debug_info,regular,debug
+Lsection_info:
+Lcu_begin0:
+.set Lset0, Ldebug_info_end0-Ldebug_info_start0 ## Length of Unit
+  .long Lset0
+Ldebug_info_start0:
+  .short  4                       ## DWARF version number
+.set Lset1, Lsection_abbrev-Lsection_abbrev ## Offset Into Abbrev. Section
+  .long Lset1
+  .byte 8                       ## Address Size (in bytes)
+  .byte 1                       ## Abbrev [1] 0xb:0x79 DW_TAG_compile_unit
+  .long 0                       ## DW_AT_producer
+  .short  12                      ## DW_AT_language
+  .long 101                     ## DW_AT_name
+  .long 105                     ## DW_AT_LLVM_sysroot
+.set Lset2, Lline_table_start0-Lsection_line ## DW_AT_stmt_list
+  .long Lset2
+  .long 107                     ## DW_AT_comp_dir
+                                        ## DW_AT_APPLE_optimized
+  .quad Lfunc_begin0            ## DW_AT_low_pc
+.set Lset3, Lfunc_end0-Lfunc_begin0     ## DW_AT_high_pc
+  .long Lset3
+  .byte 2                       ## Abbrev [2] 0x2e:0x15 DW_TAG_variable
+  .long 136                     ## DW_AT_name
+  .long 67                      ## DW_AT_type
+                                        ## DW_AT_external
+  .byte 1                       ## DW_AT_decl_file
+  .byte 1                       ## DW_AT_decl_line
+  .byte 9                       ## DW_AT_location
+  .byte 3
+  .quad _a
+  .byte 3                       ## Abbrev [3] 0x43:0x5 DW_TAG_volatile_type
+  .long 72                      ## DW_AT_type
+  .byte 4                       ## Abbrev [4] 0x48:0x7 DW_TAG_base_type
+  .long 138                     ## DW_AT_name
+  .byte 5                       ## DW_AT_encoding
+  .byte 4                       ## DW_AT_byte_size
+  .byte 5                       ## Abbrev [5] 0x4f:0x34 DW_TAG_subprogram
+  .quad Lfunc_begin0            ## DW_AT_low_pc
+.set Lset4, Lfunc_end0-Lfunc_begin0     ## DW_AT_high_pc
+  .long Lset4
+  .byte 1                       ## DW_AT_frame_base
+  .byte 86
+                                        ## DW_AT_call_all_calls
+  .long 142                     ## DW_AT_name
+  .byte 1                       ## DW_AT_decl_file
+  .byte 2                       ## DW_AT_decl_line
+  .long 72                      ## DW_AT_type
+                                        ## DW_AT_external
+                                        ## DW_AT_APPLE_optimized
+  .byte 6                       ## Abbrev [6] 0x68:0x1a DW_TAG_lexical_block
+  .quad Ltmp0                   ## DW_AT_low_pc
+.set Lset5, Ltmp1-Ltmp0                 ## DW_AT_high_pc
+  .long Lset5
+  .byte 7                       ## Abbrev [7] 0x75:0xc DW_TAG_variable
+  .byte 3                       ## DW_AT_const_value
+  .long 147                     ## DW_AT_name
+  .byte 1                       ## DW_AT_decl_file
+  .byte 4                       ## DW_AT_decl_line
+  .long 72                      ## DW_AT_type
+  .byte 0                       ## End Of Children Mark
+  .byte 0                       ## End Of Children Mark
+  .byte 0                       ## End Of Children Mark
+Ldebug_info_end0:
+  .section  __DWARF,__debug_str,regular,debug
+Linfo_string:
+  .asciz  "clang version 11.0.0 (https://github.com/llvm/llvm-project f30ebf437851d3c68fd0eee82afbc0cef7373c00)" ## string offset=0
+  .asciz  "a.c"                   ## string offset=101
+  .asciz  "/"                     ## string offset=105
+  .asciz  "/Users/davide/work/build/bin" ## string offset=107
+  .asciz  "a"                     ## string offset=136
+  .asciz  "int"                   ## string offset=138
+  .asciz  "main"                  ## string offset=142
+  .asciz  "b"                     ## string offset=147
+  .section  __DWARF,__apple_names,regular,debug
+Lnames_begin:
+  .long 1212240712              ## Header Magic
+  .short  1                       ## Header Version
+  .short  0                       ## Header Hash Function
+  .long 2                       ## Header Bucket Count
+  .long 2                       ## Header Hash Count
+  .long 12                      ## Header Data Length
+  .long 0                       ## HeaderData Die Offset Base
+  .long 1                       ## HeaderData Atom Count
+  .short  1                       ## DW_ATOM_die_offset
+  .short  6                       ## DW_FORM_data4
+  .long 0                       ## Bucket 0
+  .long -1                      ## Bucket 1
+  .long 177670                  ## Hash in Bucket 0
+  .long 2090499946              ## Hash in Bucket 0
+.set Lset6, LNames0-Lnames_begin        ## Offset in Bucket 0
+  .long Lset6
+.set Lset7, LNames1-Lnames_begin        ## Offset in Bucket 0
+  .long Lset7
+LNames0:
+  .long 136                     ## a
+  .long 1                       ## Num DIEs
+  .long 46
+  .long 0
+LNames1:
+  .long 142                     ## main
+  .long 1                       ## Num DIEs
+  .long 79
+  .long 0
+  .section  __DWARF,__apple_objc,regular,debug
+Lobjc_begin:
+  .long 1212240712              ## Header Magic
+  .short  1                       ## Header Version
+  .short  0                       ## Header Hash Function
+  .long 1                       ## Header Bucket Count
+  .long 0                       ## Header Hash Count
+  .long 12                      ## Header Data Length
+  .long 0                       ## HeaderData Die Offset Base
+  .long 1                       ## HeaderData Atom Count
+  .short  1                       ## DW_ATOM_die_offset
+  .short  6                       ## DW_FORM_data4
+  .long -1                      ## Bucket 0
+  .section  __DWARF,__apple_namespac,regular,debug
+Lnamespac_begin:
+  .long 1212240712              ## Header Magic
+  .short  1                       ## Header Version
+  .short  0                       ## Header Hash Function
+  .long 1                       ## Header Bucket Count
+  .long 0                       ## Header Hash Count
+  .long 12                      ## Header Data Length
+  .long 0                       ## HeaderData Die Offset Base
+  .long 1                       ## HeaderData Atom Count
+  .short  1                       ## DW_ATOM_die_offset
+  .short  6                       ## DW_FORM_data4
+  .long -1                      ## Bucket 0
+  .section  __DWARF,__apple_types,regular,debug
+Ltypes_begin:
+  .long 1212240712              ## Header Magic
+  .short  1                       ## Header Version
+  .short  0                       ## Header Hash Function
+  .long 1                       ## Header Bucket Count
+  .long 1                       ## Header Hash Count
+  .long 20                      ## Header Data Length
+  .long 0                       ## HeaderData Die Offset Base
+  .long 3                       ## HeaderData Atom Count
+  .short  1                       ## DW_ATOM_die_offset
+  .short  6                       ## DW_FORM_data4
+  .short  3                       ## DW_ATOM_die_tag
+  .short  5                       ## DW_FORM_data2
+  .short  4                       ## DW_ATOM_type_flags
+  .short  11                      ## DW_FORM_data1
+  .long 0                       ## Bucket 0
+  .long 193495088               ## Hash in Bucket 0
+.set Lset8, Ltypes0-Ltypes_begin        ## Offset in Bucket 0
+  .long Lset8
+Ltypes0:
+  .long 138                     ## int
+  .long 1                       ## Num DIEs
+  .long 72
+  .short  36
+  .byte 0
+  .long 0
+.subsections_via_symbols
+  .section  __DWARF,__debug_line,regular,debug
+Lsection_line:
+Lline_table_start0: