Fix resolution conflict between global and class static variables in C++.

[paulherman]

Summary

Consider having a global variable 'a' and a static variable with the same name inside a class. This resolves the arbitrary choice when resolving the name 'a'.

[clayborg]

See inlined comments.

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

7389

I would do this as:

const DWARFDebugInfoEntry *parent_die = GetDeclContextDIEContainingDIE(dwarf_cu, die);
if (parent_die->Tag() != DW_TAG_class_type && parent_die->Tag() != DW_TAG_structure_type)
{
    VariableSP var_sp (ParseVariableDIE(sc, dwarf_cu, die, LLDB_INVALID_ADDRESS));
    if (var_sp)
    {
        variables->AddVariableIfUnique (var_sp);
        ++vars_added;
    }

This will avoid parsing extra global variables by figuring out we don't need it _before_ we go and parse a global variable.

I would rather not have the SymbolFileDWARF::ParseGlobalVariableDIE(...) function because it doesn't make sense at as global variables can exist in classes and structures and I would expect a function named SymbolFileDWARF::ParseGlobalVariableDIE(...) to parse that variable, but SymbolFileDWARF:: ParseVariableDIE(...) already does this so there is really no need for SymbolFileDWARF::ParseGlobalVariableDIE().

7413–7437

Remove this. See inlined comment above.

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

325–330 ↗

(On Diff #32202)

Remove this.

[clayborg]

Please make sure that the following works after your changes:

(lldb) r
Process 35421 launched: '/private/tmp/a.out' (x86_64)
Process 35421 stopped
* thread #1: tid = 0xb659be, 0x0000000100000f9d a.out main + 13, stop reason = breakpoint 1.1, queue = com.apple.main-thread
    frame #0: 0x0000000100000f9d a.out main + 13 at main.cpp:24
   21  	
   22  	int main()
   23  	{
-> 24  	    return 0; // break here
   25  	}
(lldb) target variable 
Global variables for /tmp/main.cpp in /private/tmp/a.out:
(int) A::a = 1111
(int) B::a = 2222
(int) C::a = 3333
(int) ::a = 4444

[paulherman]

I've changed the commit so that ParseVariableDIE now detects if a variable is static inside a class or struct. Now "target variable" works since the variables are not removed from the variable list of the compile unit, but filtered when searching for a specific variable.

[clayborg]

Looks fine.

include/lldb/Symbol/Variable.h

Variable (lldb::user_id_t uid,
               const char *name, 
               const char *mangled,  // The mangled or fully qualified name of the variable.
               const lldb::SymbolFileTypeSP &symfile_type_sp,
               lldb::ValueType scope,
               SymbolContextScope *owner_scope,
               Declaration* decl,
               const DWARFExpression& location,
               bool external,
-              bool artificial);
+              bool artificial,
+              bool static_member = false);
 
     virtual
     ~Variable();
 
     void
     Dump(Stream *s, bool show_context) const;
 
     bool
public:
     }
 
     bool
     IsArtificial() const
     {
         return m_artificial;
     }
 
+    bool IsStaticMember() const
+    {
+        return m_static_member;
+    }
+
     DWARFExpression &
     LocationExpression()
     {
         return m_location;
     }
 
     const DWARFExpression &
     LocationExpression() const
protected:
     Mangled m_mangled;                  // The mangled name of the variable
     lldb::SymbolFileTypeSP m_symfile_type_sp;   // The type pointer of the variable (int, struct, class, etc)
     lldb::ValueType m_scope;            // global, parameter, local
     SymbolContextScope *m_owner_scope;  // The symbol file scope that this variable was defined in
     Declaration m_declaration;          // Declaration location for this item.
     DWARFExpression m_location;         // The location of this variable that can be fed to DWARFExpression::Evaluate()
     uint8_t m_external:1,               // Visible outside the containing compile unit?
             m_artificial:1,             // Non-zero if the variable is not explicitly declared in source
