Read ObjC class names in one large read, instead of reading them individually

[jasonmolenda]

Summary

There's a perf problem with Objective-C programs as we add more classes to the Darwin libraries over time - when lldb goes to run its first expression on one of these systems, it will load the ObjC class names from the run time, both the shared cache libraries and any app-contributed classes. lldb has two expressions it runs in the inferior to collect this information into a buffer allocated by lldb - returning the isa pointers and a hash of each class name used to id them. lldb would then read the names of the classes out of memory, which were not localized to one region of memory, and this has become a larger and larger performance issue.

This patch modifies:

1. The two jitted functions, g_get_dynamic_class_info_body and g_get_shared_cache_class_info_body, now take the address of an allocated string pool buffer and size. If the address of the string pool is 0, no names are copied. The functions will copy the class names in to the string pool buffer and add an offset to the (isa, hash) that were previously being returned. If an entry in the (isa, hash, stroffset) array does not have an entry in the string pool, UINT32_MAX is used. If lldb's pre-allocated string pool buffer is too small, entries that did not fit will get UINT32_MAX and lldb will read the class names the old slow way.

2. Modifies the two methods that call these jitted functions, AppleObjCRuntimeV2::UpdateISAToDescriptorMapDynamic and AppleObjCRuntimeV2::UpdateISAToDescriptorMapSharedCache, to allocate the string pool buffer, pass the address and size to the jitted expression, scan the array of (isa, hash, stroffset) tuples to find the highest stroffset, read that part of the string pool out of the inferior with a single read [1], pass the buffer to ParseClassInfoArray, deallocate the stringpool from the inferior.

3. Modifies AppleObjCRuntimeV2::ParseClassInfoArray to grab the class name from the stringpool buffer if the stroffset is != UINT32_MAX and is within the copied buffer size.

[1] nb: I just saw an unintended behavior on the last class name as I was writing this. Given that it uses the highest stroffset it finds to read the string pool back out of the inferior, the final class name won't be copied up. The test in ParseClassInfoArray to check that the stroffset is within the bounds of the copied buffer should mean that we read the last class name out of the inferior aka the old method. I'll double check this is handled correctly tomorrow.

This patch also includes a change that Frederic Riss wrote the other month but hadn't upstreamed yet, where we detect swift names in the shared cache and don't compute the hash in the jitted function, doing it up in lldb later.

Testing the patch shows no testsuite difference on Mac native. As an experiment, I intentionally introduced a bug where the class names in the string pool were corrupted and it caused the testsuite failures I expected. From a performance point of view, this shows the packet behavior I was aiming for.

rdar://problem/27798609

[jasonmolenda]

Fix one small bug in the jitted expressions. Increase the estimated size of class names that lldb uses to pre-allocate the string pool buffer. Added a method to read the string pool out of the inferior, to reduce code duplication, and have it read the last name to get the full size of the string pool correct.

[jasonmolenda]

I had some more revisions on this patch, but I finally understood that I was solving the wrong problem with this patchset. lldb should not fetch class names up front when scanning the objective c class tables, they should only be read lazily as we evaluate an expression. The fact that we're reading names during the table scan is the problem that needs to be fixed.

AppleObjCRuntimeV2.h

   lldb::addr_t GetISAHashTablePointer();
 
   bool UpdateISAToDescriptorMapFromMemory(RemoteNXMapTable &hash_table);
 
   DescriptorMapUpdateResult
   UpdateISAToDescriptorMapDynamic(RemoteNXMapTable &hash_table);
 
   uint32_t ParseClassInfoArray(const lldb_private::DataExtractor &data,
-                               uint32_t num_class_infos);
+                               uint32_t num_class_infos, 
+                               lldb::DataBufferSP strpool_buffer_sp);
 
   DescriptorMapUpdateResult UpdateISAToDescriptorMapSharedCache();
 
   enum class SharedCacheWarningReason {
     eExpressionExecutionFailure,
     eNotEnoughClassesRead
   };
 
   void WarnIfNoClassesCached(SharedCacheWarningReason reason);
 
   lldb::addr_t GetSharedCacheReadOnlyAddress();
 
   bool GetCFBooleanValuesIfNeeded();
 
   friend class ClassDescriptorV2;
 
+  lldb::DataBufferSP ReadClassNameStringPool (lldb::addr_t string_pool_addr, 
+                     lldb_private::DataExtractor *class_data,
+                     uint32_t num_class_infos);
+
   std::unique_ptr<UtilityFunction> m_get_class_info_code;
   lldb::addr_t m_get_class_info_args;
   std::mutex m_get_class_info_args_mutex;
 
   std::unique_ptr<UtilityFunction> m_get_shared_cache_class_info_code;
   lldb::addr_t m_get_shared_cache_class_info_args;
   std::mutex m_get_shared_cache_class_info_args_mutex;
 

