[PATCH] D103990: [lld/mac] When handling @loader_path, use realpath() of symlinks

[Nico Weber via Phabricator via llvm-commits]

thakis added inline comments.


================
Comment at: lld/MachO/InputFiles.cpp:794
   } else if (path.consume_front("@loader_path/")) {
-    path::append(newPath, sys::path::parent_path(umbrella->getName()), path);
+    fs::real_path(umbrella->getName(), newPath);
+    path::remove_filename(newPath);
----------------
int3 wrote:
> Would it make sense to call `realPath` before constructing MemoryBuffers in `readFile`, so that `getName()` always returns a canonicalized path? Or would that mess up other things?
I think that'd be good. It'd also make us not load libSystem twice when doing `-lSystem -lpthreads`. I'm a bit scared of changing that though (for no good reason), so I'd like to land the smaller change (and the test) here first if that's ok. I hereby pledge to send you a follow-up that tries moving this call earlier :)

(ld64 has the realpath call in its code corresponding to the code here -- but parts of ld64 are all over the place so that doesn't necessarily mean much.)


================
Comment at: lld/MachO/InputFiles.cpp:798
     path = newPath;
+
   } else if (path.startswith("@rpath/")) {
----------------
int3 wrote:
> intentional newline?
No, thanks :)


================
Comment at: lld/test/MachO/link-search-at-loader-path-symlink.s:24
+
+## 2. Test symlink with reexport to @loader_path-relative path directly..
+## The @loader_path-relative path is looked after resolving the symlink.
----------------
int3 wrote:
> 
Maybe I meant "..." :P


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

https://reviews.llvm.org/D103990