[wasm][refactor] Rework immediate-argument abstractions

Motivation:
The immediate-argument classes defined in function-body-decoder.h were
often adding an offset to the provided pc. This was inconsistent,
bug-prone, and counterintuitive. This CL imposes that all immediates
are passed as pc the start of the immediate argument they are parsing.
Some other smaller inconsistencies are fixed as well.

Changes:

src/wasm/:
- Enforce that all Immediates are passed the pc at the start of the
  argument they are parsing. Adapt all call sites.
- Remove unneeded offset arguments from two SIMD related immediates.
- Add a pc argument to all Validate functions for immediates instead
  of using the Decoder's current pc.
- Remove the (unused) pc argument from all Complete functions for
  immediates.
- Introduce Validate() for BranchOnExceptionImmediate.
- In WasmDecoder::Decode(), make sure len is updated before breaking out
  of the loop in case of a Validate() failure.
- Change the default prefix_len of DecodeLoadMem/DecodeStoreMem to 1.

wasm-interpreter.cc:
- Change the default prefix_len of ExecuteLoad/Store to 1.
- Adapt offsets in calls to Immediates.
- Remove redundant opcode_length argument from ExecuteSimdOp, use len
  in its place.

function-body-decoder-unittest.cc
- Adapt offsets in calls to Immediates.
- Introduce and use EXPECT_OK, as is done in other tests.

Change-Id: I534606c0e238af309804d4a7c8cec75b1e49c6ad
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2267381
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68559}

src/wasm/function-body-decoder.cc