AppleObjCRuntimeV2.cpp

     "__lldb_apple_objc_v2_get_dynamic_class_info";
 // Testing using the new C++11 raw string literals. If this breaks GCC then we
 // will need to revert to the code above...
 static const char *g_get_dynamic_class_info_body = R"(
 
 extern "C"
 {
     size_t strlen(const char *);
-    char *strncpy (char * s1, const char * s2, size_t n);
+    char *strcpy (char * dst, const char * src);
     int printf(const char * format, ...);
 }
 #define DEBUG_PRINTF(fmt, ...) if (should_log) printf(fmt, ## __VA_ARGS__)
+#define UINT32_MAX 0xffffffff
 
 typedef struct _NXMapTable {
     void *prototype;
     unsigned num_classes;
     unsigned num_buckets_minus_one;
     void *buckets;
 } NXMapTable;
 
 #define NX_MAPNOTAKEY   ((void *)(-1))
 
 typedef struct BucketInfo
 {
     const char *name_ptr;
     Class isa;
 } BucketInfo;
 
 struct ClassInfo
 {
     Class isa;
     uint32_t hash;
+    uint32_t stroffset;
 } __attribute__((__packed__));
 
 uint32_t
 __lldb_apple_objc_v2_get_dynamic_class_info (void *gdb_objc_realized_classes_ptr,
                                              void *class_infos_ptr,
                                              uint32_t class_infos_byte_size,
+                                             void *string_pool_ptr,
+                                             uint32_t string_pool_byte_size,
                                              uint32_t should_log)
 {
     DEBUG_PRINTF ("gdb_objc_realized_classes_ptr = %p\n", gdb_objc_realized_classes_ptr);
     DEBUG_PRINTF ("class_infos_ptr = %p\n", class_infos_ptr);
     DEBUG_PRINTF ("class_infos_byte_size = %u\n", class_infos_byte_size);
+    DEBUG_PRINTF ("string_pool_ptr = %p\n", string_pool_ptr);
+    DEBUG_PRINTF ("string_pool_byte_size = %u\n", string_pool_byte_size);
     const NXMapTable *grc = (const NXMapTable *)gdb_objc_realized_classes_ptr;
     if (grc)
     {
         const unsigned num_classes = grc->num_classes;
         if (class_infos_ptr)
         {
             const size_t max_class_infos = class_infos_byte_size/sizeof(ClassInfo);
             ClassInfo *class_infos = (ClassInfo *)class_infos_ptr;
             BucketInfo *buckets = (BucketInfo *)grc->buckets;
-            
+            char *string_pool_base = (char *)string_pool_ptr;
+            uint32_t current_strpool_offset = 0;
             uint32_t idx = 0;
             for (unsigned i=0; i<=grc->num_buckets_minus_one; ++i)
             {
                 if (buckets[i].name_ptr != NX_MAPNOTAKEY)
                 {
                     if (idx < max_class_infos)
                     {
-                        const char *s = buckets[i].name_ptr;
+                        const char *name = buckets[i].name_ptr;
+                        const char *s = name;
                         uint32_t h = 5381;
                         for (unsigned char c = *s; c; c = *++s)
                             h = ((h << 5) + h) + c;
                         class_infos[idx].hash = h;
                         class_infos[idx].isa = buckets[i].isa;
+                        class_infos[idx].stroffset = UINT32_MAX; // default to no strpool offset
+                        if (string_pool_base && string_pool_byte_size != 0 && name && h != 0)
+                        {
+                            const int name_len = strlen (name);
+                            const int remaining_strpool = string_pool_byte_size - current_strpool_offset;
+
+                            if (name_len > 0 && remaining_strpool > (name_len + 1))
+                            {
+                                DEBUG_PRINTF ("[%u] name %s copied into stringpool offset %u\n", idx, name, current_strpool_offset);
+                                strcpy (string_pool_base + current_strpool_offset, name);
+                                class_infos[idx].stroffset = current_strpool_offset;
+                                current_strpool_offset += name_len + 1;
+                            } 
+                        }
                     }
                     ++idx;
                 }
             }
             if (idx < max_class_infos)
             {
                 class_infos[idx].isa = NULL;
                 class_infos[idx].hash = 0;
+                class_infos[idx].stroffset = UINT32_MAX; // no strpool offset
             }
         }
         return num_classes;
     }
     return 0;
 }
 
 )";
 
 static const char *g_get_shared_cache_class_info_name =
     "__lldb_apple_objc_v2_get_shared_cache_class_info";
 // Testing using the new C++11 raw string literals. If this breaks GCC then we
 // will need to revert to the code above...
 static const char *g_get_shared_cache_class_info_body = R"(
 
 extern "C"
 {
     const char *class_getName(void *objc_class);
     size_t strlen(const char *);
-    char *strncpy (char * s1, const char * s2, size_t n);
+    char *strcpy (char * dst, const char * src);
     int printf(const char * format, ...);
 }
 
 #define DEBUG_PRINTF(fmt, ...) if (should_log) printf(fmt, ## __VA_ARGS__)
