Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

detach on linux #95

Closed
derekbruening opened this issue Nov 27, 2014 · 26 comments
Closed

detach on linux #95

derekbruening opened this issue Nov 27, 2014 · 26 comments

Comments

@derekbruening
Copy link
Contributor

From derek.br...@gmail.com on March 24, 2009 00:47:16

this was PR 212088

we support detach on Windows but not on Linux

we don't support detach with clients: that was PR 199120 (not filed here yet)

xref attach: issue #38 for detach we'll need a scheme to free the sigstack for suspended threads,
as on linux suspended threads sit on their sigstack

Original issue: http://code.google.com/p/dynamorio/issues/detail?id=95

@derekbruening
Copy link
Contributor Author

From derek.br...@gmail.com on March 25, 2009 08:21:15

we'll also need to get each thread to run its own os_tls_exit() since we
can't fully clean up from a different thread

@derekbruening
Copy link
Contributor Author

From bruen...@google.com on September 27, 2012 15:22:50

we also need each thread to do its own os_switch_lib_tls(dc, true/to
app
/). xref issue #920 .

@derekbruening
Copy link
Contributor Author

From bruen...@google.com on August 21, 2013 09:45:10

Re: running os_switch_lib_tls in each target thread being suspended:

We can just have the suspender set an ostd flag which is read in
handle_suspend_signal by the target thread being suspended (just like
ostd->terminate is read today). That thread can then run the arch_prctl or
whatever in os_switch_lib_tls() up front (fine to do so: no conflict w/
suspender cleaning up its memory) and avoid any further, separate
communication step needed.

Owner: peter.goodman
Labels: -Priority-Low Priority-Medium

@derekbruening
Copy link
Contributor Author

From bruen...@google.com on August 30, 2013 10:09:49

** TODO complications with setting the context

Although Windows has complications with stacked callbacks, Windows supports
directly setting the context of a thread, so we can resume it directly to
its native state. On Linux, however, we rely on the suspended thread
running custom code to do its own context set via sigreturn.

At first, we considered a two-step solution where the master resumes each
thread, lets them go native themselves, and then re-suspends to confirm and
once all are native it frees global state: but to handle the re-suspend we
need our handler in place, and our data structures to communicate why we're
suspending, including a futex and condition variables: so we have a problem
where the suspended thread needs the memory we want to clean up just to do
its final sigreturn.

We have to store the sigreturn context somewhere: either mmapped memory
per-thread that we never free, or on the app stack (may as well go well
beyond the redzone). We may go w/ the app stack for now for simplicity,
even though there are transparency corner cases.

Given that we have that problem, we may as well avoid the regular
re-suspend. We'll do our resume from the synchall with a flag set. The
resumed thread will see the flag and jump to special gencode. This gencode
will set the current stack pointer to the designated distance from the app
TOS and enter a special loop to wait for final resumption. This loop would
ideally do a FUTEX_WAIT on a new futex created on the app stack or
something, but for now we might do a SYS_nanosleep in a loop for simplicity
(yield and busy loop may not be good enough for high-priority threads).
The condition var for exiting the loop will be stored in an app stack
slot. Once set (after the master frees everything: global and for each
thread -- so need communication back that we're in the final loop, again
ideally w/ a futex set which we actually could do since not freed yet but
complex from gencode (could pass param in from C I guess, or use inline asm
instead of gencode)), the suspended thread invokes sigreturn.

** TODO set mask on final sigreturn to app value

@derekbruening
Copy link
Contributor Author

From zhao...@google.com on September 03, 2013 08:12:07

We might over-design.
I feel the basic step should be something like:

  1. master thread notify the slave thread to detach
  2. slave thread setup the right context, goes to the asm code with data on app stack, notifying the master thread and then sigreturn.
  3. master thread get notified, do cleanup for thread_exit and global_exit

So we do not even need synch_all as the master only need notify the slave to perform some action.
One messy part is the signal:

  1. block the signal before sigreturn
  2. need change the sigaction back to default signal handling

Owner: zhao...@google.com

@derekbruening
Copy link
Contributor Author

