Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1280616 > unrolled thread

Re: [BUG REPORT] perf tools: x86_64: Broken calllchain when sampling taken at 'callq' instruction

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [BUG REPORT] perf tools: x86_64: Broken calllchain when sampling  taken at 'callq' instruction Ingo Molnar <mingo@kernel.org> - 2015-12-01 08:30 +0100
    Re: [BUG REPORT] perf tools: x86_64: Broken calllchain when sampling  taken at 'callq' instruction Peter Zijlstra <peterz@infradead.org> - 2015-12-01 09:40 +0100
      Re: [BUG REPORT] perf tools: x86_64: Broken calllchain when sampling  taken at 'callq' instruction Ingo Molnar <mingo@kernel.org> - 2015-12-01 17:20 +0100
        Re: [BUG REPORT] perf tools: x86_64: Broken calllchain when sampling  taken at 'callq' instruction Peter Zijlstra <peterz@infradead.org> - 2015-12-01 18:30 +0100
          Re: [BUG REPORT] perf tools: x86_64: Broken calllchain when sampling  taken at 'callq' instruction Ingo Molnar <mingo@kernel.org> - 2015-12-02 11:00 +0100

#1280616 — Re: [BUG REPORT] perf tools: x86_64: Broken calllchain when sampling taken at 'callq' instruction

* Peter Zijlstra <peterz@infradead.org> wrote:

> On Fri, Nov 27, 2015 at 09:38:11AM +0100, Ingo Molnar wrote:
> > 
> > * Peter Zijlstra <peterz@infradead.org> wrote:
> > 
> > > On Thu, Nov 19, 2015 at 11:23:00AM +0100, Ingo Molnar wrote:
> > > > PEBS is an asynchronous hardware tracing mechanism, when batched PEBS is used it 
> > > > might not even result in any interruption of execution. The 'pt_regs' does not 
> > > > necessarily correspond to an interrupted, restartable context - we take the RIP 
> > > > from the PEBS machinery and also use LBR and disassembly to determine the previous 
> > > > instruction, before reporting it to user-space.
> > > 
> > > Note that modern PEBS hardware (hsw+) does the rollback in hardware.
> > > Prior to that we indeed to it manually using the LBR.
> > > 
> > > As to pt_regs, we construct a franken pt_regs based on the actual PEBS
> > > buffer overflow PMI and bits from the PEBS record (which also includes
> > > some register state). See
> > > arch/x86/kernel/cpu/perf_event_intel_ds.c:setup_pebs_sample_data().
> > > 
> > > We always copy the flags, ip, bp and sp from the PEBS record into the
> > > interrupt pt_regs.
> > > 
> > > And note that the PEBS record is constructed at instruction retirement,
> > > so it shows the state _after_ the instruction, with exception of the
> > > (hsw+) real_ip field.
> > > 
> > > So the unwinder will have to be taught that if the IP points at a stack
> > > altering instruction (call, push, etc.) it will have to 'undo' the
> > > effects on the actual stack (I appreciate this might be 'interesting'
> > > for things like: pop, ret, etc.).
> > 
> > So do we dump both the 'real' and the actual RIP, to not force tooling into having 
> > to decode instructions and such?
> 
> Nope, we only expose the corrected one.
> 
> > (Which is pretty hard and fragile and not always 
> > possible with instructions that destroy the original RIP, like JMP, etc.)
> 
> Not sure what you're getting at here. We don't need the uncorrected
> instruction.

Well, we need it for stack unwinding, as you point it out:

> But the problem here is that we rewind the instruction stream, but not
> the stack. And the stack unwinder is (obviously) interested in the stack
> state.

Unwinding the stack state would fix it as well - but an equivalent solution would 
be to pass along the original RIP would fix it as well: we'd have a 
self-consistent pair of RIP/RSP.

Especially since unwinding the RSP is probably hard:

> I'm not sure we want (or need) to go undo the specific instruction's
> stack effect in-kernel. If the !DWARF unwinders are similarly confused
> we might need to put it in kernel (expensive *groan*). If its only the
> DWARF muck then its something that can be done in userspace just
> fine, although we might need to copy slightly more of the stack than SP
> is pointing at, such that we can undo RET/POP etc. which would have data
> beyond the head of stack.
> 
> The easiest solution might be to figure out the biggest stack offset for
> any instruction and always capture that much over the head of stack.

so I think the problem here is that the RSP does not match up to the RIP. We can 
either pass along the original RIP+RSP, or the fixed up one - but what we do 
currently is that we pass along only half of it - which corrupts dwarf unwinding 
state that doesn't tolerate such errors.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1280641

On Tue, Dec 01, 2015 at 08:28:26AM +0100, Ingo Molnar wrote:
> > Not sure what you're getting at here. We don't need the uncorrected
> > instruction.
> 
> Well, we need it for stack unwinding, as you point it out:
> 
> > But the problem here is that we rewind the instruction stream, but not
> > the stack. And the stack unwinder is (obviously) interested in the stack
> > state.
> 
> Unwinding the stack state would fix it as well - but an equivalent solution would 
> be to pass along the original RIP would fix it as well: we'd have a 
> self-consistent pair of RIP/RSP.
> 
> Especially since unwinding the RSP is probably hard:
> 
> > I'm not sure we want (or need) to go undo the specific instruction's
> > stack effect in-kernel. If the !DWARF unwinders are similarly confused
> > we might need to put it in kernel (expensive *groan*). If its only the
> > DWARF muck then its something that can be done in userspace just
> > fine, although we might need to copy slightly more of the stack than SP
> > is pointing at, such that we can undo RET/POP etc. which would have data
> > beyond the head of stack.
> > 
> > The easiest solution might be to figure out the biggest stack offset for
> > any instruction and always capture that much over the head of stack.
> 
> so I think the problem here is that the RSP does not match up to the RIP. We can 
> either pass along the original RIP+RSP, or the fixed up one - but what we do 
> currently is that we pass along only half of it - which corrupts dwarf unwinding 
> state that doesn't tolerate such errors.