-            m_loc_is_const_data:1;      // The m_location expression contains the constant variable value data, not a DWARF location
+            m_loc_is_const_data:1,      // The m_location expression contains the constant variable value data, not a DWARF location
+            m_static_member:1;          // Non-zero if variable is static member of a class or struct.
 private:
     Variable(const Variable& rhs);
     Variable& operator=(const Variable& rhs);
 };
 
 } // namespace lldb_private
 
 #endif  // liblldb_Variable_h_

include/lldb/Symbol/VariableList.h

//  VariableList(const SymbolContext &symbol_context);
 
     lldb::VariableSP
     GetVariableAtIndex(size_t idx) const;
 
     lldb::VariableSP
     RemoveVariableAtIndex (size_t idx);
     
     lldb::VariableSP
-    FindVariable (const ConstString& name);
+    FindVariable (const ConstString& name, bool include_static_members = true);
     
     lldb::VariableSP
-    FindVariable (const ConstString& name, lldb::ValueType value_type);
+    FindVariable (const ConstString& name, lldb::ValueType value_type, bool include_static_members = true);
 
     uint32_t
     FindVariableIndex (const lldb::VariableSP &var_sp);
     
     size_t
     AppendVariablesIfUnique(VariableList &var_list);
     
     // Returns the actual number of unique variables that were added to the

source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

if (sc.comp_unit != NULL)
                 {
                     DWARFDebugInfo* debug_info = DebugInfo();
                     for (size_t i=0; i<num_matches; ++i)
                     {
                         const dw_offset_t die_offset = die_offsets[i];
                         die = debug_info->GetDIEPtrWithCompileUnitHint (die_offset, &match_dwarf_cu);
                         if (die)
                         {
                             VariableSP var_sp (ParseVariableDIE(sc, dwarf_cu, die, LLDB_INVALID_ADDRESS));
                             if (var_sp)
                             {
                                 variables->AddVariableIfUnique (var_sp);
                                 ++vars_added;
                             }
                         }
                         else
                         {
                             if (m_using_apple_tables)
                             {
                                 GetObjectFile()->GetModule()->ReportErrorIfModifyDetected ("the DWARF debug information has been modified (.apple_names accelerator table had bad die 0x%8.8x)\n", die_offset);
                             }
                         }            
 
                     }
                 }
             }
             return vars_added;
         }
     }
     return 0;
 }
 
