Re: [Xen-devel] [PATCH v2 2/2] x86/traps: widen condition for logging top-of-stack

[Jan Beulich <JBeulich@xxxxxxxx>]

On 03.07.2019 12:21, Andrew Cooper wrote:
> On 17/06/2019 09:13, Jan Beulich wrote:
>> Despite -fno-omit-frame-pointer the compiler may omit the frame pointer,
>> often for relatively simple leaf functions.
> 
> Actually, the problem is more widespread than this.  For every function,
> there is a non-zero quantity of time between the function starting and
> the frame pointer being set up.
> 
> However, half of this time is spent with the old %rbp on the top of the
> stack, so won't benefit from these changes.

I think the compiler typically pairs push %rbp and mov %rsp, %rbp,
but this pair may not sit at the beginning of the function. And it's
that other code that's prone to crash. The push %rbp may also fault
(most notably due to stack overrun), but that would then still have
the top of stack covered by the change here. The mov %rsp, %rbp,
otoh, won't plausibly fault. IOW I think it's far more than "half of
the time" that this change helps.

>> (To give a specific example,
>> the case I've run into this with is _pci_hide_device() and gcc 8.
>> Interestingly the even more simple neighboring iommu_has_feature() does
>> get a frame pointer set up, around just a single instruction. But this
>> may be a result of the size-of-asm() effects discussed elsewhere.)
>>
>> Log the top-of-stack value if it looks valid _or_ if RIP looks invalid.
> 
> This far, I'm happy with.
> 
>> Also annotate non-frame-pointer-based stack trace entries with a
>> question mark, to signal clearly that any one of them may not actually
>> be part of the call stack.
> 
> I'm still opposed to this.  The introduction of ? does more harm than
> good IMO, because it simply can't be trusted.
> 
> Stack traces are not guaranteed-accurate, even with frame pointers
> enabled.  The only thing we can say for certain in any trace is where
> %rip points.

Yes, I realize you still don't like this. But similarly to the
other patch set - on the v1 discussion here I was lacking
feedback, and hence I eventually timed out and sent v2. The
question is - what is your alternative proposal to distinguish
the truly guessed entry logged here from the more reliable
ones? And then similarly how to distinguish the less reliable
ones produced by the !CONFIG_FRAME_POINTER variant of
_show_trace() from their more reliable counterparts?

Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel