This is one of the sub-tasks of bug 1426134, scoped to just aarch64.

It appears that our aarch64 JIT code uses the fp register as a general-purpose register. This makes the profiler unable to unwind past these JIT frames on the stack, because the profiler expects fp values to be usable frame pointers.

Apple's arm64 guidelines request that the fp register always point to a valid frame record. This doesn't necessarily mean that every function needs to create a frame record (and point fp at it), it just means that fp should not be overwritten with anything other than a frame pointer. For example, leaf functions may opt to just ignore the fp register entirely.
JS JIT code in Firefox macOS arm64 builds currently does not conform to this guideline.

The default clang configuration on macOS arm64 preserves frame pointers, and creates frame records for every non-leaf function.

One way to test would be to recompile Firefox by adding x29 / fp in the mask of NonAllocatableMask.

Doing so will prevent all JIT backends to use the frame pointer register and it will basically skip any JIT frame unless used explicitly.

Thanks! I'll give that a try.

On its own, this does not seem to be sufficient. Here's a profile with that change applied: https://share.firefox.dev/3uKZQCJ

The experimental patch in bug 1426134 (attachment 8957345 [details] [diff] [review]) also removes lots of calls to "regs.take(BaselineFrameReg)" so I wonder if this is related to that. But I really don't know what I'm doing here.

(In reply to Markus Stange [:mstange] from comment #3)

Here's a profile with that change applied: https://share.firefox.dev/3uKZQCJ

That sample was in Ion. Here's a sample in Baseline: https://share.firefox.dev/3HWIsig

(In reply to Markus Stange [:mstange] from comment #0)

This is one of the sub-tasks of bug 1426134, scoped to just aarch64.

It appears that our aarch64 JIT code uses the fp register as a general-purpose register. This makes the profiler unable to unwind past these JIT frames on the stack, because the profiler expects fp values to be usable frame pointers.

Apple's arm64 guidelines request that the fp register always point to a valid frame record. This doesn't necessarily mean that every function needs to create a frame record (and point fp at it), it just means that fp should not be overwritten with anything other than a frame pointer. For example, leaf functions may opt to just ignore the fp register entirely.
JS JIT code in Firefox macOS arm64 builds currently does not conform to this guideline.

The default clang configuration on macOS arm64 preserves frame pointers, and creates frame records for every non-leaf function.

Do you happen to know where the definition of a "valid frame record" is documented? I've been looking at frame pointers a little bit recently, both in the context of the interaction between frame pointers and the profiler (see the comments on this patch) and in conversations with Denis from the performance team trying to get perf to work on jitcode. As far as I can tell, when we are profiling, the frame pointer is almost always pointing to something that SM thinks is a frame. However:

a) those frames may use our own custom layout: for example, this is the layout for the frame that we push when making a call from baseline code; you can see that it has an IC stub pointer between the frame pointer and the return address.
b) we don't update fp at all in Ion; we just leave it pointing at the most recent non-Ion frame, which I think will normally be a baseline stub frame or a jit entry frame. If you just follow fp, you will skip past any Ion frames entirely.
c) there are a handful of places where I think it's possible for us to clobber the frame pointer in Ion; I'm not sure how often it happens in practice. AFAICT those should be relatively easy to fix, although maybe not always easy to find.

(In reply to Iain Ireland [:iain] from comment #5)

Do you happen to know where the definition of a "valid frame record" is documented?

I couldn't find a definition either, but my guess is that a "valid frame record" means a pair of (caller frame pointer, return address). Maybe with the additional constraint that this pair is placed at the outer end of the stack frame, such that the parent stack frame starts just after the return address.

a) those frames may use our own custom layout: for example, this is the layout for the frame that we push when making a call from baseline code; you can see that it has an IC stub pointer between the frame pointer and the return address.

I see, so that would make it not a valid frame record, because the return address is not *(fp + 8).

c) there are a handful of places where I think it's possible for us to clobber the frame pointer in Ion; I'm not sure how often it happens in practice. AFAICT those should be relatively easy to fix, although maybe not always easy to find.

I see, that sounds promising. They're pretty easy to find with the profiler: Just look for the broken stacks which are missing C++ frames close to the root. Comment 3 and comment 4 have two examples: Those stacks have C++ frames only at the very deepest level, and the rest of the stack is made of frame labels and JS frames.

(In reply to Markus Stange [:mstange] from comment #3)

The experimental patch in bug 1426134 (attachment 8957345 [details] [diff] [review]) also removes lots of calls to "regs.take(BaselineFrameReg)" so I wonder if this is related to that. But I really don't know what I'm doing here.

The regs.take(BaselineFrameReg) should stay for ARM64, because BaselineFrameReg isn't the frame pointer on ARM64. The old patch removed the regs.take(BaselineFrameReg) calls, because on ARM32, x86, and x64, BaselineFrameReg is equal to the frame pointer.

Here's an updated patch series which can be applied to current central: https://treeherder.mozilla.org/jobs?repo=try&revision=a696ae0abb4ecd85e1e49b2b5dc787a33cac42af

Bug 1426134 landed and got us most of the way there.

Bug 1770365 will change JS Baseline Interpreter/JIT frames on ARM64 to use the actual FP register instead of x23. Bug 1770366 will add a proper frame pointer to Warp frames.

(In reply to Iain Ireland [:iain] from comment #5)

Do you happen to know where the definition of a "valid frame record" is documented?

"frame record" is described here. (From https://github.com/ARM-software/abi-aa/releases)