[wasm-gc] Fix interaction between AnalyzeLoopAssignment and 'let'

AnalyzeLoopAssignment did not take into account that 'let' shifts local
indexes.

Drive-by: Use gTest infrastructure in AnalyzeLoopAssignment tests
(EXPECT_*) instead of CHECKs.

Bug: v8:9495
Change-Id: Ic0ddb5edfde48acf172f4cac9bdcd0312b6121a0
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2567955
Commit-Queue: Manos Koukoutos <manoskouk@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71595}

src/wasm/function-body-decoder-impl.h

@@ -1217,7 +1217,12 @@ class WasmDecoder : public Decoder {
     // The number of locals_count is augmented by 1 so that the 'locals_count'
     // index can be used to track the instance cache.
     BitVector* assigned = zone->New<BitVector>(locals_count + 1, zone);
-    int depth = 0;
+    int depth = -1;  // We will increment the depth to 0 when we decode the
+                     // starting 'loop' opcode.
+    // Since 'let' can add additional locals at the beginning of the locals
+    // index space, we need to track this offset for every depth up to the
+    // current depth.
+    base::SmallVector<uint32_t, 8> local_offsets(8);
     // Iteratively process all AST nodes nested inside the loop.
     while (pc < decoder->end() && VALIDATE(decoder->ok())) {
       WasmOpcode opcode = static_cast<WasmOpcode>(*pc);
@@ -1226,15 +1231,29 @@ class WasmDecoder : public Decoder {
         case kExprIf:
         case kExprBlock:
         case kExprTry:
-        case kExprLet:
           depth++;
+          local_offsets.resize_no_init(depth + 1);
+          // No additional locals.
+          local_offsets[depth] = depth > 0 ? local_offsets[depth - 1] : 0;
+          break;
+        case kExprLet: {
+          depth++;
+          local_offsets.resize_no_init(depth + 1);
+          BlockTypeImmediate<validate> imm(WasmFeatures::All(), decoder, pc + 1,
+                                           nullptr);
+          uint32_t locals_length;
+          int new_locals_count = decoder->DecodeLocals(
+              pc + 1 + imm.length, &locals_length, base::Optional<uint32_t>());
+          local_offsets[depth] = local_offsets[depth - 1] + new_locals_count;
           break;
+        }
         case kExprLocalSet:
         case kExprLocalTee: {
           LocalIndexImmediate<validate> imm(decoder, pc + 1);
-          if (imm.index < locals_count) {
           // Unverified code might have an out-of-bounds index.
-            assigned->Add(imm.index);
+          if (imm.index >= local_offsets[depth] &&
+              imm.index - local_offsets[depth] < locals_count) {
+            assigned->Add(imm.index - local_offsets[depth]);
           }
           break;
         }
@@ -1254,7 +1273,7 @@ class WasmDecoder : public Decoder {
         default:
           break;
       }
-      if (depth <= 0) break;
+      if (depth < 0) break;
       pc += OpcodeLength(decoder, pc);
     }
     return VALIDATE(decoder->ok()) ? assigned : nullptr;

test/unittests/wasm/loop-assignment-analysis-unittest.cc

@@ -34,7 +34,7 @@ class WasmLoopAssignmentAnalyzerTest : public TestWithZone {
 TEST_F(WasmLoopAssignmentAnalyzerTest, Empty0) {
   byte code[] = { 0 };
   BitVector* assigned = Analyze(code, code);
-  CHECK_NULL(assigned);
+  EXPECT_EQ(assigned, nullptr);
 }
 
 TEST_F(WasmLoopAssignmentAnalyzerTest, Empty1) {
@@ -42,7 +42,7 @@ TEST_F(WasmLoopAssignmentAnalyzerTest, Empty1) {
   for (int i = 0; i < 5; i++) {
     BitVector* assigned = Analyze(code, code + arraysize(code));
     for (int j = 0; j < assigned->length(); j++) {
-      CHECK_EQ(false, assigned->Contains(j));
+      EXPECT_FALSE(assigned->Contains(j));
     }
     num_locals++;
   }
@@ -54,7 +54,7 @@ TEST_F(WasmLoopAssignmentAnalyzerTest, One) {
     byte code[] = {WASM_LOOP(WASM_SET_ZERO(i))};
     BitVector* assigned = Analyze(code, code + arraysize(code));
     for (int j = 0; j < assigned->length(); j++) {
-      CHECK_EQ(j == i, assigned->Contains(j));
+      EXPECT_EQ(j == i, assigned->Contains(j));
     }
   }
 }
@@ -65,7 +65,7 @@ TEST_F(WasmLoopAssignmentAnalyzerTest, TeeOne) {
     byte code[] = {WASM_LOOP(WASM_TEE_LOCAL(i, WASM_ZERO))};
     BitVector* assigned = Analyze(code, code + arraysize(code));
     for (int j = 0; j < assigned->length(); j++) {
-      CHECK_EQ(j == i, assigned->Contains(j));
+      EXPECT_EQ(j == i, assigned->Contains(j));
     }
   }
 }
@@ -76,7 +76,7 @@ TEST_F(WasmLoopAssignmentAnalyzerTest, OneBeyond) {
     byte code[] = {WASM_LOOP(WASM_SET_ZERO(i)), WASM_SET_ZERO(1)};
     BitVector* assigned = Analyze(code, code + arraysize(code));
     for (int j = 0; j < assigned->length(); j++) {
-      CHECK_EQ(j == i, assigned->Contains(j));
+      EXPECT_EQ(j == i, assigned->Contains(j));
     }
   }
 }
@@ -89,7 +89,7 @@ TEST_F(WasmLoopAssignmentAnalyzerTest, Two) {
       BitVector* assigned = Analyze(code, code + arraysize(code));
       for (int k = 0; k < assigned->length(); k++) {
         bool expected = k == i || k == j;
-        CHECK_EQ(expected, assigned->Contains(k));
+        EXPECT_EQ(expected, assigned->Contains(k));
       }
     }
   }
@@ -103,7 +103,7 @@ TEST_F(WasmLoopAssignmentAnalyzerTest, NestedIf) {
     BitVector* assigned = Analyze(code, code + arraysize(code));
     for (int j = 0; j < assigned->length(); j++) {
       bool expected = i == j || j == 0 || j == 1;
-      CHECK_EQ(expected, assigned->Contains(j));
+      EXPECT_EQ(expected, assigned->Contains(j));
     }
   }
 }
@@ -116,7 +116,7 @@ TEST_F(WasmLoopAssignmentAnalyzerTest, BigLocal) {
     BitVector* assigned = Analyze(code, code + arraysize(code));
     for (int j = 0; j < assigned->length(); j++) {
       bool expected = i == j;
-      CHECK_EQ(expected, assigned->Contains(j));
+      EXPECT_EQ(expected, assigned->Contains(j));
     }
   }
 }