+#define UINT32_MAX 0xffffffff
 
-
 struct objc_classheader_t {
     int32_t clsOffset;
     int32_t hiOffset;
 };
 
 struct objc_clsopt_t {
     uint32_t capacity;
     uint32_t occupied;

     int32_t headeropt_offset;
     int32_t clsopt_offset;
 };
 
 struct ClassInfo
 {
     Class isa;
     uint32_t hash;
+    uint32_t stroffset;
 }  __attribute__((__packed__));
 
 uint32_t
 __lldb_apple_objc_v2_get_shared_cache_class_info (void *objc_opt_ro_ptr,
                                                   void *class_infos_ptr,
                                                   uint32_t class_infos_byte_size,
+                                                  void *string_pool_ptr,
+                                                  uint32_t string_pool_byte_size,
                                                   uint32_t should_log)
 {
     uint32_t idx = 0;
     DEBUG_PRINTF ("objc_opt_ro_ptr = %p\n", objc_opt_ro_ptr);
     DEBUG_PRINTF ("class_infos_ptr = %p\n", class_infos_ptr);
+    DEBUG_PRINTF ("string_pool_ptr = %p\n", string_pool_ptr);
     DEBUG_PRINTF ("class_infos_byte_size = %u (%llu class infos)\n", class_infos_byte_size, (uint64_t)(class_infos_byte_size/sizeof(ClassInfo)));
+    DEBUG_PRINTF ("string_pool_byte_size = %u\n", string_pool_byte_size);
     if (objc_opt_ro_ptr)
     {
         const objc_opt_t *objc_opt = (objc_opt_t *)objc_opt_ro_ptr;
         const objc_opt_v14_t* objc_opt_v14 = (objc_opt_v14_t*)objc_opt_ro_ptr;
         const bool is_v14_format = objc_opt->version >= 14;
         if (is_v14_format)
         {
             DEBUG_PRINTF ("objc_opt->version = %u\n", objc_opt_v14->version);

             const objc_clsopt_t* clsopt = NULL;
             if (is_v14_format)
                 clsopt = (const objc_clsopt_t*)((uint8_t *)objc_opt_v14 + objc_opt_v14->clsopt_offset);
             else
                 clsopt = (const objc_clsopt_t*)((uint8_t *)objc_opt + objc_opt->clsopt_offset);
             const size_t max_class_infos = class_infos_byte_size/sizeof(ClassInfo);
             DEBUG_PRINTF("max_class_infos = %llu\n", (uint64_t)max_class_infos);
             ClassInfo *class_infos = (ClassInfo *)class_infos_ptr;
+            char *string_pool_base = (char *)string_pool_ptr;
+            uint32_t current_strpool_offset = 0;
             int32_t invalidEntryOffset = 0;
             // this is safe to do because the version field order is invariant
             if (objc_opt->version == 12)
                 invalidEntryOffset = 16;
             const uint8_t *checkbytes = &clsopt->tab[clsopt->mask+1];
             const int32_t *offsets = (const int32_t *)(checkbytes + clsopt->capacity);
             const objc_classheader_t *classOffsets = (const objc_classheader_t *)(offsets + clsopt->capacity);
             DEBUG_PRINTF ("clsopt->capacity = %u\n", clsopt->capacity);

                 {
                     class_infos[idx].isa = (Class)((uint8_t *)clsopt + clsOffset);
                     const char *name = class_getName (class_infos[idx].isa);
                     DEBUG_PRINTF ("[%u] isa = %8p %s\n", idx, class_infos[idx].isa, name);
                     // Hash the class name so we don't have to read it
                     const char *s = name;
                     uint32_t h = 5381;
                     for (unsigned char c = *s; c; c = *++s)
+                    {
+                        // See comment in ParseClassInfoArray
+                        if (c == '.') { h = 0; break; }
                         h = ((h << 5) + h) + c;
+                    }
                     class_infos[idx].hash = h;
+                    class_infos[idx].stroffset = UINT32_MAX; // default to no strpool offset
+                    if (string_pool_base && string_pool_byte_size != 0 && name && h != 0)
+                    {
+                        const int name_len = strlen (name);
+                        const int remaining_strpool = string_pool_byte_size - current_strpool_offset;
+                        if (name_len > 0 && remaining_strpool > (name_len + 1))
+                        {
+                            DEBUG_PRINTF ("[%u] name %s copied into stringpool offset %u\n", idx, name, current_strpool_offset);
+                            strcpy (string_pool_base + current_strpool_offset, name);
+                            class_infos[idx].stroffset = current_strpool_offset;
+                            current_strpool_offset += name_len + 1;
+                        } 
+                    }
                 }
                 else
                 {
                     DEBUG_PRINTF("not(class_infos && idx < max_class_infos)\n");
                 }
                 ++idx;
             }
             

                 {
                     class_infos[idx].isa = (Class)((uint8_t *)clsopt + clsOffset);
                     const char *name = class_getName (class_infos[idx].isa);
                     DEBUG_PRINTF ("[%u] isa = %8p %s\n", idx, class_infos[idx].isa, name);
                     // Hash the class name so we don't have to read it
                     const char *s = name;
                     uint32_t h = 5381;
                     for (unsigned char c = *s; c; c = *++s)
+                    {
+                        // See comment in ParseClassInfoArray
+                        if (c == '.') { h = 0; break; }
                         h = ((h << 5) + h) + c;
+                    }
                     class_infos[idx].hash = h;
+                    class_infos[idx].stroffset = UINT32_MAX; // default to no strpool offset
+                    if (string_pool_base && string_pool_byte_size != 0 && name && h != 0)
+                    {
+                        const int name_len = strlen (name);
+                        const int remaining_strpool = string_pool_byte_size - current_strpool_offset;
+                        if (name_len > 0 && remaining_strpool > (name_len + 1))
+                        {
+                            DEBUG_PRINTF ("[%u] name %s copied into stringpool offset %u\n", idx, name, current_strpool_offset);
+                            strcpy (string_pool_base + current_strpool_offset, name);
+                            class_infos[idx].stroffset = current_strpool_offset;
+                            current_strpool_offset += name_len + 1;
+                        }
+                    }
                 }
                 ++idx;
             }
         }
         DEBUG_PRINTF ("%u class_infos\n", idx);
         DEBUG_PRINTF ("done\n");
     }
     return idx;

         m_isa_hash_table_ptr = process->ReadPointerFromMemory(
             gdb_objc_realized_classes_ptr, error);
       }
     }
   }
   return m_isa_hash_table_ptr;
 }
 