From bruen...@google.com on September 03, 2013 08:32:16

Yes, the extra slave wait may not be necessary.

You do need the synchall as it's the mechanism to get a thread to a safe spot. To avoid it you'd end up duplicating essentially the same logic elsewhere. And you have to use signals to interrupted the target thread, and ensure it's at a safe spot, for critical reasons:

  1. Safety of modifying each target thread's thread-private data structures
  2. Detach in bounded length of time

@derekbruening
Copy link
Contributor Author

From bruen...@google.com on September 10, 2013 09:46:53

master can do xl8 while in synchall
thread_get_mcontext()
translate_mcontext()
thread_set_mcontext()
(preferable to translate_sigcontext b/c it grabs lock and it's internal
to unix: the above sequence will work on Windows as well as we can
change the current Windows detach code to use it)

slave can copy sigcontext to app stack. should leverage:

  • get_sigstack_frame_ptr()
  • copy_frame_to_stack()

slave must do:

  • segment restore
  • sigaltstack
  • set xsp to where sigcontext was copied
  • set flag
  • sigreturn

signal arrives during detach: how handle
xref i#

Owner: peter.goodman

@derekbruening
Copy link
Contributor Author

derekbruening commented Nov 27, 2014

From bruen...@google.com on September 10, 2013 12:47:20

continuing the final thought above about handling signals during the detach process: we'll probably want to mask all signals in the slaves (the current mask + the few we usually let through: sigsegv and the suspend signal itself or sthg), have the master restore the app signal handlers during synchall (file a separate issue on handling signal handlers not shared by the entire thread group -- won't happen for pthreads), and have the sigreturn restore the mask.

@derekbruening
Copy link
Contributor Author

From bruen...@google.com on September 19, 2013 13:54:46

** TODO discussion on start/stop all-thread vs single-thread

Discussion about how to handle changes to the address space while threads
are native led to a first proposal to split start/stop into _all_threads()
vs _cur_thread(). We'll assume we'll eventually have -native_exec_syscalls
on Linux to help find address space changes while native.

Stop should not free anything that start is not prepared to re-initialize.

For client: thread_exit should be at dr_app_cleanup. => add go-native and
resume-DR events in case client has hook or other global stuff?

What if thread is native and it exits? Feasible to we hook all calls to
SYS_exit? If client misses thread_exit: memory leak at best, incorrect
behavior at worst (think about tid reuse).

=> Challenges to figure out later? For our purposes we limit our API
usage to stop followed immediately by cleanup.

But, there's a problem if we remove our signal handler at stop: have to
re-instate it prior to cleanup to do synchall, so best to not remove
handler (have to impl propagation of signal received in native thread to
app handler for native_exec anyway). If a thread has exited while native,
that's ok: just skip the steps it does itself.

=> Conclusion: two usage models: single-thread start and stop with separate
init and cleanup, or combined init-and-takeover-all-thread plus
stop-all-threads-and-clean-up.

API routine renames:
s/dr_app_start/dr_app_start_thread/
s/dr_app_stop/dr_app_stop_thread/
s/dr_app_setup_and_start/dr_app_setup_and_start_all/
+dr_app_stop_all_and_cleanup

@derekbruening
Copy link
Contributor Author

From p...@google.com on September 20, 2013 12:21:53

I've done a bit of work on the API renaming stuff. It mostly works on Linux, but have not done extensive Windows testing (first test failed because I didn't update the CMakeLists.txt to export the renamed dr_app_start stuff).

I've renamed according to what's described, with the addition of a dr_app_start_all for temporary convenience. dr_app_start_thread and dr_app_start_all differ only in how they invoke dynamo_start, by passing true/false on whether or not to take over all threads.

Not much testing beyond this though. Going back to working on detach.

@derekbruening
Copy link
Contributor Author

From bruen...@google.com on September 23, 2013 12:57:39

** TODO dynamo_shared_exit discussion

Should still follow Windows detach general flow of detach code doing
per-thread exits and then calling just dynamo_shared_exit, as
dynamo_process_exit, when it does synch w/ all threads (doesn't in release
w/ no client or fast client), goes and forcibly kills all the threads.