@@ -244,7 +244,7 @@ bool PrintRawWasmCode(AccountingAllocator* allocator, const FunctionBody& body,
       case kExprBlock:
       case kExprTry: {
         BlockTypeImmediate<Decoder::kNoValidate> imm(WasmFeatures::All(), &i,
-                                                     i.pc());
+                                                     i.pc() + 1);
         os << " @" << i.pc_offset();
         if (decoder.Complete(imm)) {
           for (uint32_t i = 0; i < imm.out_arity(); i++) {
@@ -259,33 +259,33 @@ bool PrintRawWasmCode(AccountingAllocator* allocator, const FunctionBody& body,
         control_depth--;
         break;
       case kExprBr: {
-        BranchDepthImmediate<Decoder::kNoValidate> imm(&i, i.pc());
+        BranchDepthImmediate<Decoder::kNoValidate> imm(&i, i.pc() + 1);
         os << " depth=" << imm.depth;
         break;
       }
       case kExprBrIf: {
-        BranchDepthImmediate<Decoder::kNoValidate> imm(&i, i.pc());
+        BranchDepthImmediate<Decoder::kNoValidate> imm(&i, i.pc() + 1);
         os << " depth=" << imm.depth;
         break;
       }
       case kExprBrTable: {
-        BranchTableImmediate<Decoder::kNoValidate> imm(&i, i.pc());
+        BranchTableImmediate<Decoder::kNoValidate> imm(&i, i.pc() + 1);
         os << " entries=" << imm.table_count;
         break;
       }
       case kExprCallIndirect: {
         CallIndirectImmediate<Decoder::kNoValidate> imm(WasmFeatures::All(), &i,
-                                                        i.pc());
+                                                        i.pc() + 1);
         os << " sig #" << imm.sig_index;
-        if (decoder.Complete(i.pc(), imm)) {
+        if (decoder.Complete(imm)) {
           os << ": " << *imm.sig;
         }
         break;
       }
       case kExprCallFunction: {
-        CallFunctionImmediate<Decoder::kNoValidate> imm(&i, i.pc());
+        CallFunctionImmediate<Decoder::kNoValidate> imm(&i, i.pc() + 1);
         os << " function #" << imm.index;
-        if (decoder.Complete(i.pc(), imm)) {
+        if (decoder.Complete(imm)) {
           os << ": " << *imm.sig;
         }
         break;

src/wasm/module-decoder.cc

@@ -1610,7 +1610,7 @@ class ModuleDecoderImpl : public Decoder {
     uint32_t len = 0;
     switch (opcode) {
       case kExprGlobalGet: {
-        GlobalIndexImmediate<Decoder::kValidate> imm(this, pc() - 1);
+        GlobalIndexImmediate<Decoder::kValidate> imm(this, pc());
         if (module->globals.size() <= imm.index) {
           error("global index is out of bounds");
           expr.kind = WasmInitExpr::kNone;
@@ -1632,28 +1632,28 @@ class ModuleDecoderImpl : public Decoder {
         break;
       }
       case kExprI32Const: {
-        ImmI32Immediate<Decoder::kValidate> imm(this, pc() - 1);
+        ImmI32Immediate<Decoder::kValidate> imm(this, pc());
         expr.kind = WasmInitExpr::kI32Const;
         expr.val.i32_const = imm.value;
         len = imm.length;
         break;
       }
       case kExprF32Const: {
-        ImmF32Immediate<Decoder::kValidate> imm(this, pc() - 1);
+        ImmF32Immediate<Decoder::kValidate> imm(this, pc());
         expr.kind = WasmInitExpr::kF32Const;
         expr.val.f32_const = imm.value;
         len = imm.length;
         break;
       }
       case kExprI64Const: {
-        ImmI64Immediate<Decoder::kValidate> imm(this, pc() - 1);
+        ImmI64Immediate<Decoder::kValidate> imm(this, pc());
         expr.kind = WasmInitExpr::kI64Const;
         expr.val.i64_const = imm.value;
         len = imm.length;
         break;
       }
       case kExprF64Const: {
-        ImmF64Immediate<Decoder::kValidate> imm(this, pc() - 1);
+        ImmF64Immediate<Decoder::kValidate> imm(this, pc());
         expr.kind = WasmInitExpr::kF64Const;
         expr.val.f64_const = imm.value;
         len = imm.length;
@@ -1662,7 +1662,7 @@ class ModuleDecoderImpl : public Decoder {
       case kExprRefNull: {
         if (enabled_features_.has_reftypes() || enabled_features_.has_eh()) {
           HeapTypeImmediate<Decoder::kValidate> imm(WasmFeatures::All(), this,
-                                                    pc() - 1);
+                                                    pc());
           expr.kind = WasmInitExpr::kRefNullConst;
           len = imm.length;
           ValueType type = ValueType::Ref(imm.type, kNullable);
@@ -1677,7 +1677,7 @@ class ModuleDecoderImpl : public Decoder {
       }
       case kExprRefFunc: {
         if (enabled_features_.has_reftypes()) {
-          FunctionIndexImmediate<Decoder::kValidate> imm(this, pc() - 1);
+          FunctionIndexImmediate<Decoder::kValidate> imm(this, pc());
           if (module->functions.size() <= imm.index) {
             errorf(pc() - 1, "invalid function index: %u", imm.index);
             break;
@@ -2023,8 +2023,7 @@ class ModuleDecoderImpl : public Decoder {
     if (failed()) return index;
     switch (opcode) {
       case kExprRefNull: {
-        HeapTypeImmediate<kValidate> imm(WasmFeatures::All(), this,
-                                         this->pc() - 1);
+        HeapTypeImmediate<kValidate> imm(WasmFeatures::All(), this, this->pc());
         consume_bytes(imm.length, "ref.null immediate");
         index = WasmElemSegment::kNullIndex;
         break;

test/unittests/wasm/function-body-decoder-unittest.cc

@@ -33,6 +33,14 @@ namespace function_body_decoder_unittest {
 #define WASM_IF_OP kExprIf, kLocalVoid
 #define WASM_LOOP_OP kExprLoop, kLocalVoid
 
+#define EXPECT_OK(result)                                        \
+  do {                                                           \
+    if (!result.ok()) {                                          \
+      GTEST_NONFATAL_FAILURE_(result.error().message().c_str()); \
+      return;                                                    \
+    }                                                            \
+  } while (false)
+
 static const byte kCodeGetLocal0[] = {kExprLocalGet, 0};
 static const byte kCodeGetLocal1[] = {kExprLocalGet, 1};
 static const byte kCodeSetLocal0[] = {WASM_SET_LOCAL(0, WASM_ZERO)};
@@ -3398,14 +3406,14 @@ class BranchTableIteratorTest : public TestWithZone {
   BranchTableIteratorTest() : TestWithZone() {}
   void CheckBrTableSize(const byte* start, const byte* end) {
     Decoder decoder(start, end);
-    BranchTableImmediate<Decoder::kValidate> operand(&decoder, start);
+    BranchTableImmediate<Decoder::kValidate> operand(&decoder, start + 1);
     BranchTableIterator<Decoder::kValidate> iterator(&decoder, operand);
     EXPECT_EQ(end - start - 1u, iterator.length());
-    EXPECT_TRUE(decoder.ok());
+    EXPECT_OK(decoder);
   }
   void CheckBrTableError(const byte* start, const byte* end) {
     Decoder decoder(start, end);
-    BranchTableImmediate<Decoder::kValidate> operand(&decoder, start);
+    BranchTableImmediate<Decoder::kValidate> operand(&decoder, start + 1);
     BranchTableIterator<Decoder::kValidate> iterator(&decoder, operand);
     iterator.length();
     EXPECT_FALSE(decoder.ok());
@@ -3872,6 +3880,7 @@ TEST_F(BytecodeIteratorTest, WithLocalDecls) {
 #undef WASM_IF_OP
 #undef WASM_LOOP_OP
 #undef WASM_BRV_IF_ZERO
+#undef EXPECT_OK
 
 }  // namespace function_body_decoder_unittest
 }  // namespace wasm