Skip to content

[GR-71618] Fix PythonDebugTest with BytecodeDSL. #639

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
graalvmbot wants to merge 1 commit into from
Closed

[GR-71618] Fix PythonDebugTest with BytecodeDSL. #639

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 9 additions & 1 deletion ...e.graal.python.pegparser/src/com/oracle/graal/python/pegparser/tokenizer/SourceRange.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2026, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* The Universal Permissive License (UPL), Version 1.0
Expand Down Expand Up @@ -64,6 +64,14 @@ public SourceRange withEnd(int newEndLine, int newEndColumn) {
return new SourceRange(startLine, startColumn, newEndLine, newEndColumn);
}

public SourceRange startLineShiftColumn(int shift) {
assert shift >= 0;
if (shift == 0) {
return this;
}
return new SourceRange(startLine, startColumn, startLine, startColumn + shift);
}

public SourceRange shiftStartRight(int columns) {
assert columns >= 0;
if (columns == 0) {
Expand Down
150 changes: 86 additions & 64 deletions .../com.oracle.graal.python.test/src/com/oracle/graal/python/test/debug/PythonDebugTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2025, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2026, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* The Universal Permissive License (UPL), Version 1.0
Expand Down Expand Up @@ -59,9 +59,7 @@
import org.graalvm.polyglot.Context;
import org.graalvm.polyglot.Context.Builder;
import org.graalvm.polyglot.Source;
import org.graalvm.polyglot.Value;
import org.junit.After;
import org.junit.Assume;
import org.junit.Before;
import org.junit.Test;

Expand Down Expand Up @@ -95,11 +93,9 @@ public void dispose() {

@Test
public void testSteppingAsExpected() throws Throwable {
Value isBytecodeDLS = eval("__graalpython__.is_bytecode_dsl_interpreter");
// GR-71618
Assume.assumeFalse("TODO: wrong stacktrace", isBytecodeDLS.asBoolean());
// test that various elements step as expected, including generators, statement level atomic
// expressions, and roots
boolean isBytecodeDLS = isBytecodeDSL();
final Source source = Source.newBuilder("python", "" +
"import sys\n" +
"from sys import version\n" +
Expand Down Expand Up @@ -164,16 +160,30 @@ public void testSteppingAsExpected() throws Throwable {
assertEquals(7, frame.getSourceSection().getStartLine());
event.prepareStepOver(1);
});
// for i in >genfunc()<:
expectSuspended((SuspendedEvent event) -> {
DebugStackFrame frame = event.getTopStackFrame();
assertEquals(8, frame.getSourceSection().getStartLine());
event.prepareStepInto(1);
});
expectSuspended((SuspendedEvent event) -> {
DebugStackFrame frame = event.getTopStackFrame();
assertEquals(12, frame.getSourceSection().getStartLine());
event.prepareStepOver(1);
});
// Unlike the manual interpreter, which reports yield as onReturn, the Bytecode DSL
// correctly reports yield as onYield on the probe node, but the step over strategy
// is different for yield vs return - see SteppingStrategy$StepOver#step
if (!isBytecodeDLS) {
// steppping into genfunc()
expectSuspended((SuspendedEvent event) -> {
DebugStackFrame frame = event.getTopStackFrame();
assertEquals(12, frame.getSourceSection().getStartLine());
event.prepareStepOver(1);
});
} else {
// steppping into genfunc()
expectSuspended((SuspendedEvent event) -> {
DebugStackFrame frame = event.getTopStackFrame();
assertEquals(12, frame.getSourceSection().getStartLine());
event.prepareStepOut(1);
});
}
expectSuspended((SuspendedEvent event) -> {
DebugStackFrame frame = event.getTopStackFrame();
assertEquals(8, frame.getSourceSection().getStartLine());
Expand All @@ -190,9 +200,6 @@ public void testSteppingAsExpected() throws Throwable {

@Test
public void testException() throws Throwable {
Value isBytecodeDLS = eval("__graalpython__.is_bytecode_dsl_interpreter");
// GR-71618
Assume.assumeFalse("TODO: wrong stacktrace", isBytecodeDLS.asBoolean());
final Source source = Source.newBuilder("python", "" +
"try:\n" +
" 1 / 0\n" +
Expand All @@ -216,70 +223,52 @@ public void testException() throws Throwable {

@Test
public void testInlineEvaluation() throws Throwable {
Value isBytecodeDLS = eval("__graalpython__.is_bytecode_dsl_interpreter");
// GR-71618
Assume.assumeFalse("TODO: wrong stacktrace", isBytecodeDLS.asBoolean());
final Source source = Source.newBuilder("python", "" +
"y = 4\n" +
"def foo(x):\n" +
" a = 1\n" +
" b = 2\n" +
" def bar():\n" +
" q = 42\n" +
" def bar(z = 24):\n" +
" return a + b\n" +
" return bar() + x + y\n" +
"foo(3)", "test_inline.py").buildLiteral();

try (DebuggerSession session = tester.startSession()) {
session.install(Breakpoint.newBuilder(DebuggerTester.getSourceImpl(source)).lineIs(5).build());
session.install(Breakpoint.newBuilder(DebuggerTester.getSourceImpl(source)).lineIs(6).build());
session.install(Breakpoint.newBuilder(DebuggerTester.getSourceImpl(source)).lineIs(7).build());
session.install(Breakpoint.newBuilder(DebuggerTester.getSourceImpl(source)).lineIs(8).build());
tester.startEval(source);

// breakpoint at bar declaration
expectSuspended((SuspendedEvent event) -> {
DebugStackFrame frame = event.getTopStackFrame();
assertEquals(5, frame.getSourceSection().getStartLine());
assertEquals("42", frame.eval("q").toDisplayString());
assertEquals("3", frame.eval("a + b").toDisplayString());
assertEquals(6, frame.getSourceSection().getStartLine());
event.prepareContinue();
});
// breakpoint at bar call site
expectSuspended((SuspendedEvent event) -> {
DebugStackFrame frame = event.getTopStackFrame();
assertEquals(7, frame.getSourceSection().getStartLine());
assertEquals(8, frame.getSourceSection().getStartLine());
assertEquals("6", frame.eval("bar() * 2").toDisplayString());
event.prepareContinue();
});
assertEquals("10", tester.expectDone());
}
}

@Test
@SuppressWarnings("try")
public void testBreakpointBuiltin() throws Throwable {
Value isBytecodeDLS = eval("__graalpython__.is_bytecode_dsl_interpreter");
// GR-71618
Assume.assumeFalse("TODO: wrong stacktrace", isBytecodeDLS.asBoolean());
final Source source = Source.newBuilder("python", "" +
"def foo():\n" +
" a = 1\n" +
" breakpoint()\n" +
" return 1\n" +
"foo()\n", "test_breakpoint_builtin.py").buildLiteral();

try (DebuggerSession session = tester.startSession()) {
tester.startEval(source);

// breakpoint inside bar
expectSuspended((SuspendedEvent event) -> {
DebugStackFrame frame = event.getTopStackFrame();
assertEquals(3, frame.getSourceSection().getStartLine());
assertEquals("24", frame.eval("z").toDisplayString());
assertEquals("3", frame.eval("a + b").toDisplayString());
assertEquals(7, frame.getSourceSection().getStartLine());
event.prepareContinue();
});
assertEquals("1", tester.expectDone());
assertEquals("10", tester.expectDone());
}
}

@Test
public void testConditionalBreakpointInFunction() throws Throwable {
Value isBytecodeDLS = eval("__graalpython__.is_bytecode_dsl_interpreter");
// GR-71618
Assume.assumeFalse("TODO: wrong stacktrace", isBytecodeDLS.asBoolean());
final Source source = Source.newBuilder("python", "" +
"def fun():\n" +
" def prod(n):\n" +
Expand Down Expand Up @@ -325,9 +314,6 @@ public void testConditionalBreakpointInFunction() throws Throwable {

@Test
public void testConditionalBreakpointGlobal() throws Throwable {
Value isBytecodeDLS = eval("__graalpython__.is_bytecode_dsl_interpreter");
// GR-71618
Assume.assumeFalse("TODO: wrong stacktrace", isBytecodeDLS.asBoolean());
final Source source = Source.newBuilder("python", "" +
"values = []\n" +
"for i in range(0, 10):\n" +
Expand All @@ -349,11 +335,52 @@ public void testConditionalBreakpointGlobal() throws Throwable {
}
}

@Test
public void testGeneralLocals() throws Throwable {
final Source source = Source.newBuilder("python", """
def gen_fun(a, b):
yield 1
c = 3
yield 2
r = 0
for i in gen_fun(1,2):
r += i
r
""", "testGeneratorLocals.py").buildLiteral();

try (DebuggerSession session = tester.startSession()) {
session.install(Breakpoint.newBuilder(DebuggerTester.getSourceImpl(source)).lineIs(2).build());
session.install(Breakpoint.newBuilder(DebuggerTester.getSourceImpl(source)).lineIs(4).build());
tester.startEval(source);

expectSuspended((SuspendedEvent event) -> {
DebugStackFrame frame = event.getTopStackFrame();
assertEquals(2, frame.getSourceSection().getStartLine());
checkStack(frame, "gen_fun", "a", "1", "b", "2");
event.prepareContinue();
});
expectSuspended((SuspendedEvent event) -> {
DebugStackFrame frame = event.getTopStackFrame();
assertEquals(2, frame.getSourceSection().getStartLine());
event.prepareContinue();
});
expectSuspended((SuspendedEvent event) -> {
DebugStackFrame frame = event.getTopStackFrame();
assertEquals(4, frame.getSourceSection().getStartLine());
checkStack(frame, "gen_fun", "a", "1", "b", "2", "c", "3");
event.prepareContinue();
});
expectSuspended((SuspendedEvent event) -> {
DebugStackFrame frame = event.getTopStackFrame();
assertEquals(4, frame.getSourceSection().getStartLine());
event.prepareContinue();
});
assertEquals("3", tester.expectDone());
}
}

@Test
public void testReenterArgumentsAndValues() throws Throwable {
Value isBytecodeDLS = eval("__graalpython__.is_bytecode_dsl_interpreter");
// GR-71618
Assume.assumeFalse("TODO: wrong stacktrace", isBytecodeDLS.asBoolean());
// Test that after a re-enter, arguments are kept and variables are cleared.
final Source source = Source.newBuilder("python", "" +
"def main():\n" +
Expand Down Expand Up @@ -381,7 +408,7 @@ public void testReenterArgumentsAndValues() throws Throwable {

expectSuspended((SuspendedEvent event) -> {
DebugStackFrame frame = event.getTopStackFrame();
assertEquals(6, frame.getSourceSection().getStartLine());
assertEquals("first breakpoint at 6", 6, frame.getSourceSection().getStartLine());
checkStack(frame, "fnc", "n", "11", "m", "20");
event.prepareStepOver(4);
});
Expand All @@ -398,7 +425,7 @@ public void testReenterArgumentsAndValues() throws Throwable {
});
expectSuspended((SuspendedEvent event) -> {
DebugStackFrame frame = event.getTopStackFrame();
assertEquals(6, frame.getSourceSection().getStartLine());
assertEquals("first breakpoint at 6", 6, frame.getSourceSection().getStartLine());
checkStack(frame, "fnc", "n", "11", "m", "20");
});
assertEquals("50.0", tester.expectDone());
Expand All @@ -408,9 +435,6 @@ public void testReenterArgumentsAndValues() throws Throwable {
@Test
@SuppressWarnings("deprecation")
public void testGettersSetters() throws Throwable {
Value isBytecodeDLS = eval("__graalpython__.is_bytecode_dsl_interpreter");
// GR-71618
Assume.assumeFalse("TODO: wrong stacktrace", isBytecodeDLS.asBoolean());
final Source source = Source.newBuilder("python", "" +
"class GetterOnly:\n" +
" def __get__(self):\n" +
Expand Down Expand Up @@ -480,9 +504,6 @@ public void testGettersSetters() throws Throwable {

@Test
public void testInspectJavaArray() throws Throwable {
Value isBytecodeDLS = eval("__graalpython__.is_bytecode_dsl_interpreter");
// GR-71618
Assume.assumeFalse("TODO: wrong stacktrace", isBytecodeDLS.asBoolean());
final Source source = Source.newBuilder("python", "" +
"import java\n" +
"a_int = java.type('int[]')(3)\n" +
Expand Down Expand Up @@ -528,11 +549,8 @@ public void testInspectJavaArray() throws Throwable {

@Test
public void testSourceFileURI() throws Throwable {
Value isBytecodeDLS = eval("__graalpython__.is_bytecode_dsl_interpreter");
// GR-71618
Assume.assumeFalse("TODO: wrong stacktrace", isBytecodeDLS.asBoolean());
if (System.getProperty("os.name").toLowerCase().contains("mac")) {
// on the mac machines we run with symlinked directories and such and it's annoying to
// on the mac machines we run with symlinked directories and such, and it's annoying to
// cater for that
return;
}
Expand Down Expand Up @@ -623,4 +641,8 @@ public FileVisitResult postVisitDirectory(Path dir, IOException exc) throws IOEx
}
});
}

private static boolean isBytecodeDSL() {
return eval("__graalpython__.is_bytecode_dsl_interpreter").asBoolean();
}
}
12 changes: 10 additions & 2 deletions ...acle.graal.python/src/com/oracle/graal/python/compiler/bytecode_dsl/RootNodeCompiler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,11 @@ void endTraceLineChecked(SSTNode node, Builder b) {
* corresponding {@link #endSourceSection} call to ensure the Tag is closed.
*/
boolean beginSourceSection(SSTNode node, Builder b) {
SourceRange sourceRange = node.getSourceRange();
return beginSourceSection(node.getSourceRange(), b);
}

/** {@link #beginSourceSection(SSTNode, Builder)} */
boolean beginSourceSection(SourceRange sourceRange, Builder b) {
SourceRange oldSourceRange = this.currentLocation;
this.currentLocation = sourceRange;

Expand Down Expand Up @@ -4123,7 +4127,11 @@ public Void visit(StmtTy.FunctionDef node) {

public void emitFunctionDef(StmtTy node, String name, ArgumentsTy args, StmtTy[] body, ExprTy[] decoratorList, ExprTy returns, TypeParamTy[] typeParams) {
BytecodeLocal[] decoratorLocals = evaluateDecorators(decoratorList);
boolean newStatement = beginSourceSection(node, b);
// For instrumentation, we want to map this statement only to the declaration line, such
// that, e.g., breakpoints inside the body fire only once the body actually executes and
// not is declared. There is no simple way to get the exact line width here, so we just
// approximate it with name width.
boolean newStatement = beginSourceSection(node.getSourceRange().startLineShiftColumn(name.length()), b);
emitTraceLineChecked(node, b);

beginStoreLocal(name, b);
Expand Down
1 change: 1 addition & 0 deletions ...cle.graal.python/src/com/oracle/graal/python/nodes/bytecode_dsl/PBytecodeDSLRootNode.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,7 @@
enableSerialization = true, //
enableTagInstrumentation = true, //
boxingEliminationTypes = {int.class}, //
tagTreeNodeLibrary = PTagTreeNodeExports.class, //
storeBytecodeIndexInFrame = true //
)
@OperationProxy(PyNumberSubtractNode.class)
Expand Down
Loading
Loading