This is an archive of the discontinued LLVM Phabricator instance.

[NativeProcessLinux] Fix handling of SIGSTOP
ClosedPublic

Authored by labath on May 19 2015, 7:38 AM.

Details

Summary

Previously, NPL tried to reinject SIGSTOP into the inferior in an attempt to get the process to
start in the group-stop state. This was:
a) wrong (reinjection should be controlled by "process handle" lldb setting)
b) racy (it should use Resume for transparent resuming instead of RequestResume)
c) broken (llgs crashed on inferior SIGSTOP)

With this change, SIGSTOP is handled just like any other signal delivered to the inferior: we
stop all threads and report signal reception to lldb. SIGSTOP reinjection does not behave the
same way as it would outside the debugger, but simulating this is a hard problem and is not
normally necessary.

Diff Detail

Repository
rL LLVM

Event Timeline

labath updated this revision to Diff 26058.May 19 2015, 7:38 AM
labath retitled this revision from to [NativeProcessLinux] Fix handling of SIGSTOP.
labath updated this object.
labath edited the test plan for this revision. (Show Details)
labath added reviewers: ovyalov, chaoren.
labath added a subscriber: Unknown Object (MLST).
ovyalov edited edge metadata.May 19 2015, 1:48 PM

Please see my comments.

test/functionalities/signal/raise/TestRaise.py
1 ↗(On Diff #26058)

It will be great to make this test to verify different modes that "process handle" allows - e.g., with enabled/disabled debugger stop, ...

3 ↗(On Diff #26058)

It seems time module isn't used.

10 ↗(On Diff #26058)

Please verify the test remote invocation.

27 ↗(On Diff #26058)

It seems no need to have this method just for delegation purposes.

47 ↗(On Diff #26058)

Could you use process.GetUnixSignals().GetSignalNumberFromName('SIGSTOP') instead of signal.SIGSTOP - it matters when running test remotely on different platforms (for example, OSX <-> Linux) ?

labath updated this revision to Diff 26137.May 20 2015, 3:22 AM
labath edited edge metadata.
  • Make the test more robust and extensive.

Fixed the test suite, please take another look.

test/functionalities/signal/raise/TestRaise.py
1 ↗(On Diff #26058)

Added tests for no-stop, notify and no-stop, no-notify modes.

3 ↗(On Diff #26058)

Removed.

10 ↗(On Diff #26058)

test passes linux->android

27 ↗(On Diff #26058)

Removed.

47 ↗(On Diff #26058)

Done, thanks.

ovyalov accepted this revision.May 20 2015, 11:18 AM
ovyalov edited edge metadata.

Looks good.

This revision is now accepted and ready to land.May 20 2015, 11:18 AM
This revision was automatically updated to reflect the committed changes.