[wasm] Fix BytecodeIterator

The {BytecodeIterator} that also read locals declarations had a weird
fallback path to allow decoding of locals to fail, and just assume no
locals, decoding all bytes as code.

This CL removes that, and modifies the test that needs this
functionality to prepend a zero byte for encoding "0 locals".

R=jkummerow@chromium.org

Change-Id: I609e2bf6986eeb6380d65b03bf8512f0b09af764
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4003078
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84088}

src/wasm/function-body-decoder.cc

@@ -46,10 +46,11 @@ BytecodeIterator::BytecodeIterator(const byte* start, const byte* end,
     : Decoder(start, end) {
   DCHECK_NOT_NULL(decls);
   DCHECK_NOT_NULL(zone);
-  if (DecodeLocalDecls(WasmFeatures::All(), decls, nullptr, start, end, zone)) {
-    pc_ += decls->encoded_size;
-    if (pc_ > end_) pc_ = end_;
-  }
+  constexpr const WasmModule* kNoModule = nullptr;
+  CHECK(DecodeLocalDecls(WasmFeatures::All(), decls, kNoModule, start, end,
+                         zone));
+  pc_ += decls->encoded_size;
+  if (pc_ > end_) pc_ = end_;
 }
 
 DecodeResult ValidateFunctionBody(AccountingAllocator* allocator,

test/common/wasm/wasm-interpreter.cc

@@ -4200,7 +4200,7 @@ void WasmInterpreter::SetFunctionCodeForTesting(const WasmFunction* function,
 }
 
 ControlTransferMap WasmInterpreter::ComputeControlTransfersForTesting(
-    Zone* zone, const WasmModule* module, const byte* start, const byte* end) {
+    Zone* zone, const byte* start, const byte* end) {
   // Create some dummy structures, to avoid special-casing the implementation
   // just for testing.
   FunctionSig sig(0, 0, nullptr);
@@ -4214,7 +4214,8 @@ ControlTransferMap WasmInterpreter::ComputeControlTransfersForTesting(
   InterpreterCode code{&function, BodyLocalDecls{}, start, end, nullptr};
 
   // Now compute and return the control transfers.
-  SideTable side_table(zone, module, &code);
+  constexpr const WasmModule* kNoModule = nullptr;
+  SideTable side_table(zone, kNoModule, &code);
   return side_table.map_;
 }
 

test/common/wasm/wasm-interpreter.h

@@ -111,8 +111,9 @@ class WasmInterpreter {
 
   // Computes the control transfers for the given bytecode. Used internally in
   // the interpreter, but exposed for testing.
-  static ControlTransferMap ComputeControlTransfersForTesting(
-      Zone* zone, const WasmModule* module, const byte* start, const byte* end);
+  static ControlTransferMap ComputeControlTransfersForTesting(Zone* zone,
+                                                              const byte* start,
+                                                              const byte* end);
 
  private:
   Zone zone_;

test/unittests/wasm/control-transfer-unittest.cc

@@ -59,19 +59,23 @@ class ControlTransferMatcher
 
 class ControlTransferTest : public TestWithZone {
  public:
-  template <int code_len>
+  static constexpr pc_t kPcShiftForLocalsCount = 1;
+
+  template <int kCodeLen>
   void CheckTransfers(
-      const byte (&code)[code_len],
+      const byte (&code)[kCodeLen],
       std::initializer_list<ExpectedControlTransfer> expected_transfers) {
-    byte code_with_end[code_len + 1];  // NOLINT: code_len is a constant here
-    memcpy(code_with_end, code, code_len);
-    code_with_end[code_len] = kExprEnd;
+    // Add a leading '0' for number of locals and append the 'end' opcode.
+    byte function_body[kCodeLen + 2];
+    function_body[0] = 0;
+    memcpy(function_body + 1, code, kCodeLen);
+    function_body[kCodeLen + 1] = kExprEnd;
 
     ControlTransferMap map = WasmInterpreter::ComputeControlTransfersForTesting(
-        zone(), nullptr, code_with_end, code_with_end + code_len + 1);
+        zone(), std::begin(function_body), std::end(function_body));
     // Check all control targets in the map.
     for (auto& expected_transfer : expected_transfers) {
-      pc_t pc = expected_transfer.pc;
+      pc_t pc = expected_transfer.pc + kPcShiftForLocalsCount;
       EXPECT_TRUE(map.map.count(pc) > 0) << "expected control target @" << pc;
       if (!map.map.count(pc)) continue;
       auto& entry = map.map[pc];
@@ -80,7 +84,7 @@ class ControlTransferTest : public TestWithZone {
     }
 
     // Check there are no other control targets.
-    CheckNoOtherTargets(code_with_end, code_with_end + code_len + 1, map,
+    CheckNoOtherTargets(std::begin(function_body), std::end(function_body), map,
                         expected_transfers);
   }
 
@@ -91,7 +95,7 @@ class ControlTransferTest : public TestWithZone {
     for (pc_t pc = 0; start + pc < end; pc++) {
       bool found = false;
       for (auto& target : targets) {
-        if (target.pc == pc) {
+        if (target.pc + kPcShiftForLocalsCount == pc) {
           found = true;
           break;
         }