Registers::SyncPopulate collects information that becomes incorrect for stack-walking
Categories
(Core :: Gecko Profiler, defect, P2)
Tracking
()
People
(Reporter: mozbugz, Unassigned)
References
Details
(Whiteboard: [fxp])
Found by :glandium while debugging stack walkers for bug 1712674. Trying to summarize here, and think of solutions:
Registers::SyncPopulate
is used twice in the profiler, before calling the appropriate native stack walker.
The issue is that the call may not be inlined, in which case the register information it collects refers to the Registers::SyncPopulate
function call. So when it returns, in particular the stack register(s) (SP, and possibly BP) now refer to the Registers::SyncPopulate
function that just ended, and when calling other functions and eventually the stack walker, that stack area will be modified.
The effect may depend on the platform. For example:
On Windows, it's only used to record the "leaf function" (and it's probably not a good thing either, see bug 1695618), and the native stack walker will start from a newly-acquired correct CONTEXT
anyway.
On Mac, the FramePointerStackWalk
does use these registers. But I don't remember seeing problems around that, I'm guessing because luckily whatever function is called after Registers::SyncPopulate
happens to have the same BP (base pointer, the address where only PC and BP were pushed at the beginning of the function call), so the stack walker can proceed from there.
One solution may be to force the function to be inlined, so that its information will still be valid when calling the native stack walker from under the same function. (A quick attempt with MOZ_ALWAYS_INLINE
didn't make it inline! More work needed...)
Or maybe we could get rid of this dangerous call?
We are considering getting rid of the "leaf" profiler feature anyway (bug 1571089), in which case I think we could remove this function from the profiler code, and maybe move part of it to the stack walker(s) like FramePointerStackWalk
that require a starting point in the current thread.
Comment 1•3 years ago
•
|
||
On Mac, Registers::SyncPopulate
does get the values for the caller (which means it would not work as expected if it were inlined). On Linux x64, it probably doesn't cause a problem because LUL would find the right caller frame based on the unwind info for the Registers::SyncPopulate
function (since the PC would point there and would be correct). On other platforms, I'm not entirely sure of the status (like it might as well be borked on Linux x86, because I think we're using FramePointerStackWalk there instead of LUL ; but maybe it's also saved by the fact that BP is still saved at the right place on the call to the next function after Register::SyncPopulate
, but that's relying on tail-call optimization not happening).
Updated•2 years ago
|
Updated•2 years ago
|
This has been fixed by Bug 1779257.
Description
•