[wasm] Make signature map move-only

Signature maps should only be updated, but never copied. We had a bug
because we accidentally updated a copy of the map. This refactoring
prevents any such bugs in the future, and fixes more occurences where
we accidentally copied structs containing a signature map (the move-only
constraint also extends to all structs containing a signature map).

Drive-by: Make InstanceBuilder::NeedsWrappers const.

R=titzer@chromium.org

Bug: chromium:741750
Change-Id: Id919203d8c4078e608a1163e5c790c97d06a9753
Reviewed-on: https://chromium-review.googlesource.com/571791Reviewed-by: Ben Titzer <titzer@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46728}

src/wasm/module-compiler.cc

@@ -1524,12 +1524,12 @@ Handle<JSArrayBuffer> InstanceBuilder::AllocateMemory(uint32_t min_mem_pages) {
   return mem_buffer;
 }
 
-bool InstanceBuilder::NeedsWrappers() {
+bool InstanceBuilder::NeedsWrappers() const {
   if (module_->num_exported_functions > 0) return true;
-  for (auto table_instance : table_instances_) {
+  for (auto& table_instance : table_instances_) {
     if (!table_instance.js_wrappers.is_null()) return true;
   }
-  for (auto table : module_->function_tables) {
+  for (auto& table : module_->function_tables) {
     if (table.exported) return true;
   }
   return false;
@@ -1801,10 +1801,10 @@ void InstanceBuilder::LoadTableSegments(Handle<FixedArray> code_table,
     // since initializations are not sorted by table index.
     for (auto table_init : module_->table_inits) {
       uint32_t base = EvalUint32InitExpr(table_init.offset);
-      DCHECK(in_bounds(base, static_cast<uint32_t>(table_init.entries.size()),
+      uint32_t num_entries = static_cast<uint32_t>(table_init.entries.size());
+      DCHECK(in_bounds(base, num_entries,
                        table_instance.function_table->length()));
-      for (int i = 0, e = static_cast<int>(table_init.entries.size()); i < e;
-           ++i) {
+      for (uint32_t i = 0; i < num_entries; ++i) {
         uint32_t func_index = table_init.entries[i];
         WasmFunction* function = &module_->functions[func_index];
         int table_index = static_cast<int>(i + base);

src/wasm/module-compiler.h

@@ -289,7 +289,7 @@ class InstanceBuilder {
   // Allocate memory for a module instance as a new JSArrayBuffer.
   Handle<JSArrayBuffer> AllocateMemory(uint32_t min_mem_pages);
 
-  bool NeedsWrappers();
+  bool NeedsWrappers() const;
 
   // Process the exports, creating wrappers for functions, tables, memories,
   // and globals.

src/wasm/module-decoder.cc

@@ -444,11 +444,10 @@ class ModuleDecoder : public Decoder {
           if (!AddTable(module_.get())) break;
           import->index =
               static_cast<uint32_t>(module_->function_tables.size());
-          module_->function_tables.push_back({0, 0, false,
-                                              std::vector<int32_t>(), true,
-                                              false, SignatureMap()});
-          expect_u8("element type", kWasmAnyFunctionTypeForm);
+          module_->function_tables.emplace_back();
           WasmIndirectFunctionTable* table = &module_->function_tables.back();
+          table->imported = true;
+          expect_u8("element type", kWasmAnyFunctionTypeForm);
           consume_resizable_limits("element count", "elements",
                                    FLAG_wasm_max_table_size, &table->min_size,
                                    &table->has_max, FLAG_wasm_max_table_size,
@@ -508,8 +507,7 @@ class ModuleDecoder : public Decoder {
 
     for (uint32_t i = 0; ok() && i < table_count; i++) {
       if (!AddTable(module_.get())) break;
-      module_->function_tables.push_back(
-          {0, 0, false, std::vector<int32_t>(), false, false, SignatureMap()});
+      module_->function_tables.emplace_back();
       WasmIndirectFunctionTable* table = &module_->function_tables.back();
       expect_u8("table type", kWasmAnyFunctionTypeForm);
       consume_resizable_limits("table elements", "elements",

src/wasm/signature-map.h

@@ -19,6 +19,12 @@ namespace wasm {
 // same index.
 class V8_EXPORT_PRIVATE SignatureMap {
  public:
+  // Allow default construction and move construction (because we have vectors
+  // of objects containing SignatureMaps), but disallow copy or assign. It's
+  // too easy to get security bugs by accidentally updating a copy of the map.
+  SignatureMap() = default;
+  SignatureMap(SignatureMap&&) = default;
+
   // Gets the index for a signature, assigning a new index if necessary.
   uint32_t FindOrInsert(FunctionSig* sig);
 
@@ -32,6 +38,8 @@ class V8_EXPORT_PRIVATE SignatureMap {
   };
   uint32_t next_ = 0;
   std::map<FunctionSig*, uint32_t, CompareFunctionSigs> map_;
+
+  DISALLOW_COPY_AND_ASSIGN(SignatureMap);
 };
 
 }  // namespace wasm

src/wasm/wasm-module.h

@@ -134,13 +134,13 @@ struct WasmDataSegment {
 
 // Static representation of a wasm indirect call table.
 struct WasmIndirectFunctionTable {
-  uint32_t min_size;            // minimum table size.
-  uint32_t max_size;            // maximum table size.
-  bool has_max;                 // true if there is a maximum size.
+  uint32_t min_size = 0;  // minimum table size.
+  uint32_t max_size = 0;  // maximum table size.
+  bool has_max = false;   // true if there is a maximum size.
   // TODO(titzer): Move this to WasmInstance. Needed by interpreter only.
   std::vector<int32_t> values;  // function table, -1 indicating invalid.
-  bool imported;                // true if imported.
-  bool exported;                // true if exported.
+  bool imported = false;        // true if imported.
+  bool exported = false;        // true if exported.
   SignatureMap map;             // canonicalizing map for sig indexes.
 };
 

test/cctest/wasm/wasm-run-utils.h

@@ -251,12 +251,11 @@ class TestingModule : public ModuleEnv {
 
   void AddIndirectFunctionTable(uint16_t* function_indexes,
                                 uint32_t table_size) {
-    module_.function_tables.push_back({table_size, table_size, true,
-                                       std::vector<int32_t>(), false, false,
-                                       SignatureMap()});
+    module_.function_tables.emplace_back();
     WasmIndirectFunctionTable& table = module_.function_tables.back();
     table.min_size = table_size;
     table.max_size = table_size;
+    table.has_max = true;
     for (uint32_t i = 0; i < table_size; ++i) {
       table.values.push_back(function_indexes[i]);
       table.map.FindOrInsert(module_.functions[function_indexes[i]].sig);

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

@@ -227,10 +227,7 @@ class TestModuleEnv : public ModuleEnv {
     mod.max_mem_pages = 100;
   }
 
-  void InitializeFunctionTable() {
-    mod.function_tables.push_back(
-        {0, 0, true, std::vector<int32_t>(), false, false, SignatureMap()});
-  }
+  void InitializeFunctionTable() { mod.function_tables.emplace_back(); }
 
  private:
   WasmModule mod;