<feed xmlns='http://www.w3.org/2005/Atom'>
<title>llvm-project.git/lldb/source/Target/ThreadPlanStack.cpp, branch users/mingmingl-llvm/samplefdo-profile-format</title>
<subtitle>Unnamed repository; edit this file 'description' to name the repository.
</subtitle>
<link rel='alternate' type='text/html' href='https://git.belthelziquor.com/llvm-project.git/'/>
<entry>
<title>Convert ThreadPlanStack's mutex to a shared mutex. (#116438)</title>
<updated>2024-11-18T21:23:17+00:00</updated>
<author>
<name>jimingham</name>
<email>jingham@apple.com</email>
</author>
<published>2024-11-18T21:23:17+00:00</published>
<link rel='alternate' type='text/html' href='https://git.belthelziquor.com/llvm-project.git/commit/?id=b42a81631491571c4b78d095917ebdddee69b04f'/>
<id>b42a81631491571c4b78d095917ebdddee69b04f</id>
<content type='text'>
I have some reports of A/B inversion deadlocks between the
ThreadPlanStack and the StackFrameList accesses. There's a fair bit of
reasonable code in lldb that does "While accessing the ThreadPlanStack,
look at that threads's StackFrameList", and also plenty of "While
accessing the ThreadPlanStack, look at the StackFrameList."

In all the cases I've seen so far, there was at most one of the locks
taken that were trying to mutate the list, the other three were just
reading. So we could solve the deadlock by converting the two mutexes
over to shared mutexes.

This patch is the easy part, the ThreadPlanStack mutex.  

The tricky part was because these were originally recursive mutexes, and
recursive access to shared mutexes is undefined behavior according to
the C++ standard, I had to add a couple NoLock variants to make sure it
didn't get used recursively. Then since the only remaining calls are out
to ThreadPlans and ThreadPlans don't have access to their containing
ThreadPlanStack, converting this to a non-recursive lock should be safe.</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
I have some reports of A/B inversion deadlocks between the
ThreadPlanStack and the StackFrameList accesses. There's a fair bit of
reasonable code in lldb that does "While accessing the ThreadPlanStack,
look at that threads's StackFrameList", and also plenty of "While
accessing the ThreadPlanStack, look at the StackFrameList."

In all the cases I've seen so far, there was at most one of the locks
taken that were trying to mutate the list, the other three were just
reading. So we could solve the deadlock by converting the two mutexes
over to shared mutexes.

This patch is the easy part, the ThreadPlanStack mutex.  

The tricky part was because these were originally recursive mutexes, and
recursive access to shared mutexes is undefined behavior according to
the C++ standard, I had to add a couple NoLock variants to make sure it
didn't get used recursively. Then since the only remaining calls are out
to ThreadPlans and ThreadPlans don't have access to their containing
ThreadPlanStack, converting this to a non-recursive lock should be safe.</pre>
</div>
</content>
</entry>
<entry>
<title>[lldb] Remove unneeded .get() NFC</title>
<updated>2023-01-05T06:05:40+00:00</updated>
<author>
<name>Fangrui Song</name>
<email>i@maskray.me</email>
</author>
<published>2023-01-05T06:05:40+00:00</published>
<link rel='alternate' type='text/html' href='https://git.belthelziquor.com/llvm-project.git/commit/?id=4215a84559bb872363eaa4ac724be8ef86c2e683'/>
<id>4215a84559bb872363eaa4ac724be8ef86c2e683</id>
<content type='text'>
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
</pre>
</div>
</content>
</entry>
<entry>
<title>Revert "[lldb] Remove redundant .c_str() and .get() calls"</title>
<updated>2022-12-19T08:52:10+00:00</updated>
<author>
<name>Muhammad Omair Javaid</name>
<email>omair.javaid@linaro.org</email>
</author>
<published>2022-12-19T08:50:31+00:00</published>
<link rel='alternate' type='text/html' href='https://git.belthelziquor.com/llvm-project.git/commit/?id=58e9cc13e24f668a33abdae201d59a02e10c22c0'/>
<id>58e9cc13e24f668a33abdae201d59a02e10c22c0</id>
<content type='text'>
This reverts commit fbaf48be0ff6fb24b9aa8fe9c2284fe88a8798dd.

This has broken all LLDB buildbots:
https://lab.llvm.org/buildbot/#/builders/68/builds/44990
https://lab.llvm.org/buildbot/#/builders/96/builds/33160
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
This reverts commit fbaf48be0ff6fb24b9aa8fe9c2284fe88a8798dd.

