[8.0.0] Fix NPE in the Starlark debugger (#24453)

If we breakpoint at a line before a local (that is a cell) has been intialized, cell.x is null, which we shouldn't attempt to add to the returned map of locals. Fixes #24339 PiperOrigin-RevId: 698316990 Change-Id: Ifb45650fb41471c47605292af9873e9f66b7f7d7 Commit ae330a2 Co-authored-by: Googler <[email protected]>
bazelbuild · Nov 22, 2024 · 7b1a825 · 7b1a825
1 parent 43cb62a
commit 7b1a825
Show file tree

Hide file tree

Showing 2 changed files with 116 additions and 3 deletions.
diff --git a/src/main/java/net/starlark/java/eval/StarlarkThread.java b/src/main/java/net/starlark/java/eval/StarlarkThread.java
@@ -187,10 +187,10 @@ public ImmutableMap<String, Object> getLocals() {
       if (fn instanceof StarlarkFunction) {
         for (int i = 0; i < locals.length; i++) {
           Object local = locals[i];
+          if (local instanceof StarlarkFunction.Cell) {
+            local = ((StarlarkFunction.Cell) local).x;
+          }
           if (local != null) {
-            if (local instanceof StarlarkFunction.Cell) {
-              local = ((StarlarkFunction.Cell) local).x;
-            }
             env.put(((StarlarkFunction) fn).rfn.getLocals().get(i).getName(), local);
           }
         }

diff --git a/...test/java/com/google/devtools/build/lib/starlarkdebug/server/StarlarkDebugServerTest.java b/...test/java/com/google/devtools/build/lib/starlarkdebug/server/StarlarkDebugServerTest.java
@@ -412,6 +412,119 @@ public void testSimpleListFramesRequest() throws Exception {
             .build());
   }
 
+  @Test
+  public void testListFramesWithUninitializedCellRequest() throws Exception {
+    sendStartDebuggingRequest();
+    ParserInput bzlFile =
+        createInput(
+            "/a/build/file/foo.bzl",
+            """
+            def outer():
+               x = [1,2,3] # <- breakpoint
+
+               def inner():
+                   x.append(4)
+
+               inner()
+
+            outer()
+            """);
+
+    Location breakpoint =
+        Location.newBuilder().setLineNumber(2).setPath("/a/build/file/foo.bzl").build();
+    setBreakpoints(ImmutableList.of(breakpoint));
+
+    Module module = Module.create();
+    Thread evaluationThread = execInWorkerThread(bzlFile, module);
+    long threadId = evaluationThread.getId();
+
+    // wait for breakpoint to be hit
+    client.waitForEvent(DebugEvent::hasThreadPaused, Duration.ofSeconds(5));
+
+    ListFramesResponse frames = listFrames(threadId);
+    assertThat(frames.getFrameCount()).isEqualTo(2);
+    // critically x is not present as a local
+    assertFramesEqualIgnoringValueIdentifiers(
+        frames.getFrame(0),
+        Frame.newBuilder()
+            .setFunctionName("outer")
+            .setLocation(breakpoint.toBuilder().setColumnNumber(4))
+            .addScope(
+                Scope.newBuilder()
+                    .setName("global")
+                    .addBinding(getValueProto("outer", module.getGlobal("outer"))))
+            .build());
+    assertFramesEqualIgnoringValueIdentifiers(
+        frames.getFrame(1),
+        Frame.newBuilder()
+            .setFunctionName(StarlarkThread.TOP_LEVEL)
+            .setLocation(breakpoint.toBuilder().setLineNumber(9).setColumnNumber(6))
+            .addScope(
+                Scope.newBuilder()
+                    .setName("global")
+                    .addBinding(getValueProto("outer", module.getGlobal("outer"))))
+            .build());
+  }
+
+  @Test
+  public void testListFramesWithInitializedCellRequest() throws Exception {
+    sendStartDebuggingRequest();
+    ParserInput bzlFile =
+        createInput(
+            "/a/build/file/foo.bzl",
+            """
+            def outer():
+               x = [1]
+               pass          # <- breakpoint
+
+               def inner():
+                   x.append(4)
+
+               inner()
+
+            outer()
+            """);
+
+    Location breakpoint =
+        Location.newBuilder().setLineNumber(3).setPath("/a/build/file/foo.bzl").build();
+    setBreakpoints(ImmutableList.of(breakpoint));
+
+    Module module = Module.create();
+    Thread evaluationThread = execInWorkerThread(bzlFile, module);
+    long threadId = evaluationThread.getId();
+
+    // wait for breakpoint to be hit
+    client.waitForEvent(DebugEvent::hasThreadPaused, Duration.ofSeconds(5));
+
+    ListFramesResponse frames = listFrames(threadId);
+    assertThat(frames.getFrameCount()).isEqualTo(2);
+    // critically x is present as a local
+    assertFramesEqualIgnoringValueIdentifiers(
+        frames.getFrame(0),
+        Frame.newBuilder()
+            .setFunctionName("outer")
+            .setLocation(breakpoint.toBuilder().setColumnNumber(4))
+            .addScope(
+                Scope.newBuilder()
+                    .setName("local")
+                    .addBinding(getValueProto("x", StarlarkList.immutableOf(StarlarkInt.of(1)))))
+            .addScope(
+                Scope.newBuilder()
+                    .setName("global")
+                    .addBinding(getValueProto("outer", module.getGlobal("outer"))))
+            .build());
+    assertFramesEqualIgnoringValueIdentifiers(
+        frames.getFrame(1),
+        Frame.newBuilder()
+            .setFunctionName(StarlarkThread.TOP_LEVEL)
+            .setLocation(breakpoint.toBuilder().setLineNumber(10).setColumnNumber(6))
+            .addScope(
+                Scope.newBuilder()
+                    .setName("global")
+                    .addBinding(getValueProto("outer", module.getGlobal("outer"))))
+            .build());
+  }
+
   @Test
   public void testGetChildrenRequest() throws Exception {
     sendStartDebuggingRequest();