So:

  1. Split kernel-visible cleanup (signal mask, sigaltstack, signal handlers,
    segment) out from regular thread exit routines, with flag or sthg to
    avoid re-doing on later dynamo_other_thread_exit() call, and call that
    on each slave while suspended.

  2. Resume slaves, wait for notificatio that their DR memory can be cleaned
    up.

  3. Iterate over slaves and call dynamo_other_thread_exit().

  4. Call dynamo_shared_exit(). This is all from app stack, so ok to clean
    up dstack: verify it will be freed (we sometimes separate that part
    out).

@derekbruening
Copy link
Contributor Author

From bruen...@google.com on October 08, 2013 06:36:26

Revisiting the conclusions of comment #9:

Supporting re-takeover when stopping is tied to full cleanup is problematic
as it requires that DR fully zero all static and global variables. There
are many cases of static variables scattered around, such as inside
DO_ONCE, in the initializers for Extensions (drmgr, etc.), in memoized
functions, etc. We'd have to make all those non-global-scope static vars
exposed to get access to them, or try to zero out the whole .data and .bss.
This has performance impliciations for chains of short-lived processes. We
also have to deal with subtle things like issue #1271 , where we threw out
the .1config file under the assumption that we wouldn't re-read the options
later. Plus, even if we make DR work in this model, third-party Extensions
are unlikely to follow this: we would have to noisily demand a different
programming model than is usually assumed.

Plus, one of our use cases for attach/detach is to do instrumentation over
short periods, where it would be more efficient to not throw everything
away and re-initialize all of DR each time. It seems betterh both in
efficiency and ease of programming to implement a "re-examine the process
b/c I was native for a while" than to implement "reset every global thing
in DR and in every extension library and maybe the client too".

In fact, we already have code for re-examining the process:
retakeover_after_native() in callback.c for when we lose control on windows
cbret. For Windows, this is all much easier than for Linux, as on Windows
we have hooks for new threads or thread exit or mmap, so we just need to
flush the whole cache, throw out the module list, and re-scan the address
space on re-takeover.

For Linux we would want to add hooks for thread init+exit, mmap-related,
and signal syscalls. Long-term we want the hooks on Linux anyway, to
better support hybrid and other partially-native modes. So the conclusion
is that we should change the start/stop API back to start_all and stop_all
being separate from setup/cleanup.

@derekbruening
Copy link
Contributor Author

From bruen...@google.com on October 08, 2013 10:26:23

