Fix raw address breakpoints not resolving

[ted]

Summary

An address breakpoint of the form "b 0x1000" won't resolve if it's created while the process isn't running. This patch deletes Address::SectionWasDeleted, renames Address::SectionWasDeletedPrivate to SectionWasDeleted (and makes it public), and changes the section check in Breakpoint::ModulesChanged back to its original form

[davide]

Can you add a test for this behavior?

[ted]

Yes, I'll add a new test to TestAddressBreakpoints.py.

[ted]

Added a test to TestAddressBreakpoints.py that sets an address breakpoint before launch, launches, and checks to see if the breakpoint was hit.

[jingham]

Looks good to me.

include/lldb/Core/Address.h

#endif
 
   Block *CalculateSymbolContextBlock() const;
 
   Symbol *CalculateSymbolContextSymbol() const;
 
   bool CalculateSymbolContextLineEntry(LineEntry &line_entry) const;
 
   //------------------------------------------------------------------
-  // Returns true if the section should be valid, but isn't because the shared
-  // pointer to the section can't be reconstructed from a weak pointer that
-  // contains a valid weak reference to a section. Returns false if the section
-  // weak pointer has no reference to a section, or if the section is still
-  // valid
+  // Returns true if the m_section_wp once had a reference to a valid section
+  // shared pointer, but no longer does. This can happen if we have an address
+  // from a module that gets unloaded and deleted. This function should only be
+  // called if GetSection() returns an empty shared pointer and you want to
+  // know if this address used to have a valid section.
   //------------------------------------------------------------------
   bool SectionWasDeleted() const;
 
 protected:
   //------------------------------------------------------------------
   // Member variables.
   //------------------------------------------------------------------
   lldb::SectionWP m_section_wp; ///< The section for the address, can be NULL.
   lldb::addr_t m_offset; ///< Offset into section if \a m_section_wp is valid...
-
-  //------------------------------------------------------------------
-  // Returns true if the m_section_wp once had a reference to a valid section
-  // shared pointer, but no longer does. This can happen if we have an address
-  // from a module that gets unloaded and deleted. This function should only be
-  // called if GetSection() returns an empty shared pointer and you want to
-  // know if this address used to have a valid section.
-  //------------------------------------------------------------------
-  bool SectionWasDeletedPrivate() const;
 };
 
 //----------------------------------------------------------------------
 // NOTE: Be careful using this operator. It can correctly compare two
 // addresses from the same Module correctly. It can't compare two addresses
 // from different modules in any meaningful way, but it will compare the module
 // pointers.
 //

packages/Python/lldbsuite/test/functionalities/breakpoint/address_breakpoints/TestAddressBreakpoints.py

def address_breakpoints(self):
 
         thread = get_threads_stopped_at_breakpoint(process, breakpoint)
         self.assertTrue(
             len(threads) == 1,
             "There should be a thread stopped at our breakpoint")
 
         # The hit count for the breakpoint should now be 2.
         self.assertTrue(breakpoint.GetHitCount() == 2)
+
+
+
+    def test_address_breakpoint_set_before_launch(self):
+        """Test that an address bp set before the process is launched works correctly."""
+        self.build()
+
+        exe = self.getBuildArtifact("a.out")
+
+        # Create a target by the debugger.
+        target = self.dbg.CreateTarget(exe)
+        self.assertTrue(target, VALID_TARGET)
+
+        # get the address of the symbol "main"
+        sc_list = target.FindSymbols("main", lldb.eSymbolTypeCode)
+        symbol = sc_list.GetContextAtIndex(0).GetSymbol()
+        address = symbol.GetStartAddress().GetFileAddress()
+
+        # BreakpointCreateBySBAddress will resolve the address, causing this
+        # test to always pass, so use runCmd
+        self.runCmd("break set -a " + str(address))
+
+        # Disable ASLR.  This will allow us to actually test (on platforms that support this flag)
+        # that the breakpoint was able to track the module.
+
+        launch_info = lldb.SBLaunchInfo(None)
+        flags = launch_info.GetLaunchFlags()
+        flags &= ~lldb.eLaunchFlagDisableASLR
+        launch_info.SetLaunchFlags(flags)
+
+        error = lldb.SBError()
+
+        process = target.Launch(launch_info, error)
+        self.assertTrue(process, PROCESS_IS_VALID)
+        self.expect("process status", STOPPED_DUE_TO_BREAKPOINT,
+                    substrs=["stop reason = breakpoint 1.1"])
+

source/Breakpoint/Breakpoint.cpp

for (ModuleSP module_sp : module_list.ModulesNoLocking()) {
           continue;
         
         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 (!section_sp || section_sp->GetModule() == module_sp) {
           if (!seen)
             seen = true;
 
           if (!break_loc_sp->ResolveBreakpointSite()) {
             if (log)
               log->Printf("Warning: could not set breakpoint site for "
                           "breakpoint location %d of breakpoint %d.\n",
                           break_loc_sp->GetID(), GetID());

source/Core/Address.cpp

if (section_sp) {
     addr_t sect_file_addr = section_sp->GetFileAddress();
     if (sect_file_addr == LLDB_INVALID_ADDRESS) {
       // Section isn't resolved, we can't return a valid file address
       return LLDB_INVALID_ADDRESS;
     }
     // We have a valid file range, so we can return the file based address by
     // adding the file base address to our offset
     return sect_file_addr + m_offset;
-  } else if (SectionWasDeletedPrivate()) {
+  } else if (SectionWasDeleted()) {
     // Used to have a valid section but it got deleted so the offset doesn't
     // mean anything without the section
     return LLDB_INVALID_ADDRESS;
   }
   // No section, we just return the offset since it is the value in this case
   return m_offset;
 }
 
 addr_t Address::GetLoadAddress(Target *target) const {
   SectionSP section_sp(GetSection());
   if (section_sp) {
     if (target) {
       addr_t sect_load_addr = section_sp->GetLoadBaseAddress(target);
 
       if (sect_load_addr != LLDB_INVALID_ADDRESS) {
         // We have a valid file range, so we can return the file based address
         // by adding the file base address to our offset
         return sect_load_addr + m_offset;
       }
     }
-  } else if (SectionWasDeletedPrivate()) {
+  } else if (SectionWasDeleted()) {
     // Used to have a valid section but it got deleted so the offset doesn't
     // mean anything without the section
     return LLDB_INVALID_ADDRESS;
   } else {
     // We don't have a section so the offset is the load address
     return m_offset;
   }
   // The section isn't resolved or an invalid target was passed in so we can't
case DumpStyleResolvedPointerDescription: {
     return false;
   } break;
   }
 
   return true;
 }
 
 bool Address::SectionWasDeleted() const {
-  if (GetSection())
-    return false;
-  return SectionWasDeletedPrivate();
-}
-
-bool Address::SectionWasDeletedPrivate() const {
   lldb::SectionWP empty_section_wp;
 
   // If either call to "std::weak_ptr::owner_before(...) value returns true,
   // this indicates that m_section_wp once contained (possibly still does) a
   // reference to a valid shared pointer. This helps us know if we had a valid
   // reference to a section which is now invalid because the module it was in
   // was unloaded/deleted, or if the address doesn't have a valid reference to
   // a section.