This is an archive of the discontinued LLVM Phabricator instance.

Differential D131946

[LLDB] Handle possible resume thread error
ClosedPublic

Authored by Emmmer on Aug 16 2022, 1:05 AM.

Details

Reviewers
DavidSpickett
Commits
rG8ed3e75c96d9: [LLDB] Handle possible resume thread error
Summary

In this switch case we didn't handle possible errors in ResumeThread(), it's hard to get helpful information when it goes wrong.

Diff Detail

Event Timeline

Emmmer created this revision.Aug 16 2022, 1:05 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 16 2022, 1:05 AM
Herald added a subscriber: JDevlieghere. · View Herald Transcript
Emmmer requested review of this revision.Aug 16 2022, 1:05 AM
Harbormaster completed remote builds in B181454: Diff 452908.Aug 16 2022, 1:08 AM
DavidSpickett accepted this revision.Aug 16 2022, 1:33 AM

LGTM

lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp
943

Nit: remove the {} to fit with the lldb style of not using them for single statement if body.

This revision is now accepted and ready to land.Aug 16 2022, 1:33 AM
Emmmer updated this revision to Diff 452921.Aug 16 2022, 1:57 AM
Emmmer marked an inline comment as done.
Harbormaster completed remote builds in B181461: Diff 452921.Aug 16 2022, 2:00 AM
Closed by commit rG8ed3e75c96d9: [LLDB] Handle possible resume thread error (authored by Emmmer). · Explain WhyAug 16 2022, 8:43 AM
This revision was automatically updated to reflect the committed changes.
Emmmer added a commit: rG8ed3e75c96d9: [LLDB] Handle possible resume thread error.
Herald added a subscriber: lldb-commits. · View Herald TranscriptAug 16 2022, 8:43 AM

Revision Contents

PathSize
lldb/
source/
Plugins/
Process/
Linux/
10 lines

Diff 453030

lldb/source/Plugins/Process/Linux/NativeProcessLinux.cpp

Show First 20 Lines • Show All 927 Lines • ▼ Show 20 Lines for (const auto &thread : m_threads) {
LLDB_LOG(log, "processing resume action state {0} for pid {1} tid {2}", LLDB_LOG(log, "processing resume action state {0} for pid {1} tid {2}",
action->state, GetID(), thread->GetID()); action->state, GetID(), thread->GetID());
switch (action->state) { switch (action->state) {
case eStateRunning: case eStateRunning:
case eStateStepping: { case eStateStepping: {
// Run the thread, possibly feeding it the signal. // Run the thread, possibly feeding it the signal.
const int signo = action->signal; const int signo = action->signal;
ResumeThread(static_cast<NativeThreadLinux &>(*thread), action->state, Status error = ResumeThread(static_cast<NativeThreadLinux &>(*thread),
signo); action->state, signo);
if (error.Fail())
return Status("NativeProcessLinux::%s: failed to resume thread "
"for pid %" PRIu64 ", tid %" PRIu64 ", error = %s",
__FUNCTION__, GetID(), thread->GetID(),
error.AsCString());
DavidSpickettUnsubmitted
Done
ReplyInline Actions

Nit: remove the {} to fit with the lldb style of not using them for single statement if body.

DavidSpickett: Nit: remove the {} to fit with the lldb style of not using them for single statement if body.
break; break;
} }
case eStateSuspended: case eStateSuspended:
case eStateStopped: case eStateStopped:
break; break;
default: default:
▲ Show 20 Lines • Show All 1,054 Lines • Show Last 20 Lines