More comments on start/stop vs cleanup: it seems that for repeated
detach/attach, we really want "go native" and "back under DR". So we want
to map the start/stop API to partially-native execution (or fully native).
Perhaps we shouldn't call it "detach" then and should reserve "detach" to
mean a go native plus a full cleanup. Xref the DR_EMIT_GO_NATIVE feature
( issue #725 ) and recent work on partial-native execution on Linux ( issue #978 , issue #1242 ).

@derekbruening
Copy link
Contributor Author

From bruen...@google.com on October 08, 2013 10:27:18

Linux syscall hooks: issue #721

@derekbruening
Copy link
Contributor Author

Re: the ongoing and back-and-forth argument over having dr_app_stop do full
cleanup or just send all threads native:

It seems like we can design start/stop and attach/detach to permanently
split up the features so that we can implement them all and pick which
combination best suits each use case:

  1. Send all threads native: dr_app_stop() will trigger this.
    This leaves signal handlers and hooks in place, but they handle
    (generally via pass-through) native threads.

  2. Have dr_app_start() re-takeover known threads, and re-examine app state
    that might have changed (unless we get enough syscall hooks to not need
    this: linux syscall hooks for native_exec, hotp_only, and thin_client #721). We support making optional assumptions depending on the
    app for improved perf: that no libs were loaded, no code was modified,
    etc.

  3. Add "thread going native" and "thread re-takeover" client events, as
    these are distinct from thread_exit and thread_init in the presence of
    re-takeover. Maybe call them "thread_detach" and "thread_attach"?
    Should we invoke them in normal operation as well? I.e., whenever we
    call a thread_init event, we subsequently call the thread_attach event?

    Is this adding too much complexity? Should we step back and say we
    don't want to support re-takeover and that dr_app_stop() should do a
    full cleanup (though we've gone down that road before and rejected
    it...)

  4. Clean up and unload DR lib (if it's not static): dr_app_cleanup() will
    trigger this. This requires all threads to already be native.

  5. Re-start: for non-static DR, this would be an attach via injecting the
    DR lib. For static this gets trickier as it hits all the global
    variable issues discussed earlier.

@derekbruening
Copy link
Contributor Author

However, it is much easier to remove hooks while all threads are suspended.
Should we have a dr_app_stop_and_cleanup()?

For going native, b/c each thread has to restore its own segment, it's not
currently possible to bound the time for the whole process to go native due
to threads waiting at syscalls. We'll have to put in place a way for those
threads to restore their segments lazily while we clean up the rest of DR:
we'll have to leave tombstones that aren't cleaned up.

@derekbruening
Copy link
Contributor Author

derekbruening commented Sep 8, 2016

For taking over known-but-native threads we actually need a synchall-style loop as we see when we run the new api.startstop test with -loglevel 3:

<rank order violation event_lock(mutex)@/work/dr/git/src/core/unix/os.c:8899 acquired after memory_info_buf_lock(mutex)@/work/dr/git/src/core/unix/memquery_linux.c:71 in tid:196f>
[Switching to Thread 0xf6fffb40 (LWP 6511)]

Breakpoint 1, report_dynamorio_problem (dcontext=0x0, dumpcore_flag=8, exception_addr=0x0, report_ebp=0x0, 
    fmt=0xf72ba924 "DynamoRIO debug check failure: %s:%d %s\n(Error occurred @%d frags)") at /work/dr/git/src/core/utils.c:2168
2168        synchronize_dynamic_options();
(gdb) bt
#0  report_dynamorio_problem (dcontext=0x0, dumpcore_flag=8, exception_addr=0x0, report_ebp=0x0, 
    fmt=0xf72ba924 "DynamoRIO debug check failure: %s:%d %s\n(Error occurred @%d frags)") at /work/dr/git/src/core/utils.c:2168
#1  0xf70e3d0b in internal_error (file=0xf72ba7ef "/work/dr/git/src/core/utils.c", line=615, 
    expr=0xf72bb740 "(dcontext->thread_owned_locks->last_lock->rank < lock->rank IF_CLIENT_INTERFACE(|| first_client || both_client)) && \"rank order violation\"") at /work/dr/git/src/core/utils.c:174
#2  0xf70e5974 in deadlock_avoidance_lock (lock=0x4a810114, acquired=true, ownable=true) at /work/dr/git/src/core/utils.c:613
#3  0xf70e6215 in mutex_trylock (lock=0x4a810114) at /work/dr/git/src/core/utils.c:918
#4  0xf70e6108 in mutex_lock (lock=0x4a810114) at /work/dr/git/src/core/utils.c:857
#5  0xf7260c01 in signal_event (e=0x4a810110) at /work/dr/git/src/core/unix/os.c:8914
#6  0xf72619f8 in os_thread_take_over (mc=0x4a8fa91c) at /work/dr/git/src/core/unix/os.c:9282
#7  0xf726b4ef in sig_take_over (uc=0x4a8fab8c) at /work/dr/git/src/core/unix/signal.c:3995
#8  0xf7272913 in handle_suspend_signal (dcontext=0x4a8101c0, ucxt=0x4a8fab8c) at /work/dr/git/src/core/unix/signal.c:6138
#9  0xf726c3b1 in master_signal_handler_C (xsp=0x4a8faafc "*\275#\367\f") at /work/dr/git/src/core/unix/signal.c:4397

It looks like SIGUSR2 came in during DR code handling the sysenter hook
while this thread is native. My new code to re-takeover a known but
currently native thread then takes over.

Seems like we need a synchall-style loop there.

How about we just remove the sysenter hook while all-native? And we have a
reason now to not add a dr_app_stop_just_this_thread(). Seems fine to
not support native_exec mixed with start/stop as well.

We should also remove the signal handlers.
Xref #1921: add proper signal support while threads are native.

@derekbruening
Copy link
Contributor Author

*** TODO translate threads at syscalls: assume app will re-try interrupted syscall?

Doesn't seem good enough. Xref #1145 and adjust_syscall_for_restart().

        /* FIXME i#95: this will xl8 to a post-syscall point for a thread at
         * a syscall, and we rely on the app itself to retry a syscall interrupted
         * by our suspend signal.  This is not good enough, as this is an
         * artifical signal that the app has not planned for with SA_RESTART or
         * a loop.  We want something like adjust_syscall_for_restart().
         * Xref #1145.
         */

@derekbruening
Copy link
Contributor Author

This is a big feature with a lot of pieces and corner cases.

Recent CL's have the basics working for an app-triggered detach:

a84214e i#95 Linux detach: post-sysenter is a safe spot
a6a062d i#95 Linux detach: change dr_app_stop to send all threads native
ba6e2e6 i#95 Linux detach, i#1512 startstop flaky: rewrite api.startstop test
c7524ac i#95 Linux detach: generalize call_switch_stack
410aa08 i#95 Linux detach: translate do_syscall when method is sysenter
9592116 i#95 Linux detach: detaching via dr_app_stop_and_cleanup()
5214e9b i#95 Linux detach: merge with Windows detach

@derekbruening
Copy link
Contributor Author

dr_app_stop_and_cleanup() does a full detach today.

A separated dr_app_stop() and later dr_app_cleanup() are different: today a separate cleanup kind of assumes it's terminating the app. I have a forthcoming CL to at least remove the thread termination in the final synch. But it seems better to look like dr_app_stop_and_cleanup() and do a full detach: worst-case we can insert a dr_app_start() first?!? Or we could just drop support for dr_app_cleanup().

@derekbruening
Copy link
Contributor Author

Nudge-based detach is only supported on Windows due to the assumption of a separate thread stack. If we implement a scheme to create a temporary separate stack for a client-triggered detach #2644 we could use the same mechanism for a nudge-based detach on UNIX which I believe is the only reason this issue is still open?

@derekbruening
Copy link
Contributor Author

Pasting notes from re-examining the Windows detach status today:

The existing Windows detach nudge feature is internal to DR and does not go through a client. It looks like it is not exposed in drconfig and the old tests for it are not all enabled; the old way to trigger it is in the no-longer-supported "drcontrol" front-end, but it's just a different nudge type so it should be feasible to tweak drconfig to enable it to try it out. You can see the code paths in libutil/detach.c triggering the nudge to be sent and handling it in core/nudge.c "TEST(NUDGE_GENERIC(detach), nudge_action_mask)" calling detach_helper() which calls the shared-with-all-detach-types detach_on_permanent_stack().

@derekbruening
Copy link
Contributor Author

We'd want to merge nudgeunix into drdeploy.c so we can invoke nudges in general and detach nudges from the dronconfig front-end: that's part of #840.

@derekbruening
Copy link
Contributor Author

Re-exposing the Windows detach from drconfig is part of #725

@onroadmuwl
Copy link
Contributor

onroadmuwl commented Dec 18, 2023

The relevant code for detachment on Linux is submitted in #6513

derekbruening pushed a commit that referenced this issue Dec 30, 2023
Enable the feature of detachment on Linux. 

- I setup a temporary separate stack for the nudged thread, and resume
the normal execution of this thread after detachment.
- A new variable named `nudged_sigcxt` is used to save the `sigcxt` of
nudged thread.
- And the extra code `dcontext == GLOBAL_DCONTEXT` is added to cover the
new code path that won't be executed before during the process of thread
exit.
- What's more, I turn off the option of `sigreturn_setcontext` for the
smooth resumption of nudged thread on X64 ISA.
- Finally, the frontend, automated test cases and documentation are
modified accordingly.

Issue: [#95](#95)
@derekbruening
Copy link
Contributor Author

I think we can close this now. Any problems or enhancements can be filed as separate issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants