On 03/07/2019 11:34, Jan Beulich wrote: > 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.
My statement wasn't meant as a criticism, but more of an observation. > >>> (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? A crazy idea I've just had. Annotate all printed lines with a character identifying which source of information we used? We could have [r] for register state, [f] for "from frame pointer", and [s] for "from stack rubble". ~Andrew _______________________________________________ Xen-devel mailing list [email protected] https://lists.xenproject.org/mailman/listinfo/xen-devel
