Page MenuHomePhabricator

tsan: fix PTRACE_ATTACH handling during stop-the-world
Needs ReviewPublic

Authored by dvyukov on Feb 18 2015, 7:20 AM.

Details

Reviewers
earthdok
Summary

If the thread receives a signal concurrently with PTRACE_ATTACH,
we can get notification about the signal before notification about stop.
In such case we need to forward the signal to the thread, otherwise
the signal will be missed (as we do PTRACE_DETACH with arg=0) and
any logic relying on signals will break. After forwarding we need to
continue to wait for stopping, because the thread is not stopped yet.
We do ignore delivery of SIGSTOP, because we want to make stop-the-world
as invisible as possible.

Diff Detail

Event Timeline

dvyukov updated this revision to Diff 20170.Feb 18 2015, 7:20 AM
dvyukov updated this revision to Diff 20171.
dvyukov retitled this revision from to tsan: fix PTRACE_ATTACH handling during stop-the-world.
dvyukov updated this object.
dvyukov edited the test plan for this revision. (Show Details)
dvyukov added a reviewer: earthdok.
dvyukov added a subscriber: Unknown Object (MLST).
earthdok added inline comments.Feb 18 2015, 10:00 AM
lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
114

We do actually want to stop the threads, though.

I think you misunderstood this part:

The usual practice is to reinject these signals until SIGSTOP is seen, then suppress SIGSTOP injection.

This is "usual practice" for when you want the threads to keep running.

119

passing uptr* in place of int*

125

indent

130

How about this part:

Signal-delivery-stop is observed by the tracer as waitpid(2) returning with WIFSTOPPED(status) true, with the signal returned by WSTOPSIG(status). If the signal is SIGTRAP, this may be a different kind of ptrace-stop; see the "Syscall-stops" and "execve" sections below for details. If WSTOPSIG(status) returns a stopping signal, this may be a group-stop; see below.

and

The fact that signal injection requests may be ignored when restarting the tracee after ptrace stops that are not signal-delivery-stops is a cause of confusion among ptrace users. One typical scenario is that the tracer observes group-stop, mistakes it for signal-delivery-stop, restarts the tracee with

ptrace(PTRACE_restart, pid, 0, stopsig)

with the intention of injecting stopsig, but stopsig gets ignored and the tracee continues to run.

test/tsan/signal_segv_handler.cc
35–36

don't need this anymore

dvyukov added inline comments.Feb 18 2015, 10:11 AM
lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
114

Why do you think that my code does not stop threads?
Or what do you suggest? I don't understand.

dvyukov added inline comments.Feb 18 2015, 10:14 AM
lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
119

status has type int

125

it is 4 spaces aligned
in accordance with the Style

130

And what is about this? Please be more concrete.

test/tsan/signal_segv_handler.cc
35–36

done

dvyukov updated this revision to Diff 20199.Feb 18 2015, 10:14 AM

removed done label

earthdok added inline comments.Feb 18 2015, 10:33 AM
lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
114

nevermind, I was reading it wrong

119

oops

125

nope, that's only for those cases where even the first parameter is wrapped
otherwise you align to the first parameter

130

Nevermind

dvyukov added inline comments.Feb 19 2015, 1:03 AM
lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
125

... and what if the wrapped argument does not fit onto line if you align it to the first arg?...
ok, as you wish, done
the style guide instruction that covers this seems to be "follow the style of existing code"

Committed revision 229832

earthdok added inline comments.Feb 19 2015, 5:20 AM
lib/sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
125

and what if the wrapped argument does not fit onto line if you align it to the first arg?

Then you wrap at the opening bracket and indent with 4 spaces. It's pretty clear:

http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Function_Calls

Either write the call all on a single line, wrap the arguments at the parenthesis, or start the arguments on a new line indented by four spaces and continue at that 4 space indent. In the absence of other considerations, use the minimum number of lines, including placing multiple arguments on each line where appropriate.

Basically you can choose between

foobar(a,
       b)

and

foobar(
    a, b)

or

foobar(
    a,
    b)

But nowhere does it mention the middle ground of

foobar(a,
    b)