From 7e4e5d0fd6f29fb35d5ff3b2c300d534bf102a48 Mon Sep 17 00:00:00 2001
From: Clemens Hammacher <clemensh@chromium.org>
Date: Wed, 20 Jun 2018 08:33:57 +0200
Subject: [PATCH] [wasm][test] Fix WasmModule::num_declared_functions

Our tests currently don't set {WasmModule::num_declared_functions}
correctly. This CL fixes that.
This enables the use of {WasmModule::num_declared_functions} instead of
{NativeModule::num_functions_ - NativeModule::num_imported_functions_}.

Drive-by: Fix {std::vector} reservation to reserve enough space for all
functions during decoding.

R=titzer@chromium.org

Change-Id: I6d7783aed1c0de3275fc72787dec17c38ff8c73b
Reviewed-on: https://chromium-review.googlesource.com/1106166
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Ben Titzer <titzer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#53857}
---
 src/wasm/module-decoder.cc         |  9 +++++++--
 src/wasm/wasm-code-manager.cc      |  9 ++++++---
 src/wasm/wasm-module.h             |  2 +-
 test/cctest/wasm/wasm-run-utils.cc | 21 +++++++++++++++------
 test/cctest/wasm/wasm-run-utils.h  |  3 ++-
 5 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/src/wasm/module-decoder.cc b/src/wasm/module-decoder.cc