Still not sure what that gets you. Then you get a sample at a known
wrong location, why would you want that?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1281007

* Peter Zijlstra <peterz@infradead.org> wrote:

> > so I think the problem here is that the RSP does not match up to the RIP. We 
> > can either pass along the original RIP+RSP, or the fixed up one - but what we 
> > do currently is that we pass along only half of it - which corrupts dwarf 
> > unwinding state that doesn't tolerate such errors.
> 
> Still not sure what that gets you. Then you get a sample at a known wrong 
> location, why would you want that?

Well, we'd at least get a valid call trace - which the 'mixed' one isn't? I.e. 
this only matters with --call-graph.

But yeah, with my suggestion we'd essentially fall back from cycles:pp to 
cycles:p, ideally we'd want to have real_rsp. Does the hardware provide that?

User-space cannot compute that reliably I think, what if the 'real' instruction 
was manipulating RSP in more complex ways than doing a CALL?

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1281072

On Tue, Dec 01, 2015 at 05:11:25PM +0100, Ingo Molnar wrote:
> 
> * Peter Zijlstra <peterz@infradead.org> wrote:
> 
> > > so I think the problem here is that the RSP does not match up to the RIP. We 
> > > can either pass along the original RIP+RSP, or the fixed up one - but what we 
> > > do currently is that we pass along only half of it - which corrupts dwarf 
> > > unwinding state that doesn't tolerate such errors.
> > 
> > Still not sure what that gets you. Then you get a sample at a known wrong 
> > location, why would you want that?
> 
> Well, we'd at least get a valid call trace - which the 'mixed' one isn't? I.e. 
> this only matters with --call-graph.
> 
> But yeah, with my suggestion we'd essentially fall back from cycles:pp to 
> cycles:p, ideally we'd want to have real_rsp. Does the hardware provide that?

No, no real_rsp.

> User-space cannot compute that reliably I think, what if the 'real' instruction 
> was manipulating RSP in more complex ways than doing a CALL?

I'm not really too aware of these asm details :/ 

Jiri, what is in PERF_SAMPLE_REGS_USER.IP ? from a quick reading that is
whatever is returned from task_pt_regs(current), not the
perf_data.regs.ip field which contains the corrected IP.

Should the uwinder then not use PERF_SAMPLE_REGS_USER.{IP,SP} for a
consistent unwind?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1281568

* Peter Zijlstra <peterz@infradead.org> wrote:

> On Tue, Dec 01, 2015 at 05:11:25PM +0100, Ingo Molnar wrote:
> > 
> > * Peter Zijlstra <peterz@infradead.org> wrote:
> > 
> > > > so I think the problem here is that the RSP does not match up to the RIP. We 
> > > > can either pass along the original RIP+RSP, or the fixed up one - but what we 
> > > > do currently is that we pass along only half of it - which corrupts dwarf 
> > > > unwinding state that doesn't tolerate such errors.
> > > 
> > > Still not sure what that gets you. Then you get a sample at a known wrong 
> > > location, why would you want that?
> > 
> > Well, we'd at least get a valid call trace - which the 'mixed' one isn't? I.e. 
> > this only matters with --call-graph.
> > 
> > But yeah, with my suggestion we'd essentially fall back from cycles:pp to 
> > cycles:p, ideally we'd want to have real_rsp. Does the hardware provide that?
> 
> No, no real_rsp.
> 
> > User-space cannot compute that reliably I think, what if the 'real' instruction 
> > was manipulating RSP in more complex ways than doing a CALL?
> 
> I'm not really too aware of these asm details :/ 

So the way I understand it is that dwarf unwind relies on having consistent pairs 
of {RIP,RSP}, to be able to look at the precise instruction, look up the debuginfo 
what the function-internal stack offset at that instruction is (as there's no RBP 
frame information available so the stack offset varies).

That method really relies on having precise {RIP,RSP} pairs - it may otherwise 
lose its notion of where the next return address on the stack is, and the 
backtrace goes off into la-la-land. I suppose such broken callchains were how 
Wangnan noticed the problem?

( As a comparison, RBP based backtraces only rely on having an exact RBP available 
  as a starting point, that will define the next backtrace link and the next RBP
  value. That is why we can do the backtrace walk in the kernel. )

There are other ways a dwarf unwind can fail: for example if there's no dwarf 
debuginfo available for the binary in question, or if the stack snapshot we did is 
too small to find the next link in the backtrace. But having an off-by-few RIP is 
enough to throw the backtrace off at the very first step.

> Jiri, what is in PERF_SAMPLE_REGS_USER.IP ? from a quick reading that is 
> whatever is returned from task_pt_regs(current), not the perf_data.regs.ip field 
> which contains the corrected IP.
> 
> Should the uwinder then not use PERF_SAMPLE_REGS_USER.{IP,SP} for a consistent 
> unwind?

Yeah, if we have the real RIP belonging to that RSP value, then all should be 
golden. That way the unwinder can find the next link. (If all the other conditions 
for a successful backtrace are also there.)

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web