@@ -130,7 +130,7 @@ TEST_F(WasmLoopAssignmentAnalyzerTest, Break) {
   BitVector* assigned = Analyze(code, code + arraysize(code));
   for (int j = 0; j < assigned->length(); j++) {
     bool expected = j == 1;
-    CHECK_EQ(expected, assigned->Contains(j));
+    EXPECT_EQ(expected, assigned->Contains(j));
   }
 }
 
@@ -146,7 +146,7 @@ TEST_F(WasmLoopAssignmentAnalyzerTest, Loop1) {
   BitVector* assigned = Analyze(code, code + arraysize(code));
   for (int j = 0; j < assigned->length(); j++) {
     bool expected = j == 3;
-    CHECK_EQ(expected, assigned->Contains(j));
+    EXPECT_EQ(expected, assigned->Contains(j));
   }
 }
 
@@ -171,7 +171,7 @@ TEST_F(WasmLoopAssignmentAnalyzerTest, Loop2) {
   BitVector* assigned = Analyze(code + 2, code + arraysize(code));
   for (int j = 0; j < assigned->length(); j++) {
     bool expected = j == kIter || j == kSum;
-    CHECK_EQ(expected, assigned->Contains(j));
+    EXPECT_EQ(expected, assigned->Contains(j));
   }
 }
 
@@ -180,7 +180,7 @@ TEST_F(WasmLoopAssignmentAnalyzerTest, Malformed) {
                  'e',       'l',       'l',         'o',          ',',  ' ',
                  'w',       'o',       'r',         'l',          'd',  '!'};
   BitVector* assigned = Analyze(code, code + arraysize(code));
-  CHECK_NULL(assigned);
+  EXPECT_EQ(assigned, nullptr);
 }
 
 TEST_F(WasmLoopAssignmentAnalyzerTest, InvalidOpcode) {
@@ -197,6 +197,39 @@ TEST_F(WasmLoopAssignmentAnalyzerTest, regress_642867) {
   Analyze(code, code + arraysize(code));
 }
 
+TEST_F(WasmLoopAssignmentAnalyzerTest, LetInLoopAssigned) {
+  num_locals = 5;
+  static const byte code[] = {
+      WASM_LOOP(WASM_LET_1_V(kI32Code, WASM_I32V_1(42), WASM_SET_ZERO(3)))};
+  BitVector* assigned = Analyze(code, code + arraysize(code));
+  for (uint32_t i = 0; i <= num_locals; i++) {
+    EXPECT_EQ(assigned->Contains(i), i == 2);
+  }
+}
+
+TEST_F(WasmLoopAssignmentAnalyzerTest, LetInLoopNotAssigned) {
+  num_locals = 2;
+  static const byte code[] = {WASM_LOOP(
+      WASM_LET_1_V(kI32Code, WASM_I32V_1(42),
+                   WASM_LET_1_V(kI32Code, WASM_I32V_1(42), WASM_SET_ZERO(0),
+                                WASM_SET_ZERO(1))))};
+  BitVector* assigned = Analyze(code, code + arraysize(code));
+  for (uint32_t i = 0; i <= num_locals; i++) {
+    EXPECT_FALSE(assigned->Contains(i));
+  }
+}
+
+TEST_F(WasmLoopAssignmentAnalyzerTest, AssignmentOutsideOfLet) {
+  num_locals = 5;
+  static const byte code[] = {
+      WASM_LOOP(WASM_LET_1_V(kI32Code, WASM_I32V_1(42), WASM_SET_ZERO(3)),
+                WASM_SET_ZERO(4))};
+  BitVector* assigned = Analyze(code, code + arraysize(code));
+  for (uint32_t i = 0; i <= num_locals; i++) {
+    EXPECT_EQ(assigned->Contains(i), i == 2 || i == 4);
+  }
+}
+
 #undef WASM_SET_ZERO
 
 }  // namespace wasm