+// Iterate through the ClassInfo tuples in the class_data array,
+// find the entry with the highest stroffset value, add the length
+// of the final class name string at stroffset, and read the entire
+// block of memory in large read packets.
+DataBufferSP
+AppleObjCRuntimeV2::ReadClassNameStringPool (addr_t string_pool_addr, 
+    DataExtractor *class_infos_data, 
+    uint32_t num_class_infos) {
+  DataBufferSP strpool_buffer_sp;
+
+  if (string_pool_addr && 
+      string_pool_addr != LLDB_INVALID_ADDRESS 
+      && class_infos_data) {
+    uint32_t max_offset_seen = 0;
+    const int address_size = class_infos_data->GetAddressByteSize();
+    offset_t offset = 0;
+
+    for (uint32_t i = 0; i < num_class_infos; ++i) {
+      offset += address_size + 4; /* isa + hash */
+      uint32_t strpool_offset = class_infos_data->GetU32(&offset);
+      if (strpool_offset != UINT32_MAX & strpool_offset > max_offset_seen)
+        max_offset_seen = strpool_offset;
+    }
+    if (max_offset_seen != 0) {
+      Process *process = GetProcess();
+
+      // The highest string offset we find points to the final string in
+      // the buffer - but will not fetch the final string.  Do one extra
+      // read to find the size of that final string so we have all of them
+      // in a single buffer.
+      
+      std::string last_class_name;
+      Status err;
+      size_t string_size = process->ReadCStringFromMemory(
+          string_pool_addr + max_offset_seen, last_class_name, err);
+      if (string_size > 0 && err.Success()) {
+        max_offset_seen += last_class_name.length() + 1;
+      }
+
+      strpool_buffer_sp.reset(new DataBufferHeap (max_offset_seen, 0));
+      if (process->ReadMemory (string_pool_addr, strpool_buffer_sp->GetBytes(),
+                               strpool_buffer_sp->GetByteSize(), err) !=
+                                    strpool_buffer_sp->GetByteSize()) {
+        strpool_buffer_sp.reset();
+      }
+    }
+  }
+  return strpool_buffer_sp;
+}
+
 AppleObjCRuntimeV2::DescriptorMapUpdateResult
 AppleObjCRuntimeV2::UpdateISAToDescriptorMapDynamic(
     RemoteNXMapTable &hash_table) {
   Process *process = GetProcess();
 
   if (process == NULL)
     return DescriptorMapUpdateResult::Fail();
 

         }
         m_get_class_info_code.reset();
       }
     }
     if (!m_get_class_info_code)
       return DescriptorMapUpdateResult::Fail();
 
     // Next make the runner function for our implementation utility function.
