[lld-macho] Implement -oso_prefix

[oontvoo]

Summary

https://bugs.llvm.org/show_bug.cgi?id=50229

[oontvoo]

remove fixme

[oontvoo]

rebase

[thevinster]

lld/MachO/Driver.cpp

1100

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?

1101

Just wondering, how did you come up with 128? Realistically speaking, I haven't seen prefixes that will that much space, but I guess it's possible to have really ugly nested directories on some systems out there....

1102

nit: You can drop the llvm::sys::. It seems to be namespaced already.

lld/MachO/SyntheticSections.cpp

864

Even if it were empty, wouldn't calling consume_front be harmless?

[thakis]

Looks generally fine to me.

Do you think we could test ~ expansion by running a test with env HOME=/some/custom/path?

lld/MachO/Driver.cpp

1105

s/backslash/slash/

[int3]

lld/MachO/Driver.cpp

1100

if ld64 isn't doing it, I would prefer we not add extra logic to LLD just because :)

1101

We've been using 261 for similar things in the codebase, because the max filename length on Windows is 260. Max filename length on Linux is 255, but max *path* length is 4096, so I guess to be really safe some of these path strings should have 4096 bytes allocated...

lld/test/MachO/stabs.s

80

nit: can we keep the columns aligned? maybe call this REL-DOT

[oontvoo]

addressed comments

lld/MachO/Driver.cpp

1100

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.

1101

The 128 originally came from copy-and-paste - I'd have thought it should be plenty for most cases.

