Skip to content

Work around a bug in the interaction between newer dyld's and older simulator dyld's #78004

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 16, 2024

Conversation

jimingham
Copy link
Collaborator

There's a bad interaction between the macOS 14 dyld and the "dyld_sim" shim that comes from older (iOS 15) simulator downloads that results in dyld reporting some modules twice in the return from the dyld callback to list modules. The records were identical, but lldb wasn't happy with seeing the duplicates...

Since it's not possible to load two different modules at the same address, this change just picks the first instance of any entries that have the same load address.

There really isn't a good way to test this patch.

simulator dyld's that cause dyld to report some modules twice.
Since it's not possible to load the different modules at the same
address, this change just picks the first instance of any entries
that have the same load address.
There really isn't a good way to test this patch.
@llvmbot
Copy link
Member

llvmbot commented Jan 13, 2024

@llvm/pr-subscribers-lldb

Author: None (jimingham)

Changes

There's a bad interaction between the macOS 14 dyld and the "dyld_sim" shim that comes from older (iOS 15) simulator downloads that results in dyld reporting some modules twice in the return from the dyld callback to list modules. The records were identical, but lldb wasn't happy with seeing the duplicates...

Since it's not possible to load two different modules at the same address, this change just picks the first instance of any entries that have the same load address.

There really isn't a good way to test this patch.


Full diff: https://github.com/llvm/llvm-project/pull/78004.diff

1 Files Affected:

  • (modified) lldb/tools/debugserver/source/MacOSX/MachProcess.mm (+8)
diff --git a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
index 29fabd087ecbe1..93b976127ba45c 100644
--- a/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
+++ b/lldb/tools/debugserver/source/MacOSX/MachProcess.mm
@@ -33,6 +33,7 @@
 #include <algorithm>
 #include <chrono>
 #include <map>
+#include <unordered_set>
 
 #include <TargetConditionals.h>
 #import <Foundation/Foundation.h>
@@ -1053,9 +1054,16 @@ static bool mach_header_validity_test(uint32_t magic, uint32_t cputype) {
     dyld_process_info info =
         m_dyld_process_info_create(m_task.TaskPort(), 0, &kern_ret);
     if (info) {
+      // There's a bug in the interaction between dyld and older dyld_sim's
+      // (e.g. from the iOS 15 simulator) that causes dyld to report the same
+      // binary twice.  We use this set to eliminate the duplicates.
+      __block std::unordered_set<uint64_t> seen_header_addrs;
       m_dyld_process_info_for_each_image(
           info,
           ^(uint64_t mach_header_addr, const uuid_t uuid, const char *path) {
+            auto res_pair = seen_header_addrs.insert(mach_header_addr);
+            if (!res_pair.second)
+              return;
             struct binary_image_information image;
             image.filename = path;
             uuid_copy(image.macho_info.uuid, uuid);

Copy link
Collaborator

@jasonmolenda jasonmolenda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. debugserver never expects libdyld's API (to iterate over the binary images in a process) to return the same binary multiple times, and lldb won't handle that well if we pass that behavior along. Eliminating the incorrect duplicate here when we are getting the list of binaries from libdyld is the best place for it.

@jimingham jimingham merged commit 32dd5b2 into llvm:main Jan 16, 2024
@jimingham jimingham deleted the dyld-vrs-dyld_sim branch January 16, 2024 19:31
jimingham added a commit to jimingham/from-apple-llvm-project that referenced this pull request Jan 16, 2024
…imulator dyld's (llvm#78004)

There's a bad interaction between the macOS 14 dyld and the "dyld_sim"
shim that comes from older (iOS 15) simulator downloads that results in
dyld reporting some modules twice in the return from the dyld callback
to list modules. The records were identical, but lldb wasn't happy with
seeing the duplicates...

Since it's not possible to load two different modules at the same
address, this change just picks the first instance of any entries that
have the same load address.

There really isn't a good way to test this patch.

(cherry picked from commit 32dd5b2)
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
…imulator dyld's (llvm#78004)

There's a bad interaction between the macOS 14 dyld and the "dyld_sim"
shim that comes from older (iOS 15) simulator downloads that results in
dyld reporting some modules twice in the return from the dyld callback
to list modules. The records were identical, but lldb wasn't happy with
seeing the duplicates...

Since it's not possible to load two different modules at the same
address, this change just picks the first instance of any entries that
have the same load address.

There really isn't a good way to test this patch.
jimingham added a commit to jimingham/from-apple-llvm-project that referenced this pull request Feb 21, 2024
…imulator dyld's (llvm#78004)

There's a bad interaction between the macOS 14 dyld and the "dyld_sim"
shim that comes from older (iOS 15) simulator downloads that results in
dyld reporting some modules twice in the return from the dyld callback
to list modules. The records were identical, but lldb wasn't happy with
seeing the duplicates...

Since it's not possible to load two different modules at the same
address, this change just picks the first instance of any entries that
have the same load address.

There really isn't a good way to test this patch.

(cherry picked from commit 32dd5b2)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants