[lld-macho] Fix trailing slash in oso_prefix

[keith]

Summary

Previously if you passed -oso_prefix path/to/foo/ with a trailing
slash at the end, using real_path would remove that slash, but that
slash is necessary to make sure OSO prefix paths end up as valid
relative paths instead of starting with /.

[keith]

lld/MachO/Driver.cpp

1164

Maybe the second half of the conditional here isn't necessary?

[oontvoo]

LG Thanks!

lld/MachO/Driver.cpp

1164

I guess it's not needed if we can count on real_path to NOT change its behaviour.
But the tests below should catch it.

[int3]

lld/MachO/Driver.cpp

1164

Given that realpath is a POSIX API with very wide use, I think it is extremely unlikely to ever change its behavior...

I guess this means we can remove the check on line 1160 too

[keith]

Remove unnecessary conditions

[thevinster]

Nice find!

lld/MachO/Driver.cpp

1160–1163

We can simplify this with a || in the first if? Could we also document the behavior of real_path here (that it removes the trailing slash)?

[int3]

lgtm

lld/MachO/Driver.cpp

1160–1163

+1

[keith]

Combine conditions

lld/MachO/Driver.cpp

if (!config->osoPrefix.empty()) {
     // 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))
+      // real_path removes trailing slashes as part of the normalization, but
+      // these are meaningful for our text based stripping
+      if (config->osoPrefix.equals(".") || config->osoPrefix.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)) {

lld/test/MachO/stabs.s

 # 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 "%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-NO-SLASH
 # 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
 
 ## Check that we don't emit DWARF or stabs when -S is used
 # RUN: %lld -lSystem test.o foo.o no-debug.o -S -o %t/test-no-debug

 # 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-PATH-NO-SLASH:  (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'
+# REL-PATH-NEXT:   (N_OSO        ) 03                    0001   [[#%.16x,FOO_TIME]] '/foo.o'
+# REL-PATH-NO-SLASH-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'