-
 VariableSP
 SymbolFileDWARF::ParseVariableDIE
 (
     const SymbolContext& sc,
     DWARFCompileUnit* dwarf_cu,
     const DWARFDebugInfoEntry *die,
     const lldb::addr_t func_low_pc
 )
 {
     VariableSP var_sp (m_die_to_variable_sp[die]);
     if (var_sp)
         return var_sp;  // Already been parsed!
     
     const dw_tag_t tag = die->Tag();
     ModuleSP module = GetObjectFile()->GetModule();
     
     if ((tag == DW_TAG_variable) ||
         (tag == DW_TAG_constant) ||
         (tag == DW_TAG_formal_parameter && sc.function))
     {
         DWARFDebugInfoEntry::Attributes attributes;
         const size_t num_attributes = die->GetAttributes(this, dwarf_cu, NULL, attributes);
         if (num_attributes > 0)
         {
             const char *name = NULL;
             const char *mangled = NULL;
             Declaration decl;
             uint32_t i;
             lldb::user_id_t type_uid = LLDB_INVALID_UID;
             DWARFExpression location;
             bool is_external = false;
             bool is_artificial = false;
             bool location_is_const_value_data = false;
if ((tag == DW_TAG_variable) ||
                     case DW_AT_abstract_origin:
                     case DW_AT_sibling:
                     case DW_AT_specification:
                         break;
                     }
                 }
             }
 
+            const DWARFDebugInfoEntry *parent_context_die = GetDeclContextDIEContainingDIE(dwarf_cu, die);
+            bool is_static_member = die->GetParent()->Tag() == DW_TAG_compile_unit && (parent_context_die->Tag() == DW_TAG_class_type || parent_context_die->Tag() == DW_TAG_structure_type);
+
             ValueType scope = eValueTypeInvalid;
 
             const DWARFDebugInfoEntry *sc_parent_die = GetParentSymbolContextDIE(die);
             dw_tag_t parent_tag = sc_parent_die ? sc_parent_die->Tag() : 0;
             SymbolContextScope * symbol_context_scope = NULL;
 
             if (!mangled)
             {
if ((tag == DW_TAG_variable) ||
                                             name, 
                                             mangled,
                                             type_sp,
                                             scope, 
                                             symbol_context_scope, 
                                             &decl, 
                                             location, 
                                             is_external, 
-                                            is_artificial));
+                                            is_artificial,
+                                            is_static_member));
                 
                 var_sp->SetLocationIsConstantValueData (location_is_const_value_data);
             }
             else
             {
                 // Not ready to parse this variable yet. It might be a global
                 // or static variable that is in a function scope and the function
                 // in the symbol context wasn't filled in yet

source/Symbol/Variable.cpp

(
     const char *name, 
     const char *mangled,  // The mangled or fully qualified name of the variable.
     const lldb::SymbolFileTypeSP &symfile_type_sp,
     ValueType scope,
     SymbolContextScope *context,
     Declaration* decl_ptr,
     const DWARFExpression& location,
     bool external,
-    bool artificial
+    bool artificial,
+    bool static_member
 ) :
     UserID(uid),
     m_name(name),
     m_mangled (ConstString(mangled)),
     m_symfile_type_sp(symfile_type_sp),
     m_scope(scope),
     m_owner_scope(context),
     m_declaration(decl_ptr),
     m_location(location),
     m_external(external),
-    m_artificial(artificial)
+    m_artificial(artificial),
+    m_static_member(static_member)
 {
 }
 
 //----------------------------------------------------------------------
 // Destructor
 //----------------------------------------------------------------------
 Variable::~Variable()
 {

source/Symbol/VariableList.cpp

VariableList::FindVariableIndex (const VariableSP &var_sp)
     {
         if (pos->get() == var_sp.get())
             return std::distance (m_variables.begin(), pos);
     }
     return UINT32_MAX;
 }
 
 VariableSP
-VariableList::FindVariable(const ConstString& name)
+VariableList::FindVariable(const ConstString& name, bool include_static_members)
 {
     VariableSP var_sp;
     iterator pos, end = m_variables.end();
     for (pos = m_variables.begin(); pos != end; ++pos)
     {
         if ((*pos)->NameMatches(name))
         {
+            if (include_static_members || !(*pos)->IsStaticMember())
+            {
                 var_sp = (*pos);
                 break;
             }
         }
+    }
     return var_sp;
 }
 
 VariableSP
-VariableList::FindVariable (const ConstString& name, lldb::ValueType value_type)
+VariableList::FindVariable (const ConstString& name, lldb::ValueType value_type, bool include_static_members)
 {
     VariableSP var_sp;
     iterator pos, end = m_variables.end();
     for (pos = m_variables.begin(); pos != end; ++pos)
     {
         if ((*pos)->NameMatches(name) && (*pos)->GetScope() == value_type)
         {
+            if (include_static_members || !(*pos)->IsStaticMember())
+            {
                 var_sp = (*pos);
                 break;
             }
         }
+    }
     return var_sp;
 }
 
 size_t
 VariableList::AppendVariablesIfUnique(VariableList &var_list)
 {
     const size_t initial_size = var_list.GetSize();
     iterator pos, end = m_variables.end();

source/Target/StackFrame.cpp

if (var_expr_cstr && var_expr_cstr[0])
             StreamString var_expr_path_strm;
 
             ConstString name_const_string;
             if (separator_idx == std::string::npos)
                 name_const_string.SetCString (var_path.c_str());
             else
                 name_const_string.SetCStringWithLength (var_path.c_str(), separator_idx);
 
-            var_sp = variable_list->FindVariable(name_const_string);
+            var_sp = variable_list->FindVariable(name_const_string, false);
             
             bool synthetically_added_instance_object = false;
 
             if (var_sp)
             {
                 var_path.erase (0, name_const_string.GetLength ());
             }
             else if (options & eExpressionPathOptionsAllowDirectIVarAccess)

test/lang/cpp/scope/Makefile

+LEVEL = ../../../make
+
+CXX_SOURCES := main.cpp
+
+include $(LEVEL)/Makefile.rules

test/lang/cpp/scope/TestCppScope.py

+"""
+Test scopes in C++.
+"""
+import lldb
+from lldbtest import *
+import lldbutil
+
+class TestCppScopes(TestBase):
+    
+    mydir = TestBase.compute_mydir(__file__)
+    
+    @skipUnlessDarwin
+    @dsym_test
+    def test_with_dsym_and_run_command(self):
+        self.buildDsym()
+        self.check()
+
+    @dwarf_test
+    def test_with_dwarf_and_run_command(self):
+        self.buildDwarf()
+        self.check()
+
+    def setUp(self):
+        TestBase.setUp(self)
+
+    def check(self):
+        # Get main source file
+        src_file = "main.cpp"
+        src_file_spec = lldb.SBFileSpec(src_file)
+        self.assertTrue(src_file_spec.IsValid(), "Main source file")
+        
+        # Get the path of the executable
+        cwd = os.getcwd() 
+        exe_file = "a.out"
+        exe_path  = os.path.join(cwd, exe_file)
+        
+        # Load the executable
+        target = self.dbg.CreateTarget(exe_path)
+        self.assertTrue(target.IsValid(), VALID_TARGET)
+
+        # Break on main function
+        main_breakpoint = target.BreakpointCreateBySourceRegex("// break here", src_file_spec)
+        self.assertTrue(main_breakpoint.IsValid() and main_breakpoint.GetNumLocations() >= 1, VALID_BREAKPOINT)
+
+        # Launch the process
+        args = None
+        env = None
+        process = target.LaunchSimple(args, env, cwd)
+        self.assertTrue(process.IsValid(), PROCESS_IS_VALID)
+
+        # Get the thread of the process
+        self.assertTrue(process.GetState() == lldb.eStateStopped, PROCESS_STOPPED)
+        thread = lldbutil.get_stopped_thread(process, lldb.eStopReasonBreakpoint)
+
+        # Get current fream of the thread at the breakpoint 
+        frame = thread.GetSelectedFrame()
+
+        # Test result for scopes of variables 
+
+        global_variables = frame.GetVariables(True, True, True, False)
+        global_variables_assert = { 
+            'A::a': 1111,
+            'B::a': 2222,
+            'C::a': 3333,
+            '::a': 4444,
+            'a': 4444
+        }
+
+        self.assertTrue(global_variables.GetSize() == 4, "target variable returns all variables")
+        for variable in global_variables:
+            name = variable.GetName() 
+            self.assertTrue(name in global_variables_assert, "target variable returns wrong variable " + name)
+
+        for name in global_variables_assert:
+            value = frame.EvaluateExpression(name)
+            assert_value = global_variables_assert[name]
+            self.assertTrue(value.IsValid() and value.GetValueAsSigned() == assert_value, name + " = " + str(assert_value))
+
+if __name__ == '__main__':
+    import atexit
+    lldb.SBDebugger.Initialize()
+    atexit.register(lambda: lldb.SBDebugger.Terminate())
+    unittest2.main()

test/lang/cpp/scope/main.cpp

+class A {
+public:
+    static int a;
+    int b;
+};
+
+class B {
+public:
+    static int a;
+    int b;
+};
+
+struct C {
+    static int a;
+};
+
+int A::a = 1111;
+int B::a = 2222;
+int C::a = 3333;
+int a = 4444;
+
+int main() // break here
+{
+    return 0;
+}