Skip to content

8373110: jstack --mixed frames are broken on macOS after JDK-8371194 #28686

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
YaSuenag wants to merge 4 commits into from
+94 −29
Closed

8373110: jstack --mixed frames are broken on macOS after JDK-8371194 #28686

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6268176
8373110: jstack --mixed frames are broken on macOS after JDK-8371194
YaSuenag Dec 6, 2025
bb892eb
Override 2 "sender" method in Bsd CFrame classes
YaSuenag Dec 6, 2025
f5eeeaa
Update src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/debugger/b…
YaSuenag Dec 8, 2025
a42f8f8
Check RBP
YaSuenag Dec 9, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/debugger/bsd/BsdCDebugger.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2003, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2003, 2025, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2021, Azul Systems, Inc. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
Expand Down Expand Up @@ -86,18 +86,22 @@ public CFrame topFrameForThread(ThreadProxy thread) throws DebuggerException {
String cpu = dbg.getCPU();
if (cpu.equals("amd64") || cpu.equals("x86_64")) {
AMD64ThreadContext context = (AMD64ThreadContext) thread.getContext();
Address rsp = context.getRegisterAsAddress(AMD64ThreadContext.RSP);
if (rsp == null) return null;
Address rbp = context.getRegisterAsAddress(AMD64ThreadContext.RBP);
if (rbp == null) return null;
Address pc = context.getRegisterAsAddress(AMD64ThreadContext.RIP);
if (pc == null) return null;
return new BsdAMD64CFrame(dbg, rbp, pc);
return new BsdAMD64CFrame(dbg, rsp, rbp, pc);
} else if (cpu.equals("aarch64")) {
AARCH64ThreadContext context = (AARCH64ThreadContext) thread.getContext();
Address sp = context.getRegisterAsAddress(AARCH64ThreadContext.SP);
if (sp == null) return null;
Address fp = context.getRegisterAsAddress(AARCH64ThreadContext.FP);
if (fp == null) return null;
Address pc = context.getRegisterAsAddress(AARCH64ThreadContext.PC);
if (pc == null) return null;
return new BsdAARCH64CFrame(dbg, fp, pc);
return new BsdAARCH64CFrame(dbg, sp, fp, pc);
} else {
throw new DebuggerException(cpu + " is not yet supported");
}
Expand Down
67 changes: 54 additions & 13 deletions ...dk.hotspot.agent/share/classes/sun/jvm/hotspot/debugger/bsd/aarch64/BsdAARCH64CFrame.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2003, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2003, 2025, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2015, Red Hat Inc.
* Copyright (c) 2021, Azul Systems, Inc. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
Expand All @@ -26,15 +26,19 @@

package sun.jvm.hotspot.debugger.bsd.aarch64;

import sun.jvm.hotspot.code.*;
import sun.jvm.hotspot.debugger.*;
import sun.jvm.hotspot.debugger.aarch64.*;
import sun.jvm.hotspot.debugger.bsd.*;
import sun.jvm.hotspot.debugger.cdbg.*;
import sun.jvm.hotspot.debugger.cdbg.basic.*;
import sun.jvm.hotspot.runtime.*;
import sun.jvm.hotspot.runtime.aarch64.*;

public final class BsdAARCH64CFrame extends BasicCFrame {
public BsdAARCH64CFrame(BsdDebugger dbg, Address fp, Address pc) {
public BsdAARCH64CFrame(BsdDebugger dbg, Address sp, Address fp, Address pc) {
super(dbg.getCDebugger());
this.sp = sp;
this.fp = fp;
this.pc = pc;
this.dbg = dbg;
Expand All @@ -54,28 +58,65 @@ public Address localVariableBase() {
return fp;
}

@Override
public CFrame sender(ThreadProxy thread) {
AARCH64ThreadContext context = (AARCH64ThreadContext) thread.getContext();
Address rsp = context.getRegisterAsAddress(AARCH64ThreadContext.SP);
return sender(thread, null, null, null);
}

if ((fp == null) || fp.lessThan(rsp)) {
return null;
@Override
public CFrame sender(ThreadProxy thread, Address nextSP, Address nextFP, Address nextPC) {
Copy link
Contributor

@plummercj plummercj Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it that for aarch64 these are all "next" arguments, but for AMD64 they are not?

Copy link
Member Author

@YaSuenag YaSuenag Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to declare like sender(ThreadProxy th, Address sp, Address fp, Address pc) as defined in CFrame, however name of parameters (sp, fp, pc) have already declared as class members in BsdAARCH64CFrame. Thus I added next as a prefix of parameter name.

Copy link
Contributor

@plummercj plummercj Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. Looks like that was also the case with LunuxAARCH64CFrame.

// Check fp
// Skip if both nextFP and nextPC are given - do not need to load from fp.
if (nextFP == null && nextPC == null) {
if (fp == null) {
return null;
}

// Check alignment of fp
if (dbg.getAddressValue(fp) % (2 * ADDRESS_SIZE) != 0) {
return null;
}
}

// Check alignment of fp
if (dbg.getAddressValue(fp) % (2 * ADDRESS_SIZE) != 0) {
if (nextFP == null) {
nextFP = fp.getAddressAt(0);
}
if (nextFP == null) {
return null;
}

Address nextFP = fp.getAddressAt(0 * ADDRESS_SIZE);
if (nextFP == null || nextFP.lessThanOrEqual(fp)) {
return null;
if (nextPC == null) {
nextPC = fp.getAddressAt(ADDRESS_SIZE);
}
Address nextPC = fp.getAddressAt(1 * ADDRESS_SIZE);
if (nextPC == null) {
return null;
}
return new BsdAARCH64CFrame(dbg, nextFP, nextPC);

if (nextSP == null) {
CodeCache cc = VM.getVM().getCodeCache();
CodeBlob currentBlob = cc.findBlobUnsafe(pc());

// This case is different from HotSpot. See JDK-8371194 for details.
Copy link
Contributor

@kevinjwalls kevinjwalls Dec 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall I think.
What does "different from HotSpot" mean?
I see we used that phrase in 8371194 also in LinuxAARCH64CFrame.java
Maybe it is saying that these codeBlobs are handled differently from other code?

Copy link
Member Author

@YaSuenag YaSuenag Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. We were trying to implement similar code with NativeStackPrinter used in generating hs_err log in HotSpot, but this condition is different with it.

Comments in 8371195 says:

The special case for currentBlob.isContinuationStub() || currentBlob.isNativeMethod()) is needed compared to os::get_sender_for_C_frame because, unlike in the VM, here we are walking those frames as native C frames. Without the special case we depend on nextFP which might not be valid unless -XX:+PreserveFramePointer is used.

In os_bsd_aarch64.cpp, comment of get_sender_for_C_frame() says JVM compiled with -fno-omit-frame-pointer, but we can override CFLAGS with configure options. Thus I think it is better to align with Linux AArch64. Actually I heard this code works fine on Mac AArch64 - mixed jstack unwinded native methods successfully like java.lang.ref.Reference.waitForReferencePendingList().

Copy link
Contributor

@kevinjwalls kevinjwalls Dec 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, so this is different to the HotSpot NativeStackPrinter and the frame::next_frame() method.
As we already have the comment in a similar method maybe no need to change it here.
Thanks.

if (currentBlob != null && (currentBlob.isContinuationStub() || currentBlob.isNativeMethod())) {
// Use FP since it should always be valid for these cases.
// TODO: These should be walked as Frames not CFrames.
nextSP = fp.addOffsetTo(2 * ADDRESS_SIZE);
} else {
CodeBlob codeBlob = cc.findBlobUnsafe(nextPC);
boolean useCodeBlob = codeBlob != null && codeBlob.getFrameSize() > 0;
nextSP = useCodeBlob ? nextFP.addOffsetTo((2 * ADDRESS_SIZE) - codeBlob.getFrameSize()) : nextFP;
}
}
if (nextSP == null) {
return null;
}

return new BsdAARCH64CFrame(dbg, nextSP, nextFP, nextPC);
}

@Override
public Frame toFrame() {
return new AARCH64Frame(sp, fp, pc);
}

// package/class internals only
Expand Down
46 changes: 33 additions & 13 deletions src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/debugger/bsd/amd64/BsdAMD64CFrame.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2003, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2003, 2025, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -29,10 +29,13 @@
import sun.jvm.hotspot.debugger.bsd.*;
import sun.jvm.hotspot.debugger.cdbg.*;
import sun.jvm.hotspot.debugger.cdbg.basic.*;
import sun.jvm.hotspot.runtime.*;
import sun.jvm.hotspot.runtime.amd64.*;

public final class BsdAMD64CFrame extends BasicCFrame {
public BsdAMD64CFrame(BsdDebugger dbg, Address rbp, Address rip) {
public BsdAMD64CFrame(BsdDebugger dbg, Address rsp, Address rbp, Address rip) {
super(dbg.getCDebugger());
this.rsp = rsp;
this.rbp = rbp;
this.rip = rip;
this.dbg = dbg;
Expand All @@ -52,32 +55,49 @@ public Address localVariableBase() {
return rbp;
}

@Override
public CFrame sender(ThreadProxy thread) {
AMD64ThreadContext context = (AMD64ThreadContext) thread.getContext();
Address rsp = context.getRegisterAsAddress(AMD64ThreadContext.RSP);
return sender(thread, null, null, null);
}

if ( (rbp == null) || rbp.lessThan(rsp) ) {
return null;
@Override
public CFrame sender(ThreadProxy thread, Address sp, Address fp, Address pc) {
// Check fp
// Skip if both fp and pc are given - do not need to load from rbp.
if (fp == null && pc == null) {
if (rbp == null) {
return null;
}

// Check alignment of rbp
if (dbg.getAddressValue(rbp) % ADDRESS_SIZE != 0) {
return null;
}
}

// Check alignment of rbp
if (dbg.getAddressValue(rbp) % ADDRESS_SIZE != 0) {
Address nextRSP = sp != null ? sp : rbp.addOffsetTo(2 * ADDRESS_SIZE);
if (nextRSP == null) {
return null;
}

Address nextRBP = rbp.getAddressAt( 0 * ADDRESS_SIZE);
if (nextRBP == null || nextRBP.lessThanOrEqual(rbp)) {
Address nextRBP = fp != null ? fp : rbp.getAddressAt(0);
if (nextRBP == null) {
return null;
}
Address nextPC = rbp.getAddressAt( 1 * ADDRESS_SIZE);
Address nextPC = pc != null ? pc : rbp.getAddressAt(ADDRESS_SIZE);
if (nextPC == null) {
return null;
}
return new BsdAMD64CFrame(dbg, nextRBP, nextPC);
return new BsdAMD64CFrame(dbg, nextRSP, nextRBP, nextPC);
}

@Override
public Frame toFrame() {
return new AMD64Frame(rsp, rbp, rip);
}

// package/class internals only
private static final int ADDRESS_SIZE = 8;
private Address rsp;
private Address rip;
private Address rbp;
private BsdDebugger dbg;
Expand Down