[PATCH] D156468: [lld-macho] Fixed crashes when linking with incompatible-arch archives/

[Vy Nguyen via Phabricator via llvm-commits]

oontvoo added a comment.

In D156468#4558445 <https://reviews.llvm.org/D156468#4558445>, @MaskRay wrote:

> The summary seems to have an unneeded 4-space indentation.
>
>> - Simply ignore input archives with incompatible arch (changes from PRESIDENT810@)
>
> What is `PRESIDENT810@`?

Sorry, I meant the idea of peek at the first member in the archive to check its arch  in addLazySymbol is taken from this commit https://github.com/PRESIDENT810/llvm-project/commit/3062d0392a7ac2d3a77a303c6af7272dbdac9db6 by PRESIDENT810



================
Comment at: lld/MachO/InputFiles.cpp:1042
+
+  if (!compatArch)
+    return;
----------------
MaskRay wrote:
> `if (!compatArch)` cannot be triggered. Delete it.
I seem to have missed the TODOs on propagating this from the parent archive to its children so we don't need to parse them - implemented that now.


================
Comment at: lld/MachO/InputFiles.cpp:1044
+    return;
+  if (!(compatArch = compatWithTargetArch(this, hdr)))
+    return;
----------------
MaskRay wrote:
> `compatArch` seems no longer read after the store. Can `compatArch` be made ArchiveFile specific?
As mentioned in previous comment, this compatibility flag can be propagated from a parent archive to its children so the flag should be present for all InputFile


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156468