-    Value value;
-    value.SetValueType(Value::eValueTypeScalar);
-    value.SetCompilerType(clang_void_pointer_type);
-    arguments.PushValue(value);
-    arguments.PushValue(value);
+    Value ptr_value;
+    Value uint32_value;
+    ptr_value.SetValueType(Value::eValueTypeScalar);
+    ptr_value.SetCompilerType(clang_void_pointer_type);
+    uint32_value.SetValueType(Value::eValueTypeScalar);
+    uint32_value.SetCompilerType(clang_uint32_t_type);
 
-    value.SetValueType(Value::eValueTypeScalar);
-    value.SetCompilerType(clang_uint32_t_type);
-    arguments.PushValue(value);
-    arguments.PushValue(value);
+    arguments.PushValue(ptr_value);    // void *gdb_objc_realized_classes_ptr
+    arguments.PushValue(ptr_value);    // void *class_infos_ptr
+    arguments.PushValue(uint32_value); // uint32_t class_infos_byte_size
+    arguments.PushValue(ptr_value);    // void *string_pool_ptr
+    arguments.PushValue(uint32_value); // uint32_t string_pool_byte_size
+    arguments.PushValue(uint32_value); // uint32_t should_log
 
     get_class_info_function = m_get_class_info_code->MakeFunctionCaller(
         clang_uint32_t_type, arguments, thread_sp, error);
 
     if (error.Fail()) {
       if (log)
         log->Printf(
             "Failed to make function caller for implementation lookup: %s.",

 
       return DescriptorMapUpdateResult::Fail();
     }
     arguments = get_class_info_function->GetArgumentValues();
   }
 
   diagnostics.Clear();
 
-  const uint32_t class_info_byte_size = addr_size + 4;
+  const uint32_t class_info_byte_size = addr_size + 
+                                        4 /* hash */ + 
+                                        4 /* stroffset */;
   const uint32_t class_infos_byte_size = num_classes * class_info_byte_size;
   lldb::addr_t class_infos_addr = process->AllocateMemory(
       class_infos_byte_size, ePermissionsReadable | ePermissionsWritable, err);
 
+  // Assume classes names will be less than 48 chars on average.  If
+  // we run out of space, entries will have an offset of UINT32_MAX
+  // and lldb will read the class names out of memory individually.  
+  // Actual average is around 23 characters per class name in 2019.
+  const uint32_t string_pool_byte_size = num_classes * 48;
+  addr_t string_pool_addr = process->AllocateMemory(
+      string_pool_byte_size, ePermissionsReadable | ePermissionsWritable, err);
+
   if (class_infos_addr == LLDB_INVALID_ADDRESS) {
     if (log)
       log->Printf("unable to allocate %" PRIu32
                   " bytes in process for shared cache read",
                   class_infos_byte_size);
     return DescriptorMapUpdateResult::Fail();
   }
 
+  if (string_pool_addr == LLDB_INVALID_ADDRESS) {
+    if (log)
+      log->Printf("unable to allocate %" PRIu32
+                  " bytes in process for shared cache string_pool read, will use slow method",
+                  string_pool_byte_size);
+    // NB: representing invalid address as 0 for simplicity of checking in jitted expr
+    string_pool_addr = 0; 
+  }
+
   std::lock_guard<std::mutex> guard(m_get_class_info_args_mutex);
 
   // Fill in our function argument values
   arguments.GetValueAtIndex(0)->GetScalar() = hash_table.GetTableLoadAddress();
   arguments.GetValueAtIndex(1)->GetScalar() = class_infos_addr;
   arguments.GetValueAtIndex(2)->GetScalar() = class_infos_byte_size;
+  arguments.GetValueAtIndex(3)->GetScalar() = string_pool_addr;
+  arguments.GetValueAtIndex(4)->GetScalar() = string_pool_byte_size;
   
   // Only dump the runtime classes from the expression evaluation if the log is
   // verbose:
   Log *type_log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_TYPES);
   bool dump_log = type_log && type_log->GetVerbose();
   
