[PATCH] D76252: [lld-macho] Add basic support for linking against dylibs

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/MachO/InputFiles.cpp:78
+
+  // If this is a regular non-fat file, return it.
+  const char *buf = mbref.getBufferStart();
----------------
smeenai wrote:
> Universal binary support could technically be a diff by itself. I'd prefer to do so, although it's small enough that I'm not super opposed to folding it into this diff. We definitely need to add tests for it either way. llvm-lipo is complete enough to be used for the tests.
> 
> I'd also note in the commit message that some of this functionality is being restored from @pcc and @ruiu's initial commit, for completeness.
Fair enough, will split into another diff


================
Comment at: lld/MachO/InputFiles.cpp:228
+
+  if (const load_command *cmd = findCommand(hdr, LC_SYMTAB)) {
+    auto *c = (const symtab_command *)cmd;
----------------
smeenai wrote:
> Is it valid to not have a symtab? If not, we should error.
Not sure why, but unlike the LC_ID_DYLIB case, trying to create a dylib without LC_SYMTAB makes yaml2obj crash. Probably some offsets / indices are off.

Looking at ld64's source, it seems that not having an LC_SYMTAB is only an error if LC_DYLD_INFO(_ONLY) is also missing. The dylib I looked at had LC_DYLD_INFO_ONLY, so I'm not sure if such a case arises in practice...

Anyway, proper handling of a missing LC_SYMTAB should probably cover the ObjectFile case as well, so I'm inclined to punt on it for now.


================
Comment at: lld/test/MachO/Inputs/goodbye-dylib.yaml:108
+    tocoff:          0
+    ntoc:            0
+    modtaboff:       0
----------------
MaskRay wrote:
> Are all these fields important? I think we probably should think what fields are optional and improve yaml2obj. Otherwise the verbosity can make tests a lot more complex.
@smeenai earlier asked about deleting LC_SYMTAB, and I ran into some issues with yaml2obj while trying it (see my other comment). Given that using yaml2obj is mostly a stop-gap measure until lld itself can emit dylibs, I'd prefer not to spend too much time making the test case minimal.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76252