[wasm] Fix user properties for exported wasm functions and add extensive tests.

src/wasm/wasm-objects.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 "src/wasm/wasm-objects.h"
 #include "src/utils.h"
 
 #include "src/assembler-inl.h"
 #include "src/base/iterator.h"
 #include "src/compiler/wasm-compiler.h"
@@ skipping 467 matching lines @@
   }
   uint32_t compiled_max_pages = compiled_module()->module()->max_mem_pages;
   Isolate* isolate = GetIsolate();
   assert(compiled_module()->module()->is_wasm());
   isolate->counters()->wasm_wasm_max_mem_pages_count()->AddSample(
       compiled_max_pages);
   if (compiled_max_pages != 0) return compiled_max_pages;
   return FLAG_wasm_max_mem_pages;
 }
 
+bool WasmExportedFunction::IsWasmExportedFunction(Object* object) {
+  if (!object) return false;

[Igor Sheludko, 2017/07/18 17:32:39]

This check is not necessary. IsJSFunction() will do a Smi check anyway.

[titzer, 2017/07/18 18:07:40]

Done.

+  if (!object->IsJSFunction()) return false;
+  Handle<JSFunction> js_function(JSFunction::cast(object));
+  if (Code::JS_TO_WASM_FUNCTION != js_function->code()->kind()) return false;
+
+  Handle<Symbol> symbol(js_function->GetHeap()->wasm_instance_symbol());

[Igor Sheludko, 2017/07/18 17:32:38]

This way we will reuse an existing handle instead of creating a new one:
  Handle<Symbol> symbol(js_function->GetIsolate()->wasm_instance_symbol());

[titzer, 2017/07/18 18:07:40]

Done.

+  MaybeHandle<Object> result =
+      JSObject::GetPropertyOrElement(js_function, symbol);
+  if (result.is_null()) return false;
+  return result.ToHandleChecked()->IsWasmInstanceObject();

[Igor Sheludko, 2017/07/18 17:32:38]

Canonical way of using MaybeHandles would look like this:

  MaybeHandle<Object> maybe_result = ...
  Handle<Object> result;
  if (!maybe_result.ToHandle(&result)) return false;
  return result->IsWasmInstanceObject();

[titzer, 2017/07/18 18:07:40]

Done.

+}
+
 WasmExportedFunction* WasmExportedFunction::cast(Object* object) {
-  DCHECK(object && object->IsJSFunction());
-  DCHECK_EQ(Code::JS_TO_WASM_FUNCTION,
-            JSFunction::cast(object)->code()->kind());
-  // TODO(titzer): brand check for WasmExportedFunction.
+  DCHECK(IsWasmExportedFunction(object));
   return reinterpret_cast<WasmExportedFunction*>(object);
 }
 
