[PATCH] D77087: [lld-macho] Only mock out libSystem.dylib loading in tests

Mon Apr 13 02:07:38 PDT 2020

int3 marked 2 inline comments as done.
int3 added inline comments.

================
Comment at: lld/MachO/Writer.cpp:357-361
+  if (StringRef(getenv("LLD_FORCE_LOAD_LIBSYSTEM")) == "1" &&
+      dylibOrdinal == 1 && config->outputType == MH_EXECUTE) {
+    headerSection->addLoadCommand(
+        make<LCLoadDylib>("/usr/lib/libSystem.B.dylib"));
+  }
----------------
ruiu wrote:
> int3 wrote:
> > ruiu wrote:
> > > I may be missing something, but doesn't this force lld to always link against /usr/lib/libSystem.B.dylib when it is ran in test, which will fail on non-macOS systems that don't have the dylib file?
> > This just adds an additional load command to the output file, but it doesn't actually cause lld to read and link against libSystem. I.e. we won't create a DylibFile / InputFile for libSystem. It's just an output-time hack.
> Ah, I see. So by adding that library to the dependency list, you can execute an output of lld test run right?
> 
> `LLD_IN_TEST` is set when you are running a test. Can you use that existing environment variable instead? I'm a bit worried about defining a new environment variable as users could start using the internal flag.
> So by adding that library to the dependency list, you can execute an output of lld test run right?

Yup, that's right

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77087