Bug 12275

Summary: uretprobes break exception handling
Product: systemtap Reporter: Josh Stone <jistone>
Component: uprobesAssignee: Unassigned <systemtap>
Status: NEW ---    
Severity: normal CC: ananth, fche, jkenisto, mark, roland, srikar, tromey
Priority: P2    
Version: unspecified   
Target Milestone: ---   
Host: Target:
Build: Last reconfirmed:

Description Josh Stone 2010-11-30 23:51:13 UTC
When a uretprobe is placed on a function that throws an exception, the caller is unable to catch it.

$ cat throw.cxx
#include <stdexcept>
void foo() { throw std::runtime_error("oops!"); }
int main() {
    try { foo(); } catch (std::runtime_error&) {}
    return 0;
}
$ g++ -g -O2 throw.cxx -o throw
$ ./throw
$ stap -we 'probe process("./throw").function("foo").return {}' -c ./throw
terminate called after throwing an instance of 'std::runtime_error'
  what():  oops!
Comment 1 Mark Wielaard 2010-12-01 20:32:32 UTC
We were discussing how this relates to the longjmp case, seeing that also has "strange" stack frames. But a simple example works fine.

$ cat lj.c 
#include <setjmp.h>
#include <stdio.h>

jmp_buf b;

void
foo ()
{
  longjmp (b, 1);
  printf("impossible in func!\n");
}

int
main ()
{
  if (setjmp(b) != 0)
    {
      printf("jumped back!\n");
      return 0;
    }

  foo ();
  printf("impossible in main!\n");
  return -1;
}

$ gcc -g -Wall -o lj lj.c 