+WasmInstanceObject* WasmExportedFunction::instance() {
+  Handle<Symbol> symbol(GetHeap()->wasm_instance_symbol());

[Igor Sheludko, 2017/07/18 17:32:38]

GetInstance()->wasm_instance_symbol() to avoid handle allocation.

[titzer, 2017/07/18 18:07:40]

Done.

+  MaybeHandle<Object> result =
+      JSObject::GetPropertyOrElement(handle(this), symbol);

[Igor Sheludko, 2017/07/18 17:32:38]

You are calling handlified code from non-handlified one. Although it should be
fine in your case, in general it's a bad practice.
Could you please handlify the method or at least add DisallowHeapAllocation
scope.

[titzer, 2017/07/18 18:07:40]

Done.

+  return WasmInstanceObject::cast(*(result.ToHandleChecked()));
+}
+
+int WasmExportedFunction::function_index() {
+  Handle<Symbol> symbol(GetHeap()->wasm_function_index_symbol());

[Igor Sheludko, 2017/07/18 17:32:38]

Same here.

[titzer, 2017/07/18 18:07:40]

Done.

+  MaybeHandle<Object> result =
+      JSObject::GetPropertyOrElement(handle(this), symbol);

[Igor Sheludko, 2017/07/18 17:32:38]

And here. If the index field ever become an inobject unboxed double field (if
someone will change the way the function is created) then this function will
allocate a heap number and the caller will not expect that from a non-handlified
function.

[titzer, 2017/07/18 18:07:40]

I've added a DisallowHeapAllocation here.

+  return result.ToHandleChecked()->Number();
+}
+
 Handle<WasmExportedFunction> WasmExportedFunction::New(
     Isolate* isolate, Handle<WasmInstanceObject> instance,
     MaybeHandle<String> maybe_name, int func_index, int arity,
     Handle<Code> export_wrapper) {
+  DCHECK_EQ(Code::JS_TO_WASM_FUNCTION, export_wrapper->kind());
   Handle<String> name;
   if (!maybe_name.ToHandle(&name)) {
     EmbeddedVector<char, 16> buffer;
     int length = SNPrintF(buffer, "%d", func_index);
     name = isolate->factory()
                ->NewStringFromOneByte(
                    Vector<uint8_t>::cast(buffer.SubVector(0, length)))
                .ToHandleChecked();
   }
-  DCHECK_EQ(Code::JS_TO_WASM_FUNCTION, export_wrapper->kind());
   Handle<SharedFunctionInfo> shared =
       isolate->factory()->NewSharedFunctionInfo(name, export_wrapper, false);
   shared->set_length(arity);
   shared->set_internal_formal_parameter_count(arity);
   Handle<JSFunction> js_function = isolate->factory()->NewFunction(
-      isolate->wasm_function_map(), name, export_wrapper);
+      handle(isolate->native_context()->sloppy_function_map()), name,

[Igor Sheludko, 2017/07/18 17:32:38]

Just isolate->sloppy_function_map() to make it nicer.

[titzer, 2017/07/18 18:07:40]

Done.

+      export_wrapper);
 
-  Handle<WasmExportedFunction> function(
-      reinterpret_cast<WasmExportedFunction*>(*js_function), isolate);
+  js_function->set_shared(*shared);
+  Handle<Symbol> instance_symbol(isolate->heap()->wasm_instance_symbol());
+  JSObject::AddProperty(js_function, instance_symbol, instance, DONT_ENUM);
 
-  function->set_shared(*shared);
-  function->set_instance(*instance);
-  function->set_function_index(func_index);
+  Handle<Symbol> function_index_symbol(
+      isolate->heap()->wasm_function_index_symbol());

[Igor Sheludko, 2017/07/18 17:32:39]

s/heap()->// to avoid handle creation.

[titzer, 2017/07/18 18:07:40]

Done.

+  JSObject::AddProperty(js_function, function_index_symbol,
+                        isolate->factory()->NewNumber(func_index), DONT_ENUM);
 
-  return Handle<WasmExportedFunction>::cast(function);
+  return Handle<WasmExportedFunction>::cast(js_function);
 }
 
 bool WasmSharedModuleData::IsWasmSharedModuleData(Object* object) {
   if (!object->IsFixedArray()) return false;
   FixedArray* arr = FixedArray::cast(object);
   if (arr->length() != kFieldCount) return false;
   Isolate* isolate = arr->GetIsolate();
   if (!arr->get(kModuleWrapperIndex)->IsForeign()) return false;
   if (!arr->get(kModuleBytesIndex)->IsUndefined(isolate) &&
       !arr->get(kModuleBytesIndex)->IsSeqOneByteString())
@@ skipping 841 matching lines @@
     Isolate* isolate, Handle<WasmInstanceObject> instance, Handle<Code> caller,
     int offset, int func_index, bool patch_caller) {
   isolate->set_context(*instance->compiled_module()->native_context());
   Object* orch_obj =
       instance->compiled_module()->shared()->lazy_compilation_orchestrator();
   LazyCompilationOrchestrator* orch =
       Managed<LazyCompilationOrchestrator>::cast(orch_obj)->get();
   return orch->CompileLazy(isolate, instance, caller, offset, func_index,
                            patch_caller);
 }