[inspector] Send type as description for WasmValueObject.

Also block sending "type" as part of the ObjectPreview, but only send
the "value" property. The front-end will be updated to display
WasmValueObject's similar to what we do for wrapper objects (i.e.
StringWrapper and the like). The matching front-end change is still
pending.

Also refactor the WasmValueObject to have dedicated constructors for
the individual types (i32, i64, f32, f64, externref and v128). This
way we can just reuse the existing logic in descriptionForObject()
and we also don't need to store the "type" on the object itself (not
really performance sensitive, but fewer moving parts / things that
can go wrong).

This also addresses the crash in https://crbug.com/1166077#c16 since
the WasmValueObject instances now have a proper JSFunction in their
maps' constructor_or_backpointer slot and are thus able to locate
their creation context. Note that this doesn't generally address
https://crbug.com/1166077 itself, but only the WasmValueObject case.

Screenshot: https://imgur.com/kbd3bix.png
Bug: chromium:1170282, chromium:1071432
Bug: chromium:1159402, chromium:1166077
Change-Id: Iae649cad155efd774cfb1f4eea8cf406e413c03a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2692574
Reviewed-by: Philip Pfaffe <pfaffe@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72736}

src/debug/debug-interface.cc

@@ -1119,22 +1119,6 @@ bool WasmValueObject::IsWasmValueObject(Local<Value> that) {
   return obj->IsWasmValueObject();
 }
 
-Local<String> WasmValueObject::type() const {
-  i::Handle<i::WasmValueObject> object =
-      i::Handle<i::WasmValueObject>::cast(Utils::OpenHandle(this));
-  i::Isolate* isolate = object->GetIsolate();
-  i::Handle<i::String> type(object->type(), isolate);
-  return Utils::ToLocal(type);
-}
-
-Local<Value> WasmValueObject::value() const {
-  i::Handle<i::WasmValueObject> object =
-      i::Handle<i::WasmValueObject>::cast(Utils::OpenHandle(this));
-  i::Isolate* isolate = object->GetIsolate();
-  i::Handle<i::Object> value(object->value(), isolate);
-  return Utils::ToLocal(value);
-}
-
 MaybeLocal<Message> GetMessageFromPromise(Local<Promise> p) {
   i::Handle<i::JSPromise> promise = Utils::OpenHandle(*p);
   i::Isolate* isolate = promise->GetIsolate();

src/debug/debug-interface.h

@@ -622,9 +622,6 @@ class V8_EXPORT_PRIVATE WasmValueObject : public v8::Object {
   static bool IsWasmValueObject(v8::Local<v8::Value> obj);
   V8_INLINE static WasmValueObject* Cast(v8::Value* obj);
 
-  v8::Local<v8::String> type() const;
-  v8::Local<v8::Value> value() const;
-
  private:
   static void CheckCast(v8::Value* obj);
 };

src/debug/debug-wasm-objects-inl.h

@@ -20,7 +20,6 @@ OBJECT_CONSTRUCTORS_IMPL(WasmValueObject, JSObject)
 
 CAST_ACCESSOR(WasmValueObject)
 
-ACCESSORS(WasmValueObject, type, String, kTypeOffset)
 ACCESSORS(WasmValueObject, value, Object, kValueOffset)
 
 }  // namespace internal

src/debug/debug-wasm-objects.cc

@@ -82,8 +82,10 @@ enum DebugProxyId {
   kNumInstanceProxies = kLastInstanceProxyId + 1
 };
 