-  arguments.GetValueAtIndex(3)->GetScalar() = dump_log ? 1 : 0;
+  arguments.GetValueAtIndex(5)->GetScalar() = dump_log ? 1 : 0;
 
   bool success = false;
 
   diagnostics.Clear();
 
   // Write our function arguments into the process so we can run our function
   if (get_class_info_function->WriteFunctionArguments(
           exe_ctx, m_get_class_info_args, arguments, diagnostics)) {

         // Read the ClassInfo structures
         DataBufferHeap buffer(num_class_infos * class_info_byte_size, 0);
         if (process->ReadMemory(class_infos_addr, buffer.GetBytes(),
                                 buffer.GetByteSize(),
                                 err) == buffer.GetByteSize()) {
           DataExtractor class_infos_data(buffer.GetBytes(),
                                          buffer.GetByteSize(),
                                          process->GetByteOrder(), addr_size);
-          ParseClassInfoArray(class_infos_data, num_class_infos);
+
+          DataBufferSP strpool_buffer_sp = ReadClassNameStringPool (string_pool_addr, 
+                                                  &class_infos_data, num_class_infos);
+
+          ParseClassInfoArray(class_infos_data, num_class_infos, strpool_buffer_sp);
         }
       }
       success = true;
     } else {
       if (log) {
         log->Printf("Error evaluating our find class name function.");
         diagnostics.Dump(log);
       }
     }
   } else {
     if (log) {
       log->Printf("Error writing function arguments.");
       diagnostics.Dump(log);
     }
   }
 
   // Deallocate the memory we allocated for the ClassInfo array
   process->DeallocateMemory(class_infos_addr);
+  process->DeallocateMemory(string_pool_addr);
 
   return DescriptorMapUpdateResult(success, num_class_infos);
 }
 
 uint32_t AppleObjCRuntimeV2::ParseClassInfoArray(const DataExtractor &data,
-                                                 uint32_t num_class_infos) {
+                                                 uint32_t num_class_infos,
+                                                 DataBufferSP strpool_buffer_sp) {
   // Parses an array of "num_class_infos" packed ClassInfo structures:
   //
   //    struct ClassInfo
   //    {
   //        Class isa;
   //        uint32_t hash;
+  //        uint32_t stroffset;
   //    } __attribute__((__packed__));
 
   Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_TYPES));
   bool should_log = log && log->GetVerbose();
 
   uint32_t num_parsed = 0;
-
   // Iterate through all ClassInfo structures
   lldb::offset_t offset = 0;
   for (uint32_t i = 0; i < num_class_infos; ++i) {
     ObjCISA isa = data.GetPointer(&offset);
 
     if (isa == 0) {
       if (should_log)
         log->Printf(
             "AppleObjCRuntimeV2 found NULL isa, ignoring this class info");
       continue;
     }
     // Check if we already know about this ISA, if we do, the info will never
     // change, so we can just skip it.
     if (ISAIsCached(isa)) {
       if (should_log)
         log->Printf("AppleObjCRuntimeV2 found cached isa=0x%" PRIx64
                     ", ignoring this class info",
                     isa);
-      offset += 4;
+      offset += 4; // hash
+      offset += 4; // stroffset
     } else {
       // Read the 32 bit hash for the class name
       const uint32_t name_hash = data.GetU32(&offset);
-      ClassDescriptorSP descriptor_sp(new ClassDescriptorV2(*this, isa, NULL));
-      AddClass(isa, descriptor_sp, name_hash);
+      const uint32_t strpool_offset = data.GetU32(&offset);
+
+      bool name_read_from_strpool = false;
+      const char *name = nullptr;
+      if (strpool_buffer_sp.get() 
+          && strpool_offset != UINT32_MAX 
+          && strpool_buffer_sp->GetByteSize() > strpool_offset) {
+        name = (const char*) strpool_buffer_sp->GetBytes() + strpool_offset;
+        name_read_from_strpool = true;
+      }
+      ClassDescriptorSP descriptor_sp(new ClassDescriptorV2(*this, isa, name));
+
+      // The code in g_get_shared_cache_class_info_body sets the value of the hash
+      // to 0 to signal a mangled symbol. We use class_getName() in that code to
+      // find the class name, but this returns a demangled name for Swift symbols.
+      // For those symbols, recompute the hash here by extracing their name from the
+      // runtime (this is slow and cannot be done generally for the 45000+ symbols
+      // of the shared cache.
+
+      if (name_hash) {
+        AddClass(isa, descriptor_sp, name_hash);
+      } else {
+        if (name == nullptr) {
+          name = descriptor_sp->GetClassName().AsCString(nullptr);
+        }
+        AddClass(isa, descriptor_sp, name);
+      }
       num_parsed++;
-      if (should_log)
-        log->Printf("AppleObjCRuntimeV2 added isa=0x%" PRIx64
-                    ", hash=0x%8.8x, name=%s",
-                    isa, name_hash,
-                    descriptor_sp->GetClassName().AsCString("<unknown>"));
+      if (should_log) {
+        if (name == nullptr)
+          name = descriptor_sp->GetClassName().AsCString(nullptr);
+        char name_from_strpool_buf[80];
+        if (name_read_from_strpool) {
+          snprintf (name_from_strpool_buf, sizeof (name_from_strpool_buf),
+                    "read from strpool offset %u", strpool_offset);
+        } else {
+          strcpy (name_from_strpool_buf, "read directly from memory, not via strpool");
+        }
+        log->Printf("AppleObjCRuntimeV2 added [%d] isa=0x%" PRIx64
+                    ", hash=0x%8.8x, name=%s %s",
+                    i, isa, name_hash,
+                    name, name_from_strpool_buf);
+      }
     }
   }
   if (should_log)
     log->Printf("AppleObjCRuntimeV2 parsed %" PRIu32 " class infos",
                 num_parsed);
   return num_parsed;
 }
 

         m_get_shared_cache_class_info_code.reset();
       }
     }
 
     if (!m_get_shared_cache_class_info_code)
       return DescriptorMapUpdateResult::Fail();
 
     // Next make the function caller for our implementation utility function.