index 75903f5f6fa..8fc75b18951 100644
--- a/src/wasm/module-decoder.cc
+++ b/src/wasm/module-decoder.cc
@@ -525,9 +525,12 @@ class ModuleDecoderImpl : public Decoder {
     auto counter =
         SELECT_WASM_COUNTER(GetCounters(), origin_, wasm_functions_per, module);
     counter->AddSample(static_cast<int>(functions_count));
-    module_->functions.reserve(functions_count);
+    DCHECK_EQ(module_->functions.size(), module_->num_imported_functions);
+    uint32_t total_function_count =
+        module_->num_imported_functions + functions_count;
+    module_->functions.reserve(total_function_count);
     module_->num_declared_functions = functions_count;
-    for (uint32_t i = 0; ok() && i < functions_count; ++i) {
+    for (uint32_t i = 0; i < functions_count; ++i) {
       uint32_t func_index = static_cast<uint32_t>(module_->functions.size());
       module_->functions.push_back({nullptr,     // sig
                                     func_index,  // func_index
@@ -537,7 +540,9 @@ class ModuleDecoderImpl : public Decoder {
                                     false});     // exported
       WasmFunction* function = &module_->functions.back();
       function->sig_index = consume_sig_index(module_.get(), &function->sig);
+      if (!ok()) return;
     }
+    DCHECK_EQ(module_->functions.size(), total_function_count);
   }
 
   void DecodeTableSection() {
diff --git a/src/wasm/wasm-code-manager.cc b/src/wasm/wasm-code-manager.cc
index 9b3a36fe871..3542ed23ffc 100644
--- a/src/wasm/wasm-code-manager.cc
+++ b/src/wasm/wasm-code-manager.cc
@@ -881,9 +881,12 @@ size_t WasmCodeManager::EstimateNativeModuleSize(const WasmModule* module) {
 std::unique_ptr<NativeModule> WasmCodeManager::NewNativeModule(
     Isolate* isolate, const WasmModule& module, ModuleEnv& env) {
   size_t memory_estimate = EstimateNativeModuleSize(&module);
-  return NewNativeModule(
-      isolate, memory_estimate, static_cast<uint32_t>(module.functions.size()),
-      module.num_imported_functions, kModuleCanAllocateMoreMemory, env);
+  uint32_t num_wasm_functions =
+      module.num_imported_functions + module.num_declared_functions;
+  DCHECK_EQ(module.functions.size(), num_wasm_functions);
+  return NewNativeModule(isolate, memory_estimate, num_wasm_functions,
+                         module.num_imported_functions,
+                         kModuleCanAllocateMoreMemory, env);
 }
 
 std::unique_ptr<NativeModule> WasmCodeManager::NewNativeModule(
diff --git a/src/wasm/wasm-module.h b/src/wasm/wasm-module.h
index 303779dcb13..fdbb000e815 100644
--- a/src/wasm/wasm-module.h
+++ b/src/wasm/wasm-module.h
@@ -152,7 +152,7 @@ struct V8_EXPORT_PRIVATE WasmModule {
   uint32_t globals_buffer_size = 0;
   uint32_t num_imported_mutable_globals = 0;
   uint32_t num_imported_functions = 0;
-  uint32_t num_declared_functions = 0;
+  uint32_t num_declared_functions = 0;  // excluding imported
   uint32_t num_exported_functions = 0;
   WireBytesRef name = {0, 0};
   std::vector<FunctionSig*> signatures;  // by signature index
diff --git a/test/cctest/wasm/wasm-run-utils.cc b/test/cctest/wasm/wasm-run-utils.cc
index fe728932742..e920c7a0c92 100644
--- a/test/cctest/wasm/wasm-run-utils.cc
+++ b/test/cctest/wasm/wasm-run-utils.cc
@@ -29,12 +29,10 @@ TestingModuleBuilder::TestingModuleBuilder(
   uint32_t maybe_import_index = 0;
   if (maybe_import) {
     // Manually add an imported function before any other functions.
-    // This must happen before the instance objectis created, since the
+    // This must happen before the instance object is created, since the
     // instance object allocates import entries.
-    maybe_import_index = AddFunction(maybe_import->sig, nullptr);
+    maybe_import_index = AddFunction(maybe_import->sig, nullptr, kImport);
     DCHECK_EQ(0, maybe_import_index);
-    test_module_->num_imported_functions = 1;
-    test_module_->functions[0].imported = true;
   }
 
   instance_object_ = InitInstanceObject();
@@ -93,7 +91,8 @@ byte* TestingModuleBuilder::AddMemory(uint32_t size) {
   return mem_start_;
 }
 
-uint32_t TestingModuleBuilder::AddFunction(FunctionSig* sig, const char* name) {
+uint32_t TestingModuleBuilder::AddFunction(FunctionSig* sig, const char* name,
+                                           FunctionType type) {
   if (test_module_->functions.size() == 0) {
     // TODO(titzer): Reserving space here to avoid the underlying WasmFunction
     // structs from moving.
@@ -104,6 +103,16 @@ uint32_t TestingModuleBuilder::AddFunction(FunctionSig* sig, const char* name) {
     native_module_->SetNumFunctionsForTesting(index + 1);
   }
   test_module_->functions.push_back({sig, index, 0, {0, 0}, false, false});
+  if (type == kImport) {
+    DCHECK_EQ(0, test_module_->num_declared_functions);
+    ++test_module_->num_imported_functions;
+    test_module_->functions.back().imported = true;
+  } else {
+    ++test_module_->num_declared_functions;
+  }
+  DCHECK_EQ(test_module_->functions.size(),
+            test_module_->num_imported_functions +
+                test_module_->num_declared_functions);
   if (name) {
     Vector<const byte> name_vec = Vector<const byte>::cast(CStrVector(name));
     test_module_->AddNameForTesting(
@@ -452,7 +461,7 @@ WasmFunctionCompiler::WasmFunctionCompiler(Zone* zone, FunctionSig* sig,
       source_position_table_(this->graph()),
       interpreter_(builder->interpreter()) {
   // Get a new function from the testing module.
-  int index = builder->AddFunction(sig, name);
+  int index = builder->AddFunction(sig, name, TestingModuleBuilder::kWasm);
   function_ = builder_->GetFunctionAt(index);
 }
 
diff --git a/test/cctest/wasm/wasm-run-utils.h b/test/cctest/wasm/wasm-run-utils.h
index 26948638750..a64f9378953 100644
--- a/test/cctest/wasm/wasm-run-utils.h
+++ b/test/cctest/wasm/wasm-run-utils.h
@@ -181,7 +181,8 @@ class TestingModuleBuilder {
 
   void SetHasSharedMemory() { test_module_->has_shared_memory = true; }
 
-  uint32_t AddFunction(FunctionSig* sig, const char* name);
+  enum FunctionType { kImport, kWasm };
+  uint32_t AddFunction(FunctionSig* sig, const char* name, FunctionType type);
 
   Handle<JSFunction> WrapCode(uint32_t index);
 
-- 
2.18.1