[PATCH] D97641: [lld/mac] Add support for -flat_namespace

Nico Weber via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Mar 1 10:42:30 PST 2021


thakis added a comment.

Thanks!



================
Comment at: lld/MachO/InputFiles.cpp:705
+          reinterpret_cast<const char *>(c) + read32le(&c->dylib.name);
+      ::loadDylib(dylibPath, umbrella);
+    }
----------------
int3 wrote:
> int3 wrote:
> > hm is the `::` prefix necessary?
> judging from the test failures, I guess we need to account for syslibroot here too
> hm is the :: prefix necessary?

Yes, else ADL makes this try to use lld::macho::loadDyib() (from Driver.h, in DriverUtils.cpp)


================
Comment at: lld/MachO/InputFiles.cpp:705
+          reinterpret_cast<const char *>(c) + read32le(&c->dylib.name);
+      ::loadDylib(dylibPath, umbrella);
+    }
----------------
thakis wrote:
> int3 wrote:
> > int3 wrote:
> > > hm is the `::` prefix necessary?
> > judging from the test failures, I guess we need to account for syslibroot here too
> > hm is the :: prefix necessary?
> 
> Yes, else ADL makes this try to use lld::macho::loadDyib() (from Driver.h, in DriverUtils.cpp)
> judging from the test failures, I guess we need to account for syslibroot here too

ah, annoying. will try to do that…


================
Comment at: lld/test/MachO/flat-namespace.s:38-39
+
+# No "(dynamically looked up)" because llvm-nm -m doesn't print that
+# for files without MH_TWOLEVEL for some reason.
+# FLATSYM: (undefined) external _bar
----------------
int3 wrote:
> does `llvm-objdump --syms` or `llvm-readobj` print anything better?
`llvm-objdump --syms` doesn't print it. `llvm-readobj` has it as `Flags [ (0xFE00)`. obj2yaml also prints it as `n_desc:          65024`.

I think we definitely want to test `nm` here since that's what end users are most likely to use. Want me to add a readobj/yaml2obj test in addition to what's here, or nah?


================
Comment at: lld/test/MachO/header.s:15-16
 
 # CHECK:      magic        cputype cpusubtype  caps    filetype
-# CHECK-NEXT: MH_MAGIC_64  {{.*}}         ALL  [[CAPS]]   {{.*}}
+# CHECK-NEXT: MH_MAGIC_64  {{.*}}         ALL  [[CAPS]]   {{.*}} TWOLEVEL
 
----------------
int3 wrote:
> ultra nit: align the header
Done, but it's not aligned in llvm-objdump output :P


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

https://reviews.llvm.org/D97641



More information about the llvm-commits mailing list