Ok, yakshaving here :) , but isn't 4096 too much (if the prefix is 4096 ie., max length, then you're trimming everything, no?)
How about 512? (based on slightly unscientific observation that the top 10 longest paths in lld-macho's test output are 253 to 266 chars long, on my machine)

 vyng@vyng:/mnt/ssd/repo/llvm-project/build_lld$ ls -R /mnt/ssd/repo/llvm-project/build_lld | awk '
/:$/&&f{s=$0;f=0}
/:$/&&!f{sub(/:$/,"");s=$0;f=1;next}
NF&&f{ print s"/"$0 }' | awk '{print length, $0}' | sort -nr | head -n10
266 /mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/repro6/mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/libfoo.dylib
266 /mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/repro6/mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/libbar.dylib
266 /mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/repro5/mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/libbar.dylib
266 /mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/repro4/mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/libbar.dylib
266 /mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/repro3/mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/libbar.dylib
259 /mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/repro2/mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/bar.a
259 /mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/repro1/mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/bar.a
253 /mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/repro6/mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp
253 /mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/repro5/mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp
253 /mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/repro4/mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp/mnt/ssd/repo/llvm-project/build_lld/tools/lld/test/MachO/Output/reroot-path.s.tmp

[oontvoo]

typo + rebase again( seems parallel-libs has been removed - why is it still being referenced?)

CMake Error at CMakeLists.txt:76 (MESSAGE):
  parallel-libs isn't a known project:
  clang;clang-tools-extra;compiler-rt;cross-project-tests;libc;libclc;libcxx;libcxxabi;libunwind;lld;lldb;mlir;openmp;polly;pstl;flang

[oontvoo]

adding test with env HOME=/some/custom/path

[int3]

lgtm

lld/MachO/Driver.cpp

1100

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.

1101

what's 4k bytes between friends? :p

yeah, looking at the codebase, I see there are a bunch of SmallString<128>s, but also 260s and 261s...

feel free to put whatever here; I think I'll codemod them to something more consistent later

lld/MachO/SyntheticSections.cpp

864

worth addressing

lld/test/MachO/stabs.s

80

don't forget about this :)

[thevinster]

lld/MachO/Driver.cpp

1100

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.

[oontvoo]

lld/MachO/Driver.cpp

1100

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

lld/MachO/SyntheticSections.cpp

864

done - forgot to update

[oontvoo]

addressed previously missed comments.

[oontvoo]

The windows build is red but it doesn't look related ... committing now.
Thanks for the review!

[oontvoo]

This revision was not accepted when it landed; it landed in state Needs

Review.

This is soooo not true ! 🤔 I've got 3 LGTM

lld/MachO/Config.h

struct Configuration {
   SectionRenameMap sectionRenameMap;
   SegmentRenameMap segmentRenameMap;
 
   SymbolPatterns exportedSymbols;
   SymbolPatterns unexportedSymbols;
 
   bool zeroModTime = false;
 
+  llvm::StringRef osoPrefix;
+
   llvm::MachO::Architecture arch() const { return platformInfo.target.Arch; }
 
   llvm::MachO::PlatformKind platform() const {
     return platformInfo.target.Platform;
   }
 };
 
 // The symbol with the highest priority should be ordered first in the output

lld/MachO/Driver.cpp

bool macho::link(ArrayRef<const char *> argsArr, bool canExitEarly,
   }
 
   config = make<Configuration>();
   symtab = make<SymbolTable>();
   target = createTargetInfo(args);
   depTracker =
       make<DependencyTracker>(args.getLastArgValue(OPT_dependency_info));
 
+  config->osoPrefix = args.getLastArgValue(OPT_oso_prefix);
+  if (!config->osoPrefix.empty()) {
+    // Expand special characters, such as ".", "..", or  "~", if present.
+    // Note: LD64 only expands "." and not other special characters.
+    // That seems silly to imitate so we will not try to follow it, but rather
+    // just use real_path() to do it.
+
+    // The max path length is 4096, in theory. However that seems quite long
+    // and seems unlikely that any one would want to strip everything from the
+    // path. Hence we've picked a reasonably large number here.
+    SmallString<1024> expanded;
+    if (!fs::real_path(config->osoPrefix, expanded,
+                       /*expand_tilde=*/true)) {
+      // Note: LD64 expands "." to be `<current_dir>/`
+      // (ie., it has a slash suffix) whereas real_path() doesn't.
+      // So we have to append '/' to be consistent.
+      StringRef sep = sys::path::get_separator();
+      if (config->osoPrefix.equals(".") && !expanded.endswith(sep))
+        expanded += sep;
+      config->osoPrefix = saver.save(expanded.str());
+    }
+  }
+
   // Must be set before any InputSections and Symbols are created.
   config->deadStrip = args.hasArg(OPT_dead_strip);
 
   config->systemLibraryRoots = getSystemLibraryRoots(args);
   if (const char *path = getReproduceOption(args)) {
     // Note that --reproduce is a debug option so you can ignore it
     // if you are trying to understand the whole picture of the code.
     Expected<std::unique_ptr<TarWriter>> errOrWriter =

lld/MachO/Options.td

 def non_global_symbols_no_strip_list : Separate<["-"], "non_global_symbols_no_strip_list">,
     MetaVarName<"<path>">,
     HelpText<"Specify in <path> the non-global symbols that should remain in the output symbol table">,
     Flags<[HelpHidden]>,
     Group<grp_symtab>;
 def oso_prefix : Separate<["-"], "oso_prefix">,
     MetaVarName<"<path>">,
     HelpText<"Remove the prefix <path> from OSO symbols in the debug map">,
-    Flags<[HelpHidden]>,
     Group<grp_symtab>;
 def add_ast_path : Separate<["-"], "add_ast_path">,
     MetaVarName<"<path>">,
     HelpText<"AST paths will be emitted as STABS">,
     Group<grp_symtab>;
 
 def grp_bitcode : OptionGroup<"bitcode">, HelpText<"BITCODE BUILD FLOW">;
 

lld/MachO/SyntheticSections.cpp

SmallString<261> path(!file->archiveName.empty() ? file->archiveName
                                                    : file->getName());
   std::error_code ec = sys::fs::make_absolute(path);
   if (ec)
     fatal("failed to get absolute path for " + path);
 
   if (!file->archiveName.empty())
     path.append({"(", file->getName(), ")"});
 
-  stab.strx = stringTableSection.addString(saver.save(path.str()));
+  StringRef adjustedPath = saver.save(path.str());
+  adjustedPath.consume_front(config->osoPrefix);
+
+  stab.strx = stringTableSection.addString(adjustedPath);
   stab.desc = 1;
   stab.value = file->modTime;
   stabs.emplace_back(std::move(stab));
 }
 
 void SymtabSection::emitEndFunStab(Defined *defined) {
   StabsEntry stab(N_FUN);
   stab.value = defined->size;

lld/test/MachO/stabs.s

 
 ## Check that we emit absolute paths to the object files in our OSO entries
 ## even if our inputs are relative paths.
 # RUN: cd %t && %lld -lSystem test.o foo.o no-debug.o -o test
 # RUN: (llvm-objdump --section-headers %t/test; dsymutil -s %t/test) | \
 # RUN:   FileCheck %s -DDIR=%t -DFOO_PATH=%t/foo.o \
 # RUN:       -D#TEST_TIME=0x10 -D#FOO_TIME=0x20
 
+## Check that we emit relative path to object files in OSO entries
+## when -oso_prefix <path> is used.
+# RUN: cd %t && %lld -lSystem test.o foo.o no-debug.o -oso_prefix "%t" -o %t/test-rel
+# RUN: dsymutil -s  %t/test-rel | grep 'N_OSO' | FileCheck %s  -D#TEST_TIME=0x10 -D#FOO_TIME=0x20 --check-prefix=REL-PATH 
+# RUN: cd %t && %lld -lSystem test.o foo.o no-debug.o -oso_prefix "." -o %t/test-rel-dot
+# RUN: dsymutil -s  %t/test-rel-dot | grep 'N_OSO' | FileCheck %s  -D#TEST_TIME=0x10 -D#FOO_TIME=0x20 --check-prefix=REL-DOT 
+## Set HOME to %t (for ~ to expand to)
+# RUN: cd %t && env HOME=%t %lld -lSystem test.o foo.o no-debug.o -oso_prefix "~" -o %t/test-rel-tilde
+# RUN: dsymutil -s  %t/test-rel-tilde | grep 'N_OSO' | FileCheck %s  -D#TEST_TIME=0x10 -D#FOO_TIME=0x20 --check-prefix=REL-PATH
+
+
 # RUN: cd %t && %lld -lSystem test.o foo.a no-debug.o -o %t/test
 # RUN: (llvm-objdump --section-headers %t/test; dsymutil -s %t/test) | \
 # RUN:   FileCheck %s -DDIR=%t -DFOO_PATH=%t/foo.a\(foo.o\) \
 # RUN:       -D#TEST_TIME=0x10 -D#FOO_TIME=0x20
 
 # CHECK:       Sections:
 # CHECK-NEXT:  Idx                Name
 # CHECK-NEXT:  [[#TEXT_ID:]]      __text
 # CHECK-NEXT:  [[#DATA_ID:]]      __data
 # CHECK-NEXT:  [[#MORE_DATA_ID:]] more_data
 # CHECK-NEXT:  [[#COMM_ID:]]      __common
 # CHECK-NEXT:  [[#MORE_TEXT_ID:]] more_text
 
 # CHECK:      (N_SO         ) 00                         0000   0000000000000000   '/tmp/test.cpp'
 # CHECK-NEXT: (N_OSO        ) 03                         0001   [[#%.16x,TEST_TIME]] '[[DIR]]/test.o'
+# REL-PATH:   (N_OSO        ) 03                         0001   [[#%.16x,TEST_TIME]] '/test.o'
+# REL-DOT:    (N_OSO        ) 03                         0001   [[#%.16x,TEST_TIME]] 'test.o'        
 # CHECK-NEXT: (N_STSYM      ) [[#%.2d,MORE_DATA_ID + 1]] 0000   [[#%.16x,STATIC:]] '_static_var'
 # CHECK-NEXT: (N_FUN        ) [[#%.2d,TEXT_ID + 1]]      0000   [[#%.16x,MAIN:]]   '_main'
 # CHECK-NEXT: (N_FUN        ) 00                         0000   0000000000000006{{$}}
 # CHECK-NEXT: (N_FUN        ) [[#%.2d,TEXT_ID + 1]]      0000   [[#%.16x,BAR:]]    '_bar'
 # CHECK-NEXT: (N_FUN        ) 00                         0000   0000000000000000{{$}}
 # CHECK-NEXT: (N_FUN        ) [[#%.2d,TEXT_ID + 1]]      0000   [[#%.16x,BAR2:]]   '_bar2'
 # CHECK-NEXT: (N_FUN        ) 00                         0000   0000000000000001{{$}}
 # CHECK-NEXT: (N_FUN        ) [[#%.2d,TEXT_ID + 1]]      0000   [[#%.16x,BAZ:]]    '_baz'
 # CHECK-NEXT: (N_FUN        ) 00                         0000   0000000000000000{{$}}
 # CHECK-NEXT: (N_FUN        ) [[#%.2d,TEXT_ID + 1]]      0000   [[#%.16x,BAZ2:]]   '_baz2'
 # CHECK-NEXT: (N_FUN        ) 00                         0000   0000000000000002{{$}}
 # CHECK-NEXT: (N_FUN        ) [[#%.2d,TEXT_ID + 1]]      0000   [[#%.16x,QUX:]]    '_qux'
 # CHECK-NEXT: (N_FUN        ) 00                         0000   0000000000000003{{$}}
 # CHECK-NEXT: (N_FUN        ) [[#%.2d,TEXT_ID + 1]]      0000   [[#%.16x,QUUX:]]   '_quux'
 # CHECK-NEXT: (N_FUN        ) 00                         0000   0000000000000004{{$}}
 # CHECK-NEXT: (N_GSYM       ) [[#%.2d,DATA_ID + 1]]      0000   [[#%.16x,GLOB:]]   '_global_var'
 # CHECK-NEXT: (N_GSYM       ) [[#%.2d,COMM_ID + 1]]      0000   [[#%.16x,ZERO:]]   '_zero'
 # CHECK-NEXT: (N_FUN        ) [[#%.2d,MORE_TEXT_ID + 1]] 0000   [[#%.16x,FUN:]]    '_fun'
 # CHECK-NEXT: (N_FUN        ) 00                         0000   0000000000000001{{$}}
 # CHECK-NEXT: (N_SO         ) 01                         0000   0000000000000000{{$}}
 # CHECK-NEXT: (N_SO         ) 00                         0000   0000000000000000   '/foo.cpp'
 # CHECK-NEXT: (N_OSO        ) 03                         0001   [[#%.16x,FOO_TIME]] '[[FOO_PATH]]'
+# REL-PATH-NEXT:   (N_OSO        ) 03                         0001   [[#%.16x,FOO_TIME]] '/foo.o'
+# REL-DOT-NEXT:    (N_OSO        ) 03                         0001   [[#%.16x,FOO_TIME]] 'foo.o'
 # CHECK-NEXT: (N_FUN        ) [[#%.2d,TEXT_ID + 1]]      0000   [[#%.16x,FOO:]]    '_foo'
 # CHECK-NEXT: (N_FUN        ) 00                         0000   0000000000000001{{$}}
 # CHECK-NEXT: (N_SO         ) 01                         0000   0000000000000000{{$}}
 # CHECK-DAG:  (     SECT    ) [[#%.2d,MORE_DATA_ID + 1]] 0000   [[#STATIC]]        '_static_var'
 # CHECK-DAG:  (     SECT EXT) [[#%.2d,TEXT_ID + 1]]      0000   [[#MAIN]]          '_main'
 # CHECK-DAG:  (     ABS  EXT) 00                         0000   {{[0-9af]+}}       '_abs'
 # CHECK-DAG:  (     SECT EXT) [[#%.2d,TEXT_ID + 1]]      0000   [[#FOO]]           '_foo'
 # CHECK-DAG:  (     SECT EXT) [[#%.2d,TEXT_ID + 1]]      0000   [[#BAR]]           '_bar'