Bug 10836 - uprobes-provided pt_regs* are unreliable
Summary: uprobes-provided pt_regs* are unreliable
Status: RESOLVED WONTFIX
Alias: None
Product: systemtap
Classification: Unclassified
Component: uprobes (show other bugs)
Version: unspecified
: P2 normal
Target Milestone: ---
Assignee: Unassigned
URL:
Keywords:
Depends on:
Blocks: blockers-1.1
  Show dependency treegraph
 
Reported: 2009-10-23 16:25 UTC by Frank Ch. Eigler
Modified: 2011-03-16 21:19 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Frank Ch. Eigler 2009-10-23 16:25:29 UTC
Several registers appearing in a utrace-oriented pt_regs do not accurately
represent the state of the user-space task.  As per bug #10601, this breaks
some dwarf-based $context variables.

It seems to me that either uprobes should not pass pt_regs at all to its
callbacks, or else populate it with synthetic (but accurate) values pulled
out of utrace regset calls.  (The latter would be slower and require per-arch
code, but would make it more convenient for the clients.  The former would
be faster, and still require per-arch code at the client, but would be less
convenient.)
Comment 1 Srikar Dronamraju 2009-10-27 11:44:31 UTC
I am not sure if I understand the problem completely.

> Several registers appearing in a utrace-oriented pt_regs do not accurately
> represent the state of the user-space task. 

Is this concern about instruction pointer pointing past the breakpoint?
or do we have additional concerns? If yes do we have specific registers in mind?
                                        
uprobes passes the pt_regs it gets from utrace's report_signal callback as is to
the handler.

> As per bug #10601, this breaks
> some dwarf-based $context variables.

This bug refers to two other bugs which point to problems in user space markers.
 So is this problem only seen on user space markers? or can we see this problem
on plain uprobes probe points too.

> It seems to me that either uprobes should not pass pt_regs at all to its
> callbacks, or else populate it with synthetic (but accurate) values pulled
> out of utrace regset calls.  (The latter would be slower and require per-arch
> code, but would make it more convenient for the clients.  The former would
> be faster, and still require per-arch code at the client, but would be less
> convenient.)

