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

src/wasm/wasm-objects.cc

Index: src/wasm/wasm-objects.cc
diff --git a/src/wasm/wasm-objects.cc b/src/wasm/wasm-objects.cc
index e5b3bfdbf82c5ce4feff8bdc9fbcb7793517befc..a09717d252aa0ca764fcdb463b239d8e814fb964 100644
--- a/src/wasm/wasm-objects.cc
+++ b/src/wasm/wasm-objects.cc
@@ -485,18 +485,43 @@ uint32_t WasmInstanceObject::GetMaxMemoryPages() {
   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;
@@ -506,22 +531,24 @@ Handle<WasmExportedFunction> WasmExportedFunction::New(
                    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) {