This has broken all LLDB buildbots:
https://lab.llvm.org/buildbot/#/builders/68/builds/44990
https://lab.llvm.org/buildbot/#/builders/96/builds/33160
</pre>
</div>
</content>
</entry>
<entry>
<title>[lldb] Remove redundant .c_str() and .get() calls</title>
<updated>2022-12-18T01:15:25+00:00</updated>
<author>
<name>Fangrui Song</name>
<email>i@maskray.me</email>
</author>
<published>2022-12-18T01:15:25+00:00</published>
<link rel='alternate' type='text/html' href='https://git.belthelziquor.com/llvm-project.git/commit/?id=fbaf48be0ff6fb24b9aa8fe9c2284fe88a8798dd'/>
<id>fbaf48be0ff6fb24b9aa8fe9c2284fe88a8798dd</id>
<content type='text'>
Removing .c_str() has a semantics difference, but the use scenarios
likely do not matter as we don't have NUL in the strings.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Removing .c_str() has a semantics difference, but the use scenarios
likely do not matter as we don't have NUL in the strings.
</pre>
</div>
</content>
</entry>
<entry>
<title>Add a mutex to the ThreadPlanStackMap class.</title>
<updated>2022-04-29T18:07:22+00:00</updated>
<author>
<name>Jim Ingham</name>
<email>jingham@apple.com</email>
</author>
<published>2022-04-29T18:07:22+00:00</published>
<link rel='alternate' type='text/html' href='https://git.belthelziquor.com/llvm-project.git/commit/?id=dca2bc408186667346ab3bbb951adab44feba5bd'/>
<id>dca2bc408186667346ab3bbb951adab44feba5bd</id>
<content type='text'>
We've seen very occasional crashes that we can only explain by
simultaneous access to the ThreadPlanStackMap, so I'm adding a
mutex to protect it.

Differential Revision: https://reviews.llvm.org/D124029
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
We've seen very occasional crashes that we can only explain by
simultaneous access to the ThreadPlanStackMap, so I'm adding a
mutex to protect it.

Differential Revision: https://reviews.llvm.org/D124029
</pre>
</div>
</content>
</entry>
<entry>
<title>Remove redundant return and continue statements (NFC)</title>
<updated>2021-12-25T07:17:54+00:00</updated>
<author>
<name>Kazu Hirata</name>
<email>kazu@google.com</email>
</author>
<published>2021-12-25T07:17:53+00:00</published>
<link rel='alternate' type='text/html' href='https://git.belthelziquor.com/llvm-project.git/commit/?id=2d303e678152fddb88dea4199c8872223232b406'/>
<id>2d303e678152fddb88dea4199c8872223232b406</id>
<content type='text'>
Identified with readability-redundant-control-flow.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Identified with readability-redundant-control-flow.
</pre>
</div>
</content>
</entry>
<entry>
<title>[lldb][NFC] Inclusive Language: rename master plan to controlling plan</title>
<updated>2021-11-11T21:04:44+00:00</updated>
<author>
<name>Quinn Pham</name>
<email>Quinn.Pham@ibm.com</email>
</author>
<published>2021-11-02T13:58:19+00:00</published>
<link rel='alternate' type='text/html' href='https://git.belthelziquor.com/llvm-project.git/commit/?id=04cbfa950e0221ac334f802407a9b766df33eee5'/>
<id>04cbfa950e0221ac334f802407a9b766df33eee5</id>
<content type='text'>
[NFC] As part of using inclusive language within the llvm project, this patch
renames master plan to controlling plan in lldb.

Reviewed By: jingham

Differential Revision: https://reviews.llvm.org/D113019
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
[NFC] As part of using inclusive language within the llvm project, this patch
renames master plan to controlling plan in lldb.

Reviewed By: jingham

Differential Revision: https://reviews.llvm.org/D113019
</pre>
</div>
</content>
</entry>
<entry>
<title>[lldb] Avoid moving ThreadPlanSP from plans vector</title>
<updated>2021-08-01T17:40:04+00:00</updated>
<author>
<name>Dave Lee</name>
<email>davelee.com@gmail.com</email>
</author>
<published>2021-07-16T18:04:27+00:00</published>
<link rel='alternate' type='text/html' href='https://git.belthelziquor.com/llvm-project.git/commit/?id=41d0b20cc90f2aea25b4306f6c8f6c258ca3d377'/>
<id>41d0b20cc90f2aea25b4306f6c8f6c258ca3d377</id>
<content type='text'>
Change `ThreadPlanStack::PopPlan` and `::DiscardPlan` to not do the following:

