Skip to content

Commit f932a26

Browse files
DSouzaMDSouzaM
committed
Fix: Assert that call node used in stack traces is not a BytecodeRootNode.
1 parent bc63fb1 commit f932a26

File tree

6 files changed

+82
-20
lines changed

6 files changed

+82
-20
lines changed

‎truffle/src/com.oracle.truffle.api.bytecode.test/src/com/oracle/truffle/api/bytecode/test/BytecodeDSLTestLanguage.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,15 @@
4040
*/
4141
package com.oracle.truffle.api.bytecode.test;
4242

43+
import org.graalvm.polyglot.Context;
44+
4345
import com.oracle.truffle.api.TruffleLanguage;
4446
import com.oracle.truffle.api.instrumentation.ProvidedTags;
4547
import com.oracle.truffle.api.instrumentation.StandardTags.ExpressionTag;
4648
import com.oracle.truffle.api.instrumentation.StandardTags.RootBodyTag;
4749
import com.oracle.truffle.api.instrumentation.StandardTags.RootTag;
4850
import com.oracle.truffle.api.instrumentation.StandardTags.StatementTag;
51+
import com.oracle.truffle.tck.tests.TruffleTestAssumptions;
4952

5053
/**
5154
* Placeholder language for Bytecode DSL test interpreters.
@@ -60,5 +63,19 @@ protected Object createContext(Env env) {
6063
return new Object();
6164
}
6265

66+
/**
67+
* Ensures compilation is disabled (when supported). This allows tests to validate the behaviour
68+
* of assertions, which are optimized away in compiled code.
69+
*/
70+
public static Context createPolyglotContextWithCompilationDisabled() {
71+
var builder = Context.newBuilder(ID);
72+
if (TruffleTestAssumptions.isOptimizingRuntime()) {
73+
builder.option("engine.Compilation", "false");
74+
}
75+
Context result = builder.build();
76+
result.enter();
77+
return result;
78+
}
79+
6380
public static final LanguageReference<BytecodeDSLTestLanguage> REF = LanguageReference.create(BytecodeDSLTestLanguage.class);
6481
}

‎truffle/src/com.oracle.truffle.api.bytecode.test/src/com/oracle/truffle/api/bytecode/test/StackTraceTest.java‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
import static org.junit.Assert.assertNotNull;
4646
import static org.junit.Assert.assertNull;
4747
import static org.junit.Assert.assertSame;
48+
import static org.junit.Assert.assertThrows;
4849
import static org.junit.Assert.assertTrue;
4950
import static org.junit.Assert.fail;
5051
import static org.junit.Assume.assumeTrue;
@@ -87,6 +88,7 @@
8788
import com.oracle.truffle.api.frame.FrameDescriptor;
8889
import com.oracle.truffle.api.frame.FrameInstance;
8990
import com.oracle.truffle.api.frame.FrameInstanceVisitor;
91+
import com.oracle.truffle.api.frame.VirtualFrame;
9092
import com.oracle.truffle.api.interop.ArityException;
9193
import com.oracle.truffle.api.interop.InteropLibrary;
9294
import com.oracle.truffle.api.interop.TruffleObject;
@@ -97,6 +99,7 @@
9799
import com.oracle.truffle.api.library.ExportMessage;
98100
import com.oracle.truffle.api.nodes.EncapsulatingNodeReference;
99101
import com.oracle.truffle.api.nodes.Node;
102+
import com.oracle.truffle.api.nodes.RootNode;
100103
import com.oracle.truffle.api.source.Source;
101104

102105
@RunWith(Parameterized.class)
@@ -450,6 +453,52 @@ private static void assertStackElementNoLocation(TruffleStackTraceElement elemen
450453
assertNull(BytecodeNode.get(element));
451454
}
452455

456+
@Test
457+
public void testBadLocationFrameInstance() {
458+
try (Context c = BytecodeDSLTestLanguage.createPolyglotContextWithCompilationDisabled()) {
459+
CallTarget collectBytecodeNodes = new RootNode(null) {
460+
@Override
461+
public Object execute(VirtualFrame frame) {
462+
List<BytecodeNode> bytecodeNodes = new ArrayList<>();
463+
Truffle.getRuntime().iterateFrames(f -> {
464+
bytecodeNodes.add(BytecodeNode.get(f));
465+
return null;
466+
});
467+
return bytecodeNodes;
468+
}
469+
}.getCallTarget();
470+
471+
StackTraceTestRootNode caller = parse(b -> {
472+
b.beginRoot();
473+
b.emitCallWithBadLocation(collectBytecodeNodes);
474+
b.endRoot();
475+
});
476+
477+
assertThrows(AssertionError.class, () -> caller.getCallTarget().call());
478+
}
479+
}
480+
481+
@Test
482+
public void testBadLocationTruffleStackTraceElement() {
483+
try (Context c = BytecodeDSLTestLanguage.createPolyglotContextWithCompilationDisabled()) {
484+
StackTraceTestRootNode capturesStack = parse(b -> {
485+
b.beginRoot();
486+
b.beginReturn();
487+
b.emitCaptureStack();
488+
b.endReturn();
489+
b.endRoot();
490+
});
491+
492+
StackTraceTestRootNode caller = parse(b -> {
493+
b.beginRoot();
494+
b.emitCallWithBadLocation(capturesStack.getCallTarget());
495+
b.endRoot();
496+
});
497+
498+
assertThrows(AssertionError.class, () -> caller.getCallTarget().call());
499+
}
500+
}
501+
453502
private StackTraceTestRootNode[] chainCalls(int depth, BytecodeParser<StackTraceTestRootNodeBuilder> innerParser, boolean includeLocation, boolean includeSources) {
454503
StackTraceTestRootNode[] nodes = new StackTraceTestRootNode[depth];
455504
nodes[0] = parse(innerParser);
@@ -599,6 +648,17 @@ static Object doDefault(CallTarget target) {
599648
}
600649
}
601650