-    Value value;
-    value.SetValueType(Value::eValueTypeScalar);
-    // value.SetContext (Value::eContextTypeClangType, clang_void_pointer_type);
-    value.SetCompilerType(clang_void_pointer_type);
-    arguments.PushValue(value);
-    arguments.PushValue(value);
+    Value ptr_value;
+    Value uint32_value;
+    ptr_value.SetValueType(Value::eValueTypeScalar);
+    ptr_value.SetCompilerType(clang_void_pointer_type);
+    uint32_value.SetValueType(Value::eValueTypeScalar);
+    uint32_value.SetCompilerType(clang_uint32_t_type);
 
-    value.SetValueType(Value::eValueTypeScalar);
-    // value.SetContext (Value::eContextTypeClangType, clang_uint32_t_type);
-    value.SetCompilerType(clang_uint32_t_type);
-    arguments.PushValue(value);
-    arguments.PushValue(value);
+    arguments.PushValue(ptr_value);    // void *gdb_objc_realized_classes_ptr
+    arguments.PushValue(ptr_value);    // void *class_infos_ptr
+    arguments.PushValue(uint32_value); // uint32_t class_infos_byte_size
+    arguments.PushValue(ptr_value);    // void *string_pool_ptr
+    arguments.PushValue(uint32_value); // uint32_t string_pool_byte_size
+    arguments.PushValue(uint32_value); // uint32_t should_log
 
     get_shared_cache_class_info_function =
         m_get_shared_cache_class_info_code->MakeFunctionCaller(
             clang_uint32_t_type, arguments, thread_sp, error);
 
     if (get_shared_cache_class_info_function == nullptr)
       return DescriptorMapUpdateResult::Fail();
 
   } else {
     get_shared_cache_class_info_function =
         m_get_shared_cache_class_info_code->GetFunctionCaller();
     if (get_shared_cache_class_info_function == nullptr)
       return DescriptorMapUpdateResult::Fail();
     arguments = get_shared_cache_class_info_function->GetArgumentValues();
   }
 
   diagnostics.Clear();
 
-  const uint32_t class_info_byte_size = addr_size + 4;
+  const uint32_t class_info_byte_size = addr_size + 
+                                        4 /* hash */ + 
+                                        4 /* stroffset */;
   const uint32_t class_infos_byte_size = num_classes * class_info_byte_size;
   lldb::addr_t class_infos_addr = process->AllocateMemory(
       class_infos_byte_size, ePermissionsReadable | ePermissionsWritable, err);
 
+  // Assume classes names will be less than 48 chars on average.  If
+  // we run out of space, entries will have an offset of UINT32_MAX
+  // and lldb will read the class names out of memory individually.  
+  // Actual average is around 23 characters per class name in 2019.
+  const uint32_t string_pool_byte_size = num_classes * 48;
+  addr_t string_pool_addr = process->AllocateMemory(
+      string_pool_byte_size, ePermissionsReadable | ePermissionsWritable, err);
+
   if (class_infos_addr == LLDB_INVALID_ADDRESS) {
     if (log)
       log->Printf("unable to allocate %" PRIu32
                   " bytes in process for shared cache read",
                   class_infos_byte_size);
     return DescriptorMapUpdateResult::Fail();
   }
 