$ stap -we 'probe process("./lj").function("foo").return {}' -c ./lj
jumped back!
Comment 2 Josh Stone 2010-12-01 21:06:08 UTC
(In reply to comment #0)
> When a uretprobe is placed on a function that throws an exception, the caller
> is unable to catch it.

Roland was not surprised that this fails.  My rough understanding from him is that C++ dynamically looks at eh_frame data to figure out how to unwind.  It will see the uretprobe trampoline address on the stack, for which it has no eh_frame data.  So it assumes that we've reached a point of code compiled without exception handling (kind of correct) and terminates the app.

I think it's fine that the .return probe doesn't fire for this case, but ideally I think we'll want a new .except probe so script writers can still have balanced call+return/except semantics if needed.  But our #1 priority needs to be getting out of the way of the app's normal execution.

(In reply to comment #1)
> We were discussing how this relates to the longjmp case, seeing that also has
> "strange" stack frames. But a simple example works fine.

In this case the app is fine, so it's much less worrisome.  The .return probe doesn't fire, which is semantically correct, but we also need to make sure that uretprobes still cleans up sanely after having been so gracefully avoided.  My guess is that we'll leak a uretprobe_instance, recovered once the app exits, but I need to get more familiar with the uretprobes bookkeeping.
Comment 3 Jim Keniston 2010-12-01 22:13:29 UTC
(In reply to comment #2)
> > We were discussing how this relates to the longjmp case, seeing that also has
> > "strange" stack frames. But a simple example works fine.
> 
> In this case the app is fine, so it's much less worrisome.  The .return probe
> doesn't fire, which is semantically correct, but we also need to make sure that
> uretprobes still cleans up sanely after having been so gracefully avoided.  My
> guess is that we'll leak a uretprobe_instance, recovered once the app exits,
> but I need to get more familiar with the uretprobes bookkeeping.

A longjmp shouldn't cause uretprobe_instances to leak.  See uretprobe_bypass_instances() in uprobes.c.  Our intent was to handle longjmps correctly, but we didn't consider that C++ exception handling is a different beast.
Comment 4 Josh Stone 2010-12-01 22:45:29 UTC
(In reply to comment #3)
> A longjmp shouldn't cause uretprobe_instances to leak.  See
> uretprobe_bypass_instances() in uprobes.c.  Our intent was to handle longjmps
> correctly, but we didn't consider that C++ exception handling is a different
> beast.

Sorry, "leak" isn't quite the right term.  They are left in a sort of limbo until reclaimed.  This will happen at task exit, but it's good to know that the heuristic you point out should reclaim them much sooner.  It relies on this claim:

  "A bypassed uretprobe_instance's stack_ptr is beyond the current stack."

This isn't strictly guaranteed, as future uretprobes that invoke this check might be at a deeper nesting than the limbo uretprobe.  But since the stack can't grow forever, it's a pretty good heuristic.  I expect this would work after C++ exception handling too, if we can just get the unwinder not to trip on the trampoline address.
Comment 5 Josh Stone 2010-12-02 02:25:46 UTC
Here's a very rough outline of what I think the trampoline needs:

1:  /* As before, the trampoline return address points here.
     * When hit, run the uretprobe handler, then fixup the IP
     * back to the original return address. */
    BREAKPOINT_INSTRUCTION

2:  /* The eh_frame magic metadata for the trampoline points here
     * for a "catch" clause.  When hit, run an exception probe handler,
     * set the regs/stack so we could return to the original caller,
     * then fall through. */
    BREAKPOINT_INSTRUCTION

3: /* Now that we're ready to return to the original caller,
    * rethrow the exception so C++ can take care of the rest. */
    CALL __cxa_rethrow


We currently do [1] in an SSOL slot, but we may need a different mechanism since we're expanding to a sequence of instructions.  I call it "magic" to get us to [2], because I don't know how that stuff works -- those in the know, please help flesh that out, if it's even possible.  And from looking at the asm of little test programs that rethrow, I know that [3] is more complicated than a single call, but we can expand that.
Comment 6 Roland McGrath 2010-12-03 03:00:48 UTC
A "catch and rethrow" is not necessarily what you need to model.
It really depends on what you are trying to get.

For merely making exception handling work, you don't need the retprobe trampoline to appear to be a frame that catches.  It can just appear to be a frame that does nothing with exceptions, but that can be unwound through to its caller (i.e., the real caller of the probed function).  That is also what you need for generic unwinding for backtraces (including by userland unwinders/debuggers, not just ubacktrace()) not to be so confused by the presence of the uretprobe trampoline.  That is far simpler metadata, just a tiny bit that says how to derive the true caller's address.  If that address is not already stored somewhere findable in user space, you can just stick the address in the trampoline after the breakpoint instruction, so the metadata can say to extract the address from the memory near the trampoline PC.  Or, if you are generating metadata on the fly rather than using fixed metadata, it can just include the literal address directly in the metadata.

Something more like what you suggest might be required to have a probe hit for exceptions, though with some more thought we might come up with a way to intercept that without actually using rethrow.

What so far still seems like the harder problem is getting the infrastructure to find your metadata at all.  The trampoline lives in an SSOL slot, which is some dynamically-allocated text page not envisioned before runtime.  To get that found, you'd have to somehow wedge a new record into the list of objects examined to find .eh_frame data, as if it were a DSO you'd dlopen'd or suchlike.  That is doable with sufficient knowledge of the dynamic linker bookkeeping and/or the EH code's hooks, but it seems fragile and not immediately obvious.  It may be an easier tack to modify the vDSO's .eh_frame data to claim to cover the SSOL area, since the vDSO is already in that list.  But that is also fragile and nonobvious unless you modify the kernel's vDSO source beforehand to leave some space to use for it or something.
Comment 7 Frank Ch. Eigler 2010-12-06 21:05:36 UTC
For reference, another short-term hack would be to disarm
some or all uretprobes while an exception dispatch is in
progress.
Comment 8 Josh Stone 2010-12-07 03:20:53 UTC
The current design shares a single SSOL slot as the trampoline address of uretprobes throughout the process.  If we want a user-accessible "real" return address stored predictably nearby, then we'll have to use a separate SSOL slot as trampoline for each uretprobe hit.  That also means eh_frame metadata would have to be available for all of those.

That's partly why I came up with having a "catch" handler of our own, so in a single trampoline we can hook uprobes to inject the real return address from its in-kernel uretprobe_instance to wherever is convenient.  And now that I've read more of the exception ABI, I think it wouldn't actually have to be a full C++ catch-rethrow.  I think just having a low-level cleanup routine is sufficient, which just needs our breakpoint hook, then call _Unwind_Resume.  I don't know if we could actually link _Unwind_Resume from something like a VDSO handler though.

As a different strategy with the existing simple trampoline, maybe the eh_frame metadata could inject a different kind of fault?  Perhaps tell the unwinder that the next return address is at *(MAGIC_BAD_ADDR), and let uprobes fix it from there?  We still have the pesky problem of hooking up our eh_frame, of course.

I like the idea of putting the trampoline in the VDSO, including eh_frame data, but it seems much more plausible with an in-kernel uprobes.  Someday, perhaps...
Comment 9 Josh Stone 2010-12-07 03:41:21 UTC
(In reply to comment #7)
> For reference, another short-term hack would be to disarm
> some or all uretprobes while an exception dispatch is in
> progress.

I mentioned this idea to Jim, and he told me that it's not a simple thing to disarm uretprobes.  The current design doesn't really disarm anything, even after unregister_uretprobe -- it just zeros the handler so the next hit does nothing.  On x86, we could actually restore the return addresses throughout the stack, but on other architectures like powerpc we can't generally predict where the fixup is needed.  Perhaps if uprobes had a full unwinder...

Then there's the matter of hooking exception dispatch itself.  I suppose this needs probes on _Unwind_RaiseException and _Unwind_ForcedUnwind, maybe also _Unwind_Resume.  And some point also to know that exception dispatch is finished.

Maybe stap can register for these extra probepoints, provided enough debuginfo, and also perform the unwinding to figure out how to restore the return addresses.  This means "proper" uretprobes behavior would be depending on its client (stap), but I'm not sure we want uprobes to grow all this functionality itself.

I could be making a mountain out of a molehill, but I doubt that this disarm-hack is really any easier to accomplish.
Comment 10 Frank Ch. Eigler 2010-12-07 03:46:08 UTC
(In reply to comment #9)
> (In reply to comment #7)
> > For reference, another short-term hack would be to disarm
> > some or all uretprobes while an exception dispatch is in
> > progress.
> 
> I mentioned this idea to Jim, and he told me that it's not a simple thing to
> disarm uretprobes.  

(Sacrificing more performance then, "temporarily remove" instead of "disarm".)

> Then there's the matter of hooking exception dispatch itself.  [...]
> And some point also to know that exception dispatch is finished.

Yes, a few extra uprobes would have to be placed, with awareness of
libstdc++/libgcc internals.
Comment 11 Josh Stone 2010-12-07 04:17:01 UTC
(In reply to comment #10)
> > I mentioned this idea to Jim, and he told me that it's not a simple thing to
> > disarm uretprobes.  
> 
> (Sacrificing more performance then, "temporarily remove" instead of "disarm".)

I clarified this with Frank, but for reference -- we have no such mechanism, no matter what you would call it.  Even when stap completely removes its probes, uprobes must hang on to in-flight uretprobe instances, since there's no easy way to clean up all the return addresses except to let them trigger.
Comment 12 Josh Stone 2010-12-08 21:12:22 UTC
PR10056 may be another solution, probing the actual return path rather than the current trampoline-return approach.
Comment 13 Jim Keniston 2010-12-08 22:07:24 UTC
(In reply to comment #9)
> (In reply to comment #7)
> > For reference, another short-term hack would be to disarm
> > some or all uretprobes while an exception dispatch is in
> > progress.
> 
> I mentioned this idea to Jim, and he told me that it's not a simple thing to
> disarm uretprobes.  The current design doesn't really disarm anything, even
> after unregister_uretprobe -- it just zeros the handler so the next hit does
> nothing.  On x86, we could actually restore the return addresses throughout the
> stack, but on other architectures like powerpc we can't generally predict where
> the fixup is needed.  Perhaps if uprobes had a full unwinder...

Adding Ananth and Srikar to the cc list.  Is ppc64 unwind data sufficient to enable stap to figure out where a function's return address is stored? Seems pretty basic.

...
> 
> Maybe stap can register for these extra probepoints, provided enough debuginfo,
> and also perform the unwinding to figure out how to restore the return
> addresses.  This means "proper" uretprobes behavior would be depending on its
> client (stap), but I'm not sure we want uprobes to grow all this functionality
> itself.

I wouldn't let that stand in your way.  The number of uprobes users who aren't also stap users can't be very big.
Comment 14 Tom Tromey 2011-06-03 18:01:29 UTC
Note that libgcc has an unwinder hook for debuggers nowadays.  GDB
uses this to implement "next-over-throw" in a nice way.

The hook comes in two forms.  See gcc/unwind-dw2.c in the GCC tree.

One form is a function, _Unwind_DebugHook.  This is called with two
arguments, the thrower's CFA, and the PC where the exception will
land.

The other form is an sdt.h-style probe.  It has the same arguments; it
is called as:

  STAP_PROBE2 (libgcc, unwind, cfa, handler);

We added the probe form because it will work without debuginfo being
installed.
Comment 15 Josh Stone 2011-06-06 21:17:10 UTC
(In reply to comment #14)
> Note that libgcc has an unwinder hook for debuggers nowadays.  GDB
> uses this to implement "next-over-throw" in a nice way.
> 
> The hook comes in two forms.  See gcc/unwind-dw2.c in the GCC tree.
> 
> One form is a function, _Unwind_DebugHook.  This is called with two
> arguments, the thrower's CFA, and the PC where the exception will
> land.
> 
> The other form is an sdt.h-style probe.  It has the same arguments; it
> is called as:
> 
>   STAP_PROBE2 (libgcc, unwind, cfa, handler);
> 
> We added the probe form because it will work without debuginfo being
> installed.

Unfortunately, I think that libgcc probe is placed too late for us to react and get out of the way.  In a run without any uretprobe, I see handler=0x400869.  With a uretprobe, I see handler=0x7f5c32ad1dc2, which is an address within libstdc++ (near _ZN9__gnu_cxx27__verbose_@@CXXABI_1.3).  So I think by the time that SDT probe fires, the damage to unwinding is already done.
Comment 16 Josh Stone 2015-06-24 20:30:39 UTC
I just realized the obvious fact that this is really breaking *all* unwinding, not just exceptions.  So print_ubacktrace() will end at the uretprobe trampoline, and _caller_match() used by .callee probes will be disrupted too.

Perhaps those cases could poke around in kernel memory to find the true return address.  The old utrace-based uprobes.ko had uprobe_get_pc(), and runtime/stack.c conditionally uses this, but I think even that only worked for the first unwind level.