Is there any reason why this synthesis should be done at the uprobes end and not
at the client end?  Do you see all uprobe clients facing this problem?
If its a problem faced by all uprobe clients, then is it worth checking if
utrace should send the synthesized pt_regs as a parameter to report_signal.
Comment 2 Frank Ch. Eigler 2009-10-27 12:27:11 UTC
(In reply to comment #1)
> > Several registers appearing in a utrace-oriented pt_regs do not accurately
> > represent the state of the user-space task. 
> 
> Is this concern about instruction pointer pointing past the breakpoint?

No, not only.

> or do we have additional concerns? If yes do we have specific registers in mind?

It depends.  Sometimes esp, sometimes ebp, and I think I've seen other
registers with inconsistent values too.  Compare a systemtap print_regs
with a gdb breakpoint "info regs" at the same spot.

                                     
> uprobes passes the pt_regs it gets from utrace's report_signal callback as is to
> the handler.

Yes.  Unfortunately, these registers do not completely & correctly match the
state of the user-space thread.


> This bug refers to two other bugs which point to problems in user space markers.
>  So is this problem only seen on user space markers? or can we see this problem
> on plain uprobes probe points too.

In this context, user-space markers are a special case of uprobes.  Statement
uprobes (bypassing prologue heuristics) at the function entry point also
display this problem.


> Is there any reason why this synthesis should be done at the uprobes end and not
> at the client end?

I believe I summarized some pros & cons already.  Hiding the regset
stuff from uprobes clients would be the main benefit.  Perhaps the
run-time costs of doing this could be controlled by a struct-uprobes
flag that tells uprobes whether the client is interested in raw
pt_regs, nothing, or regset-filled pt_regs.

> Do you see all uprobe clients facing this problem?

Yes.

> If its a problem faced by all uprobe clients, then is it worth checking if
> utrace should send the synthesized pt_regs as a parameter to report_signal.

Roland?
Comment 3 Roland McGrath 2009-10-27 18:40:13 UTC
"synthesized pt_regs" is a loony concept considered only by systemtap.
It has no place in the general utrace world.  user_regset is there, it's what
should be used.  The only reason pt_regs is passed into some callbacks is
because the pointer is handy and in some circumstances a few of its fields might
be sufficiently useful for particular code that knows exactly what it is looking
at.  General-case code can use the asm/syscall.h macros on it, for example.  For
any generalized register access, user_regset is the only right thing to use.
Comment 4 Frank Ch. Eigler 2009-10-27 19:16:53 UTC
irc transcript:

mjw So, for syscall things, like getting at the arguments, we can use asm/syscall.h
mjw But that is only enough for syscall related callbacks isn't it?
mjw For anything else, like uprobes, we need to go through linux/regset.h
mjw and some arch specific mapping of the regsets to arch specific register 
values

roland i think instruction_pointer(regs) and user_stack_pointer(regs) can always
be used
roland for other particular things where you know arch-specifically that the reg
is ok you can use it
roland but yes, the general case is user_regset
Comment 5 Mark Wielaard 2009-10-28 12:29:40 UTC
This is also a problem for the dwarf based unwinder.

In runtime/unwind.c we feed unwind_frame() a pt_regs struct describing the
current state of the registers (plus some unwind tables, stp_module, etc.) Then
that uses its internal unwind_state regs stack to keep track of what is going to
happen with the pt_regs given the cfa ops found in the table. And when it is all
processed it spits out a new pt_regs struct describing the state of one unwind
step. In runtime/stack-<arch>.c we are feeding unwind_frame() initially the
pt_regs given by either kprobes or uprobes (arch_unw_init_frame_info(&info,
regs)). But the uprobes pt_regs are the "wrong kind" for user space although the
do work most of the time.

This could be fixed without having to change much by having the "loony concept"
of synthesized pt_regs without having to change too much of this code. Otherwise
we would need to make arch_unw_init_frame_info() smarter and detect we want the
user space registers (which we know in stp_stack_print because tsk != NULL) and
use the appropriate regsets to instantiate the unwind_info->regs.
Comment 6 Jim Keniston 2009-10-28 16:53:13 UTC
(In reply to comment #3)
> "synthesized pt_regs" is a loony concept considered only by systemtap.
> It has no place in the general utrace world.  user_regset is there, it's what
> should be used.  The only reason pt_regs is passed into some callbacks is
> because the pointer is handy and in some circumstances a few of its fields might
> be sufficiently useful for particular code that knows exactly what it is looking
> at.  General-case code can use the asm/syscall.h macros on it, for example.  For
> any generalized register access, user_regset is the only right thing to use.

Previous advice (as I understood it) from Roland during the uprobes 2 port
(~August '08) was to continue using the pt_regs pointer passed to
uprobe_report_signal().  So before Srikar adds a lot of user_regset code to
uprobes, it'd be nice clarify what Roland means by "any generalized register
access."  For example, at least some architectures' user_regset code boils down
to references to the pt_regs pointer provided by task_pt_regs().  For the
registers that SystemTap actually references, would the pointer provided by
task_pt_regs() be sufficient?
Comment 7 Frank Ch. Eigler 2009-10-28 16:56:20 UTC
(In reply to comment #6)

> [...] For the registers that SystemTap actually references, would the pointer 
> provided by task_pt_regs() be sufficient?

For sake of print_regs(), unwinding, or general dwarf-based (eg. fbreg-relative)
$context variable access, it looks like utrace signal pt_regs == task_pt_regs ==
not accurate.  So we do have a pervasive problem.
Comment 8 Frank Ch. Eigler 2009-10-28 16:58:54 UTC
(In reply to comment #6)
> [...] So before Srikar adds a lot of user_regset code to
> uprobes, [...]

By the way, systemtap could do its own user_regset thing
independent of uprobes -- we'd just ignore the incoming pt_regs
almost entirely and add a bunch of code to uprobe/utrace probe
prologues and/or multiple places the runtime.
Comment 9 Roland McGrath 2009-10-28 18:22:34 UTC
uprobes machinery is not "generalized register access".  It is arch-specific
magic that knows plenty of deep magic about machine and kernel arch
implementation details.
Comment 10 Ananth Mavinakayanahalli 2009-10-29 06:20:27 UTC
What are the main registers one needs to know here? Helpers like
regset_instruction_pointer(task), regset_stack_pointer(task) would be a useful
addition with or without uprobes, since they can hide the underlying
architecture details and offsets.
Comment 11 Frank Ch. Eigler 2009-10-29 14:29:11 UTC
(In reply to comment #10)
> What are the main registers one needs to know here?

In the context of uprobes, apprx. all of them, since they may
contain $variable addresses / values.

> Helpers like regset_instruction_pointer(task), regset_stack_pointer(task)
> would be a useful addition with or without uprobes [...]

While I've been thinking about this mainly for uprobes, we get related
complications from utrace & timer.profile type probes, if someone were
to invoke print_ubacktrace().
Comment 12 Frank Ch. Eigler 2009-11-30 20:35:58 UTC
systemtap will learn to ignore uprobes-provided pt_regs for most purposes as a
part of bug #10601.