651+
@Operation(storeBytecodeIndex = true)
652+
@ConstantOperand(type = CallTarget.class)
653+
static final class CallWithBadLocation {
654+
@Specialization
655+
static Object doDefault(CallTarget target, @Bind StackTraceTestRootNode root) {
656+
// This is incorrect. The tests using this operation exert paths that should trigger
657+
// assertions.
658+
return target.call(root);
659+
}
660+
}
661+
602662
@Operation(storeBytecodeIndex = true)
603663
static final class Resume {
604664
@Specialization

‎truffle/src/com.oracle.truffle.api.bytecode.test/src/com/oracle/truffle/api/bytecode/test/basic_interpreter/BasicInterpreterTest.java‎

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1406,7 +1406,7 @@ public void testMaterializedFrameAccessesDeadVariable() {
14061406
assumeTrue(run.storesBciInFrame());
14071407

14081408
// This test relies on an assertion. Explicitly open a context with compilation disabled.
1409-
try (Context c = createContextWithCompilationDisabled()) {
1409+
try (Context c = BytecodeDSLTestLanguage.createPolyglotContextWithCompilationDisabled()) {
14101410
BytecodeRootNodes<BasicInterpreter> nodes = createNodes(BytecodeConfig.DEFAULT, b -> {
14111411
b.beginRoot();
14121412

@@ -1480,16 +1480,6 @@ public void testMaterializedFrameAccessesDeadVariable() {
14801480

14811481
}
14821482

1483-
private static Context createContextWithCompilationDisabled() {
1484-
var builder = Context.newBuilder(BytecodeDSLTestLanguage.ID);
1485-
if (TruffleTestAssumptions.isOptimizingRuntime()) {
1486-
builder.option("engine.Compilation", "false");
1487-
}
1488-
Context result = builder.build();
1489-
result.enter();
1490-
return result;
1491-
}
1492-
14931483
/*
14941484
* In this test we check that accessing a local from the wrong frame throws an assertion.
14951485
*/

‎truffle/src/com.oracle.truffle.api.bytecode/src/com/oracle/truffle/api/bytecode/BytecodeLocation.java‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -278,14 +278,7 @@ public static BytecodeLocation get(FrameInstance frameInstance) {
278278
* into the frame before any operation that might call another node. This incurs a bit of
279279
* overhead during regular execution (but just for the uncached interpreter).
280280
*/
281-
Node location = frameInstance.getCallNode();
282-
BytecodeNode foundBytecodeNode = null;
283-
for (Node current = location; current != null; current = current.getParent()) {
284-
if (current instanceof BytecodeNode bytecodeNode) {
285-
foundBytecodeNode = bytecodeNode;
286-
break;
287-
}
288-
}
281+
BytecodeNode foundBytecodeNode = BytecodeNode.get(frameInstance);
289282
if (foundBytecodeNode == null) {
290283
return null;
291284
}

‎truffle/src/com.oracle.truffle.api.bytecode/src/com/oracle/truffle/api/bytecode/BytecodeNode.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1289,6 +1289,7 @@ private static Frame resolveFrame(FrameInstance frameInstance, FrameAccess acces
12891289
*/
12901290
@TruffleBoundary
12911291
public static BytecodeNode get(FrameInstance frameInstance) {
1292+
assert !(frameInstance.getCallNode() instanceof BytecodeRootNode) : "A BytecodeRootNode should not be used as a call location.";
12921293
return get(frameInstance.getCallNode());
12931294
}
12941295

‎truffle/src/com.oracle.truffle.dsl.processor/src/com/oracle/truffle/dsl/processor/bytecode/generator/BytecodeRootNodeElement.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -865,6 +865,7 @@ private Element createFindBytecodeIndex() {
865865
CodeExecutableElement ex = overrideImplementRootNodeMethod(model, "findBytecodeIndex", new String[]{"node", "frame"});
866866
mergeSuppressWarnings(ex, "hiding");
867867
CodeTreeBuilder b = ex.createBuilder();
868+
b.startAssert().string("!(node instanceof ").type(types.BytecodeRootNode).string("): ").doubleQuote("A BytecodeRootNode should not be used as a call location.").end();
868869
if (model.storeBciInFrame) {
869870
b.startIf().string("node == null").end().startBlock();
870871
b.statement("return -1");
@@ -882,7 +883,7 @@ private Element createFindBytecodeIndex() {
882883
b.declaration(types.Node, "prev", "node");
883884
b.declaration(types.Node, "current", "node");
884885
b.startWhile().string("current != null").end().startBlock();
885-
b.startIf().string("current ").instanceOf(abstractBytecodeNode.asType()).string(" b").end().startBlock();
886+
b.startIf().string("current").instanceOf(abstractBytecodeNode.asType()).string(" b").end().startBlock();
886887
b.statement("bytecode = b");
887888
b.statement("break");
888889
b.end();

0 commit comments

Comments
 (0)