Skip to content

Commit e7aba15

Browse files
committed
Fix: stop overriding countsTowardsStackTraceLimit.
The default `countsTowardsStackTraceLimit` impl invokes `isInternal`, which used to force materialization of sources in the Bytecode DSL. To prevent this eager materialization, we overrode `countsTowardsStackTraceLimit` to `true` when the user did not implement it. This is incorrect: the user could override `isInternal`, but the `countsTowardsStackTraceLimit` override would ignore it. Since eager materialization no longer occurs, we can stop generating an override for this method.
1 parent ea608d2 commit e7aba15

File tree

2 files changed

+57
-23
lines changed

2 files changed

+57
-23
lines changed

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,10 @@
4747
import static org.junit.Assert.assertSame;
4848
import static org.junit.Assert.assertTrue;
4949
import static org.junit.Assert.fail;
50+
import static org.junit.Assume.assumeTrue;
5051

5152
import java.util.ArrayList;
53+
import java.util.Arrays;
5254
import java.util.List;
5355

5456
import org.graalvm.polyglot.Context;
@@ -337,6 +339,40 @@ public void testValidateResumedFrameInstances() {
337339
}
338340
}
339341

342+
@Test
343+
@SuppressWarnings("unchecked")
344+
public void testCaptureWithLimit() {
345+
int depth = run.depth;
346+
assumeTrue("Cannot test stack trace limits with less than two frames in the trace.", depth >= 2);
347+
int stackTraceLimit = depth / 2;
348+
StackTraceTestRootNode[] nodes = chainCalls(depth, b -> {
349+
b.beginRoot();
350+
b.emitDummy();
351+
b.beginReturn();
352+
b.emitCaptureStackWithLimit(stackTraceLimit);
353+
b.endReturn();
354+
b.endRoot();
355+
}, true, false);
356+
StackTraceTestRootNode outer = nodes[nodes.length - 1];
357+
358+
// First, invoke as usual. The stack trace should consist of the top stackTraceLimit frames.
359+
StackTraceTestRootNode[] topFrames = Arrays.copyOfRange(nodes, 0, stackTraceLimit);
360+
for (int repeat = 0; repeat < REPEATS; repeat++) {
361+
List<TruffleStackTraceElement> elements = (List<TruffleStackTraceElement>) outer.getCallTarget().call();
362+
assertStackElements(topFrames, elements, "c.CaptureStackWithLimit", "c.Call", false);
363+
}
364+
365+
// Next, mark the top (depth - stackTraceLimit) roots as internal so they do not "count".
366+
for (int i = 0; i < depth - stackTraceLimit; i++) {
367+
nodes[i].internal = true;
368+
}
369+
// Since they do not "count", we should capture every frame.
370+
for (int repeat = 0; repeat < REPEATS; repeat++) {
371+
List<TruffleStackTraceElement> elements = (List<TruffleStackTraceElement>) outer.getCallTarget().call();
372+
assertStackElements(nodes, elements, "c.CaptureStackWithLimit", "c.Call", false);
373+
}
374+
}
375+
340376
private void assertStackElements(StackTraceTestRootNode[] nodes, List<TruffleStackTraceElement> elements, String firstElementInstruction, String chainedElementInstruction, boolean checkSources) {
341377
assertEquals(nodes.length, elements.size());
342378
assertStackElement(elements.get(0), nodes[0], firstElementInstruction, checkSources);
@@ -515,6 +551,7 @@ protected StackTraceTestRootNode(BytecodeDSLTestLanguage language, FrameDescript
515551

516552
public String name;
517553
public int depth;
554+
public boolean internal = false;
518555

519556
public void setName(String name) {
520557
this.name = name;
@@ -525,6 +562,11 @@ public String getName() {
525562
return name;
526563
}
527564

565+
@Override
566+
public boolean isInternal() {
567+
return internal;
568+
}
569+
528570
@Override
529571
public String toString() {
530572
return "StackTest[name=" + name + ", depth=" + depth + "]";
@@ -606,6 +648,17 @@ static Object doDefault(@Bind Node node) {
606648
}
607649
}
608650

651+
@Operation(storeBytecodeIndex = true)
652+
@ConstantOperand(type = int.class, name = "stackTraceLimit")
653+
static final class CaptureStackWithLimit {
654+
655+
@Specialization
656+
static Object doDefault(int stackTraceLimit, @Bind Node node) {
657+
TestException ex = new TestException(node, stackTraceLimit);
658+
return TruffleStackTrace.getStackTrace(ex);
659+
}
660+
}
661+
609662
@SuppressWarnings("truffle-interpreted-performance")
610663
@Operation(storeBytecodeIndex = true)
611664
// The parser should populate this array after parsing. This is a hack to expose the roots.
@@ -689,6 +742,10 @@ static class TestException extends AbstractTruffleException {
689742
super(resolveLocation(location));
690743
}
691744

745+
TestException(Node location, int stackTraceElementLimit) {
746+
super(null, null, stackTraceElementLimit, resolveLocation(location));
747+
}
748+
692749
private static Node resolveLocation(Node location) {
693750
if (location == null) {
694751
return null;

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

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -500,7 +500,6 @@ final class BytecodeRootNodeElement extends CodeTypeElement {
500500
this.add(createInvalidate());
501501

502502
this.add(createGetRootNodes());
503-
this.addOptional(createCountTowardsStackTraceLimit());
504503
this.add(createGetSourceSection());
505504
CodeExecutableElement translateStackTraceElement = this.addOptional(createTranslateStackTraceElement());
506505
if (translateStackTraceElement != null) {
@@ -1137,28 +1136,6 @@ private CodeExecutableElement createCreateStackTraceElement() {
11371136
return ex;
11381137
}
11391138

1140-
private CodeExecutableElement createCountTowardsStackTraceLimit() {
1141-
ExecutableElement executable = ElementUtils.findOverride(ElementUtils.findMethod(types.RootNode, "countsTowardsStackTraceLimit"), model.templateType);
1142-
if (executable != null) {
1143-
return null;
1144-
}
1145-
CodeExecutableElement ex = overrideImplementRootNodeMethod(model, "countsTowardsStackTraceLimit");
1146-
if (ex.getModifiers().contains(Modifier.FINAL)) {
1147-
// already overridden by the root node.
1148-
return null;
1149-
}
1150-
1151-
ex.getModifiers().remove(Modifier.ABSTRACT);
1152-
ex.getModifiers().add(Modifier.FINAL);
1153-
CodeTreeBuilder b = ex.createBuilder();
1154-
/*
1155-
* We do override with false by default to avoid materialization of sources during stack
1156-
* walking.
1157-
*/
1158-
b.returnTrue();
1159-
return ex;
1160-
}
1161-
11621139
private CodeExecutableElement createGetSourceSection() {
11631140
CodeExecutableElement ex = GeneratorUtils.override(types.Node, "getSourceSection");
11641141
CodeTreeBuilder b = ex.createBuilder();

0 commit comments

Comments
 (0)