[PATCH] D140592: [lld-macho] Skip re-loading archive if already loaded

Tue Dec 27 00:22:43 PST 2022

thevinster added inline comments.

================
Comment at: lld/MachO/InputFiles.cpp:2110

+  if (!skipCache && !seen.insert(mb->getBuffer()).second)
+    return Error::success();
----------------
thakis wrote:
> We're doing this based on buffer contents? Where in ld64's source does it do that? That seems like very surprising semantics (and kinda bad for perf).
I don't know if there will be any significant perf regressions. I'll double check that on my end first before saying anything else. From what I can tell, it looks like ld64 hashes based on the contents of the [[ https://github.com/apple-oss-distributions/ld64/blob/main/src/ld/parsers/archive_file.cpp#L99-L111 | Entry ]] class. I agree that these are surprising semantics (especially the behavior of force loading two of the same object files from an archive), but this strange behavior is also implicitly depended upon by some of our builds. Whether or not this is the correct implementation, I'm merely trying to follow ld64's behavior. As far as implementation is concerned, this is the path of least resistance that'll solve our current duplicate symbol issue. I'm open to suggestions on how else this can be done (I've tried approaches where I only load the symbol if it hasn't been loaded but that ran into many edge cases). 

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140592/new/

https://reviews.llvm.org/D140592