[PATCH] D112291: [lld-macho] Implement -oso_prefix

Vy Nguyen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 22 13:11:55 PDT 2021


oontvoo requested review of this revision.
oontvoo marked 6 inline comments as done.
oontvoo added inline comments.


================
Comment at: lld/MachO/Driver.cpp:1100
+  if (!config->osoPrefix.empty()) {
+    // Expand "." and "~", if present.
+    SmallString<128> expanded;
----------------
thevinster wrote:
> int3 wrote:
> > oontvoo wrote:
> > > int3 wrote:
> > > > thevinster wrote:
> > > > > I wasn't able to find where ld64 did this transformation. In fact, I don't see ld64 doing any transformation for the prefix. It looks like it just strips from the debug path if it has the raw prefix. That said, it seems nice to have this extra functionality for LLD. Do you know if real_path also expands `..` as well?
> > > > if ld64 isn't doing it, I would prefer we not add extra logic to LLD just because :)
> > > LD64 doesn't expand anything other than the `.`
> > > But IMO, that seems weird to handle only one special notation and not the others .  I think we should just use `real_path` to do it - and if it happens to be able to do more transformations than LD64 then so be it. I can't think of a reason why someone would pass `~`, for eg., and not expect it to be expanded - especially given that the path we emit to stabs never contains these special characters anyway.
> > > 
> > Gotcha. Yeah, if we have to handle ".", it makes sense to use `real_path` and handle all the cases. Worth leaving a comment here that ld64 only handles "." though.
> I did a quick test and it looks like the apple provided ld64 does seem to work with `~` and `.`, but a "built-from-source" ld64 won't work with it.
> 
> ```
> $ cat main.c
> main(){}
> $ clang -c -g main.c
> $ clang -fuse-ld=<path-to-internally-built-ld64> -o a.out -Wl,-oso_prefix,. -g main.o
> $ clang -o b.out -Wl,-oso_prefix,. -g main.o
> $ nm -a a.out | grep OSO
> 0000000061730ca3 - 03 0001   OSO /Users/leevince/sandbox/test2/main.o
> $ nm -a b.out | grep OSO
> 0000000061730ca3 - 03 0001   OSO main.o
> ```
> 
> I don't know where this leaves us, but it's probably fine as is written here. 
if you invoke the linker from the clang driver, I suspect this `~` is expanded by the shell (or possibly clang) and not ld64.
invoking LD64 directly (and from looking at the its  code), it doesn't expand ~ at all :

```
$  ld -arch x86_64 -platform_version macos 10.15 11.0 -syslibroot /Users/vyng/repo/llvm/llvm-project/lld/test/MachO/Inputs/MacOSX.sdk -fatal_warnings -lSystem test.o foo.o no-debug.o -oso_prefix "~" -o /Users/vyng/repo/llvm/llvm-project/build_lld/tools/lld/test/MachO/Output/stabs.s.tmp/test-rel-tilde     
vyng-macbookpro2 /Users/vyng/repo/llvm/llvm-project/build_lld/tools/lld/test/MachO/Output/stabs.s.tmp$ nm -a test-rel-tilde| grep 'OSO'
0000000000000020 - 03 0001   OSO /Users/vyng/repo/llvm/llvm-project/build_lld/tools/lld/test/MachO/Output/stabs.s.tmp/foo.o
0000000000000010 - 03 0001   OSO /Users/vyng/repo/llvm/llvm-project/build_lld/tools/lld/test/MachO/Output/stabs.s.tmp/test.o
```




================
Comment at: lld/MachO/SyntheticSections.cpp:864
+  StringRef adjustedPath = saver.save(path.str());
+  if (!config->osoPrefix.empty())
+    adjustedPath.consume_front(config->osoPrefix);
----------------
int3 wrote:
> thevinster wrote:
> > Even if it were empty, wouldn't calling `consume_front` be harmless?
> worth addressing
done - forgot to update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112291



More information about the llvm-commits mailing list