-constexpr int kWasmValueMapIndex = kNumProxies;
-constexpr int kNumDebugMaps = kWasmValueMapIndex + 1;
+constexpr int kFirstWasmValueMapIndex = kNumProxies;
+constexpr int kLastWasmValueMapIndex =
+    kFirstWasmValueMapIndex + WasmValueObject::kNumTypes - 1;
+constexpr int kNumDebugMaps = kLastWasmValueMapIndex + 1;
 
 Handle<FixedArray> GetOrCreateDebugMaps(Isolate* isolate) {
   Handle<FixedArray> maps = isolate->wasm_debug_maps();
@@ -751,23 +753,53 @@ Handle<String> WasmSimd128ToString(Isolate* isolate, wasm::Simd128 s128) {
   return isolate->factory()->NewStringFromAsciiChecked(buffer.data());
 }
 
+Handle<String> Type2String(Isolate* isolate, WasmValueObject::Type type) {
+  switch (type) {
+    case WasmValueObject::kExternRef:
+      return isolate->factory()->InternalizeString(
+          StaticCharVector("externref"));
+    case WasmValueObject::kF32:
+      return isolate->factory()->InternalizeString(StaticCharVector("f32"));
+    case WasmValueObject::kF64:
+      return isolate->factory()->InternalizeString(StaticCharVector("f64"));
+    case WasmValueObject::kI32:
+      return isolate->factory()->InternalizeString(StaticCharVector("i32"));
+    case WasmValueObject::kI64:
+      return isolate->factory()->InternalizeString(StaticCharVector("i64"));
+    case WasmValueObject::kV128:
+      return isolate->factory()->InternalizeString(StaticCharVector("v128"));
+    case WasmValueObject::kNumTypes:
+      break;
+  }
+  UNREACHABLE();
+}
+
 }  // namespace
 
 // static
