[Lldb-commits] [lldb] r322348 - Fix Breakpoint::RemoveInvalidLocations to fix the exec testcase.

Jim Ingham via lldb-commits lldb-commits at lists.llvm.org
Thu Jan 11 19:03:23 PST 2018


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;
         }
       }




More information about the lldb-commits mailing list