[tools][torque] Include string values in GetObjectProperties responses

This change provides a quick way to see string contents in postmortem
debugging sessions, without digging through a (possibly very large, in
the case of ConsString) tree of properties. As well as being convenient
for inspecting String objects, this functionality will also be necessary
for displaying property names on JSReceiver objects. In order to support
custom behaviors for specific classes, this change extends the existing
generated debug reader classes with a visitor pattern.

Bug: v8:9376
Change-Id: I70eab9ea4e74ca0fab39bf5998d6a602716a4202
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1771939
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#63485}

src/torque/class-debug-reader-generator.cc

@@ -12,7 +12,7 @@ namespace torque {
 
 namespace {
 void GenerateClassDebugReader(const ClassType& type, std::ostream& h_contents,
-                              std::ostream& cc_contents,
+                              std::ostream& cc_contents, std::ostream& visitor,
                               std::unordered_set<const ClassType*>* done) {
   // Make sure each class only gets generated once.
   if (!type.IsExtern() || !done->insert(&type).second) return;
@@ -21,7 +21,8 @@ void GenerateClassDebugReader(const ClassType& type, std::ostream& h_contents,
   // We must emit the classes in dependency order. If the super class hasn't
   // been emitted yet, go handle it first.
   if (super_type != nullptr) {
-    GenerateClassDebugReader(*super_type, h_contents, cc_contents, done);
+    GenerateClassDebugReader(*super_type, h_contents, cc_contents, visitor,
+                             done);
   }
 
   const std::string name = type.name();
@@ -32,7 +33,24 @@ void GenerateClassDebugReader(const ClassType& type, std::ostream& h_contents,
   h_contents << "  inline Tq" << name << "(uintptr_t address) : Tq"
              << super_name << "(address) {}\n";
   h_contents << "  std::vector<std::unique_ptr<ObjectProperty>> "
-                "GetProperties(d::MemoryAccessor accessor);\n";
+                "GetProperties(d::MemoryAccessor accessor) const override;\n";
+  h_contents << "  const char* GetName() const override;\n";
+  h_contents << "  void Visit(TqObjectVisitor* visitor) const override;\n";
+
+  cc_contents << "\nconst char* Tq" << name << "::GetName() const {\n";
+  cc_contents << "  return \"v8::internal::" << name << "\";\n";
+  cc_contents << "}\n";
+
+  cc_contents << "\nvoid Tq" << name
+              << "::Visit(TqObjectVisitor* visitor) const {\n";
+  cc_contents << "  visitor->Visit" << name << "(this);\n";
+  cc_contents << "}\n";
+
+  visitor << "  virtual void Visit" << name << "(const Tq" << name
+          << "* object) {\n";
+  visitor << "    Visit" << super_name << "(object);\n";
+  visitor << "  }\n";
+
   std::stringstream get_props_impl;
 
   for (const Field& field : type.fields()) {
@@ -86,6 +104,8 @@ void GenerateClassDebugReader(const ClassType& type, std::ostream& h_contents,
         "Get" + CamelifyString(field_name) + "Address";
 
     std::string indexed_field_info;
+    std::string index_param;
+    std::string index_offset;
     if (field.index) {
       const Type* index_type = (*field.index)->name_and_type.type;
       std::string index_type_name;
@@ -113,6 +133,8 @@ void GenerateClassDebugReader(const ClassType& type, std::ostream& h_contents,
                      << "Value(accessor);\n";
       indexed_field_info =
           ", " + index_value + ", GetArrayKind(indexed_field_count.validity)";
+      index_param = ", size_t offset";
+      index_offset = " + offset * sizeof(value)";
     }
     get_props_impl
         << "  result.push_back(v8::base::make_unique<ObjectProperty>(\""
@@ -120,20 +142,21 @@ void GenerateClassDebugReader(const ClassType& type, std::ostream& h_contents,
         << field_cc_type << "\", " << address_getter << "()"
         << indexed_field_info << "));\n";
 
-    h_contents << "  uintptr_t " << address_getter << "();\n";
+    h_contents << "  uintptr_t " << address_getter << "() const;\n";
     h_contents << "  Value<" << field_value_type << "> " << field_getter
-               << "(d::MemoryAccessor accessor);\n";
+               << "(d::MemoryAccessor accessor " << index_param << ") const;\n";
     cc_contents << "\nuintptr_t Tq" << name << "::" << address_getter
-                << "() {\n";
+                << "() const {\n";
     cc_contents << "  return address_ - i::kHeapObjectTag + " << field.offset
                 << ";\n";
     cc_contents << "}\n";
     cc_contents << "\nValue<" << field_value_type << "> Tq" << name
-                << "::" << field_getter << "(d::MemoryAccessor accessor) {\n";
+                << "::" << field_getter << "(d::MemoryAccessor accessor"
+                << index_param << ") const {\n";
     cc_contents << "  " << field_value_type_compressed << " value{};\n";
     cc_contents << "  d::MemoryAccessResult validity = accessor("
-                << address_getter
+                << address_getter << "()" << index_offset
-                << "(), reinterpret_cast<uint8_t*>(&value), sizeof(value));\n";
+                << ", reinterpret_cast<uint8_t*>(&value), sizeof(value));\n";
     cc_contents << "  return {validity, "
                 << (is_field_tagged ? "Decompress(value, address_)" : "value")
                 << "};\n";
@@ -143,7 +166,7 @@ void GenerateClassDebugReader(const ClassType& type, std::ostream& h_contents,
   h_contents << "};\n";
 
   cc_contents << "\nstd::vector<std::unique_ptr<ObjectProperty>> Tq" << name
-              << "::GetProperties(d::MemoryAccessor accessor) {\n";
+              << "::GetProperties(d::MemoryAccessor accessor) const {\n";
   cc_contents << "  std::vector<std::unique_ptr<ObjectProperty>> result = Tq"
               << super_name << "::GetProperties(accessor);\n";
   cc_contents << get_props_impl.str();
@@ -176,11 +199,19 @@ void ImplementationVisitor::GenerateClassDebugReaders(
     NamespaceScope h_namespaces(h_contents, {"v8_debug_helper_internal"});
     NamespaceScope cc_namespaces(cc_contents, {"v8_debug_helper_internal"});
 
+    std::stringstream visitor;
+    visitor << "\nclass TqObjectVisitor {\n";
+    visitor << " public:\n";
+    visitor << "  virtual void VisitObject(const TqObject* object) {}\n";
+
     std::unordered_set<const ClassType*> done;
     for (const TypeAlias* alias : GlobalContext::GetClasses()) {
       const ClassType* type = ClassType::DynamicCast(alias->type());
-      GenerateClassDebugReader(*type, h_contents, cc_contents, &done);
+      GenerateClassDebugReader(*type, h_contents, cc_contents, visitor, &done);
     }
+
+    visitor << "};\n";
+    h_contents << visitor.str();
   }
   WriteFile(output_directory + "/" + file_name + ".h", h_contents.str());
   WriteFile(output_directory + "/" + file_name + ".cc", cc_contents.str());

test/cctest/BUILD.gn

@@ -4,6 +4,13 @@
 
 import("../../gni/v8.gni")
 
+config("cctest_config") {
+  # Work around a bug in the gold linker.
+  if (use_gold && target_cpu == "x86") {
+    ldflags = [ "-Wl,--icf=none" ]
+  }
+}
+
 v8_executable("cctest") {
   testonly = true
 
@@ -28,6 +35,7 @@ v8_executable("cctest") {
   configs = [
     "../..:external_config",
     "../..:internal_config_base",
+    ":cctest_config",
   ]
 
   ldflags = []

test/cctest/test-debug-helper.cc

@@ -109,8 +109,9 @@ TEST(GetObjectProperties) {
     props = d::GetObjectProperties(properties_or_hash, &ReadMemory, roots);
     CHECK(props->type_check_result ==
           d::TypeCheckResult::kObjectPointerValidButInaccessible);
-    CHECK(props->type == std::string("v8::internal::Object"));
+    CHECK(props->type == std::string("v8::internal::HeapObject"));
-    CHECK_EQ(props->num_properties, 0);
+    CHECK_EQ(props->num_properties, 1);
+    CheckProp(*props->properties[0], "v8::internal::Map", "map");
     CHECK(std::string(props->brief).substr(0, 21) ==
           std::string("maybe EmptyFixedArray"));
 
@@ -123,8 +124,9 @@ TEST(GetObjectProperties) {
     props = d::GetObjectProperties(properties_or_hash, &ReadMemory, roots);
     CHECK(props->type_check_result ==
           d::TypeCheckResult::kObjectPointerValidButInaccessible);
-    CHECK(props->type == std::string("v8::internal::Object"));
+    CHECK(props->type == std::string("v8::internal::HeapObject"));
-    CHECK_EQ(props->num_properties, 0);
+    CHECK_EQ(props->num_properties, 1);
+    CheckProp(*props->properties[0], "v8::internal::Map", "map");
     CHECK(std::string(props->brief).substr(0, 15) ==
           std::string("EmptyFixedArray"));
   }
@@ -192,6 +194,33 @@ TEST(GetObjectProperties) {
   CheckProp(*props2->properties[1], "uint32_t", "hash_field",
             *reinterpret_cast<i::Tagged_t*>(props->properties[1]->address));
   CheckProp(*props2->properties[2], "int32_t", "length", 2);
+
+  // Build a complicated string (multi-level cons with slices inside) to test
+  // string printing.
+  v = CompileRun(R"(
+    const alphabet = "abcdefghijklmnopqrstuvwxyz";
+    alphabet.substr(3,20) + alphabet.toUpperCase().substr(5,15) + "7")");
+  o = v8::Utils::OpenHandle(*v);
+  props = d::GetObjectProperties(o->ptr(), &ReadMemory, roots);
+  CHECK(std::string(props->brief).substr(0, 38) ==
+        std::string("\"defghijklmnopqrstuvwFGHIJKLMNOPQRST7\""));
+
+  // Cause a failure when reading the "second" pointer within the top-level
+  // ConsString.
+  {
+    CheckProp(*props->properties[4], "v8::internal::String", "second");
+    uintptr_t second_address = props->properties[4]->address;
+    MemoryFailureRegion failure(second_address, second_address + 4);
+    props = d::GetObjectProperties(o->ptr(), &ReadMemory, roots);
+    CHECK(std::string(props->brief).substr(0, 40) ==
+          std::string("\"defghijklmnopqrstuvwFGHIJKLMNOPQRST...\""));
+  }
+
+  // Build a very long string.
+  v = CompileRun("'a'.repeat(1000)");
+  o = v8::Utils::OpenHandle(*v);
+  props = d::GetObjectProperties(o->ptr(), &ReadMemory, roots);
+  CHECK(std::string(props->brief).substr(79, 7) == std::string("aa...\" "));
 }
 
 }  // namespace internal

tools/debug_helper/debug-helper-internal.cc

@@ -4,6 +4,7 @@
 
 #include "debug-helper-internal.h"
 #include "src/common/ptr-compr-inl.h"
+#include "torque-generated/class-debug-readers-tq.h"
 
 namespace i = v8::internal;
 
@@ -44,8 +45,14 @@ d::PropertyKind GetArrayKind(d::MemoryAccessResult mem_result) {
 }
 
 std::vector<std::unique_ptr<ObjectProperty>> TqObject::GetProperties(
-    d::MemoryAccessor accessor) {
+    d::MemoryAccessor accessor) const {
   return std::vector<std::unique_ptr<ObjectProperty>>();
 }
 
+const char* TqObject::GetName() const { return "v8::internal::Object"; }
+
+void TqObject::Visit(TqObjectVisitor* visitor) const {
+  visitor->VisitObject(this);
+}
+
 }  // namespace v8_debug_helper_internal

tools/debug_helper/debug-helper-internal.h

@@ -104,13 +104,18 @@ class ObjectPropertiesResult {
   std::vector<d::ObjectProperty*> properties_raw_;
 };
 
+class TqObjectVisitor;
+
 // Base class representing a V8 object in the debuggee's address space.
 // Subclasses for specific object types are generated by the Torque compiler.
 class TqObject {
  public:
   inline TqObject(uintptr_t address) : address_(address) {}
-  std::vector<std::unique_ptr<ObjectProperty>> GetProperties(
+  virtual ~TqObject() = default;
-      d::MemoryAccessor accessor);
+  virtual std::vector<std::unique_ptr<ObjectProperty>> GetProperties(
+      d::MemoryAccessor accessor) const;
+  virtual const char* GetName() const;
+  virtual void Visit(TqObjectVisitor* visitor) const;
 
  protected:
   uintptr_t address_;

tools/debug_helper/get-object-properties.cc

@@ -9,6 +9,7 @@
 #include "include/v8-internal.h"
 #include "src/common/ptr-compr-inl.h"
 #include "src/objects/string-inl.h"
+#include "src/strings/unicode-inl.h"
 #include "torque-generated/class-debug-readers-tq.h"
 
 namespace i = v8::internal;
@@ -142,26 +143,20 @@ std::string AppendAddressAndType(const std::string& brief, uintptr_t address,
                        : brief + " (" + brief_stream.str() + ")";
 }
 
-struct TypeNameAndProps {
+struct TypedObject {
-  TypeNameAndProps(d::TypeCheckResult type_check_result, std::string&& type,
+  TypedObject(d::TypeCheckResult type_check_result,
-                   std::vector<std::unique_ptr<ObjectProperty>>&& properties)
+              std::unique_ptr<TqObject> object)
-      : type_check_result(type_check_result),
+      : type_check_result(type_check_result), object(std::move(object)) {}
-        type_name(std::move(type)),
-        props(std::move(properties)) {}
-  explicit TypeNameAndProps(d::TypeCheckResult type_check_result)
-      : type_check_result(type_check_result) {}
   d::TypeCheckResult type_check_result;
-  std::string type_name;
+  std::unique_ptr<TqObject> object;
-  std::vector<std::unique_ptr<ObjectProperty>> props;
 };
 
-TypeNameAndProps GetTypeNameAndPropsByHint(uintptr_t address,
+TypedObject GetTypedObjectByHint(uintptr_t address,
-                                           d::MemoryAccessor accessor,
+                                 std::string type_hint_string) {
-                                           std::string type_hint_string) {
+#define TYPE_NAME_CASE(ClassName, ...)                      \
-#define TYPE_NAME_CASE(ClassName, ...)                       \
+  if (type_hint_string == "v8::internal::" #ClassName) {    \
-  if (type_hint_string == "v8::internal::" #ClassName) {     \
+    return {d::TypeCheckResult::kUsedTypeHint,              \
-    return {d::TypeCheckResult::kUsedTypeHint, #ClassName,   \
+            v8::base::make_unique<Tq##ClassName>(address)}; \
-            Tq##ClassName(address).GetProperties(accessor)}; \
   }
 
   TQ_INSTANCE_TYPES_SINGLE(TYPE_NAME_CASE)
@@ -169,47 +164,53 @@ TypeNameAndProps GetTypeNameAndPropsByHint(uintptr_t address,
 
 #undef TYPE_NAME_CASE
 
-  return TypeNameAndProps(d::TypeCheckResult::kUnknownTypeHint);
+  return {d::TypeCheckResult::kUnknownTypeHint,
+          v8::base::make_unique<TqHeapObject>(address)};
 }
 
-TypeNameAndProps GetTypeNameAndPropsForString(uintptr_t address,
+TypedObject GetTypedObjectForString(uintptr_t address, i::InstanceType type) {
-                                              d::MemoryAccessor accessor,
-                                              i::InstanceType type) {
   class StringGetDispatcher : public i::AllStatic {
    public:
-#define DEFINE_METHOD(ClassName)                             \
+#define DEFINE_METHOD(ClassName)                                   \
-  static inline TypeNameAndProps Handle##ClassName(          \
+  static inline TypedObject Handle##ClassName(uintptr_t address) { \
-      uintptr_t address, d::MemoryAccessor accessor) {       \
+    return {d::TypeCheckResult::kUsedMap,                          \
-    return {d::TypeCheckResult::kUsedMap, #ClassName,        \
+            v8::base::make_unique<Tq##ClassName>(address)};        \
-            Tq##ClassName(address).GetProperties(accessor)}; \
   }
     STRING_CLASS_TYPES(DEFINE_METHOD)
 #undef DEFINE_METHOD
-    static inline TypeNameAndProps HandleInvalidString(
+    static inline TypedObject HandleInvalidString(uintptr_t address) {
-        uintptr_t address, d::MemoryAccessor accessor) {
+      return {d::TypeCheckResult::kUnknownInstanceType,
-      return TypeNameAndProps(d::TypeCheckResult::kUnknownInstanceType);
+              v8::base::make_unique<TqString>(address)};
     }
   };
 
   return i::StringShape(type)
-      .DispatchToSpecificTypeWithoutCast<StringGetDispatcher, TypeNameAndProps>(
+      .DispatchToSpecificTypeWithoutCast<StringGetDispatcher, TypedObject>(
-          address, accessor);
+          address);
 }
 
-std::unique_ptr<ObjectPropertiesResult> GetHeapObjectProperties(
+TypedObject GetTypedHeapObject(uintptr_t address, d::MemoryAccessor accessor,
-    uintptr_t address, d::MemoryAccessor accessor, Value<i::InstanceType> type,
+                               const char* type_hint) {
-    const char* type_hint, std::string brief) {
+  auto heap_object = v8::base::make_unique<TqHeapObject>(address);
-  TypeNameAndProps tnp(d::TypeCheckResult::kUsedMap);
+  Value<uintptr_t> map_ptr = heap_object->GetMapValue(accessor);
+
+  if (map_ptr.validity != d::MemoryAccessResult::kOk) {
+    return {map_ptr.validity == d::MemoryAccessResult::kAddressNotValid
+                ? d::TypeCheckResult::kObjectPointerInvalid
+                : d::TypeCheckResult::kObjectPointerValidButInaccessible,
+            std::move(heap_object)};
+  }
+  Value<i::InstanceType> type =
+      TqMap(map_ptr.value).GetInstanceTypeValue(accessor);
 
   if (type.validity == d::MemoryAccessResult::kOk) {
     // Dispatch to the appropriate method for each instance type. After calling
     // the generated method to fetch properties, we can add custom properties.
     switch (type.value) {
-#define INSTANCE_TYPE_CASE(ClassName, INSTANCE_TYPE)            \
+#define INSTANCE_TYPE_CASE(ClassName, INSTANCE_TYPE) \
-  case i::INSTANCE_TYPE:                                        \
+  case i::INSTANCE_TYPE:                             \
-    tnp.type_name = #ClassName;                                 \
+    return {d::TypeCheckResult::kUsedMap,            \
-    tnp.props = Tq##ClassName(address).GetProperties(accessor); \
+            v8::base::make_unique<Tq##ClassName>(address)};
-    break;
       TQ_INSTANCE_TYPES_SINGLE(INSTANCE_TYPE_CASE)
 #undef INSTANCE_TYPE_CASE
 
@@ -219,54 +220,36 @@ std::unique_ptr<ObjectPropertiesResult> GetHeapObjectProperties(
         // main instance type list because they use the low bits of the instance
         // type enum as flags.
         if (type.value <= i::LAST_STRING_TYPE) {
-          tnp = GetTypeNameAndPropsForString(address, accessor, type.value);
+          return GetTypedObjectForString(address, type.value);
-          break;
         }
 
 #define INSTANCE_RANGE_CASE(ClassName, FIRST_TYPE, LAST_TYPE)      \
   if (type.value >= i::FIRST_TYPE && type.value <= i::LAST_TYPE) { \
-    tnp.type_name = #ClassName;                                    \
+    return {d::TypeCheckResult::kUsedMap,                          \
-    tnp.props = Tq##ClassName(address).GetProperties(accessor);    \
+            v8::base::make_unique<Tq##ClassName>(address)};        \
-    break;                                                         \
   }
         TQ_INSTANCE_TYPES_RANGE(INSTANCE_RANGE_CASE)
 #undef INSTANCE_RANGE_CASE
 
-        tnp.type_check_result = d::TypeCheckResult::kUnknownInstanceType;
+        return {d::TypeCheckResult::kUnknownInstanceType,
+                std::move(heap_object)};
         break;
     }
   } else if (type_hint != nullptr) {
     // Try to use the provided type hint, since the real instance type is
     // unavailable.
-    tnp = GetTypeNameAndPropsByHint(address, accessor, type_hint);
+    return GetTypedObjectByHint(address, type_hint);
   } else {
     // TODO(v8:9376): Use known maps here. If known map is just a guess (because
     // root pointers weren't provided), then create a synthetic property with
     // the more specific type. Then the caller could presumably ask us again
     // with the type hint we provided. Otherwise, just go ahead and use it to
     // generate properties.
-    tnp.type_check_result =
+    return {type.validity == d::MemoryAccessResult::kAddressNotValid
-        type.validity == d::MemoryAccessResult::kAddressNotValid
+                ? d::TypeCheckResult::kMapPointerInvalid
-            ? d::TypeCheckResult::kMapPointerInvalid
+                : d::TypeCheckResult::kMapPointerValidButInaccessible,
-            : d::TypeCheckResult::kMapPointerValidButInaccessible;
+            std::move(heap_object)};
   }
-
-  if (tnp.type_name.empty()) {
-    tnp.type_name = "Object";
-  }
-
-  // TODO(v8:9376): Many object types need additional data that is not included
-  // in their Torque layout definitions. For example, JSObject has an array of
-  // in-object properties after its Torque-defined fields, which at a minimum
-  // should be represented as an array in this response. If the relevant memory
-  // is available, we should instead represent those properties (and any out-of-
-  // object properties) using their JavaScript property names.
-
-  brief = AppendAddressAndType(brief, address, tnp.type_name.c_str());
-
-  return v8::base::make_unique<ObjectPropertiesResult>(
-      tnp.type_check_result, brief, "v8::internal::" + tnp.type_name,
-      std::move(tnp.props));
 }
 
 #undef STRUCT_INSTANCE_TYPE_ADAPTER
@@ -275,6 +258,194 @@ std::unique_ptr<ObjectPropertiesResult> GetHeapObjectProperties(
 #undef TQ_INSTANCE_TYPES_SINGLE_NOSTRUCTS
 #undef TQ_INSTANCE_TYPES_RANGE
 
+// An object visitor that accumulates the first few characters of a string.
+class ReadStringVisitor : public TqObjectVisitor {
+ public:
+  ReadStringVisitor(d::MemoryAccessor accessor)
+      : accessor_(accessor), index_(0), limit_(INT32_MAX), done_(false) {}
+
+  // Returns the result as UTF-8 once visiting is complete.
+  std::string GetString() {
+    std::vector<char> result(
+        string_.size() * unibrow::Utf16::kMaxExtraUtf8BytesForOneUtf16CodeUnit);
+    unsigned write_index = 0;
+    int prev_char = unibrow::Utf16::kNoPreviousCharacter;
+    for (size_t read_index = 0; read_index < string_.size(); ++read_index) {
+      uint16_t character = string_[read_index];
+      write_index +=
+          unibrow::Utf8::Encode(result.data() + write_index, character,
+                                prev_char, /*replace_invalid=*/true);
+      prev_char = character;
+    }
+    return {result.data(), write_index};
+  }
+
+  template <typename T>
+  void ReadSeqString(const T* object) {
+    int32_t length = GetOrFinish(object->GetLengthValue(accessor_));
+    for (; index_ < length && index_ < limit_ && !done_; ++index_) {
+      char16_t c = static_cast<char16_t>(
+          GetOrFinish(object->GetCharsValue(accessor_, index_)));
+      if (!done_) AddCharacter(c);
+    }
+  }
+
+  void VisitSeqOneByteString(const TqSeqOneByteString* object) override {
+    ReadSeqString(object);
+  }
+
+  void VisitSeqTwoByteString(const TqSeqTwoByteString* object) override {
+    ReadSeqString(object);
+  }
+
+  void VisitConsString(const TqConsString* object) override {
+    uintptr_t first_address = GetOrFinish(object->GetFirstValue(accessor_));
+    if (done_) return;
+    auto first = GetTypedHeapObject(first_address, accessor_, nullptr).object;
+    first->Visit(this);
+    if (done_) return;
+    int32_t first_length = GetOrFinish(
+        static_cast<TqString*>(first.get())->GetLengthValue(accessor_));
+    uintptr_t second = GetOrFinish(object->GetSecondValue(accessor_));
+    if (done_) return;
+    IndexModifier modifier(this, -first_length, -first_length);
+    GetTypedHeapObject(second, accessor_, nullptr).object->Visit(this);
+  }
+
+  void VisitSlicedString(const TqSlicedString* object) override {
+    uintptr_t parent = GetOrFinish(object->GetParentValue(accessor_));
+    int32_t length = GetOrFinish(object->GetLengthValue(accessor_));
+    int32_t offset = i::PlatformSmiTagging::SmiToInt(
+        GetOrFinish(object->GetOffsetValue(accessor_)));
+    if (done_) return;
+    int32_t limit_adjust = offset + length - limit_;
+    IndexModifier modifier(this, offset, limit_adjust < 0 ? limit_adjust : 0);
+    GetTypedHeapObject(parent, accessor_, nullptr).object->Visit(this);
+  }
+
+  void VisitThinString(const TqThinString* object) override {
+    uintptr_t actual = GetOrFinish(object->GetActualValue(accessor_));
+    if (done_) return;
+    GetTypedHeapObject(actual, accessor_, nullptr).object->Visit(this);
+  }
+
+  void VisitExternalString(const TqExternalString* object) override {
+    // TODO(v8:9376): External strings are very common and important when
+    // attempting to print the source of a function in the browser. For now
+    // we're just ignoring them, but eventually we'll want some kind of
+    // mechanism where the user of this library can provide a callback function
+    // that fetches data from external strings.
+    AddEllipsisAndFinish();
+  }
+
+  void VisitObject(const TqObject* object) override {
+    // If we fail to find a specific type for a sub-object within a cons string,
+    // sliced string, or thin string, we will end up here.
+    AddEllipsisAndFinish();
+  }
+
+ private:
+  // Unpacks a value that was fetched from the debuggee. If the value indicates
+  // that it couldn't successfully fetch memory, then prevents further work.
+  template <typename T>
+  T GetOrFinish(Value<T> value) {
+    if (value.validity != d::MemoryAccessResult::kOk) {
+      AddEllipsisAndFinish();
+    }
+    return value.value;
+  }
+
+  void AddEllipsisAndFinish() {
+    if (!done_) {
+      string_ += u"...";
+      done_ = true;
+    }
+  }
+
+  void AddCharacter(char16_t c) {
+    if (string_.size() >= kMaxCharacters) {
+      AddEllipsisAndFinish();
+    } else {
+      string_.push_back(c);
+    }
+  }
+
+  // Temporarily adds offsets to both index_ and limit_, to handle ConsString
+  // and SlicedString.
+  class IndexModifier {
+   public:
+    IndexModifier(ReadStringVisitor* that, int32_t index_adjust,
+                  int32_t limit_adjust)
+        : that_(that),
+          index_adjust_(index_adjust),
+          limit_adjust_(limit_adjust) {
+      that_->index_ += index_adjust_;
+      that_->limit_ += limit_adjust_;
+    }
+    ~IndexModifier() {
+      that_->index_ -= index_adjust_;
+      that_->limit_ -= limit_adjust_;
+    }
+
+   private:
+    ReadStringVisitor* that_;
+    int32_t index_adjust_;
+    int32_t limit_adjust_;
+    DISALLOW_COPY_AND_ASSIGN(IndexModifier);
+  };
+
+  static constexpr int kMaxCharacters = 80;  // How many characters to print.
+
+  std::u16string string_;  // Result string.
+  d::MemoryAccessor accessor_;
+  int32_t index_;  // Index of next char to read.
+  int32_t limit_;  // Don't read past this index (set by SlicedString).
+  bool done_;      // Whether to stop further work.
+};
+
+// An object visitor that adds extra debugging information for some types.
+class AddInfoVisitor : public TqObjectVisitor {
+ public:
+  AddInfoVisitor(const std::string& brief, d::MemoryAccessor accessor)
+      : accessor_(accessor), brief_(brief) {}
+
+  // Returns the brief object description, once visiting is complete.
+  const std::string& GetBrief() { return brief_; }
+
+  void VisitString(const TqString* object) override {
+    ReadStringVisitor visitor(accessor_);
+    object->Visit(&visitor);
+    if (!brief_.empty()) brief_ += " ";
+    brief_ += "\"" + visitor.GetString() + "\"";
+  }
+
+ private:
+  d::MemoryAccessor accessor_;
+  std::string brief_;
+};
+
+std::unique_ptr<ObjectPropertiesResult> GetHeapObjectProperties(
+    uintptr_t address, d::MemoryAccessor accessor, const char* type_hint,
+    std::string brief) {
+  TypedObject typed = GetTypedHeapObject(address, accessor, type_hint);
+
+  // TODO(v8:9376): Many object types need additional data that is not included
+  // in their Torque layout definitions. For example, JSObject has an array of
+  // in-object properties after its Torque-defined fields, which at a minimum
+  // should be represented as an array in this response. If the relevant memory
+  // is available, we should instead represent those properties (and any out-of-
+  // object properties) using their JavaScript property names.
+  AddInfoVisitor visitor(brief, accessor);
+  typed.object->Visit(&visitor);
+  brief = visitor.GetBrief();
+
+  brief = AppendAddressAndType(brief, address, typed.object->GetName());
+
+  return v8::base::make_unique<ObjectPropertiesResult>(
+      typed.type_check_result, brief, typed.object->GetName(),
+      typed.object->GetProperties(accessor));
+}
+
 std::unique_ptr<ObjectPropertiesResult> GetHeapObjectProperties(
     uintptr_t address, d::MemoryAccessor memory_accessor, const d::Roots& roots,
     const char* type_hint) {
@@ -306,22 +477,7 @@ std::unique_ptr<ObjectPropertiesResult> GetHeapObjectProperties(
   // Regardless of whether we can read the object itself, maybe we can find its
   // pointer in the list of known objects.
   std::string brief = FindKnownObject(address, roots);
-
+  return GetHeapObjectProperties(address, memory_accessor, type_hint, brief);
-  TqHeapObject heap_object(address);
-  Value<uintptr_t> map_ptr = heap_object.GetMapValue(memory_accessor);
-  if (map_ptr.validity != d::MemoryAccessResult::kOk) {
-    brief = AppendAddressAndType(brief, address, "v8::internal::Object");
-    return v8::base::make_unique<ObjectPropertiesResult>(
-        map_ptr.validity == d::MemoryAccessResult::kAddressNotValid
-            ? d::TypeCheckResult::kObjectPointerInvalid
-            : d::TypeCheckResult::kObjectPointerValidButInaccessible,
-        brief, "v8::internal::Object",
-        std::vector<std::unique_ptr<ObjectProperty>>());
-  }
-  Value<i::InstanceType> instance_type =
-      TqMap(map_ptr.value).GetInstanceTypeValue(memory_accessor);
-  return GetHeapObjectProperties(address, memory_accessor, instance_type,
-                                 type_hint, brief);
 }
 
 std::unique_ptr<ObjectPropertiesResult> GetObjectPropertiesImpl(