RFR: 8339307: jhsdb jstack could not trace FFM upcall frame [v4]

[Jorn Vernee]

On Mon, 2 Sep 2024 14:50:10 GMT, Yasumasa Suenaga <ysuenaga at openjdk.org> wrote:

> > Note how there is also special handling for (JNI) entry frames in the SA.
> 
> Do you mean `JavaCallWrapper` (`X86Frame::senderForEntryFrame` in SA) ?

Yes. Internally it loads the fields of `JavaFrameAnchor`, which points at the previous Java frame.

> > I'm guessing because we end up walking the native frames until we get back to Java, and the native frames are simply ignored. I'm not sure if that will always work for arbitrary native code though.
> > I think the right fix here is to implement handling for upcall stub frames in the SA agent, since that's also how entry frames are handled. I don't think setting the frame size in hotspot is actually needed if we do that.
> 
> If we add some frame info (return address and FP) like `JavaCallWrapper` to `UpcallStub` and process it in SA, we do not need frame size of `UpcallStub` as you said. But I think it should be fixed in all of upcall implementation. `UpcallStub` is "Stub", so it compliant native calling convention. Thus I believe native frame unwinder like `X86Frame` should always work if frame size is set in `UpcallStub`.

The problem is not the upcall stub frame itself. We know which ABI that is using. The problems is any intermediate frames between the upcall stub frame and last Java frame before that. These intermediate native frames can have any ABI. There is no single 'native calling convention'. We know that we are interfacing with something that follows the C ABI, but that code may switch to a different ABI (e.g. Rust, C++, or some other language) which may have a different stack frame layout. Stack walking those frames might break. The frame anchor used by entry/upcall frames helps to avoid this by letting the stack walker jump over all the native frames, and continue walking at the last java frame before the upcall stub instead. That means it doesn't have to deal with the foreign stack layout of frames in between.

> We need to fix all of upcall implementation in both case, and zero frame size is not nature. In addition adding frame size is simpler than add special handling for `UpcallStub` and SA. Thus I give +1 to add frame size to `UpcallStub`.

I'm not necessarily opposed to adding a frame size to upcall stubs, but as a fix for SA stack walking this seems like a band-aid.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/20789#issuecomment-2324964613