1. Move the last plan, leaving a moved `ThreadPlanSP` in the plans vector
2. Operate on the last plan
3. Pop the last plan off the plans vector

This leaves a period of time where the last element in the plans vector has been moved. I am not sure what, if any, guarantees there are when doing this, but it seems like it would/could leave a null `ThreadPlanSP` in the container. There are asserts in place to prevent empty/null `ThreadPlanSP` instances from being pushed on to the stack, and so this could break that invariant during multithreaded access to the thread plan stack.

An open question is whether this use of `std::move` was the result of a measure performance problem.

Differential Revision: https://reviews.llvm.org/D106171
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Change `ThreadPlanStack::PopPlan` and `::DiscardPlan` to not do the following:

1. Move the last plan, leaving a moved `ThreadPlanSP` in the plans vector
2. Operate on the last plan
3. Pop the last plan off the plans vector

This leaves a period of time where the last element in the plans vector has been moved. I am not sure what, if any, guarantees there are when doing this, but it seems like it would/could leave a null `ThreadPlanSP` in the container. There are asserts in place to prevent empty/null `ThreadPlanSP` instances from being pushed on to the stack, and so this could break that invariant during multithreaded access to the thread plan stack.

An open question is whether this use of `std::move` was the result of a measure performance problem.

Differential Revision: https://reviews.llvm.org/D106171
</pre>
</div>
</content>
</entry>
<entry>
<title>Add a mutex to guard access to the ThreadPlanStack class</title>
<updated>2021-07-16T22:40:58+00:00</updated>
<author>
<name>Jim Ingham</name>
<email>jingham@apple.com</email>
</author>
<published>2021-07-16T03:39:15+00:00</published>
<link rel='alternate' type='text/html' href='https://git.belthelziquor.com/llvm-project.git/commit/?id=6eb576dcff45fa53e8ceb30c7648b903c1413e05'/>
<id>6eb576dcff45fa53e8ceb30c7648b903c1413e05</id>
<content type='text'>
We've seen reports of crashes (none we've been able to reproduce
locally) that look like they are caused by concurrent access to a
thread plan stack.  It looks like there are error paths when an
interrupt request to debugserver times out that cause this problem.

The thread plan stack access is never in a hot loop, and there
aren't enough of them for the extra data member to matter, so
there's really no good reason not to protect the access.

Adding the mutex revealed a couple of places where we were
using "auto" in an iteration when we should have been using
"auto &amp;" - we didn't intend to copy the stack - and I fixed
those as well.

Except for preventing crashes this should be NFC.

Differential Revision: https\://reviews.llvm.org/D106122
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
We've seen reports of crashes (none we've been able to reproduce
locally) that look like they are caused by concurrent access to a
thread plan stack.  It looks like there are error paths when an
interrupt request to debugserver times out that cause this problem.

The thread plan stack access is never in a hot loop, and there
aren't enough of them for the extra data member to matter, so
there's really no good reason not to protect the access.

Adding the mutex revealed a couple of places where we were
using "auto" in an iteration when we should have been using
"auto &amp;" - we didn't intend to copy the stack - and I fixed
those as well.

Except for preventing crashes this should be NFC.

Differential Revision: https\://reviews.llvm.org/D106122
</pre>
</div>
</content>
</entry>
<entry>
<title>[lldb] Rename QueueFundamentalPlan to QueueBasePlan (NFC)</title>
<updated>2021-03-05T05:46:49+00:00</updated>
<author>
<name>Dave Lee</name>
<email>davelee.com@gmail.com</email>
</author>
<published>2021-03-02T21:18:35+00:00</published>
<link rel='alternate' type='text/html' href='https://git.belthelziquor.com/llvm-project.git/commit/?id=e7361c8eccb7663146096622549dc03240414157'/>
<id>e7361c8eccb7663146096622549dc03240414157</id>
<content type='text'>
Minor change for naming consistency.

Differential Revision: https://reviews.llvm.org/D97985
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Minor change for naming consistency.

Differential Revision: https://reviews.llvm.org/D97985
</pre>
</div>
</content>
</entry>
</feed>
