Skip to content

[Driver] Infer target-specific dylib names #12443

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

modocache
Copy link
Contributor

When the Swift driver is invoked with the -emit-library option, but without an -o option that specifies the emitted library's filename, logic in the getOutputFilename() function derives a filename: "lib" + <a plasible base name>", and then the value of the LTDL_SHLIB_EXT macro.

There are two problems here:

  1. Windows shared library file names, by convention, do not begin with "lib".
  2. The LTDL_SHLIB_EXT macro is set by llvm/cmake/modules/HandleLLVMOptions.cmake, based on CMAKE_SHARED_LIBRARY_SUFFIX, a built-in CMake variable that is set at the time LLVM is configured to be built. So, if LLVM and Swift were built on a Linux machine, but the swiftc executable that was built was then invoked to produce a shared library for a Darwin target, the library would have a ".so" suffix, not ".dylib". (It's for this reason that the tests for this name inference, in test/Driver/linker.swift, must use a regular expression that matches both ".dylib" and ".so", despite specifying a Darwin -target.)

In order to produce conventionally correct prefixes and suffixes based on the target, modify the getOutputFilename() function to take an llvm::Triple argument.

When the Swift driver is invoked with the `-emit-library` option, but
without an `-o` option that specifies the emitted library's filename,
logic in the `getOutputFilename()` function derives a filename:
`"lib" + <a plasible base name>"`, and then the value of the
`LTDL_SHLIB_EXT` macro.

There are two problems here:

1. Windows shared library file names, by convention, do not begin with "lib".
2. The `LTDL_SHLIB_EXT` macro is set by
   `llvm/cmake/modules/HandleLLVMOptions.cmake`, based on
   `CMAKE_SHARED_LIBRARY_SUFFIX`, a built-in CMake variable that is set
   at the time LLVM is configured to be built. So, if LLVM and Swift
   were built on a Linux machine, but the `swiftc` executable that was
   built was then invoked to produce a shared library for a Darwin target,
   the library would have a ".so" suffix, not ".dylib". (It's for this
   reason that the tests for this name inference, in
   `test/Driver/linker.swift`, must use a regular expression that
   matches both ".dylib" and ".so", despite specifying a Darwin
   `-target`.)

In order to produce conventionally correct prefixes and suffixes based
on the target, modify the `getOutputFilename()` function to take an
`llvm::Triple` argument.
@modocache modocache requested a review from jrose-apple October 15, 2017 04:41
@modocache
Copy link
Contributor Author

@swift-ci please smoke test

@@ -1814,10 +1815,17 @@ static StringRef getOutputFilename(Compilation &C,
BaseName = llvm::sys::path::stem(BaseInput);
if (auto link = dyn_cast<LinkJobAction>(JA)) {
if (link->getKind() == LinkKind::DynamicLibrary) {
// FIXME: This should be target-specific.
Buffer = "lib";
if (Triple.isOSWindows())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drive by food for thought: Why not hoist this into LLVM? How does clang do this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, absolutely, I felt the same! I didn't look where the best place for this to live would be, though. I'll check what Clang does and comment back here.

@jrose-apple
Copy link
Contributor

Seems reasonable to me. I think Michael's right that this ought to live somewhere else in the long run, but it'd be okay to take it here for now if it looks annoying to factor out of Clang (or if Clang doesn't even do this yet).

@modocache
Copy link
Contributor Author

modocache commented Nov 1, 2017

Sorry it took me a while to circle back to this. As far as I can tell, I don't think Clang tries nearly as hard as Swift to come up with a reasonable default output name. Instead, it simply chooses between a.out or a.exe, based on whether it's targeting Windows. I tried running clang -### with various combinations of -target and -shared and it seemed like I always ended up with either -o a.out or -o a.exe.

I did find a few other spots that determine whether to use a "lib" prefix, or what file extension to use, but none have to do with determining an output file name:

  • This bit of code appears to be determining the path to a compiler-rt shared library.
  • This uses compile-time constant macros to determine the location of the LLVM gold plugin.
  • The Python bindings to LLVM use basically the same logic as I wrote in this pull request in order to infer a path to LLVM libraries.

So, thanks for pointing this out, it's got me wondering whether I could contribute to Clang the same sort of output file name inference as Swift uses, at which point I think it'd be a great idea to move this logic up to llvm::Triple. For now, though, maybe this PR is good enough?

@modocache
Copy link
Contributor Author

@swift-ci please smoke test and merge

@swift-ci swift-ci merged commit b382fa1 into swiftlang:master Nov 1, 2017
@modocache modocache deleted the driver-inferred-dylib-name-fixme branch November 2, 2017 02:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants