[wasm][debug] Make some control opcodes non-breakable

Most control structures in WebAssembly do not have a clear execution
semantics, they are more like markers. Hence the execute state, and the
change in the state, when breaking on them and stepping over them is
unclear.
Hence this CL just makes them non-breakable. If the user tries to set a
breakpoint on them, this breakpoint will automatically be propagated to
the first instruction after the respective control opcode (this is
tested for other cases in existing tests).

R=thibaudm@chromium.org

Bug: v8:10326
Change-Id: Iaf540a94789c9cbc87d23ddfb794e4b01776b49f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2122017
Reviewed-by: Thibaud Michaud <thibaudm@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66876}

src/wasm/baseline/liftoff-compiler.cc

@@ -691,8 +691,10 @@ class LiftoffCompiler {
       if (*next_breakpoint_ptr_ == 0) {
         // A single breakpoint at offset 0 indicates stepping.
         DCHECK_EQ(next_breakpoint_ptr_ + 1, next_breakpoint_end_);
-        breakpoint = true;
-        EmitBreakpoint(decoder);
+        if (WasmOpcodes::IsBreakable(opcode)) {
+          breakpoint = true;
+          EmitBreakpoint(decoder);
+        }
       } else {
         while (next_breakpoint_ptr_ != next_breakpoint_end_ &&
                *next_breakpoint_ptr_ < decoder->position()) {
@@ -702,6 +704,7 @@ class LiftoffCompiler {
         if (next_breakpoint_ptr_ == next_breakpoint_end_) {
           next_breakpoint_ptr_ = next_breakpoint_end_ = nullptr;
         } else if (*next_breakpoint_ptr_ == decoder->position()) {
+          DCHECK(WasmOpcodes::IsBreakable(opcode));
           breakpoint = true;
           EmitBreakpoint(decoder);
         }

src/wasm/wasm-debug.cc

@@ -1248,8 +1248,10 @@ int FindNextBreakablePosition(wasm::NativeModule* native_module, int func_index,
                                   &locals);
   DCHECK_LT(0, locals.encoded_size);
   if (offset_in_func < 0) return 0;
-  for (uint32_t offset : iterator.offsets()) {
-    if (offset >= static_cast<uint32_t>(offset_in_func)) return offset;
+  for (; iterator.has_next(); iterator.next()) {
+    if (iterator.pc_offset() < static_cast<uint32_t>(offset_in_func)) continue;
+    if (!wasm::WasmOpcodes::IsBreakable(iterator.current())) continue;
+    return static_cast<int>(iterator.pc_offset());
   }
   return 0;
 }
@@ -1598,13 +1600,14 @@ bool WasmScript::GetPossibleBreakpoints(
                                     module_start + func.code.end_offset(),
                                     &locals);
     DCHECK_LT(0u, locals.encoded_size);
-    for (uint32_t offset : iterator.offsets()) {
-      uint32_t total_offset = func.code.offset() + offset;
+    for (; iterator.has_next(); iterator.next()) {
+      uint32_t total_offset = func.code.offset() + iterator.pc_offset();
       if (total_offset >= end_offset) {
         DCHECK_EQ(end_func_index, func_idx);
         break;
       }
       if (total_offset < start_offset) continue;
+      if (!wasm::WasmOpcodes::IsBreakable(iterator.current())) continue;
       locations->emplace_back(0, total_offset, debug::kCommonBreakLocation);
     }
   }

src/wasm/wasm-interpreter.cc

@@ -3059,8 +3059,10 @@ class ThreadImpl {
       }
 
       // If max is 0, break. If max is positive (a limit is set), decrement it.
-      if (max == 0) break;
-      if (max > 0) --max;
+      if (max >= 0 && WasmOpcodes::IsBreakable(opcode)) {
+        if (max == 0) break;
+        --max;
+      }
 
       USE(skip);
       TRACE("@%-3zu: %s%-24s:", pc, skip, WasmOpcodes::OpcodeName(opcode));

src/wasm/wasm-opcodes.cc

@@ -409,6 +409,19 @@ bool WasmOpcodes::IsUnconditionalJump(WasmOpcode opcode) {
   }
 }
 
+bool WasmOpcodes::IsBreakable(WasmOpcode opcode) {
+  switch (opcode) {
+    case kExprBlock:
+    case kExprTry:
+    case kExprCatch:
+    case kExprLoop:
+    case kExprElse:
+      return false;
+    default:
+      return true;
+  }
+}
+
 bool WasmOpcodes::IsAnyRefOpcode(WasmOpcode opcode) {
   switch (opcode) {
     case kExprRefNull:

src/wasm/wasm-opcodes.h

@@ -676,20 +676,21 @@ enum TrapReason {
 // A collection of opcode-related static methods.
 class V8_EXPORT_PRIVATE WasmOpcodes {
  public:
-  static const char* OpcodeName(WasmOpcode opcode);
-  static const FunctionSig* Signature(WasmOpcode opcode);
-  static const FunctionSig* AsmjsSignature(WasmOpcode opcode);
-  static bool IsPrefixOpcode(WasmOpcode opcode);
-  static bool IsControlOpcode(WasmOpcode opcode);
-  static bool IsAnyRefOpcode(WasmOpcode opcode);
-  static bool IsThrowingOpcode(WasmOpcode opcode);
-  static bool IsSimdPostMvpOpcode(WasmOpcode opcode);
+  static const char* OpcodeName(WasmOpcode);
+  static const FunctionSig* Signature(WasmOpcode);
+  static const FunctionSig* AsmjsSignature(WasmOpcode);
+  static bool IsPrefixOpcode(WasmOpcode);
+  static bool IsControlOpcode(WasmOpcode);
+  static bool IsAnyRefOpcode(WasmOpcode);
+  static bool IsThrowingOpcode(WasmOpcode);
+  static bool IsSimdPostMvpOpcode(WasmOpcode);
   // Check whether the given opcode always jumps, i.e. all instructions after
   // this one in the current block are dead. Returns false for |end|.
-  static bool IsUnconditionalJump(WasmOpcode opcode);
+  static bool IsUnconditionalJump(WasmOpcode);
+  static bool IsBreakable(WasmOpcode);
 
-  static MessageTemplate TrapReasonToMessageId(TrapReason reason);
-  static const char* TrapReasonMessage(TrapReason reason);
+  static MessageTemplate TrapReasonToMessageId(TrapReason);
+  static const char* TrapReasonMessage(TrapReason);
 };
 
 // Representation of an initializer expression.

test/inspector/debugger/wasm-get-breakable-locations-byte-offsets-expected.txt

@@ -7,31 +7,29 @@ This is a wasm script (nr 0).
 Querying breakable locations for all wasm scripts now...
 Requesting all breakable locations in wasm script 0
 Bytecode matches!
-11 breakable location(s):
+10 breakable location(s):
 [0] 0:40 || byte=1
 [1] 0:41 || byte=65
 [2] 0:43 || byte=33
 [3] 0:45 || byte=11
 [4] 0:48 || byte=32
 [5] 0:50 || byte=4
-[6] 0:52 || byte=2
-[7] 0:54 || byte=16
-[8] 0:56 || byte=11
-[9] 0:57 || byte=11
-[10] 0:58 || byte=11
+[6] 0:54 || byte=16
+[7] 0:56 || byte=11
+[8] 0:57 || byte=11
+[9] 0:58 || byte=11
 Requesting breakable locations in offsets [0,45)
 3 breakable location(s):
 [0] 0:40 || byte=1
 [1] 0:41 || byte=65
 [2] 0:43 || byte=33
 Requesting breakable locations in offsets [50,60)
-6 breakable location(s):
+5 breakable location(s):
 [0] 0:50 || byte=4
-[1] 0:52 || byte=2
-[2] 0:54 || byte=16
-[3] 0:56 || byte=11
-[4] 0:57 || byte=11
-[5] 0:58 || byte=11
+[1] 0:54 || byte=16
+[2] 0:56 || byte=11
+[3] 0:57 || byte=11
+[4] 0:58 || byte=11
 Setting a breakpoint on each breakable location...
 Setting at wasm://wasm/354ada0e:0:40
 Success!
@@ -45,8 +43,6 @@ Setting at wasm://wasm/354ada0e:0:48
 Success!
 Setting at wasm://wasm/354ada0e:0:50
 Success!
-Setting at wasm://wasm/354ada0e:0:52
-Success!
 Setting at wasm://wasm/354ada0e:0:54
 Success!
 Setting at wasm://wasm/354ada0e:0:56
@@ -56,13 +52,11 @@ Success!
 Setting at wasm://wasm/354ada0e:0:58
 Success!
 Running wasm code...
-Missing breakpoints: 11
+Missing breakpoints: 10
 Script nr 3 parsed. URL: v8://test/runWasm
 Stopped at wasm://wasm/354ada0e:0:48
-Missing breakpoints: 10
-Stopped at wasm://wasm/354ada0e:0:50
 Missing breakpoints: 9
-Stopped at wasm://wasm/354ada0e:0:52
+Stopped at wasm://wasm/354ada0e:0:50
 Missing breakpoints: 8
 Stopped at wasm://wasm/354ada0e:0:54
 Missing breakpoints: 7

test/inspector/debugger/wasm-stepping-byte-offsets-expected.txt

@@ -101,8 +101,8 @@ at (anonymous) (0:17):
  - scope (global):
    -- skipped
 Debugger.stepInto called
-Script wasm://wasm/befe41aa byte offset 44: Wasm opcode 0x3
-at wasm_B (0:44):
+Script wasm://wasm/befe41aa byte offset 46: Wasm opcode 0x20
+at wasm_B (0:46):
  - scope (global):
    -- skipped
  - scope (local):
@@ -161,19 +161,6 @@ at (anonymous) (0:17):
  - scope (global):
    -- skipped
 Debugger.stepInto called
-Script wasm://wasm/befe41aa byte offset 44: Wasm opcode 0x3
-at wasm_B (0:44):
- - scope (global):
-   -- skipped
- - scope (local):
-   locals: {"var0":1}
-   stack: {}
- - scope (wasm-expression-stack):
-   {}
-at (anonymous) (0:17):
- - scope (global):
-   -- skipped
-Debugger.stepInto called
 Script wasm://wasm/befe41aa byte offset 46: Wasm opcode 0x20
 at wasm_B (0:46):
  - scope (global):

test/inspector/debugger/wasm-stepping-byte-offsets.js

@@ -83,8 +83,8 @@ function instantiate(bytes) {
   await waitForPauseAndStep('resume');    // to next breakpoint (3rd iteration)
   await waitForPauseAndStep('stepInto');  // into wasm_A
   await waitForPauseAndStep('stepOut');   // out to wasm_B
-  // Now step 10 times, until we are in wasm_A again.
-  for (let i = 0; i < 10; ++i) await waitForPauseAndStep('stepInto');
+  // Now step 9 times, until we are in wasm_A again.
+  for (let i = 0; i < 9; ++i) await waitForPauseAndStep('stepInto');
   // 3 more times, back to wasm_B.
   for (let i = 0; i < 3; ++i) await waitForPauseAndStep('stepInto');
   // Then just resume.

test/inspector/debugger/wasm-stepping-with-source-map-expected.txt

@@ -187,19 +187,6 @@ at (anonymous) (0:17):
  - scope (global):
    -- skipped
 Debugger.stepInto called
-Script wasm://wasm/9b4bf87e byte offset 43: Wasm opcode 0x3
-at wasm_B (0:43):
- - scope (global):
-   -- skipped
- - scope (local):
-   locals: {"var0":1}
-   stack: {}
- - scope (wasm-expression-stack):
-   {}
-at (anonymous) (0:17):
- - scope (global):
-   -- skipped
-Debugger.stepInto called
 Script wasm://wasm/9b4bf87e byte offset 45: Wasm opcode 0x20
 at wasm_B (0:45):
  - scope (global):

test/inspector/debugger/wasm-stepping-with-source-map.js

@@ -81,8 +81,8 @@ function instantiate(bytes) {
   await waitForPauseAndStep('stepInto');  // to call
   await waitForPauseAndStep('stepInto');  // into wasm_A
   await waitForPauseAndStep('stepOut');   // out to wasm_B
-  // now step 9 times, until we are in wasm_A again.
-  for (let i = 0; i < 9; ++i) await waitForPauseAndStep('stepInto');
+  // Now step 8 times, until we are in wasm_A again.
+  for (let i = 0; i < 8; ++i) await waitForPauseAndStep('stepInto');
   // 3 more times, back to wasm_B.
   for (let i = 0; i < 3; ++i) await waitForPauseAndStep('stepInto');
   // then just resume.