Bugzilla

Found by :glandium while debugging stack walkers. Trying to summarize here, and think of solutions:

`Registers::SyncPopulate` [is used twice in the profiler](https://searchfox.org/mozilla-central/search?q=symbol:_ZN9Registers12SyncPopulateEv), 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 during the function call), so the stack walker can proceed from there.

One solution may be to force the function to be inlined, so that it's 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.

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](https://searchfox.org/mozilla-central/search?q=symbol:_ZN9Registers12SyncPopulateEv), 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 during the function call), so the stack walker can proceed from there.

One solution may be to force the function to be inlined, so that it's 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.

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](https://searchfox.org/mozilla-central/search?q=symbol:_ZN9Registers12SyncPopulateEv), 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 during 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.

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](https://searchfox.org/mozilla-central/search?q=symbol:_ZN9Registers12SyncPopulateEv), 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.