From 08a04327a94bbfa0d2d804c4ee845ef8a7c6cf54 Mon Sep 17 00:00:00 2001 From: Enrico Granata Date: Tue, 18 Feb 2014 23:48:11 +0000 Subject: [PATCH] Fix a bug where calling SBFrame::FindValue() would cause a copy of all variables in the block to be inserted in the frame's variable list, regardless of whether those same variables were there or not - which means one could end up with a frame with lots of duplicate copies of the same variables llvm-svn: 201614 --- lldb/include/lldb/Symbol/VariableList.h | 3 + lldb/source/API/SBFrame.cpp | 20 +++---- lldb/source/Symbol/VariableList.cpp | 16 +++++ lldb/test/python_api/findvalue_duplist/Makefile | 8 +++ .../findvalue_duplist/TestSBFrameFindValue.py | 70 ++++++++++++++++++++++ lldb/test/python_api/findvalue_duplist/main.cpp | 7 +++ 6 files changed, 111 insertions(+), 13 deletions(-) create mode 100644 lldb/test/python_api/findvalue_duplist/Makefile create mode 100644 lldb/test/python_api/findvalue_duplist/TestSBFrameFindValue.py create mode 100644 lldb/test/python_api/findvalue_duplist/main.cpp diff --git a/lldb/include/lldb/Symbol/VariableList.h b/lldb/include/lldb/Symbol/VariableList.h index 2ce6146..7abb2c6 100644 --- a/lldb/include/lldb/Symbol/VariableList.h +++ b/lldb/include/lldb/Symbol/VariableList.h @@ -49,6 +49,9 @@ public: lldb::VariableSP FindVariable (const ConstString& name); + + lldb::VariableSP + FindVariable (const ConstString& name, lldb::ValueType value_type); uint32_t FindVariableIndex (const lldb::VariableSP &var_sp); diff --git a/lldb/source/API/SBFrame.cpp b/lldb/source/API/SBFrame.cpp index cff4602..7d0a03f 100644 --- a/lldb/source/API/SBFrame.cpp +++ b/lldb/source/API/SBFrame.cpp @@ -852,7 +852,7 @@ SBFrame::FindValue (const char *name, ValueType value_type, lldb::DynamicValueTy case eValueTypeVariableArgument: // function argument variables case eValueTypeVariableLocal: // function local variables { - VariableList *variable_list = frame->GetVariableList(true); + VariableList variable_list; SymbolContext sc (frame->GetSymbolContext (eSymbolContextBlock)); @@ -863,21 +863,15 @@ SBFrame::FindValue (const char *name, ValueType value_type, lldb::DynamicValueTy if (sc.block && sc.block->AppendVariables (can_create, get_parent_variables, stop_if_block_is_inlined_function, - variable_list)) + &variable_list)) { ConstString const_name(name); - const uint32_t num_variables = variable_list->GetSize(); - for (uint32_t i = 0; i < num_variables; ++i) + VariableSP variable_sp(variable_list.FindVariable(const_name,value_type)); + if (variable_sp) { - VariableSP variable_sp (variable_list->GetVariableAtIndex(i)); - if (variable_sp && - variable_sp->GetScope() == value_type && - variable_sp->GetName() == const_name) - { - value_sp = frame->GetValueObjectForFrameVariable (variable_sp, eNoDynamicValues); - sb_value.SetSP (value_sp, use_dynamic); - break; - } + value_sp = frame->GetValueObjectForFrameVariable (variable_sp, eNoDynamicValues); + sb_value.SetSP (value_sp, use_dynamic); + break; } } } diff --git a/lldb/source/Symbol/VariableList.cpp b/lldb/source/Symbol/VariableList.cpp index 3451166..15f2b29 100644 --- a/lldb/source/Symbol/VariableList.cpp +++ b/lldb/source/Symbol/VariableList.cpp @@ -115,6 +115,22 @@ VariableList::FindVariable(const ConstString& name) return var_sp; } +VariableSP +VariableList::FindVariable (const ConstString& name, lldb::ValueType value_type) +{ + 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) + { + var_sp = (*pos); + break; + } + } + return var_sp; +} + size_t VariableList::AppendVariablesIfUnique (const RegularExpression& regex, VariableList &var_list, size_t& total_matches) { diff --git a/lldb/test/python_api/findvalue_duplist/Makefile b/lldb/test/python_api/findvalue_duplist/Makefile new file mode 100644 index 0000000..ddffdcf --- /dev/null +++ b/lldb/test/python_api/findvalue_duplist/Makefile @@ -0,0 +1,8 @@ +LEVEL = ../../make + +CXX_SOURCES := main.cpp + +# Clean renamed executable on 'make clean' +clean: OBJECTS+=no_synth + +include $(LEVEL)/Makefile.rules diff --git a/lldb/test/python_api/findvalue_duplist/TestSBFrameFindValue.py b/lldb/test/python_api/findvalue_duplist/TestSBFrameFindValue.py new file mode 100644 index 0000000..accd88f --- /dev/null +++ b/lldb/test/python_api/findvalue_duplist/TestSBFrameFindValue.py @@ -0,0 +1,70 @@ +"""Test that SBFrame::FindValue finds things but does not duplicate the entire variables list""" + +import os, sys, time +import unittest2 +import lldb +from lldbtest import * +import lldbutil + +class SBFrameFindValueTestCase(TestBase): + + mydir = TestBase.compute_mydir(__file__) + + @unittest2.skipUnless(sys.platform.startswith("darwin"), "requires Darwin") + @python_api_test + @dsym_test + def test_with_dsym_formatters_api(self): + """Test that SBFrame::FindValue finds things but does not duplicate the entire variables list""" + self.buildDsym() + self.setTearDownCleanup() + self.commands() + + @python_api_test + @dwarf_test + def test_with_dwarf_formatters_api(self): + """Test that SBFrame::FindValue finds things but does not duplicate the entire variables list""" + self.buildDwarf() + self.setTearDownCleanup() + self.commands() + + def setUp(self): + # Call super's setUp(). + TestBase.setUp(self) + + def commands(self): + """Test that SBFrame::FindValue finds things but does not duplicate the entire variables list""" + exe_name = "a.out" + exe = os.path.join(os.getcwd(), exe_name) + + # Create the target + target = self.dbg.CreateTarget(exe) + self.assertTrue(target, VALID_TARGET) + + # Set the breakpoints + breakpoint = target.BreakpointCreateBySourceRegex('Set breakpoint here', lldb.SBFileSpec("main.cpp")) + self.assertTrue(breakpoint.GetNumLocations() > 0, VALID_BREAKPOINT) + + # Launch the process, and do not stop at the entry point. + process = target.LaunchSimple(None, None, os.getcwd()) + + self.assertTrue(process, PROCESS_IS_VALID) + + # Frame #0 should be at our breakpoint. + threads = lldbutil.get_threads_stopped_at_breakpoint (process, breakpoint) + + self.assertTrue(len(threads) == 1) + self.thread = threads[0] + self.frame = self.thread.frames[0] + self.assertTrue(self.frame, "Frame 0 is valid.") + + self.assertTrue(self.frame.GetVariables(True,True,False,True).GetSize() == 2, "variable count is off") + self.assertFalse(self.frame.FindValue("NoSuchThing",lldb.eValueTypeVariableArgument,lldb.eDynamicCanRunTarget).IsValid(), "found something that should not be here") + self.assertTrue(self.frame.GetVariables(True,True,False,True).GetSize() == 2, "variable count is off after failed FindValue()") + self.assertTrue(self.frame.FindValue("a",lldb.eValueTypeVariableArgument,lldb.eDynamicCanRunTarget).IsValid(), "FindValue() didn't find an argument") + self.assertTrue(self.frame.GetVariables(True,True,False,True).GetSize() == 2, "variable count is off after successful FindValue()") + +if __name__ == '__main__': + import atexit + lldb.SBDebugger.Initialize() + atexit.register(lambda: lldb.SBDebugger.Terminate()) + unittest2.main() diff --git a/lldb/test/python_api/findvalue_duplist/main.cpp b/lldb/test/python_api/findvalue_duplist/main.cpp new file mode 100644 index 0000000..7058d46 --- /dev/null +++ b/lldb/test/python_api/findvalue_duplist/main.cpp @@ -0,0 +1,7 @@ +int foo(int a, int b) { + return a + b; // Set breakpoint here +} + +int main() { + return foo(1,3); +} -- 2.7.4