[PATCH] [lld] [mach-o] Support -l and -syslibroot options

kledzik at apple.com kledzik at apple.com
Mon Jul 7 13:50:17 PDT 2014


This sort of testing won't scale up to all the combinations of searching the linker needs to do.  

My suggestion is to add a hidden linker option* and change all uses of llvm::sys::fs::exists() to call a new MachOLinkingContext::fileExists(StringRef) method.  Normally, fileExists() calls through to  llvm::sys::fs::exists(), but if the new option is used, instead it prints out the path and returns false.  Now you can write test cases with various combinations of options and then use FileCheck to verify all the correct files and checked for in the right order (for instance -r mode should not look for .dylib files).

 * There are various ways expose the option.  It could be a command line arg, or an environment variable, or a DEBUG_WITH_TYPE() thing. Each has pros and cons.  Also, since this option causes all libraries to fail to be found (so the full search is shown), we may want to have the option also trigger the linker to stop early (setDoNothing()) so the test cases don't have to deal with the errors from "missing" files.

================
Comment at: include/lld/ReaderWriter/MachOLinkingContext.h:82-83
@@ -81,1 +81,4 @@
 
+  ErrorOr<StringRef> searchPathForLibrary(StringRef path,
+                                          StringRef libName) const;
+
----------------
"path" is ambiguous is that it can refer to a file or directory.  It would clearer to me to have this method as:

 ErrorOr<StringRef> searchDirForLibrary(StringRef dir, StringRef libName) const


================
Comment at: lib/ReaderWriter/MachO/MachOLinkingContext.cpp:271
@@ +270,3 @@
+  SmallString<128> fullPath;
+  if (libName.endswith(".o")) {
+    // A request ending in .o is special: just search for the file directly.
----------------
Tim Northover wrote:
> Shankar Kalpathi Easwaran wrote:
> >   - Can the the system paths be prepended to a set of directories that are being looked up, This could help adding for -L.
> >   - We can possibly search for a library name outside the loop.
> > 
> > 
> > 
> > 
> I started with that system, but specifically moved to this one for two reasons:
> 
> + -syslibroot only applies to the system directories, so I'd need some way of making sure they stay special anyway.
> + Similarly there's a -Z option that removes just these from the search path. This could be handled by dealing with that before any -L or similar, of course.
> 
> On the whole, I think the static array is the neatest solution.
Tim , -syslibroot applies to all -L paths - not just the built in ones, or the "system directories".  

The algorithm is that if  -syslibroot is used, to walk all search paths and if prepending that path with the syslibroot (aka SDK) path results in a directory, than switch that search path to use the SDK one.

You can add the -v option to ld64 and see it print out the final search paths.

================
Comment at: lib/ReaderWriter/MachO/MachOLinkingContext.cpp:275-276
@@ +274,4 @@
+    llvm::sys::path::append(fullPath, libName);
+    if (llvm::sys::fs::exists(fullPath.str()))
+      return StringRef(*new (_allocator) std::string(fullPath.str()));
+    return make_error_code(llvm::errc::no_such_file_or_directory);
----------------
You can avoid the copy-thru-std::string by using:
    return fullPath.str().copy(_allocator);

http://reviews.llvm.org/D4409






More information about the llvm-commits mailing list