From da172451c6ae422931e019c6ca56b5ca0f1a15be Mon Sep 17 00:00:00 2001
From: Clemens Hammacher <clemensh@chromium.org>
Date: Mon, 8 May 2017 14:45:20 +0200
Subject: [PATCH] [wasm] Fix memory management for Result types

Make ModuleResult and FunctionResult return Result<std::unique_ptr<X>>.
This makes memory ownership and transfer of ownership more clear and
avoids a lot of manual releases of the referenced native heap object.

R=ahaas@chromium.org

Change-Id: I7a3f5bd7761b6ae1ebdc7d17ff1b96a8df599871
Reviewed-on: https://chromium-review.googlesource.com/498352
Reviewed-by: Andreas Haas <ahaas@chromium.org>
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#45160}
---
 src/wasm/module-decoder.cc                    | 49 +++++++++----------
 src/wasm/module-decoder.h                     |  4 +-
 src/wasm/wasm-module.cc                       | 49 ++++++++++++-------
 src/wasm/wasm-objects.cc                      |  4 +-
 src/wasm/wasm-result.h                        |  2 +-
 test/common/wasm/wasm-module-runner.cc        | 12 ++---
 test/common/wasm/wasm-module-runner.h         |  2 +-
 .../unittests/wasm/module-decoder-unittest.cc | 48 ++----------------
 8 files changed, 70 insertions(+), 100 deletions(-)

