[lldb-dev] PATCH for Review: Fix Rendezvous breakpoint to only be set once, resolve addr in BreakpointLocationList::FindByAddress

[Greg Clayton]

You might think about storing the breakpoint ID instead of a shared pointer to the breakpoint.

On Jul 15, 2013, at 3:24 PM, Michael Sartain <mikesart at gmail.com> wrote:

> This should fix the issue where multiple Rendezvous breakpoints could get set, get confused about the Rendezvous state, and then things head south.
> 
> Also resolve the address in BreakpointLocationList FindByAddress (thanks Jim for that pointer).
> 
> All the Linux tests run fine with this patch. Please let me know if it's ok to commit. Thanks.
>  -Mike
> 
> http://llvm-reviews.chandlerc.com/D1145
> 
> Files:
>   source/Breakpoint/BreakpointLocationList.cpp
>   source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
>   source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
> 
> Index: source/Breakpoint/BreakpointLocationList.cpp
> ===================================================================
> --- source/Breakpoint/BreakpointLocationList.cpp
> +++ source/Breakpoint/BreakpointLocationList.cpp
> @@ -14,7 +14,9 @@
>  // Project includes
>  #include "lldb/Breakpoint/BreakpointLocationList.h"
>  #include "lldb/Breakpoint/BreakpointLocation.h"
> +#include "lldb/Breakpoint/Breakpoint.h"
>  #include "lldb/Core/Section.h"
> +#include "lldb/Target/Target.h"
> 
>  using namespace lldb;
>  using namespace lldb_private;
> @@ -114,7 +116,24 @@
>      BreakpointLocationSP bp_loc_sp;
>      if (!m_locations.empty())
>      {
> -        addr_map::const_iterator pos = m_address_to_location.find (addr);
> +        Address so_addr;
> +
> +        if (addr.IsSectionOffset())
> +        {
> +            so_addr = addr;
> +        }
> +        else
> +        {
> +            // Try and resolve as a load address if possible.
> +            m_owner.GetTarget().GetSectionLoadList().ResolveLoadAddress (addr.GetOffset(), so_addr);
> +            if (!so_addr.IsValid())
> +            {
> +                // The address didn't resolve, so just set to passed in addr.
> +                so_addr = addr;
> +            }
> +        }
> +
> +        addr_map::const_iterator pos = m_address_to_location.find (so_addr);
>          if (pos != m_address_to_location.end())
>              bp_loc_sp = pos->second;
>      }
> Index: source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
> ===================================================================
> --- source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
> +++ source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.cpp
> @@ -263,13 +263,19 @@
>  void
>  DynamicLoaderPOSIXDYLD::SetRendezvousBreakpoint()
>  {
> -    Breakpoint *dyld_break;
> -    addr_t break_addr;
> +    addr_t break_addr = m_rendezvous.GetBreakAddress();
> 
> -    break_addr = m_rendezvous.GetBreakAddress();
> -    dyld_break = m_process->GetTarget().CreateBreakpoint(break_addr, true).get();
> -    dyld_break->SetCallback(RendezvousBreakpointHit, this, true);
> -    dyld_break->SetBreakpointKind ("shared-library-event");
> +    if (m_dyld_break_bp_sp)
> +    {
> +        // If we've already got a breakpoint, make sure it's pointing to the right address.
> +        assert (m_dyld_break_bp_sp->FindLocationIDByAddress(break_addr) != LLDB_INVALID_BREAK_ID);
> +        return;
> +    }
> +
> +    // Breakpoint *dyld_break;
> +    m_dyld_break_bp_sp = m_process->GetTarget().CreateBreakpoint(break_addr, true);
> +    m_dyld_break_bp_sp->SetCallback(RendezvousBreakpointHit, this, true);
> +    m_dyld_break_bp_sp->SetBreakpointKind ("shared-library-event");
>  }
> 
>  bool
> Index: source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
> ===================================================================
> --- source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
> +++ source/Plugins/DynamicLoader/POSIX-DYLD/DynamicLoaderPOSIXDYLD.h
> @@ -92,6 +92,9 @@
>      /// Auxiliary vector of the inferior process.
>      std::unique_ptr<AuxVector> m_auxv;
> 
> +    /// Rendezvous breakpoint.
> +    lldb::BreakpointSP m_dyld_break_bp_sp;
> +
>      /// Enables a breakpoint on a function called by the runtime
>      /// linker each time a module is loaded or unloaded.
>      void
> 
> <D1145.1.patch>_______________________________________________
> lldb-dev mailing list
> lldb-dev at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev