Skip to content

Commit d57e88a

Browse files
committed
[GR-66421] Wrongly merged polyglot isolate stack traces.
PullRequest: graal/21211
2 parents 9ab5967 + b245aa9 commit d57e88a

File tree

8 files changed

+290
-47
lines changed

8 files changed

+290
-47
lines changed

sdk/src/org.graalvm.jniutils/src/org/graalvm/jniutils/JNIExceptionWrapper.java

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ public static StackTraceElement[] mergeStackTraces(
322322
return hotSpotStackTrace;
323323
}
324324
} else {
325-
if (containsJNIHostCall(nativeStackTrace)) {
325+
if (containsHostToIsolateTransition(nativeStackTrace, hotSpotStackTrace)) {
326326
// Already merged
327327
return nativeStackTrace;
328328
}
@@ -363,7 +363,8 @@ private static StackTraceElement[] mergeStackTraces(
363363
} else {
364364
useHotSpotStack = true;
365365
}
366-
while (nativeStackIndex < nativeStackTrace.length && (startingnativeFrame || !isJNIHostCall(nativeStackTrace[nativeStackIndex]))) {
366+
while (nativeStackIndex < nativeStackTrace.length &&
367+
(startingnativeFrame || !(isJNIAPICall(nativeStackTrace[nativeStackIndex]) && isJNIHostCall(nativeStackTrace[nativeStackIndex + 1])))) {
367368
startingnativeFrame = false;
368369
merged[targetIndex++] = nativeStackTrace[nativeStackIndex++];
369370
}
@@ -410,6 +411,28 @@ private static boolean containsJNIHostCall(StackTraceElement[] stackTrace) {
410411
return false;
411412
}
412413

414+
/**
415+
* Determines if {@code nativeStackTrace} is already merged, contains an to isolate entry point
416+
* from {@code hotSpotStackTrace}.
417+
*/
418+
private static boolean containsHostToIsolateTransition(StackTraceElement[] nativeStackTrace, StackTraceElement[] hotSpotStackTrace) {
419+
StackTraceElement transition = null;
420+
for (StackTraceElement element : hotSpotStackTrace) {
421+
if (element.isNativeMethod()) {
422+
transition = element;
423+
break;
424+
}
425+
}
426+
if (transition != null) {
427+
for (StackTraceElement element : nativeStackTrace) {
428+
if (transition.getClassName().equals(element.getClassName()) && transition.getMethodName().equals(element.getMethodName())) {
429+
return true;
430+
}
431+
}
432+
}
433+
return false;
434+
}
435+
413436
private static JThrowable updateStackTrace(JNIEnv env, JThrowable throwableHandle, StackTraceElement[] mergedStackTrace) {
414437
ByteArrayOutputStream bout = new ByteArrayOutputStream();
415438
try (DataOutputStream out = new DataOutputStream(bout)) {
@@ -592,6 +615,14 @@ private static JNI.JClass getEntryPoints(JNIEnv env) {
592615
return res;
593616
}
594617

618+
/**
619+
* Determines if {@code frame} is for a method denoting a JNI API call using
620+
* {@code CFunctionPointer}.
621+
*/
622+
private static boolean isJNIAPICall(StackTraceElement frame) {
623+
return frame.getClassName().startsWith(JNI_FUNCTION_POINTER_CLASS_PREFIX);
624+
}
625+
595626
/**
596627
* Determines if {@code frame} is for a method denoting a call into HotSpot.
597628
*/
@@ -604,6 +635,7 @@ private static boolean isJNIHostCall(StackTraceElement frame) {
604635
*/
605636
private static final Set<String> JNI_TRANSITION_METHODS;
606637
private static final String JNI_TRANSITION_CLASS;
638+
private static final String JNI_FUNCTION_POINTER_CLASS_PREFIX;
607639
static {
608640
Map<String, Method> entryPoints = new HashMap<>();
609641
Map<String, Method> others = new HashMap<>();
@@ -627,5 +659,6 @@ private static boolean isJNIHostCall(StackTraceElement frame) {
627659
}
628660
JNI_TRANSITION_CLASS = JNICalls.class.getName();
629661
JNI_TRANSITION_METHODS = Set.copyOf(entryPoints.keySet());
662+
JNI_FUNCTION_POINTER_CLASS_PREFIX = JNI.class.getName() + '$';
630663
}
631664
}

sdk/src/org.graalvm.nativebridge.processor/src/org/graalvm/nativebridge/processor/NativeToHotSpotServiceGenerator.java

Lines changed: 35 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -386,9 +386,12 @@ private void generateNativeToHSStartMethod(CodeBuilder builder, CacheSnippet cac
386386
TypeMirror returnType = methodData.type.getReturnType();
387387
boolean voidMethod = returnType.getKind() == TypeKind.VOID;
388388
MarshallerSnippet resultMarshallerSnippets = marshallerSnippets(resultMarshallerData);
389-
CharSequence isolateVar = "hsIsolate";
389+
CharSequence isolateVar;
390390
if (resultMarshallerData.isCustom() || firstOutMarshalledParameter != null) {
391-
builder.lineStart().write(typeCache.hsIsolate).space().write(isolateVar).write(" = ").invoke(PEER_FIELD, "getIsolate").lineEnd(";");
391+
isolateVar = "hsIsolate";
392+
builder.lineStart().write(typeCache.hsIsolate).space().write(isolateVar).write(" = ").write(createGetIsolate(builder, receiver)).lineEnd(";");
393+
} else {
394+
isolateVar = null;
392395
}
393396
if (cacheData != null) {
394397
if (hasOutParameters) {
@@ -440,7 +443,7 @@ private void generateNativeToHSStartMethod(CodeBuilder builder, CacheSnippet cac
440443
CharSequence foreignException = "foreignException";
441444
builder.lineStart("} catch (").write(typeCache.foreignException).space().write(foreignException).lineEnd(") {");
442445
builder.indent();
443-
builder.lineStart("throw ").invoke(foreignException, "throwOriginalException", "null", getDefinition().getCustomMarshaller(typeCache.throwable, null, types).name).lineEnd(";");
446+
generateThrowOriginalException(builder, receiver, foreignException, isolateVar);
444447
builder.dedent();
445448
builder.line("}");
446449
builder.dedent();
@@ -514,14 +517,41 @@ private void generateNativeToHSStartMethod(CodeBuilder builder, CacheSnippet cac
514517
CharSequence foreignException = "foreignException";
515518
builder.lineStart("} catch (").write(typeCache.foreignException).space().write(foreignException).lineEnd(") {");
516519
builder.indent();
517-
builder.lineStart("throw ").invoke(foreignException, "throwOriginalException", "null", getDefinition().getCustomMarshaller(typeCache.throwable, null, types).name).lineEnd(";");
520+
generateThrowOriginalException(builder, receiver, foreignException, isolateVar);
518521
builder.dedent();
519522
builder.line("}");
520523
}
521524
builder.dedent();
522525
builder.line("}");
523526
}
524527

528+
private void generateThrowOriginalException(CodeBuilder builder, CharSequence receiver, CharSequence foreignException, CharSequence hsIsolate) {
529+
CharSequence isolate;
530+
if (hsIsolate != null) {
531+
isolate = hsIsolate;
532+
} else {
533+
isolate = createGetIsolate(builder, receiver);
534+
}
535+
builder.lineStart("throw ").invoke(foreignException, "throwOriginalException", isolate, getDefinition().getCustomMarshaller(typeCache.throwable, null, types).name).lineEnd(";");
536+
}
537+
538+
private CharSequence createGetIsolate(CodeBuilder builder, CharSequence receiver) {
539+
return new CodeBuilder(builder).invoke(createGetPeer(builder, receiver, false), "getIsolate").build();
540+
}
541+
542+
private CharSequence createGetPeer(CodeBuilder builder, CharSequence receiver, boolean castToHSPeer) {
543+
if (getDefinition().hasCustomDispatch()) {
544+
CharSequence foreignObject = new CodeBuilder(builder).cast(typeCache.foreignObject, receiver, true).build();
545+
CharSequence result = new CodeBuilder(builder).invoke(foreignObject, "getPeer").build();
546+
if (castToHSPeer) {
547+
result = new CodeBuilder(builder).cast(typeCache.hSPeer, result, true).build();
548+
}
549+
return result;
550+
} else {
551+
return receiver;
552+
}
553+
}
554+
525555
private static CharSequence generateStoreEndPointResult(CodeBuilder builder, MarshallerSnippet snippets, TypeMirror returnType, CharSequence nativeCall, CharSequence jniEnvFieldName) {
526556
return snippets.storeRawResult(builder, returnType, nativeCall, jniEnvFieldName);
527557
}
@@ -708,14 +738,7 @@ private CharSequence generatePushArgs(CodeBuilder builder, MethodData methodData
708738
builder.lineStart().write(typeCache.jValue).space().write(jniArgs).write(" = ").invokeStatic(typeCache.stackValue, "get", Integer.toString(argumentCount),
709739
jValueClassLiteral.build()).lineEnd(";");
710740
CharSequence address = new CodeBuilder(builder).invoke(jniArgs, "addressOf", "0").build();
711-
CharSequence receiverHSPeer;
712-
if (hasExplicitReceiver) {
713-
CharSequence foreignObject = new CodeBuilder(builder).cast(typeCache.foreignObject, receiver, true).build();
714-
CharSequence getPeer = new CodeBuilder(builder).invoke(foreignObject, "getPeer").build();
715-
receiverHSPeer = new CodeBuilder(builder).cast(typeCache.hSPeer, getPeer, true).build();
716-
} else {
717-
receiverHSPeer = receiver;
718-
}
741+
CharSequence receiverHSPeer = createGetPeer(builder, receiver, true);
719742
CharSequence value = new CodeBuilder(builder).invoke(receiverHSPeer, "getJObject").build();
720743
builder.lineStart().invoke(address, "setJObject", value).lineEnd(";");
721744
int stackIndex;

sdk/src/org.graalvm.nativebridge/src/org/graalvm/nativebridge/DefaultThrowableMarshaller.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public Throwable read(Isolate<?> isolate, BinaryInput in) {
5050
String foreignExceptionClassName = in.readUTF();
5151
String foreignExceptionMessage = (String) in.readTypedValue();
5252
StackTraceElement[] foreignExceptionStack = stackTraceMarshaller.read(isolate, in);
53-
return new MarshalledException(foreignExceptionClassName, foreignExceptionMessage, ForeignException.mergeStackTrace(foreignExceptionStack));
53+
return new MarshalledException(foreignExceptionClassName, foreignExceptionMessage, ForeignException.mergeStackTrace(isolate, foreignExceptionStack));
5454
}
5555

5656
@Override

sdk/src/org.graalvm.nativebridge/src/org/graalvm/nativebridge/ForeignException.java

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2021, 2024, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 2021, 2025, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* The Universal Permissive License (UPL), Version 1.0
@@ -153,21 +153,28 @@ public RuntimeException throwOriginalException(Isolate<?> isolate, BinaryMarshal
153153
* @param foreignExceptionStack the stack trace marshalled into the {@link ForeignException}
154154
* @return the stack trace combining both local and foreign stack trace elements
155155
*/
156-
public static StackTraceElement[] mergeStackTrace(StackTraceElement[] foreignExceptionStack) {
156+
public static StackTraceElement[] mergeStackTrace(Isolate<?> isolate, StackTraceElement[] foreignExceptionStack) {
157157
if (foreignExceptionStack.length == 0) {
158158
// Exception has no stack trace, nothing to merge.
159159
return foreignExceptionStack;
160160
}
161161
ForeignException localException = pendingException.get();
162162
if (localException != null) {
163-
switch (localException.kind) {
164-
case HOST_TO_GUEST:
165-
return JNIExceptionWrapper.mergeStackTraces(localException.getStackTrace(), foreignExceptionStack, false);
166-
case GUEST_TO_HOST:
167-
return JNIExceptionWrapper.mergeStackTraces(foreignExceptionStack, localException.getStackTrace(), true);
168-
default:
169-
throw new IllegalStateException("Unsupported kind " + localException.kind);
163+
StackMerger merger;
164+
if (isolate instanceof ProcessIsolate) {
165+
merger = ProcessIsolateStack::mergeStackTraces;
166+
} else if (isolate instanceof NativeIsolate) {
167+
merger = JNIExceptionWrapper::mergeStackTraces;
168+
} else if (isolate instanceof HSIsolate) {
169+
merger = JNIExceptionWrapper::mergeStackTraces;
170+
} else {
171+
throw new IllegalArgumentException("Unsupported isolate type: " + isolate);
170172
}
173+
return switch (localException.kind) {
174+
case HOST_TO_GUEST -> merger.merge(localException.getStackTrace(), foreignExceptionStack, false);
175+
case GUEST_TO_HOST -> merger.merge(foreignExceptionStack, localException.getStackTrace(), true);
176+
default -> throw new IllegalStateException("Unsupported kind " + localException.kind);
177+
};
171178
} else {
172179
return foreignExceptionStack;
173180
}
@@ -364,4 +371,9 @@ public void handleException(ExceptionHandlerContext context) {
364371
}
365372
}
366373
}
374+
375+
@FunctionalInterface
376+
private interface StackMerger {
377+
StackTraceElement[] merge(StackTraceElement[] hostStack, StackTraceElement[] isolateStack, boolean originatedInHost);
378+
}
367379
}

sdk/src/org.graalvm.nativebridge/src/org/graalvm/nativebridge/ProcessIsolate.java

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,10 @@ boolean doIsolateShutdown(ProcessIsolateThread shutdownThread) {
270270
return success;
271271
}
272272

273+
boolean isHost() {
274+
return threadSupport.isInitiator();
275+
}
276+
273277
static Collection<? extends ProcessIsolate> getAllProcessIsolates() {
274278
return isolates.values();
275279
}
Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
/*
2+
* Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* The Universal Permissive License (UPL), Version 1.0
6+
*
7+
* Subject to the condition set forth below, permission is hereby granted to any
8+
* person obtaining a copy of this software, associated documentation and/or
9+
* data (collectively the "Software"), free of charge and under any and all
10+
* copyright rights in the Software, and any and all patent rights owned or
11+
* freely licensable by each licensor hereunder covering either (i) the
12+
* unmodified Software as contributed to or provided by such licensor, or (ii)
13+
* the Larger Works (as defined below), to deal in both
14+
*
15+
* (a) the Software, and
16+
*
17+
* (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
18+
* one is included with the Software each a "Larger Work" to which the Software
19+
* is contributed by such licensors),
20+
*
21+
* without restriction, including without limitation the rights to copy, create
22+
* derivative works of, display, perform, and distribute the Software and make,
23+
* use, sell, offer for sale, import, export, have made, and have sold the
24+
* Software and the Larger Work(s), and to sublicense the foregoing rights on
25+
* either these or other terms.
26+
*
27+
* This license is subject to the following condition:
28+
*
29+
* The above copyright notice and either this complete permission notice or at a
30+
* minimum a reference to the UPL must be included in all copies or substantial
31+
* portions of the Software.
32+
*
33+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
34+
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
35+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
36+
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
37+
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
38+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
39+
* SOFTWARE.
40+
*/
41+
package org.graalvm.nativebridge;
42+
43+
/**
44+
* Provides utility methods for merging stack traces for process isolate.
45+
*/
46+
final class ProcessIsolateStack {
47+
48+
private static final String THREAD_CHANNEL_CLASS = ProcessIsolateThreadSupport.ThreadChannel.class.getName();
49+
private static final String DISPATCH_RUNNABLE = ProcessIsolateThreadSupport.DispatchRunnable.class.getName();
50+
private static final String FOREIGN_EXCEPTION_CLASS = ForeignException.class.getName();
51+
private static final String SEND_RECEIVE_METHOD = "sendAndReceive";
52+
private static final String CREATE_METHOD = "create";
53+
private static final String DISPATCH_METHOD = "dispatch";
54+
private static final String RUN_METHOD = "run";
55+
56+
private ProcessIsolateStack() {
57+
}
58+
59+
/**
60+
* Merges {@code host} and {@code isolate} stack traces respecting process isolate transition
61+
* boundaries.
62+
*/
63+
static StackTraceElement[] mergeStackTraces(StackTraceElement[] host, StackTraceElement[] isolate, boolean originatedInHost) {
64+
int hostStackEndIndex;
65+
int isolateStackEndIndex;
66+
if (originatedInHost) {
67+
MirrorThreadInfo mirrorThreadInfo = getMirrorThreadEntryMethodInfo(host);
68+
hostStackEndIndex = mirrorThreadInfo.runnableIndex;
69+
if (hostStackEndIndex == host.length && mirrorThreadInfo.dispatchLoopIndex != -1) {
70+
// Already merged
71+
return host;
72+
}
73+
isolateStackEndIndex = getMirrorThreadEntryMethodInfo(isolate).runnableIndex;
74+
} else {
75+
MirrorThreadInfo mirrorThreadInfo = getMirrorThreadEntryMethodInfo(isolate);
76+
isolateStackEndIndex = mirrorThreadInfo.runnableIndex;
77+
if (isolateStackEndIndex == isolate.length && mirrorThreadInfo.dispatchLoopIndex != -1) {
78+
// Already merged
79+
return isolate;
80+
}
81+
hostStackEndIndex = getMirrorThreadEntryMethodInfo(host).runnableIndex;
82+
}
83+
StackTraceElement[] merged = mergeStackTraces(host, isolate, getTransitionIndex(host), hostStackEndIndex, getTransitionIndex(isolate), isolateStackEndIndex, originatedInHost);
84+
return merged;
85+
}
86+
87+
private static StackTraceElement[] mergeStackTraces(
88+
StackTraceElement[] hostStackTrace,
89+
StackTraceElement[] isolateStackTrace,
90+
int hostStackStartIndex,
91+
int hostStackEndIndex,
92+
int isolateStackStartIndex,
93+
int isolateStackEndIndex,
94+
boolean originatedInHotSpot) {
95+
int targetIndex = 0;
96+
StackTraceElement[] merged = new StackTraceElement[hostStackEndIndex - hostStackStartIndex + isolateStackEndIndex - isolateStackStartIndex];
97+
boolean startingHostFrame = true;
98+
boolean startingIsolateFrame = true;
99+
boolean useHostStack = originatedInHotSpot;
100+
int hostStackIndex = hostStackStartIndex;
101+
int isolateStackIndex = isolateStackStartIndex;
102+
while (hostStackIndex < hostStackEndIndex || isolateStackIndex < isolateStackEndIndex) {
103+
if (useHostStack) {
104+
while (hostStackIndex < hostStackEndIndex && (startingHostFrame || !isBoundary(hostStackTrace[hostStackIndex]))) {
105+
startingHostFrame = false;
106+
merged[targetIndex++] = hostStackTrace[hostStackIndex++];
107+
}
108+
startingHostFrame = true;
109+
} else {
110+
useHostStack = true;
111+
}
112+
while (isolateStackIndex < isolateStackEndIndex && (startingIsolateFrame || !isBoundary(isolateStackTrace[isolateStackIndex]))) {
113+
startingIsolateFrame = false;
114+
merged[targetIndex++] = isolateStackTrace[isolateStackIndex++];
115+
}
116+
startingIsolateFrame = true;
117+
}
118+
return merged;
119+
}
120+
121+
private static boolean isBoundary(StackTraceElement frame) {
122+
return THREAD_CHANNEL_CLASS.equals(frame.getClassName()) && SEND_RECEIVE_METHOD.equals(frame.getMethodName());
123+
}
124+
125+
private static int getTransitionIndex(StackTraceElement[] stack) {
126+
return FOREIGN_EXCEPTION_CLASS.equals(stack[0].getClassName()) && CREATE_METHOD.equals(stack[0].getMethodName()) ? 1 : 0;
127+
}
128+
129+
private static MirrorThreadInfo getMirrorThreadEntryMethodInfo(StackTraceElement[] stack) {
130+
int dispatchIndex = -1;
131+
int index = 0;
132+
for (; index < stack.length; index++) {
133+
StackTraceElement frame = stack[index];
134+
if (THREAD_CHANNEL_CLASS.equals(frame.getClassName()) && DISPATCH_METHOD.equals(frame.getMethodName())) {
135+
dispatchIndex = index;
136+
}
137+
if (DISPATCH_RUNNABLE.equals(frame.getClassName()) && RUN_METHOD.equals(frame.getMethodName())) {
138+
break;
139+
}
140+
}
141+
return new MirrorThreadInfo(index, dispatchIndex);
142+
}
143+
144+
/**
145+
* Holds the positions of key method frames in a mirror thread stack trace.
146+
*
147+
* @param runnableIndex the index of the mirror thread's {@code run()} method; this frame and
148+
* all frames below it are excluded during stack trace merging
149+
* @param dispatchLoopIndex the index of the dispatch loop method; this is the first frame from
150+
* the mirror thread included in a merged stack trace. Its presence may also indicate
151+
* that the stack trace has already been merged.
152+
*/
153+
private record MirrorThreadInfo(int runnableIndex, int dispatchLoopIndex) {
154+
}
155+
}

0 commit comments

Comments
 (0)