+  if (string_pool_addr == LLDB_INVALID_ADDRESS) {
+    if (log)
+      log->Printf("unable to allocate %" PRIu32
+                  " bytes in process for shared cache string_pool read, will use slow method",
+                  string_pool_byte_size);
+    // NB: representing invalid address as 0 for simplicity of checking in jitted expr
+    string_pool_addr = 0;
+  }
+
   std::lock_guard<std::mutex> guard(m_get_shared_cache_class_info_args_mutex);
 
   // Fill in our function argument values
   arguments.GetValueAtIndex(0)->GetScalar() = objc_opt_ptr;
   arguments.GetValueAtIndex(1)->GetScalar() = class_infos_addr;
   arguments.GetValueAtIndex(2)->GetScalar() = class_infos_byte_size;
+  arguments.GetValueAtIndex(3)->GetScalar() = string_pool_addr;
+  arguments.GetValueAtIndex(4)->GetScalar() = string_pool_byte_size;
   // Only dump the runtime classes from the expression evaluation if the log is
   // verbose:
   Log *type_log = GetLogIfAllCategoriesSet(LIBLLDB_LOG_TYPES);
   bool dump_log = type_log && type_log->GetVerbose();
   
-  arguments.GetValueAtIndex(3)->GetScalar() = dump_log ? 1 : 0;
+  arguments.GetValueAtIndex(5)->GetScalar() = dump_log ? 1 : 0;
 
   bool success = false;
 
   diagnostics.Clear();
 
   // Write our function arguments into the process so we can run our function
   if (get_shared_cache_class_info_function->WriteFunctionArguments(
           exe_ctx, m_get_shared_cache_class_info_args, arguments,

         DataBufferHeap buffer(num_class_infos * class_info_byte_size, 0);
         if (process->ReadMemory(class_infos_addr, buffer.GetBytes(),
                                 buffer.GetByteSize(),
                                 err) == buffer.GetByteSize()) {
           DataExtractor class_infos_data(buffer.GetBytes(),
                                          buffer.GetByteSize(),
                                          process->GetByteOrder(), addr_size);
 
-          ParseClassInfoArray(class_infos_data, num_class_infos);
+          DataBufferSP strpool_buffer_sp = ReadClassNameStringPool (string_pool_addr, 
+                                                  &class_infos_data, num_class_infos);
+
+          ParseClassInfoArray(class_infos_data, num_class_infos, strpool_buffer_sp);
         }
       } else {
         success = true;
       }
     } else {
       if (log) {
         log->Printf("Error evaluating our find class name function.");
         diagnostics.Dump(log);
       }
     }
   } else {
     if (log) {
       log->Printf("Error writing function arguments.");
       diagnostics.Dump(log);
     }
   }
 
   // Deallocate the memory we allocated for the ClassInfo array
   process->DeallocateMemory(class_infos_addr);
+  process->DeallocateMemory(string_pool_addr);
 
   return DescriptorMapUpdateResult(success, num_class_infos);
 }
 
 bool AppleObjCRuntimeV2::UpdateISAToDescriptorMapFromMemory(
     RemoteNXMapTable &hash_table) {
   Log *log(GetLogIfAnyCategoriesSet(LIBLLDB_LOG_PROCESS | LIBLLDB_LOG_TYPES));
 

     m_cache[real_isa] = descriptor_sp;
   return descriptor_sp;
 }
 
 bool AppleObjCRuntimeV2::NonPointerISACache::EvaluateNonPointerISA(
     ObjCISA isa, ObjCISA &ret_isa) {
   Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_TYPES));
 
+  if ((isa & ~m_objc_debug_isa_class_mask) == 0)
+    return false;
+
   if (log)
     log->Printf("AOCRT::NPI Evalulate(isa = 0x%" PRIx64 ")", (uint64_t)isa);
 
-  if ((isa & ~m_objc_debug_isa_class_mask) == 0)
-    return false;
-
   // If all of the indexed ISA variables are set, then its possible that this
   // ISA is indexed, and we should first try to get its value using the index.
   // Note, we check these variables first as the ObjC runtime will set at least
   // one of their values to 0 if they aren't needed.
   if (m_objc_debug_indexed_isa_magic_mask &&
       m_objc_debug_indexed_isa_magic_value &&
       m_objc_debug_indexed_isa_index_mask &&
       m_objc_debug_indexed_isa_index_shift && m_objc_indexed_classes) {