Page MenuHomePhabricator

Add a setting to always run all threads when stepping
ClosedPublic

Authored by jingham on Aug 4 2020, 6:15 PM.

Details

Summary

There are some platforms (the xnu kernel being one) where trying to run just one thread can cause problems, and you need to always run all threads whenever you do more than a single step-i.

This patch adds a target.process.run-all-threads setting which overrides the default "run-mode" value for all the stepping commands.

I also needed to come up with a way to test this. The easiest way to do that was to use a scripted thread plan so that I could check the incoming value. But to do that I needed to plumb the stop others through the scripted-step & the ThreadPlanPython, which this patch also does.

Diff Detail

Event Timeline

jingham created this revision.Aug 4 2020, 6:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 4 2020, 6:16 PM
jingham requested review of this revision.Aug 4 2020, 6:16 PM
jingham updated this revision to Diff 283767.Aug 6 2020, 4:52 PM

clang-format

jingham updated this revision to Diff 283769.Aug 6 2020, 4:57 PM

Remove a file that got inadvertently included from another commit.

friss added a comment.Aug 7 2020, 8:55 AM

This LGTM. The only comment I had is about a comment you added on preexisting code:

lldb/source/Commands/CommandObjectThread.cpp
486–490

It feels like the last sentence shouldn't be there.

Also, the comment doesn't explain *why* "we need to pretend".

jingham updated this revision to Diff 283945.Aug 7 2020, 10:26 AM

Clarify comment

jingham added inline comments.Aug 7 2020, 10:27 AM
lldb/source/Commands/CommandObjectThread.cpp
486–490

Is this better?

friss added inline comments.Aug 7 2020, 11:40 AM
lldb/source/Commands/CommandObjectThread.cpp
486–490

Yes, although it still doesn't explain why this implementation detail matters. The comment makes it sound it shouldn't matter. It would be great to document what would break if we didn't set it.

jingham updated this revision to Diff 284031.Aug 7 2020, 1:44 PM

reduce comment to the part appropriate to this change.

jingham added inline comments.Aug 7 2020, 1:48 PM
lldb/source/Commands/CommandObjectThread.cpp
486–490

I don't know why the non-stop mode does things the way it does. That and the fact that it is most likely not very reliable makes me not what to try to put down how it does work here.

All that's relevant to this patch is that if non-stop mode is on, you are running all threads always, so whether you want to run them only when stepping is not relevant, and that's why in this case we aren't checking the setting.

friss accepted this revision.Aug 7 2020, 2:12 PM

Sure. As I said I know this is unrelated to the path. this LGTM

This revision is now accepted and ready to land.Aug 7 2020, 2:12 PM
This revision was landed with ongoing or failed builds.Aug 7 2020, 2:47 PM
This revision was automatically updated to reflect the committed changes.