[wasm] Exit loop once wasm code in JS_TO_WASM is found

src/wasm/wasm-module.cc

 // Copyright 2015 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <memory>
 
 #include "src/assembler-inl.h"
 #include "src/base/adapters.h"
 #include "src/base/atomic-utils.h"
 #include "src/code-stubs.h"
@@ skipping 842 matching lines @@
     if (func->code()->kind() == Code::JS_TO_WASM_FUNCTION) {
       auto exported = Handle<WasmExportedFunction>::cast(func);
       Handle<WasmInstanceObject> other_instance(exported->instance(), isolate);
       int func_index = exported->function_index();
       return &other_instance->module()->functions[func_index];
     }
   }
   return nullptr;
 }
 
-static Handle<Code> UnwrapImportWrapper(Handle<Object> target) {
-  Handle<JSFunction> func = Handle<JSFunction>::cast(target);
+Code* UnwrapImportWrapper(Object* target) {

[titzer, 2017/02/28 15:07:31]

I think it'd be better if you left this as using handles.

[titzer, 2017/02/28 15:07:31]

any reason for dropping the static qualifier?

[Clemens Hammacher, 2017/02/28 15:14:52]

No, this is a mistake. This CL (https://codereview.chromium.org/2714373003) will
wrap it in an anonymous namespace, but without this, we should re-add the
static.
Hm, any reason for this? I know that the performance impact is minimal, but it
exists (handle(obj) also needs to gather the isolate first by
obj->GetIsolate()).
Handles generally make code less readable IMO.

[Clemens Hammacher, 2017/03/01 15:31:15]

Done.

+  JSFunction* func = JSFunction::cast(target);
   Handle<Code> export_wrapper_code = handle(func->code());
-  int found = 0;
   int mask = RelocInfo::ModeMask(RelocInfo::CODE_TARGET);
   Handle<Code> code;
-  for (RelocIterator it(*export_wrapper_code, mask); !it.done(); it.next()) {
-    RelocInfo* rinfo = it.rinfo();
-    Address target_address = rinfo->target_address();
-    Code* target = Code::GetCodeFromTargetAddress(target_address);
-    if (target->kind() == Code::WASM_FUNCTION ||
-        target->kind() == Code::WASM_TO_JS_FUNCTION) {
-      ++found;
-      code = handle(target);
+  for (RelocIterator it(*export_wrapper_code, mask);; it.next()) {
+    DCHECK(!it.done());
+    Code* target = Code::GetCodeFromTargetAddress(it.rinfo()->target_address());
+    if (target->kind() != Code::WASM_FUNCTION &&
+        target->kind() != Code::WASM_TO_JS_FUNCTION)

[ahaas, 2017/02/28 18:40:35]

What is the reason why code->builtin_index() != Builtins::kIllegal is not
included here?

[Clemens Hammacher, 2017/03/01 15:31:14]

The import wrapper we are unwrapping here belongs to another module, which must
be fully compiled at that stage, so kIllegal cannot occur here.
If it ever would, the DCHECK(!it.done()) would fail.

+      continue;
+// There should only be this one call to wasm code.
+#ifdef DEBUG
+    for (it.next(); !it.done(); it.next()) {
+      Code* code = Code::GetCodeFromTargetAddress(it.rinfo()->target_address());
+      DCHECK(code->kind() != Code::WASM_FUNCTION &&
+             code->kind() != Code::WASM_TO_JS_FUNCTION);
     }
+#endif
+    return target;
   }
-  DCHECK_EQ(1, found);
-  return code;
+  UNREACHABLE();
+  return nullptr;
 }
 
 static Handle<Code> CompileImportWrapper(Isolate* isolate, int index,
                                          FunctionSig* sig,
                                          Handle<JSReceiver> target,
                                          Handle<String> module_name,
                                          MaybeHandle<String> import_name,
                                          ModuleOrigin origin) {
   WasmFunction* other_func = GetWasmFunctionForImportWrapper(isolate, target);
   if (other_func) {
     if (sig->Equals(other_func->sig)) {
       // Signature matched. Unwrap the JS->WASM wrapper and return the raw
       // WASM function code.
-      return UnwrapImportWrapper(target);
+      return handle(UnwrapImportWrapper(*target), isolate);
     } else {
       return Handle<Code>::null();
     }
   } else {
     // Signature mismatch. Compile a new wrapper for the new signature.
     return compiler::CompileWasmToJSWrapper(isolate, target, sig, index,
                                             module_name, import_name, origin);
   }
 }
 
@@ skipping 29 matching lines @@
 void wasm::UpdateDispatchTables(Isolate* isolate,
                                 Handle<FixedArray> dispatch_tables, int index,
                                 Handle<JSFunction> function) {
   if (function.is_null()) {
     UpdateDispatchTablesInternal(isolate, dispatch_tables, index, nullptr,
                                  Handle<Code>::null());
   } else {
     UpdateDispatchTablesInternal(
         isolate, dispatch_tables, index,
         GetWasmFunctionForImportWrapper(isolate, function),
-        UnwrapImportWrapper(function));
+        handle(UnwrapImportWrapper(*function), isolate));
   }
 }
 
 // A helper class to simplify instantiating a module from a compiled module.
 // It closes over the {Isolate}, the {ErrorThrower}, the {WasmCompiledModule},
 // etc.
 class InstantiationHelper {
  public:
   InstantiationHelper(Isolate* isolate, ErrorThrower* thrower,
                       Handle<WasmModuleObject> module_object,
@@ skipping 621 matching lines @@
             if (!val->IsJSFunction()) continue;
             WasmFunction* function =
                 GetWasmFunctionForImportWrapper(isolate_, val);
             if (function == nullptr) {
               thrower_->LinkError("table import %d[%d] is not a WASM function",
                                   index, i);
               return -1;
             }
             int sig_index = table.map.FindOrInsert(function->sig);
             table_instance.signature_table->set(i, Smi::FromInt(sig_index));
-            table_instance.function_table->set(i, *UnwrapImportWrapper(val));
+            table_instance.function_table->set(i, UnwrapImportWrapper(*val));
           }
 
           num_imported_tables++;
           break;
         }
         case kExternalMemory: {
           // Validation should have failed if more than one memory object was
           // provided.
           DCHECK(!instance->has_memory_object());
           if (!WasmJs::IsWasmMemoryObject(isolate_, value)) {
@@ skipping 1113 matching lines @@
   Handle<String> module_property_name =
       isolate->factory()->InternalizeUtf8String("module");
   Handle<String> instance_property_name =
       isolate->factory()->InternalizeUtf8String("instance");
   JSObject::AddProperty(ret, module_property_name, module, NONE);
   JSObject::AddProperty(ret, instance_property_name,
                         instance_object.ToHandleChecked(), NONE);
 
   ResolvePromise(isolate, promise, ret);
 }