[wasm] Refactor decoding of local declarations and make more robust.

R=ahaas@chromium.org
BUG=

Review URL: https://codereview.chromium.org/1856413002

Cr-Commit-Position: refs/heads/master@{#35272}

src/wasm/ast-decoder.cc

@@ -412,15 +412,23 @@ class SR_WasmDecoder : public WasmDecoder {
     return toResult(tree);
   }
 
-  std::vector<LocalType>* DecodeLocalDeclsForTesting() {
+  bool DecodeLocalDecls(AstLocalDecls& decls) {
     DecodeLocalDecls();
-    if (failed()) return nullptr;
-    auto result = new std::vector<LocalType>();
-    result->reserve(local_type_vec_.size());
-    for (size_t i = 0; i < local_type_vec_.size(); i++) {
-      result->push_back(local_type_vec_[i]);
+    if (failed()) return false;
+    decls.decls_encoded_size = pc_offset();
+    decls.total_local_count = 0;
+    decls.local_types.reserve(local_type_vec_.size());
+    for (size_t pos = 0; pos < local_type_vec_.size();) {
+      uint32_t count = 0;
+      LocalType type = local_type_vec_[pos];
+      while (pos < local_type_vec_.size() && local_type_vec_[pos] == type) {
+        pos++;
+        count++;
+      }
+      decls.total_local_count += count;
+      decls.local_types.push_back(std::pair<LocalType, uint32_t>(type, count));
     }
-    return result;
+    return true;
   }
 
   BitVector* AnalyzeLoopAssignmentForTesting(const byte* pc,
@@ -1626,12 +1634,13 @@ class SR_WasmDecoder : public WasmDecoder {
   }
 };
 
-std::vector<LocalType>* DecodeLocalDeclsForTesting(
-    base::AccountingAllocator* allocator, const byte* start, const byte* end) {
-  Zone zone(allocator);
+bool DecodeLocalDecls(AstLocalDecls& decls, const byte* start,
+                      const byte* end) {
+  base::AccountingAllocator allocator;
+  Zone tmp(&allocator);
   FunctionBody body = {nullptr, nullptr, nullptr, start, end};
-  SR_WasmDecoder decoder(&zone, nullptr, body);
-  return decoder.DecodeLocalDeclsForTesting();
+  SR_WasmDecoder decoder(&tmp, nullptr, body);
+  return decoder.DecodeLocalDecls(decls);
 }
 
 TreeResult VerifyWasmCode(base::AccountingAllocator* allocator,
@@ -1700,19 +1709,14 @@ void PrintAst(base::AccountingAllocator* allocator, FunctionBody& body) {
   }
 
   // Print the local declarations.
-  std::vector<LocalType>* decls = decoder.DecodeLocalDeclsForTesting();
+  AstLocalDecls decls(&zone);
+  decoder.DecodeLocalDecls(decls);
   const byte* pc = decoder.pc();
   if (body.start != decoder.pc()) {
     printf("// locals:");
-    size_t pos = 0;
-    while (pos < decls->size()) {
-      LocalType type = decls->at(pos++);
-      size_t count = 1;
-      while (pos < decls->size() && decls->at(pos) == type) {
-        pos++;
-        count++;
-      }
-
+    for (auto p : decls.local_types) {
+      LocalType type = p.first;
+      uint32_t count = p.second;
       os << " " << count << " " << WasmOpcodes::TypeName(type);
     }
     os << std::endl;
@@ -1722,7 +1726,6 @@ void PrintAst(base::AccountingAllocator* allocator, FunctionBody& body) {
     }
     printf("\n");
   }
-  delete decls;
 
   printf("// body: \n");
   std::vector<int> arity_stack;

src/wasm/ast-decoder.h

@@ -220,8 +220,22 @@ enum ReadUnsignedLEB128ErrorCode { kNoError, kInvalidLEB128, kMissingLEB128 };
 ReadUnsignedLEB128ErrorCode ReadUnsignedLEB128Operand(const byte*, const byte*,
                                                       int*, uint32_t*);
 
-std::vector<LocalType>* DecodeLocalDeclsForTesting(
-    base::AccountingAllocator* allocator, const byte* start, const byte* end);
+struct AstLocalDecls {
+  // The size of the encoded declarations.
+  uint32_t decls_encoded_size;  // size of encoded declarations
+
+  // Total number of locals.
+  uint32_t total_local_count;
+
+  // List of {local type, count} pairs.
+  ZoneVector<std::pair<LocalType, uint32_t>> local_types;
+
+  // Constructor initializes the vector.
+  explicit AstLocalDecls(Zone* zone)
+      : decls_encoded_size(0), total_local_count(0), local_types(zone) {}
+};
+
+bool DecodeLocalDecls(AstLocalDecls& decls, const byte* start, const byte* end);
 BitVector* AnalyzeLoopAssignmentForTesting(Zone* zone, size_t num_locals,
                                            const byte* start, const byte* end);
 

src/wasm/decoder.h

@@ -329,7 +329,7 @@ class Decoder {
   }
 
   bool ok() const { return error_pc_ == nullptr; }
-  bool failed() const { return error_pc_ != nullptr; }
+  bool failed() const { return !error_msg_.is_empty(); }
   bool more() const { return pc_ < limit_; }
 
   const byte* start() { return start_; }

test/unittests/wasm/ast-decoder-unittest.cc

@@ -2197,26 +2197,41 @@ TEST_F(WasmOpcodeArityTest, SimpleExpressions) {
   EXPECT_ARITY(1, kExprI64ReinterpretF64);
 }
 
-typedef std::vector<LocalType>* LocalTypeMap;
+typedef ZoneVector<LocalType> LocalTypeMap;
 
 class LocalDeclDecoderTest : public TestWithZone {
  public:
+  base::AccountingAllocator allocator;
+
   size_t ExpectRun(LocalTypeMap map, size_t pos, LocalType expected,
                    size_t count) {
     for (size_t i = 0; i < count; i++) {
-      EXPECT_EQ(expected, map->at(pos++));
+      EXPECT_EQ(expected, map[pos++]);
     }
     return pos;
   }
+
+  LocalTypeMap Expand(AstLocalDecls& decls) {
+    ZoneVector<LocalType> map(zone());
+    for (auto p : decls.local_types) {
+      map.insert(map.end(), p.second, p.first);
+    }
+    return map;
+  }
 };
 
+TEST_F(LocalDeclDecoderTest, EmptyLocals) {
+  AstLocalDecls decls(zone());
+  bool result = DecodeLocalDecls(decls, nullptr, nullptr);
+  EXPECT_FALSE(result);
+}
+
 TEST_F(LocalDeclDecoderTest, NoLocals) {
   static const byte data[] = {0};
-  base::AccountingAllocator allocator;
-  LocalTypeMap map =
-      DecodeLocalDeclsForTesting(&allocator, data, data + sizeof(data));
-  EXPECT_EQ(0, map->size());
-  if (map) delete map;
+  AstLocalDecls decls(zone());
+  bool result = DecodeLocalDecls(decls, data, data + sizeof(data));
+  EXPECT_TRUE(result);
+  EXPECT_EQ(0, decls.total_local_count);
 }
 
 TEST_F(LocalDeclDecoderTest, OneLocal) {
@@ -2224,12 +2239,14 @@ TEST_F(LocalDeclDecoderTest, OneLocal) {
     LocalType type = kLocalTypes[i];
     const byte data[] = {
         1, 1, static_cast<byte>(WasmOpcodes::LocalTypeCodeFor(type))};
-    base::AccountingAllocator allocator;
-    LocalTypeMap map =
-        DecodeLocalDeclsForTesting(&allocator, data, data + sizeof(data));
-    EXPECT_EQ(1, map->size());
-    EXPECT_EQ(type, map->at(0));
-    if (map) delete map;
+    AstLocalDecls decls(zone());
+    bool result = DecodeLocalDecls(decls, data, data + sizeof(data));
+    EXPECT_TRUE(result);
+    EXPECT_EQ(1, decls.total_local_count);
+
+    LocalTypeMap map = Expand(decls);
+    EXPECT_EQ(1, map.size());
+    EXPECT_EQ(type, map.at(0));
   }
 }
 
@@ -2238,12 +2255,15 @@ TEST_F(LocalDeclDecoderTest, FiveLocals) {
     LocalType type = kLocalTypes[i];
     const byte data[] = {
         1, 5, static_cast<byte>(WasmOpcodes::LocalTypeCodeFor(type))};
-    base::AccountingAllocator allocator;
-    LocalTypeMap map =
-        DecodeLocalDeclsForTesting(&allocator, data, data + sizeof(data));
-    EXPECT_EQ(5, map->size());
+    AstLocalDecls decls(zone());
+    bool result = DecodeLocalDecls(decls, data, data + sizeof(data));
+    EXPECT_TRUE(result);
+    EXPECT_EQ(sizeof(data), decls.decls_encoded_size);
+    EXPECT_EQ(5, decls.total_local_count);
+
+    LocalTypeMap map = Expand(decls);
+    EXPECT_EQ(5, map.size());
     ExpectRun(map, 0, type, 5);
-    if (map) delete map;
   }
 }
 
@@ -2254,18 +2274,20 @@ TEST_F(LocalDeclDecoderTest, MixedLocals) {
         for (byte d = 0; d < 3; d++) {
           const byte data[] = {4, a,         kLocalI32, b,        kLocalI64,
                                c, kLocalF32, d,         kLocalF64};
-          base::AccountingAllocator allocator;
-          LocalTypeMap map =
-              DecodeLocalDeclsForTesting(&allocator, data, data + sizeof(data));
-          EXPECT_EQ(a + b + c + d, map->size());
+          AstLocalDecls decls(zone());
+          bool result = DecodeLocalDecls(decls, data, data + sizeof(data));
+          EXPECT_TRUE(result);
+          EXPECT_EQ(sizeof(data), decls.decls_encoded_size);
+          EXPECT_EQ(a + b + c + d, decls.total_local_count);
+
+          LocalTypeMap map = Expand(decls);
+          EXPECT_EQ(a + b + c + d, map.size());
 
           size_t pos = 0;
           pos = ExpectRun(map, pos, kAstI32, a);
           pos = ExpectRun(map, pos, kAstI64, b);
           pos = ExpectRun(map, pos, kAstF32, c);
           pos = ExpectRun(map, pos, kAstF64, d);
-
-          if (map) delete map;
         }
       }
     }
@@ -2282,14 +2304,16 @@ TEST_F(LocalDeclDecoderTest, UseEncoder) {
   local_decls.AddLocals(212, kAstI64);
   local_decls.Prepend(&data, &end);
 
-  base::AccountingAllocator allocator;
-  LocalTypeMap map = DecodeLocalDeclsForTesting(&allocator, data, end);
+  AstLocalDecls decls(zone());
+  bool result = DecodeLocalDecls(decls, data, end);
+  EXPECT_TRUE(result);
+  EXPECT_EQ(5 + 1337 + 212, decls.total_local_count);
+
+  LocalTypeMap map = Expand(decls);
   size_t pos = 0;
   pos = ExpectRun(map, pos, kAstF32, 5);
   pos = ExpectRun(map, pos, kAstI32, 1337);
   pos = ExpectRun(map, pos, kAstI64, 212);
-
-  if (map) delete map;
   delete[] data;
 }