diff --git a/src/wasm/module-decoder.cc b/src/wasm/module-decoder.cc
index c9c71f3b62..68d9fe09d2 100644
--- a/src/wasm/module-decoder.cc
+++ b/src/wasm/module-decoder.cc
@@ -275,7 +275,7 @@ class ModuleDecoder : public Decoder {
   // Decodes an entire module.
   ModuleResult DecodeModule(bool verify_functions = true) {
     pc_ = start_;
-    WasmModule* module = new WasmModule(module_zone_);
+    std::unique_ptr<WasmModule> module(new WasmModule(module_zone_));
     module->min_mem_pages = 0;
     module->max_mem_pages = 0;
     module->mem_export = false;
@@ -357,12 +357,13 @@ class ModuleDecoder : public Decoder {
                                          true,           // imported
                                          false});        // exported
             WasmFunction* function = &module->functions.back();
-            function->sig_index = consume_sig_index(module, &function->sig);
+            function->sig_index =
+                consume_sig_index(module.get(), &function->sig);
             break;
           }
           case kExternalTable: {
             // ===== Imported table ==========================================
-            if (!AddTable(module)) break;
+            if (!AddTable(module.get())) break;
             import->index =
                 static_cast<uint32_t>(module->function_tables.size());
             module->function_tables.push_back({0, 0, false,
@@ -378,7 +379,7 @@ class ModuleDecoder : public Decoder {
           }
           case kExternalMemory: {
             // ===== Imported memory =========================================
-            if (!AddMemory(module)) break;
+            if (!AddMemory(module.get())) break;
             consume_resizable_limits(
                 "memory", "pages", FLAG_wasm_max_mem_pages,
                 &module->min_mem_pages, &module->has_max_mem,
@@ -424,7 +425,7 @@ class ModuleDecoder : public Decoder {
                                      false,       // imported
                                      false});     // exported
         WasmFunction* function = &module->functions.back();
-        function->sig_index = consume_sig_index(module, &function->sig);
+        function->sig_index = consume_sig_index(module.get(), &function->sig);
       }
       section_iter.advance();
     }
@@ -434,7 +435,7 @@ class ModuleDecoder : public Decoder {
       uint32_t table_count = consume_count("table count", kV8MaxWasmTables);
 
       for (uint32_t i = 0; ok() && i < table_count; i++) {
-        if (!AddTable(module)) break;
+        if (!AddTable(module.get())) break;
         module->function_tables.push_back({0, 0, false, std::vector<int32_t>(),
                                            false, false, SignatureMap()});
         WasmIndirectFunctionTable* table = &module->function_tables.back();
@@ -452,7 +453,7 @@ class ModuleDecoder : public Decoder {
       uint32_t memory_count = consume_count("memory count", kV8MaxWasmMemories);
 
       for (uint32_t i = 0; ok() && i < memory_count; i++) {
-        if (!AddMemory(module)) break;
+        if (!AddMemory(module.get())) break;
         consume_resizable_limits("memory", "pages", FLAG_wasm_max_mem_pages,
                                  &module->min_mem_pages, &module->has_max_mem,
                                  kSpecMaxWasmMemoryPages,
@@ -474,7 +475,7 @@ class ModuleDecoder : public Decoder {
         module->globals.push_back(
             {kWasmStmt, false, WasmInitExpr(), 0, false, false});
         WasmGlobal* global = &module->globals.back();
-        DecodeGlobalInModule(module, i + imported_globals, global);
+        DecodeGlobalInModule(module.get(), i + imported_globals, global);
       }
       section_iter.advance();
     }
@@ -503,14 +504,14 @@ class ModuleDecoder : public Decoder {
         switch (exp->kind) {
           case kExternalFunction: {
             WasmFunction* func = nullptr;
-            exp->index = consume_func_index(module, &func);
+            exp->index = consume_func_index(module.get(), &func);
             module->num_exported_functions++;
             if (func) func->exported = true;
             break;
           }
           case kExternalTable: {
             WasmIndirectFunctionTable* table = nullptr;
-            exp->index = consume_table_index(module, &table);
+            exp->index = consume_table_index(module.get(), &table);
             if (table) table->exported = true;
             break;
           }
@@ -526,7 +527,7 @@ class ModuleDecoder : public Decoder {
           }
           case kExternalGlobal: {
             WasmGlobal* global = nullptr;
-            exp->index = consume_global_index(module, &global);
+            exp->index = consume_global_index(module.get(), &global);
             if (global) {
               if (global->mutability) {
                 error("mutable globals cannot be exported");
@@ -573,7 +574,7 @@ class ModuleDecoder : public Decoder {
     if (section_iter.section_code() == kStartSectionCode) {
       WasmFunction* func;
       const byte* pos = pc_;
-      module->start_function_index = consume_func_index(module, &func);
+      module->start_function_index = consume_func_index(module.get(), &func);
       if (func &&
           (func->sig->parameter_count() > 0 || func->sig->return_count() > 0)) {
         error(pos,
@@ -598,7 +599,7 @@ class ModuleDecoder : public Decoder {
           break;
         }
         table = &module->function_tables[table_index];
-        WasmInitExpr offset = consume_init_expr(module, kWasmI32);
+        WasmInitExpr offset = consume_init_expr(module.get(), kWasmI32);
         uint32_t num_elem =
             consume_count("number of elements", kV8MaxWasmTableEntries);
         std::vector<uint32_t> vector;
@@ -606,7 +607,7 @@ class ModuleDecoder : public Decoder {
         WasmTableInit* init = &module->table_inits.back();
         for (uint32_t j = 0; ok() && j < num_elem; j++) {
           WasmFunction* func = nullptr;
-          uint32_t index = consume_func_index(module, &func);
+          uint32_t index = consume_func_index(module.get(), &func);
           DCHECK_EQ(func != nullptr, ok());
           if (!func) break;
           DCHECK_EQ(index, func->func_index);
@@ -634,7 +635,7 @@ class ModuleDecoder : public Decoder {
         function->code_start_offset = pc_offset();
         function->code_end_offset = pc_offset() + size;
         if (verify_functions) {
-          ModuleBytesEnv module_env(module, nullptr,
+          ModuleBytesEnv module_env(module.get(), nullptr,
                                     ModuleWireBytes(start_, end_));
           VerifyFunctionBody(i + module->num_imported_functions, &module_env,
                              function);
@@ -662,7 +663,7 @@ class ModuleDecoder : public Decoder {
             0                // source_size
         });
         WasmDataSegment* segment = &module->data_segments.back();
-        DecodeDataSegmentInModule(module, segment);
+        DecodeDataSegmentInModule(module.get(), segment);
       }
       section_iter.advance();
     }
@@ -717,10 +718,9 @@ class ModuleDecoder : public Decoder {
     }
 
     if (ok()) {
-      CalculateGlobalOffsets(module);
+      CalculateGlobalOffsets(module.get());
     }
-    const WasmModule* finished_module = module;
-    ModuleResult result = toResult(finished_module);
+    ModuleResult result = toResult(std::move(module));
     if (verify_functions && result.ok()) {
       // Copy error code and location.
       result.MoveErrorFrom(intermediate_result_);
@@ -731,7 +731,7 @@ class ModuleDecoder : public Decoder {
 
   // Decodes a single anonymous function starting at {start_}.
   FunctionResult DecodeSingleFunction(ModuleBytesEnv* module_env,
-                                      WasmFunction* function) {
+                                      std::unique_ptr<WasmFunction> function) {
     pc_ = start_;
     function->sig = consume_sig();            // read signature
     function->name_offset = 0;                // ---- name
@@ -739,12 +739,11 @@ class ModuleDecoder : public Decoder {
     function->code_start_offset = off(pc_);   // ---- code start
     function->code_end_offset = off(end_);    // ---- code end
 
-    if (ok()) VerifyFunctionBody(0, module_env, function);
+    if (ok()) VerifyFunctionBody(0, module_env, function.get());
 
-    FunctionResult result;
+    FunctionResult result(std::move(function));
     // Copy error code and location.
     result.MoveErrorFrom(intermediate_result_);
-    result.val = function;
     return result;
   }
 
@@ -1199,9 +1198,9 @@ FunctionResult DecodeWasmFunction(Isolate* isolate, Zone* zone,
   (is_wasm ? isolate->counters()->wasm_wasm_function_size_bytes()
            : isolate->counters()->wasm_asm_function_size_bytes())
       ->AddSample(static_cast<int>(size));
-  WasmFunction* function = new WasmFunction();
   ModuleDecoder decoder(zone, function_start, function_end, kWasmOrigin);
-  return decoder.DecodeSingleFunction(module_env, function);
+  return decoder.DecodeSingleFunction(
+      module_env, std::unique_ptr<WasmFunction>(new WasmFunction()));
 }
 
 AsmJsOffsetsResult DecodeAsmJsOffsets(const byte* tables_start,
diff --git a/src/wasm/module-decoder.h b/src/wasm/module-decoder.h
index b29dfb196b..85577ad16a 100644
--- a/src/wasm/module-decoder.h
+++ b/src/wasm/module-decoder.h
@@ -44,8 +44,8 @@ inline bool IsValidSectionCode(uint8_t byte) {
 
 const char* SectionName(SectionCode code);
 
-typedef Result<const WasmModule*> ModuleResult;
-typedef Result<WasmFunction*> FunctionResult;
+typedef Result<std::unique_ptr<WasmModule>> ModuleResult;
+typedef Result<std::unique_ptr<WasmFunction>> FunctionResult;
 typedef std::vector<std::pair<int, int>> FunctionOffsets;
 typedef Result<FunctionOffsets> FunctionOffsetsResult;
 struct AsmJsOffsetEntry {
diff --git a/src/wasm/wasm-module.cc b/src/wasm/wasm-module.cc
index fc2ddb6ee9..3d02c02a54 100644
--- a/src/wasm/wasm-module.cc
+++ b/src/wasm/wasm-module.cc
@@ -300,6 +300,12 @@ bool compile_lazy(const WasmModule* module) {
 // A helper for compiling an entire module.
 class CompilationHelper {
  public:
+  // The compilation helper itself does not take ownership of the {WasmModule},
+  // since there are use cases in which is must remain owned by the caller.
+  // The CompileToModuleObject method transfers ownership to the generated
+  // {WasmModuleWrapper}, so each user or this method must release ownership.
+  // TODO(clemensh): Fix this by storing the WasmModule as unique_ptr and having
+  // an explicit release_module() method.
   CompilationHelper(Isolate* isolate, WasmModule* module)
       : isolate_(isolate), module_(module) {}
 
@@ -499,6 +505,7 @@ class CompilationHelper {
     }
   }
 
+  // This method takes ownership of the {WasmModule} stored in {module_}.
   MaybeHandle<WasmModuleObject> CompileToModuleObject(
       ErrorThrower* thrower, const ModuleWireBytes& wire_bytes,
       Handle<Script> asm_js_script,
@@ -649,7 +656,7 @@ class CompilationHelper {
     }
 
     return WasmModuleObject::New(isolate_, compiled_module);
-}
+  }
 };
 
 static void MemoryInstanceFinalizer(Isolate* isolate,
@@ -2470,7 +2477,6 @@ bool wasm::SyncValidate(Isolate* isolate, const ModuleWireBytes& bytes) {
   if (bytes.start() == nullptr || bytes.length() == 0) return false;
   ModuleResult result =
       DecodeWasmModule(isolate, bytes.start(), bytes.end(), true, kWasmOrigin);
-  if (result.val) delete result.val;
   return result.ok();
 }
 
@@ -2482,13 +2488,13 @@ MaybeHandle<WasmModuleObject> wasm::SyncCompileTranslatedAsmJs(
   ModuleResult result = DecodeWasmModule(isolate, bytes.start(), bytes.end(),
                                          false, kAsmJsOrigin);
   if (result.failed()) {
-    // TODO(titzer): use Result<std::unique_ptr<const WasmModule*>>?
-    if (result.val) delete result.val;
     thrower->CompileFailed("Wasm decoding failed", result);
     return {};
   }
 
-  CompilationHelper helper(isolate, const_cast<WasmModule*>(result.val));
+  // Transfer ownership to the {WasmModuleWrapper} generated in
+  // {CompileToModuleObject}.
+  CompilationHelper helper(isolate, result.val.release());
   return helper.CompileToModuleObject(thrower, bytes, asm_js_script,
                                       asm_js_offset_table_bytes);
 }
@@ -2504,12 +2510,13 @@ MaybeHandle<WasmModuleObject> wasm::SyncCompile(Isolate* isolate,
   ModuleResult result =
       DecodeWasmModule(isolate, bytes.start(), bytes.end(), false, kWasmOrigin);
   if (result.failed()) {
-    if (result.val) delete result.val;
     thrower->CompileFailed("Wasm decoding failed", result);
     return {};
   }
 
-  CompilationHelper helper(isolate, const_cast<WasmModule*>(result.val));
+  // Transfer ownership to the {WasmModuleWrapper} generated in
+  // {CompileToModuleObject}.
+  CompilationHelper helper(isolate, result.val.release());
   return helper.CompileToModuleObject(thrower, bytes, Handle<Script>(),
                                       Vector<const byte>());
 }
@@ -2601,9 +2608,11 @@ class AsyncCompileJob {
   ModuleWireBytes wire_bytes_;
   Handle<Context> context_;
   Handle<JSPromise> module_promise_;
+  // The WasmModule is owned by the WasmModuleWrapper created in step 2.
+  // It is {nullptr} before.
   WasmModule* module_ = nullptr;
-  std::unique_ptr<CompilationHelper> helper_ = nullptr;
-  std::unique_ptr<ModuleBytesEnv> module_bytes_env_ = nullptr;
+  std::unique_ptr<CompilationHelper> helper_;
+  std::unique_ptr<ModuleBytesEnv> module_bytes_env_;
 
   bool failed_ = false;
   std::vector<DeferredHandles*> deferred_handles_;
@@ -2701,13 +2710,10 @@ class AsyncCompileJob {
       }
       if (result.failed()) {
         // Decoding failure; reject the promise and clean up.
-        if (result.val) delete result.val;
-        result.val = nullptr;
         job_->DoSync<DecodeFail>(std::move(result));
       } else {
         // Decode passed.
-        job_->module_ = const_cast<WasmModule*>(result.val);
-        job_->DoSync<PrepareAndStartCompile>();
+        job_->DoSync<PrepareAndStartCompile>(std::move(result.val));
       }
     }
   };
@@ -2735,6 +2741,12 @@ class AsyncCompileJob {
   // Step 2 (sync): Create heap-allocated data and start compile.
   //==========================================================================
   class PrepareAndStartCompile : public SyncCompileTask {
+   public:
+    explicit PrepareAndStartCompile(std::unique_ptr<WasmModule> module)
+        : module_(std::move(module)) {}
+
+   private:
+    std::unique_ptr<WasmModule> module_;
     void RunImpl() override {
       TRACE_COMPILE("(2) Prepare and start compile...\n");
       HandleScope scope(job_->isolate_);
@@ -2743,9 +2755,9 @@ class AsyncCompileJob {
       // The {module_wrapper} will take ownership of the {WasmModule} object,
       // and it will be destroyed when the GC reclaims the wrapper object.
       job_->module_wrapper_ =
-          WasmModuleWrapper::New(job_->isolate_, job_->module_);
-      job_->temp_instance_ =
-          std::unique_ptr<WasmInstance>(new WasmInstance(job_->module_));
+          WasmModuleWrapper::New(job_->isolate_, module_.release());
+      job_->module_ = job_->module_wrapper_->get();
+      job_->temp_instance_.reset(new WasmInstance(job_->module_));
       job_->temp_instance_->context = job_->context_;
       job_->temp_instance_->mem_size =
           WasmModule::kPageSize * job_->module_->min_mem_pages;
@@ -2810,9 +2822,8 @@ class AsyncCompileJob {
                   Min(static_cast<size_t>(FLAG_wasm_num_compilation_tasks),
                       V8::GetCurrentPlatform()
                           ->NumberOfAvailableBackgroundThreads())));
-      job_->module_bytes_env_ =
-          std::unique_ptr<ModuleBytesEnv>(new ModuleBytesEnv(
-              job_->module_, job_->temp_instance_.get(), job_->wire_bytes_));
+      job_->module_bytes_env_.reset(new ModuleBytesEnv(
+          job_->module_, job_->temp_instance_.get(), job_->wire_bytes_));
       job_->outstanding_units_ = job_->helper_->InitializeParallelCompilation(
           job_->module_->functions, *job_->module_bytes_env_);
 
diff --git a/src/wasm/wasm-objects.cc b/src/wasm/wasm-objects.cc
index 0ca5c92d22..93d0e3e3d2 100644
--- a/src/wasm/wasm-objects.cc
+++ b/src/wasm/wasm-objects.cc
@@ -812,7 +812,9 @@ void WasmSharedModuleData::ReinitializeAfterDeserialization(
         DecodeWasmModule(isolate, start, end, false, kWasmOrigin);
     CHECK(result.ok());
     CHECK_NOT_NULL(result.val);
-    module = const_cast<WasmModule*>(result.val);
+    // Take ownership of the WasmModule and immediately transfer it to the
+    // WasmModuleWrapper below.
+    module = result.val.release();
   }
 
   Handle<WasmModuleWrapper> module_wrapper =
diff --git a/src/wasm/wasm-result.h b/src/wasm/wasm-result.h
index ca9f646882..848170c80e 100644
--- a/src/wasm/wasm-result.h
+++ b/src/wasm/wasm-result.h
@@ -69,7 +69,7 @@ class Result : public ResultBase {
   Result() = default;
 
   template <typename S>
-  explicit Result(S&& value) : val(value) {}
+  explicit Result(S&& value) : val(std::forward<S>(value)) {}
 
   template <typename S>
   Result(Result<S>&& other)
diff --git a/test/common/wasm/wasm-module-runner.cc b/test/common/wasm/wasm-module-runner.cc
index 72c80cdc52..fb2066a4bf 100644
--- a/test/common/wasm/wasm-module-runner.cc
+++ b/test/common/wasm/wasm-module-runner.cc
@@ -25,7 +25,7 @@ uint32_t GetMinModuleMemSize(const WasmModule* module) {
   return WasmModule::kPageSize * module->min_mem_pages;
 }
 
-const WasmModule* DecodeWasmModuleForTesting(
+std::unique_ptr<WasmModule> DecodeWasmModuleForTesting(
     Isolate* isolate, ErrorThrower* thrower, const byte* module_start,
     const byte* module_end, ModuleOrigin origin, bool verify_functions) {
   // Decode the module, but don't verify function bodies, since we'll
@@ -39,11 +39,7 @@ const WasmModule* DecodeWasmModuleForTesting(
                           decoding_result.error_msg().c_str());
   }
 
-  if (thrower->error()) {
-    if (decoding_result.val) delete decoding_result.val;
-    return nullptr;
-  }
-  return decoding_result.val;
+  return std::move(decoding_result.val);
 }
 
 const Handle<WasmInstanceObject> InstantiateModuleForTesting(
@@ -78,8 +74,8 @@ const Handle<WasmInstanceObject> InstantiateModuleForTesting(
 const Handle<WasmInstanceObject> CompileInstantiateWasmModuleForTesting(
     Isolate* isolate, ErrorThrower* thrower, const byte* module_start,
     const byte* module_end, ModuleOrigin origin) {
-  std::unique_ptr<const WasmModule> module(DecodeWasmModuleForTesting(
-      isolate, thrower, module_start, module_end, origin));
+  std::unique_ptr<WasmModule> module = DecodeWasmModuleForTesting(
+      isolate, thrower, module_start, module_end, origin);
 
   if (module == nullptr) {
     thrower->CompileError("Wasm module decoding failed");
diff --git a/test/common/wasm/wasm-module-runner.h b/test/common/wasm/wasm-module-runner.h
index 4d58513c14..b3d9b8f908 100644
--- a/test/common/wasm/wasm-module-runner.h
+++ b/test/common/wasm/wasm-module-runner.h
@@ -23,7 +23,7 @@ namespace wasm {
 namespace testing {
 
 // Decodes the given encoded module.
-const WasmModule* DecodeWasmModuleForTesting(
+std::unique_ptr<WasmModule> DecodeWasmModuleForTesting(
     Isolate* isolate, ErrorThrower* thrower, const byte* module_start,
     const byte* module_end, ModuleOrigin origin, bool verify_functions = false);
 
diff --git a/test/unittests/wasm/module-decoder-unittest.cc b/test/unittests/wasm/module-decoder-unittest.cc
index 1bbf3dd216..c4cce31ec0 100644
--- a/test/unittests/wasm/module-decoder-unittest.cc
+++ b/test/unittests/wasm/module-decoder-unittest.cc
@@ -89,14 +89,12 @@ namespace wasm {
   do {                                                             \
     ModuleResult result = DecodeModule(data, data + sizeof(data)); \
     EXPECT_TRUE(result.ok());                                      \
-    if (result.val) delete result.val;                             \
   } while (false)
 
 #define EXPECT_FAILURE_LEN(data, length)                     \
   do {                                                       \
     ModuleResult result = DecodeModule(data, data + length); \
     EXPECT_FALSE(result.ok());                               \
-    if (result.val) delete result.val;                       \
   } while (false)
 
 #define EXPECT_FAILURE(data) EXPECT_FAILURE_LEN(data, sizeof(data))
@@ -108,13 +106,10 @@ namespace wasm {
     }                                                   \
   } while (false)
 
-#define EXPECT_OK(result)                \
-  do {                                   \
-    EXPECT_TRUE(result.ok());            \
-    if (!result.ok()) {                  \
-      if (result.val) delete result.val; \
-      return;                            \
-    }                                    \
+#define EXPECT_OK(result)     \
+  do {                        \
+    EXPECT_TRUE(result.ok()); \
+    if (!result.ok()) return; \
   } while (false)
 
 static size_t SizeOfVarInt(size_t value) {
@@ -161,7 +156,6 @@ TEST_F(WasmModuleVerifyTest, WrongMagic) {
     const byte data[] = {U32_LE(kWasmMagic ^ x), U32_LE(kWasmVersion)};
     ModuleResult result = DecodeModuleNoHeader(data, data + sizeof(data));
     EXPECT_FALSE(result.ok());
-    if (result.val) delete result.val;
   }
 }
 
@@ -170,14 +164,12 @@ TEST_F(WasmModuleVerifyTest, WrongVersion) {
     const byte data[] = {U32_LE(kWasmMagic), U32_LE(kWasmVersion ^ x)};
     ModuleResult result = DecodeModuleNoHeader(data, data + sizeof(data));
     EXPECT_FALSE(result.ok());
-    if (result.val) delete result.val;
   }
 }
 
 TEST_F(WasmModuleVerifyTest, DecodeEmpty) {
   ModuleResult result = DecodeModule(nullptr, 0);
   EXPECT_TRUE(result.ok());
-  if (result.val) delete result.val;
 }
 
 TEST_F(WasmModuleVerifyTest, OneGlobal) {
@@ -204,8 +196,6 @@ TEST_F(WasmModuleVerifyTest, OneGlobal) {
     EXPECT_FALSE(global->mutability);
     EXPECT_EQ(WasmInitExpr::kI32Const, global->init.kind);
     EXPECT_EQ(13, global->init.val.i32_const);
-
-    if (result.val) delete result.val;
   }
 
   EXPECT_OFF_END_FAILURE(data, 1, sizeof(data));
@@ -222,7 +212,6 @@ TEST_F(WasmModuleVerifyTest, Global_invalid_type) {
 
   ModuleResult result = DecodeModule(data, data + sizeof(data));
   EXPECT_FALSE(result.ok());
-  if (result.val) delete result.val;
 }
 
 TEST_F(WasmModuleVerifyTest, Global_invalid_type2) {
@@ -236,7 +225,6 @@ TEST_F(WasmModuleVerifyTest, Global_invalid_type2) {
 
   ModuleResult result = DecodeModule(data, data + sizeof(data));
   EXPECT_FALSE(result.ok());
-  if (result.val) delete result.val;
 }
 
 TEST_F(WasmModuleVerifyTest, ZeroGlobals) {
@@ -246,7 +234,6 @@ TEST_F(WasmModuleVerifyTest, ZeroGlobals) {
   };
   ModuleResult result = DecodeModule(data, data + sizeof(data));
   EXPECT_OK(result);
-  if (result.val) delete result.val;
 }
 
 TEST_F(WasmModuleVerifyTest, ExportMutableGlobal) {
@@ -325,7 +312,6 @@ TEST_F(WasmModuleVerifyTest, NGlobals) {
 
     ModuleResult result = DecodeModule(&buffer[0], &buffer[0] + buffer.size());
     EXPECT_OK(result);
-    if (result.val) delete result.val;
   }
 }
 
@@ -371,8 +357,6 @@ TEST_F(WasmModuleVerifyTest, TwoGlobals) {
     EXPECT_EQ(8u, g1->offset);
     EXPECT_TRUE(g1->mutability);
     EXPECT_EQ(WasmInitExpr::kF64Const, g1->init.kind);
-
-    if (result.val) delete result.val;
   }
 
   EXPECT_OFF_END_FAILURE(data, 1, sizeof(data));
@@ -413,7 +397,6 @@ TEST_F(WasmModuleVerifyTest, MultipleSignatures) {
     EXPECT_EQ(1u, result.val->signatures[1]->parameter_count());
     EXPECT_EQ(2u, result.val->signatures[2]->parameter_count());
   }
-  if (result.val) delete result.val;
 
   EXPECT_OFF_END_FAILURE(data, 1, sizeof(data));
 }
@@ -456,7 +439,6 @@ TEST_F(WasmModuleVerifyTest, DataSegmentWithImmutableImportedGlobal) {
   WasmInitExpr expr = result.val->data_segments.back().dest_addr;
   EXPECT_EQ(WasmInitExpr::kGlobalIndex, expr.kind);
   EXPECT_EQ(1u, expr.val.global_index);
-  if (result.val) delete result.val;
 }
 
 TEST_F(WasmModuleVerifyTest, DataSegmentWithMutableImportedGlobal) {
@@ -544,8 +526,6 @@ TEST_F(WasmModuleVerifyTest, OneDataSegment) {
     EXPECT_EQ(0x9bbaa, segment->dest_addr.val.i32_const);
     EXPECT_EQ(kDataSegmentSourceOffset, segment->source_offset);
     EXPECT_EQ(3u, segment->source_size);
-
-    if (result.val) delete result.val;
   }
 
   EXPECT_OFF_END_FAILURE(data, 14, sizeof(data));
@@ -604,8 +584,6 @@ TEST_F(WasmModuleVerifyTest, TwoDataSegments) {
     EXPECT_EQ(0x6ddcc, s1->dest_addr.val.i32_const);
     EXPECT_EQ(kDataSegment1SourceOffset, s1->source_offset);
     EXPECT_EQ(10u, s1->source_size);
-
-    if (result.val) delete result.val;
   }
 
   EXPECT_OFF_END_FAILURE(data, 14, sizeof(data));
@@ -679,7 +657,6 @@ TEST_F(WasmModuleVerifyTest, OneIndirectFunction) {
     EXPECT_EQ(1u, result.val->function_tables.size());
     EXPECT_EQ(1u, result.val->function_tables[0].min_size);
   }
-  if (result.val) delete result.val;
 }
 
 TEST_F(WasmModuleVerifyTest, OneIndirectFunction_one_entry) {
@@ -705,7 +682,6 @@ TEST_F(WasmModuleVerifyTest, OneIndirectFunction_one_entry) {
     EXPECT_EQ(1u, result.val->function_tables.size());
     EXPECT_EQ(1u, result.val->function_tables[0].min_size);
   }
-  if (result.val) delete result.val;
 }
 
 TEST_F(WasmModuleVerifyTest, MultipleIndirectFunctions) {
@@ -742,7 +718,6 @@ TEST_F(WasmModuleVerifyTest, MultipleIndirectFunctions) {
     EXPECT_EQ(1u, result.val->function_tables.size());
     EXPECT_EQ(8u, result.val->function_tables[0].min_size);
   }
-  if (result.val) delete result.val;
 }
 
 TEST_F(WasmModuleVerifyTest, IndirectFunctionNoFunctions) {
@@ -955,7 +930,7 @@ TEST_F(WasmFunctionVerifyTest, Ok_v_v_empty) {
   EXPECT_OK(result);
 
   if (result.val && result.ok()) {
-    WasmFunction* function = result.val;
+    WasmFunction* function = result.val.get();
     EXPECT_EQ(0u, function->sig->parameter_count());
     EXPECT_EQ(0u, function->sig->return_count());
     EXPECT_EQ(0u, function->name_offset);
@@ -964,8 +939,6 @@ TEST_F(WasmFunctionVerifyTest, Ok_v_v_empty) {
     EXPECT_EQ(sizeof(data), function->code_end_offset);
     // TODO(titzer): verify encoding of local declarations
   }
-
-  if (result.val) delete result.val;
 }
 
 TEST_F(WasmModuleVerifyTest, SectionWithoutNameLength) {
@@ -1070,8 +1043,6 @@ TEST_F(WasmModuleVerifyTest, UnknownSectionSkipped) {
 
   EXPECT_EQ(kWasmI32, global->type);
   EXPECT_EQ(0u, global->offset);
-
-  if (result.val) delete result.val;
 }
 
 TEST_F(WasmModuleVerifyTest, ImportTable_empty) {
@@ -1213,8 +1184,6 @@ TEST_F(WasmModuleVerifyTest, ExportTable_empty1) {
 
   EXPECT_EQ(1u, result.val->functions.size());
   EXPECT_EQ(0u, result.val->export_table.size());
-
-  if (result.val) delete result.val;
 }
 
 TEST_F(WasmModuleVerifyTest, ExportTable_empty2) {
@@ -1244,8 +1213,6 @@ TEST_F(WasmModuleVerifyTest, ExportTableOne) {
 
   EXPECT_EQ(1u, result.val->functions.size());
   EXPECT_EQ(1u, result.val->export_table.size());
-
-  if (result.val) delete result.val;
 }
 
 TEST_F(WasmModuleVerifyTest, ExportNameWithInvalidStringLength) {
@@ -1288,8 +1255,6 @@ TEST_F(WasmModuleVerifyTest, ExportTableTwo) {
 
   EXPECT_EQ(1u, result.val->functions.size());
   EXPECT_EQ(2u, result.val->export_table.size());
-
-  if (result.val) delete result.val;
 }
 
 TEST_F(WasmModuleVerifyTest, ExportTableThree) {
@@ -1316,8 +1281,6 @@ TEST_F(WasmModuleVerifyTest, ExportTableThree) {
 
   EXPECT_EQ(3u, result.val->functions.size());
   EXPECT_EQ(3u, result.val->export_table.size());
-
-  if (result.val) delete result.val;
 }
 
 TEST_F(WasmModuleVerifyTest, ExportTableThreeOne) {
@@ -1357,7 +1320,6 @@ TEST_F(WasmModuleVerifyTest, ExportTableOne_off_end) {
   for (size_t length = 33; length < sizeof(data); length++) {
     ModuleResult result = DecodeModule(data, data + length);
     EXPECT_FALSE(result.ok());
-    if (result.val) delete result.val;
   }
 }