Author: jingham Date: Thu Jan 11 19:03:23 2018 New Revision: 322348 URL: http://llvm.org/viewvc/llvm-project?rev=322348&view=rev Log: Fix Breakpoint::RemoveInvalidLocations to fix the exec testcase.
RemoveInvalidLocations was clearing out the m_locations in the breakpoint by hand, and it wasn't also clearing the locations from the address->location map, which confused us when we went to update breakpoint locations. I also made Breakpoint::ModulesChanged check the Location's Section to make sure it hadn't been deleted. This shouldn't strictly be necessary, but if the DynamicLoaderPlugin doesn't do it's job right (I'm looking at you new Darwin DynamicLoader...) then it can end up leaving stale locations on rerun. It doesn't hurt to clean them up here as a backstop. <rdar://problem/36134350> Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointLocationList.h lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py lldb/trunk/source/Breakpoint/Breakpoint.cpp lldb/trunk/source/Breakpoint/BreakpointLocationList.cpp Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointLocationList.h URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointLocationList.h?rev=322348&r1=322347&r2=322348&view=diff ============================================================================== --- lldb/trunk/include/lldb/Breakpoint/BreakpointLocationList.h (original) +++ lldb/trunk/include/lldb/Breakpoint/BreakpointLocationList.h Thu Jan 11 19:03:23 2018 @@ -235,6 +235,8 @@ protected: lldb::BreakpointLocationSP from_location_sp); bool RemoveLocation(const lldb::BreakpointLocationSP &bp_loc_sp); + + void RemoveLocationByIndex(size_t idx); void RemoveInvalidLocations(const ArchSpec &arch); Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py?rev=322348&r1=322347&r2=322348&view=diff ============================================================================== --- lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py (original) +++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/exec/TestExec.py Thu Jan 11 19:03:23 2018 @@ -29,14 +29,12 @@ class ExecTestCase(TestBase): mydir = TestBase.compute_mydir(__file__) @skipUnlessDarwin - @expectedFailureAll(oslist=['macosx'], bugnumber="rdar://36134350") # when building with cmake on green gragon or on ci.swift.org, this test fails. @expectedFailureAll(archs=['i386'], bugnumber="rdar://28656532") @expectedFailureAll(oslist=["ios", "tvos", "watchos", "bridgeos"], bugnumber="rdar://problem/34559552") # this exec test has problems on ios systems def test_hitting_exec (self): self.do_test(False) @skipUnlessDarwin - @expectedFailureAll(oslist=['macosx'], bugnumber="rdar://36134350") # when building with cmake on green gragon or on ci.swift.org, this test fails. @expectedFailureAll(archs=['i386'], bugnumber="rdar://28656532") @expectedFailureAll(oslist=["ios", "tvos", "watchos", "bridgeos"], bugnumber="rdar://problem/34559552") # this exec test has problems on ios systems def test_skipping_exec (self): @@ -74,7 +72,7 @@ class ExecTestCase(TestBase): self.assertTrue(process, PROCESS_IS_VALID) if skip_exec: - self.debugger.HandleCommand("settings set target.process.stop-on-exec false") + self.dbg.HandleCommand("settings set target.process.stop-on-exec false") def cleanup(): self.runCmd("settings set target.process.stop-on-exec false", check=False) Modified: lldb/trunk/source/Breakpoint/Breakpoint.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/Breakpoint.cpp?rev=322348&r1=322347&r2=322348&view=diff ============================================================================== --- lldb/trunk/source/Breakpoint/Breakpoint.cpp (original) +++ lldb/trunk/source/Breakpoint/Breakpoint.cpp Thu Jan 11 19:03:23 2018 @@ -23,6 +23,7 @@ #include "lldb/Core/ModuleList.h" #include "lldb/Core/SearchFilter.h" #include "lldb/Core/Section.h" +#include "lldb/Target/SectionLoadList.h" #include "lldb/Symbol/CompileUnit.h" #include "lldb/Symbol/Function.h" #include "lldb/Symbol/Symbol.h" @@ -535,12 +536,29 @@ void Breakpoint::ModulesChanged(ModuleLi if (!m_filter_sp->ModulePasses(module_sp)) continue; + BreakpointLocationCollection locations_with_no_section; for (BreakpointLocationSP break_loc_sp : m_locations.BreakpointLocations()) { + + // If the section for this location was deleted, that means + // it's Module has gone away but somebody forgot to tell us. + // Let's clean it up here. + Address section_addr(break_loc_sp->GetAddress()); + if (section_addr.SectionWasDeleted()) { + locations_with_no_section.Add(break_loc_sp); + continue; + } + if (!break_loc_sp->IsEnabled()) continue; - SectionSP section_sp(break_loc_sp->GetAddress().GetSection()); - if (!section_sp || section_sp->GetModule() == module_sp) { + + SectionSP section_sp(section_addr.GetSection()); + + // If we don't have a Section, that means this location is a raw address + // that we haven't resolved to a section yet. So we'll have to look + // in all the new modules to resolve this location. + // Otherwise, if it was set in this module, re-resolve it here. + if (section_sp && section_sp->GetModule() == module_sp) { if (!seen) seen = true; @@ -552,6 +570,11 @@ void Breakpoint::ModulesChanged(ModuleLi } } } + + size_t num_to_delete = locations_with_no_section.GetSize(); + + for (size_t i = 0; i < num_to_delete; i++) + m_locations.RemoveLocation(locations_with_no_section.GetByIndex(i)); if (!seen) new_modules.AppendIfNeeded(module_sp); Modified: lldb/trunk/source/Breakpoint/BreakpointLocationList.cpp URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/BreakpointLocationList.cpp?rev=322348&r1=322347&r2=322348&view=diff ============================================================================== --- lldb/trunk/source/Breakpoint/BreakpointLocationList.cpp (original) +++ lldb/trunk/source/Breakpoint/BreakpointLocationList.cpp Thu Jan 11 19:03:23 2018 @@ -246,10 +246,10 @@ bool BreakpointLocationList::RemoveLocat m_address_to_location.erase(bp_loc_sp->GetAddress()); - collection::iterator pos, end = m_locations.end(); - for (pos = m_locations.begin(); pos != end; ++pos) { - if ((*pos).get() == bp_loc_sp.get()) { - m_locations.erase(pos); + size_t num_locations = m_locations.size(); + for (size_t idx = 0; idx < num_locations; idx++) { + if (m_locations[idx].get() == bp_loc_sp.get()) { + RemoveLocationByIndex(idx); return true; } } @@ -257,6 +257,12 @@ bool BreakpointLocationList::RemoveLocat return false; } +void BreakpointLocationList::RemoveLocationByIndex(size_t idx) { + assert (idx < m_locations.size()); + m_address_to_location.erase(m_locations[idx]->GetAddress()); + m_locations.erase(m_locations.begin() + idx); +} + void BreakpointLocationList::RemoveInvalidLocations(const ArchSpec &arch) { std::lock_guard<std::recursive_mutex> guard(m_mutex); size_t idx = 0; @@ -267,7 +273,7 @@ void BreakpointLocationList::RemoveInval if (bp_loc->GetAddress().SectionWasDeleted()) { // Section was deleted which means this breakpoint comes from a module // that is no longer valid, so we should remove it. - m_locations.erase(m_locations.begin() + idx); + RemoveLocationByIndex(idx); continue; } if (arch.IsValid()) { @@ -276,7 +282,7 @@ void BreakpointLocationList::RemoveInval if (!arch.IsCompatibleMatch(module_sp->GetArchitecture())) { // The breakpoint was in a module whose architecture is no longer // compatible with "arch", so we need to remove it - m_locations.erase(m_locations.begin() + idx); + RemoveLocationByIndex(idx); continue; } } _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits