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

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/MachO/Driver.cpp:1100
+  if (!config->osoPrefix.empty()) {
+    // Expand "." and "~", if present.
+    SmallString<128> expanded;
----------------
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 :)


================
Comment at: lld/MachO/Driver.cpp:1101
+    // Expand "." and "~", if present.
+    SmallString<128> expanded;
+    if (!llvm::sys::fs::real_path(config->osoPrefix, expanded,
----------------
thevinster wrote:
> 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....
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...


================
Comment at: lld/test/MachO/stabs.s:76
+# REL-PATH:   (N_OSO        ) 03                         0001   [[#%.16x,TEST_TIME]] '/test.o'
+# REL-PATH-DOT:   (N_OSO        ) 03                         0001   [[#%.16x,TEST_TIME]] 'test.o'        
 # CHECK-NEXT: (N_STSYM      ) [[#%.2d,MORE_DATA_ID + 1]] 0000   [[#%.16x,STATIC:]] '_static_var'
----------------
nit: can we keep the columns aligned? maybe call this `REL-DOT`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112291