On Wed, Mar 27, 2019 at 6:49 PM Jim Ingham via lldb-commits <lldb-commits@lists.llvm.org> wrote: > > Author: jingham > Date: Wed Mar 27 18:51:33 2019 > New Revision: 357141 > > URL: http://llvm.org/viewvc/llvm-project?rev=357141&view=rev > Log: > Copy the breakpoint site owner's collection so we can drop > the collection lock before we iterate over the owners calling ShouldStop. > > BreakpointSite::ShouldStop can do a lot of work, and might by chance hit the > same breakpoint > site again on another thread. So instead of holding the site's owners lock > while iterating over them calling ShouldStop, I make a local copy of the > list, drop the lock > and then iterate over the copy calling BreakpointLocation::ShouldStop. > > It's actually quite difficult to make this cause problems because usually all > the > action happens on the private state thread, and the lock is recursive. > > I have a report where some code hit the ASAN error breakpoint, went to > compile the ASAN error gathering expression, in the course of compiling > that we went to fetch the ObjC runtime data, but the state of the program > was such that the ObjC runtime grubbing function triggered an ASAN error and > we were executing that function on another thread. > > I couldn't figure out a way to reproduce that situation in a test. But this > is an > NFC change anyway, it just makes the locking strategy more narrowly focused. > > <rdar://problem/49074093> > > Modified: > lldb/trunk/include/lldb/Breakpoint/BreakpointLocationCollection.h > lldb/trunk/source/Breakpoint/BreakpointLocationCollection.cpp > lldb/trunk/source/Breakpoint/BreakpointSite.cpp > > Modified: lldb/trunk/include/lldb/Breakpoint/BreakpointLocationCollection.h > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/BreakpointLocationCollection.h?rev=357141&r1=357140&r2=357141&view=diff > ============================================================================== > --- lldb/trunk/include/lldb/Breakpoint/BreakpointLocationCollection.h > (original) > +++ lldb/trunk/include/lldb/Breakpoint/BreakpointLocationCollection.h Wed Mar > 27 18:51:33 2019 > @@ -22,6 +22,8 @@ public: > BreakpointLocationCollection(); > > ~BreakpointLocationCollection(); > + > + BreakpointLocationCollection &operator=(const BreakpointLocationCollection > &rhs); > > //------------------------------------------------------------------ > /// Add the breakpoint \a bp_loc_sp to the list. > > Modified: lldb/trunk/source/Breakpoint/BreakpointLocationCollection.cpp > URL: > http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Breakpoint/BreakpointLocationCollection.cpp?rev=357141&r1=357140&r2=357141&view=diff > ============================================================================== > --- lldb/trunk/source/Breakpoint/BreakpointLocationCollection.cpp (original) > +++ lldb/trunk/source/Breakpoint/BreakpointLocationCollection.cpp Wed Mar 27 > 18:51:33 2019 > @@ -178,3 +178,20 @@ void BreakpointLocationCollection::GetDe > (*pos)->GetDescription(s, level); > } > } > + > +BreakpointLocationCollection &BreakpointLocationCollection::operator=( > + const BreakpointLocationCollection &rhs) { > + // Same trick as in ModuleList to avoid lock inversion. > + if (this != &rhs) { > + if (uintptr_t(this) > uintptr_t(&rhs)) { > + std::lock_guard<std::mutex> lhs_guard(m_collection_mutex); > + std::lock_guard<std::mutex> rhs_guard(rhs.m_collection_mutex); > + m_break_loc_collection = rhs.m_break_loc_collection; > + } else { > + std::lock_guard<std::mutex> lhs_guard(m_collection_mutex); > + std::lock_guard<std::mutex> rhs_guard(rhs.m_collection_mutex); > + m_break_loc_collection = rhs.m_break_loc_collection; > + } > + }
The two branches are identical, is this intentional? -- Davide _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