-Handle<WasmValueObject> WasmValueObject::New(Isolate* isolate,
-                                             Handle<String> type,
+Handle<WasmValueObject> WasmValueObject::New(Isolate* isolate, Type type,
                                              Handle<Object> value) {
+  int map_index = kFirstWasmValueMapIndex + type;
+  DCHECK_LE(kFirstWasmValueMapIndex, map_index);
+  DCHECK_LE(map_index, kLastWasmValueMapIndex);
   auto maps = GetOrCreateDebugMaps(isolate);
-  if (maps->is_the_hole(isolate, kWasmValueMapIndex)) {
+  if (maps->is_the_hole(isolate, map_index)) {
+    auto type_name = Type2String(isolate, type);
+    auto shared = isolate->factory()->NewSharedFunctionInfoForBuiltin(
+        type_name, Builtins::kIllegal);
+    shared->set_language_mode(LanguageMode::kStrict);
+    auto constructor =
+        Factory::JSFunctionBuilder{isolate, shared, isolate->native_context()}
+            .set_map(isolate->strict_function_map())
+            .Build();
     Handle<Map> map = isolate->factory()->NewMap(
         WASM_VALUE_OBJECT_TYPE, WasmValueObject::kSize,
-        TERMINAL_FAST_ELEMENTS_KIND, 2);
+        TERMINAL_FAST_ELEMENTS_KIND, 1);
     Map::EnsureDescriptorSlack(isolate, map, 2);
     {  // type
-      Descriptor d = Descriptor::DataField(
-          isolate,
+      Descriptor d = Descriptor::DataConstant(
           isolate->factory()->InternalizeString(StaticCharVector("type")),
-          WasmValueObject::kTypeIndex, FROZEN, Representation::Tagged());
+          type_name, FROZEN);
       map->AppendDescriptor(isolate, &d);
     }
     {  // value
@@ -777,14 +809,13 @@ Handle<WasmValueObject> WasmValueObject::New(Isolate* isolate,
           WasmValueObject::kValueIndex, FROZEN, Representation::Tagged());
       map->AppendDescriptor(isolate, &d);
     }
+    map->set_constructor_or_back_pointer(*constructor);
     map->set_is_extensible(false);
-    maps->set(kWasmValueMapIndex, *map);
+    maps->set(map_index, *map);
   }
-  Handle<Map> value_map =
-      handle(Map::cast(maps->get(kWasmValueMapIndex)), isolate);
+  Handle<Map> value_map(Map::cast(maps->get(map_index)), isolate);
   Handle<WasmValueObject> object = Handle<WasmValueObject>::cast(
       isolate->factory()->NewJSObjectFromMap(value_map));
-  object->set_type(*type);
   object->set_value(*value);
   return object;
 }
@@ -792,44 +823,22 @@ Handle<WasmValueObject> WasmValueObject::New(Isolate* isolate,
 // static
 Handle<WasmValueObject> WasmValueObject::New(Isolate* isolate,
                                              const wasm::WasmValue& value) {
-  Handle<String> t;
-  Handle<Object> v;
   switch (value.type().kind()) {
-    case wasm::ValueType::kI32: {
-      t = isolate->factory()->InternalizeString(StaticCharVector("i32"));
-      v = isolate->factory()->NewNumberFromInt(value.to_i32_unchecked());
-      break;
-    }
-    case wasm::ValueType::kI64: {
-      t = isolate->factory()->InternalizeString(StaticCharVector("i64"));
-      v = BigInt::FromInt64(isolate, value.to_i64_unchecked());
-      break;
-    }
-    case wasm::ValueType::kF32: {
-      t = isolate->factory()->InternalizeString(StaticCharVector("f32"));
-      v = isolate->factory()->NewNumber(value.to_f32_unchecked());
-      break;
-    }
-    case wasm::ValueType::kF64: {
-      t = isolate->factory()->InternalizeString(StaticCharVector("f64"));
-      v = isolate->factory()->NewNumber(value.to_f64_unchecked());
-      break;
-    }
-    case wasm::ValueType::kS128: {
-      t = isolate->factory()->InternalizeString(StaticCharVector("v128"));
-      v = WasmSimd128ToString(isolate, value.to_s128_unchecked());
-      break;
-    }
-    case wasm::ValueType::kRef: {
-      t = isolate->factory()->InternalizeString(StaticCharVector("externref"));
-      v = value.to_externref();
-      break;
-    }
+    case wasm::ValueType::kF32:
+      return New(isolate, kF32, isolate->factory()->NewNumber(value.to_f32()));
+    case wasm::ValueType::kF64:
+      return New(isolate, kF64, isolate->factory()->NewNumber(value.to_f64()));
+    case wasm::ValueType::kI32:
+      return New(isolate, kI32, isolate->factory()->NewNumber(value.to_i32()));
+    case wasm::ValueType::kI64:
+      return New(isolate, kI64, BigInt::FromInt64(isolate, value.to_i64()));
+    case wasm::ValueType::kRef:
+      return New(isolate, kExternRef, value.to_externref());
+    case wasm::ValueType::kS128:
+      return New(isolate, kV128, WasmSimd128ToString(isolate, value.to_s128()));
     default:
       UNREACHABLE();
   }
-
-  return New(isolate, t, v);
 }
 
 Handle<JSObject> GetWasmDebugProxy(WasmFrame* frame) {

src/debug/debug-wasm-objects.h

@@ -32,7 +32,6 @@ class WasmValueObject : public JSObject {
  public:
   DECL_CAST(WasmValueObject)
 
-  DECL_ACCESSORS(type, String)
   DECL_ACCESSORS(value, Object)
 
   // Dispatched behavior.
@@ -41,17 +40,17 @@ class WasmValueObject : public JSObject {
 
 // Layout description.
 #define WASM_VALUE_FIELDS(V)   \
-  V(kTypeOffset, kTaggedSize)  \
   V(kValueOffset, kTaggedSize) \
   V(kSize, 0)
   DEFINE_FIELD_OFFSET_CONSTANTS(JSObject::kHeaderSize, WASM_VALUE_FIELDS)
 #undef WASM_VALUE_FIELDS
 
   // Indices of in-object properties.
-  static constexpr int kTypeIndex = 0;
-  static constexpr int kValueIndex = 1;
+  static constexpr int kValueIndex = 0;
 
-  static Handle<WasmValueObject> New(Isolate* isolate, Handle<String> type,
+  enum Type { kExternRef, kF32, kF64, kI32, kI64, kV128, kNumTypes };
+
+  static Handle<WasmValueObject> New(Isolate* isolate, Type type,
                                      Handle<Object> value);
   static Handle<WasmValueObject> New(Isolate* isolate,
                                      const wasm::WasmValue& value);

src/diagnostics/objects-debug.cc

@@ -1548,7 +1548,6 @@ void WasmInstanceObject::WasmInstanceObjectVerify(Isolate* isolate) {
 void WasmValueObject::WasmValueObjectVerify(Isolate* isolate) {
   JSObjectVerify(isolate);
   CHECK(IsWasmValueObject());
-  CHECK(type().IsString());
 }
 
 void WasmExportedFunctionData::WasmExportedFunctionDataVerify(

src/diagnostics/objects-printer.cc

@@ -1902,7 +1902,6 @@ void WasmTableObject::WasmTableObjectPrint(std::ostream& os) {  // NOLINT
 
 void WasmValueObject::WasmValueObjectPrint(std::ostream& os) {  // NOLINT
   PrintHeader(os, "WasmValueObject");
-  os << "\n - type: " << Brief(type());
   os << "\n - value: " << Brief(value());
   os << "\n";
 }

src/inspector/value-mirror.cc

@@ -305,22 +305,6 @@ String16 descriptionForCollection(v8::Isolate* isolate,
   return String16::concat(className, '(', String16::fromInteger(length), ')');
 }
 
-String16 descriptionForWasmValueObject(
-    v8::Local<v8::Context> context,
-    v8::Local<v8::debug::WasmValueObject> object) {
-  v8::Isolate* isolate = context->GetIsolate();
-  auto type = toProtocolString(isolate, object->type());
-  auto value = object->value();
-  if (type == "i32" || type == "f32" || type == "f64") {
-    return String16::fromDouble(v8::Local<v8::Number>::Cast(value)->Value());
-  } else if (type == "i64") {
-    return descriptionForBigInt(context, v8::Local<v8::BigInt>::Cast(value));
-  } else if (type == "v128") {
-    return toProtocolString(isolate, v8::Local<v8::String>::Cast(value));
-  }
-  return type;
-}
-
 String16 descriptionForEntry(v8::Local<v8::Context> context,
                              v8::Local<v8::Object> object) {
   v8::Isolate* isolate = context->GetIsolate();
@@ -809,6 +793,8 @@ bool getPropertiesForPreview(v8::Local<v8::Context> context,
   if (object->IsArray() || isArrayLike(context, object, &length) ||
       object->IsStringObject()) {
     blocklist.push_back("length");
+  } else if (v8::debug::WasmValueObject::IsWasmValueObject(object)) {
+    blocklist.push_back("type");
   } else {
     auto clientSubtype = clientFor(context)->valueSubtype(object);
     if (clientSubtype && toString16(clientSubtype->string()) == "array") {
@@ -1708,11 +1694,11 @@ std::unique_ptr<ValueMirror> ValueMirror::create(v8::Local<v8::Context> context,
             isolate, memory, memory->Buffer()->ByteLength() / kWasmPageSize));
   }
   if (v8::debug::WasmValueObject::IsWasmValueObject(value)) {
-    v8::Local<v8::debug::WasmValueObject> v =
+    v8::Local<v8::debug::WasmValueObject> object =
         value.As<v8::debug::WasmValueObject>();
     return std::make_unique<ObjectMirror>(
         value, RemoteObject::SubtypeEnum::Wasmvalue,
-        descriptionForWasmValueObject(context, v));
+        descriptionForObject(isolate, object));
   }
   V8InternalValueType internalType =
       v8InternalValueTypeFrom(context, value.As<v8::Object>());

test/inspector/debugger/wasm-evaluate-on-call-frame-expected.txt

@@ -18,33 +18,33 @@ Debugger paused in main.
 > globals = Globals
 > typeof globals = "object"
 > Object.keys(globals) = Array(2)
-> globals[0] = 0
-> globals[1] = 1
-> globals[2] = 2n
-> globals[3] = 3n
-> globals["$global0"] = 0
-> $global0 = 0
-> globals["$global3"] = 2n
-> $global3 = 2n
+> globals[0] = i32 {0}
+> globals[1] = i32 {1}
+> globals[2] = i64 {2n}
+> globals[3] = i64 {3n}
+> globals["$global0"] = i32 {0}
+> $global0 = i32 {0}
+> globals["$global3"] = i64 {2n}
+> $global3 = i64 {2n}
 Stepping twice in main.
 Debugger paused in main.
-> globals[0] = 0
-> globals[1] = 1
-> globals[2] = 2n
-> globals[3] = 42n
-> globals["$global0"] = 0
-> $global0 = 0
-> globals["$global3"] = 2n
-> $global3 = 2n
+> globals[0] = i32 {0}
+> globals[1] = i32 {1}
+> globals[2] = i64 {2n}
+> globals[3] = i64 {42n}
+> globals["$global0"] = i32 {0}
+> $global0 = i32 {0}
+> globals["$global3"] = i64 {2n}
+> $global3 = i64 {2n}
 Changing global from JavaScript.
-> globals[0] = 0
-> globals[1] = 21
-> globals[2] = 2n
-> globals[3] = 42n
-> globals["$global0"] = 0
-> $global0 = 0
-> globals["$global3"] = 2n
-> $global3 = 2n
+> globals[0] = i32 {0}
+> globals[1] = i32 {21}
+> globals[2] = i64 {2n}
+> globals[3] = i64 {42n}
+> globals["$global0"] = i32 {0}
+> $global0 = i32 {0}
+> globals["$global3"] = i64 {2n}
+> $global3 = i64 {2n}
 
 Running test: testFunctions
 Compile module.
@@ -77,22 +77,22 @@ Debugger paused in main.
 > locals = Locals
 > typeof locals = "object"
 > Object.keys(locals) = Array(2)
-> locals[0] = 3
-> locals[1] = 6
-> locals[2] = 0
-> locals["$x"] = 3
-> $x = 3
-> locals["$var2"] = 0
-> $var2 = 0
+> locals[0] = i32 {3}
+> locals[1] = i32 {6}
+> locals[2] = i32 {0}
+> locals["$x"] = i32 {3}
+> $x = i32 {3}
+> locals["$var2"] = i32 {0}
+> $var2 = i32 {0}
 Stepping twice in main.
 Debugger paused in main.
-> locals[0] = 3
-> locals[1] = 6
-> locals[2] = 42
-> locals["$x"] = 3
-> $x = 3
-> locals["$var2"] = 42
-> $var2 = 42
+> locals[0] = i32 {3}
+> locals[1] = i32 {6}
+> locals[2] = i32 {42}
+> locals["$x"] = i32 {3}
+> $x = i32 {3}
+> locals["$var2"] = i32 {42}
+> $var2 = i32 {42}
 
 Running test: testMemories
 Compile module.
@@ -133,5 +133,5 @@ Stepping twice in main.
 Debugger paused in main.
 > stack = Stack
 > Object.keys(stack) = Array(2)
-> stack[0] = 5
-> stack[1] = 42
+> stack[0] = i32 {5}
+> stack[1] = i32 {42}

test/inspector/debugger/wasm-evaluate-on-call-frame.js

@@ -31,13 +31,17 @@ async function instantiateModule({objectId}, importObject) {
 }
 
 async function dumpOnCallFrame(callFrameId, expression) {
-  const {result: {result}} = await Protocol.Debugger.evaluateOnCallFrame({
+  const {result: {result: object}} = await Protocol.Debugger.evaluateOnCallFrame({
     callFrameId, expression
   });
-  if ('description' in result) {
-    InspectorTest.log(`> ${expression} = ${result.description}`);
+  if (object.type === 'object' && object.subtype === 'wasmvalue') {
+    const {result: {result: properties}} = await Protocol.Runtime.getProperties({objectId: object.objectId, ownProperties: true})
+    const valueProperty = properties.find(p => p.name === 'value');
+    InspectorTest.log(`> ${expression} = ${object.description} {${valueProperty.value.description}}`);
+  } else if ('description' in object) {
+    InspectorTest.log(`> ${expression} = ${object.description}`);
   } else {
-    InspectorTest.log(`> ${expression} = ${JSON.stringify(result.value)}`);
+    InspectorTest.log(`> ${expression} = ${JSON.stringify(object.value)}`);
   }
 }