[Lldb-commits] [PATCH] D153648: [lldb] Remove broken shared cache duplicate class handling

[Dave Lee via Phabricator via lldb-commits]

kastiglione created this revision.
kastiglione added reviewers: JDevlieghere, jingham, bulbazord.
Herald added a project: All.
kastiglione requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

The shared cache list of ObjC classes contains classes that are duplicate in name (which
may or may not be duplicate in definition).

Currently, lldb does not handle this list of duplicates correctly. The bugs in the
handling of duplicate classes are in the for loop over the duplicates, which has the
following issues:

1. Indexing into the unique class list (`classOffsets`), not the duplicate list (`duplicateClassOffsets`)
2. Not incrementing `idx`

The result is that the first N unique classes are wastefully rehashed, where N is
`duplicates_count` (which can be large).

Note that this change removes the loop over the duplicates altogether, to avoid paying
the cost of ultimately doing nothing.

Ideally, the above bugs could be addressed, however lldb doesn't know which of the
duplicates the ObjC runtime has chosen. When the ObjC runtime registers duplicate
classes, it emits the following error:

> Class <class> is implemented in both libA.dylib (0x1048800b8) and libB.dylib
> (0x1048700b8). One of the two will be used. Which one is undefined.

In lldb, the code that uses results of this scan does class lookup by name, and doesn't
handle the case where there are multiple classes for a given name. Also, lldb doesn't
know which of the duplicates the ObjC runtime has chosen.

The ultimate fix is to determine which duplicate the ObjC runtime has chosen, and have
lldb use that too.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D153648

Files:
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp


Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
===================================================================
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntimeV2.cpp
@@ -527,48 +527,7 @@
             DEBUG_PRINTF ("duplicate_count = %u\n", duplicate_count);
             DEBUG_PRINTF ("duplicateClassOffsets = %p\n", duplicateClassOffsets);
 
-            for (uint32_t i=0; i<duplicate_count; ++i)
-            {
-                const uint64_t objectCacheOffset = classOffsets[i].objectCacheOffset;
-                DEBUG_PRINTF("objectCacheOffset[%u] = %u\n", i, objectCacheOffset);
-
-                if (classOffsets[i].isDuplicate) {
-                    DEBUG_PRINTF("isDuplicate = true\n");
-                    continue; // duplicate
-                }
-
-                if (objectCacheOffset == 0) {
-                    DEBUG_PRINTF("objectCacheOffset == invalidEntryOffset\n");
-                    continue; // invalid offset
-                }
-
-                if (class_infos && idx < max_class_infos)
-                {
-                    class_infos[idx].isa = (Class)((uint8_t *)shared_cache_base_ptr + objectCacheOffset);
-
-                    // Lookup the class name.
-                    const char *name = class_name_lookup_func(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)
-                    {
-                        // class_getName demangles swift names and the hash must
-                        // be calculated on the mangled name.  hash==0 means lldb
-                        // will fetch the mangled name and compute the hash in
-                        // ParseClassInfoArray.
-                        if (c == '.')
-                        {
-                            h = 0;
-                            break;
-                        }
-                        h = ((h << 5) + h) + c;
-                    }
-                    class_infos[idx].hash = h;
-                }
-            }
+            // TODO: Handle "duplicate" classes. Duplicate in name, not necessarily in definition.
         }
         else if (objc_opt->version >= 12 && objc_opt->version <= 15)
         {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D153648.534013.patch
Type: text/x-patch
Size: 2579 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20230623/1a0fb357/attachment.bin>