[sourcekitd-test] Remove LLVM Core static library that is getting linked twice

[finagolfin]

On Linux and Android, this Core library is presumably already supplied by the LLVM CMake config, so adding it here causes the CommandLine option parser to fail as it registers the same option twice.

I have not dug into exactly where the previous dependency's coming from or why it changed, but I can consistently reproduce since late last year on an Android host and in the one time I tried it on Arch Linux x86_64. What happens is that any invocation of sourcekitd-test fails with this error, after building with this command on Linux x86_64:

> ./swift/utils/build-script -R --no-assertions --llvm-targets-to-build="X86;ARM;AArch64" -j9 -T

> ./build/Ninja-Release/swift-linux-x86_64/bin/sourcekitd-test --help
: CommandLine Error: Option 'non-global-value-max-name-size' registered more than once!
LLVM ERROR: inconsistency in registered CommandLine options

@drodriguez says he's seen this intermittently in Linux too, "Yes, I have seen this in one my setups (a chrooted Ubuntu in a CentOS), but I think only from time to time. I couldn't find a reason why this is not happening in CI."

@compnerd, you last modified this line to remove another LLVM library, so maybe you know what's going on.

[drodriguez]

@swift-ci please test

[finagolfin]

The fact this passes the CI implies this library is already linked elsewhere on the CI too (or extremely unlikely, the Swift compiler tests never invoke anything from it 😉 ). That means there's some linking issue where linking this static library twice will sometimes cause problems, as it did for me and Daniel, though I don't know what that ELF or linker issue would be.

[gribozavr]

BTW, I am also seeing the same issue in my local Linux builds (Option 'non-global-value-max-name-size' registered more than once).

[drodriguez]

/cc @benlangmuir @rintaro which has been working in this lately

@compnerd: seems that you added the original core in 2016 f0e06df, but seems that adding support already includes core, and in some configurations seems to bring the same options with different symbols (?) to the same binary.

[compnerd]

This seems wrong. LLVMCore definitely should be linked in, since that is what provides things like DL. LLVMSupport definitely shouldn't include LLVMCore, as LLVMCore uses LLVMSupport.

[finagolfin]

I just experimented with this on an Android host by removing each library on this line one by one, and it turns out that simply deleting this line completely had no effect on linking and running sourcekitd-test --help, ie that also fixed the problem and it ran fine. Looking at the list of LLVM static libraries that remained even after this line was deleted, these 9 LLVM static libraries were left in the build.ninja link command, including LLVMCore and LLVMSupport:

build/Ninja-Release/llvm-android-aarch64/lib/libLLVMCore.a build/Ninja-Release/llvm-android-aarch64/lib/libLLVMRemarks.a build/Ninja-Release/llvm-android-aarch64/lib/libLLVMBitstreamReader.a build/Ninja-Release/llvm-android-aarch64/lib/libLLVMMC.a build/Ninja-Release/llvm-android-aarch64/lib/libLLVMBinaryFormat.a build/Ninja-Release/llvm-android-aarch64/lib/libLLVMDebugInfoCodeView.a build/Ninja-Release/llvm-android-aarch64/lib/libLLVMDebugInfoMSF.a build/Ninja-Release/llvm-android-aarch64/lib/libLLVMSupport.a build/Ninja-Release/llvm-android-aarch64/lib/libLLVMDemangle.a

Maybe this line can be deleted altogether because of the recent LLVM monorepo or other config changes?

[drodriguez]

@compnerd: it might seem wrong, but as the build results prove, it doesn't seem necessary, and fixes the build for several people. I don't understand where the double linking is coming from, but looking at the code of add_sourcekit_executable it might be already adding this components thru target_link_libraries(${name} PRIVATE ${LLVM_COMMON_LIBS}).

@swift-ci please test

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - 7ba01e9998f44b0cd2863ccd72dad9cf0f8e46a7

[swift-ci]

Build failed
Swift Test Linux Platform
Git Sha - 7ba01e9998f44b0cd2863ccd72dad9cf0f8e46a7

[finagolfin]

CI failures are spurious and it says they were restarted, but I don't know where those are.

[drodriguez]

This is the failure:

22:07:59 FAILED: bin/sourcekitd-test 
22:07:59 : && /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang++  -Wno-unknown-warning-option -Werror=unguarded-availability-new -fno-stack-protector -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -std=c++14 -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -Wimplicit-fallthrough -Wcovered-switch-default -Wno-class-memaccess -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color -Werror=switch -Wdocumentation -Wimplicit-fallthrough -Wunreachable-code -Woverloaded-virtual -DOBJC_OLD_DISPATCH_PROTOTYPES=0 -O3  -arch x86_64 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk -Wl,-search_paths_first -Wl,-headerpad_max_install_names -mmacosx-version-min=10.9    -target x86_64-apple-macosx10.9 -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk -arch x86_64 -F /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk/../../../Developer/Library/Frameworks -mmacosx-version-min=10.9 -Wl,-dead_strip -Xlinker -exported_symbol -Xlinker _main tools/SourceKit/tools/sourcekitd-test/CMakeFiles/sourcekitd-test.dir/sourcekitd-test.cpp.o tools/SourceKit/tools/sourcekitd-test/CMakeFiles/sourcekitd-test.dir/TestOptions.cpp.o  -o bin/sourcekitd-test -L/Users/buildnode/jenkins/workspace/swift-PR-osx/branch-master/buildbot_incremental/llvm-macosx-x86_64/./lib -Wl,-rpath,/Users/buildnode/jenkins/workspace/swift-PR-osx/branch-master/buildbot_incremental/llvm-macosx-x86_64/./lib -Wl,-rpath,/Users/buildnode/jenkins/workspace/swift-PR-osx/branch-master/buildbot_incremental/swift-macosx-x86_64/lib lib/libSourceKitSupport.a /Users/buildnode/jenkins/workspace/swift-PR-osx/branch-master/buildbot_incremental/llvm-macosx-x86_64/lib/libclangRewrite.a /Users/buildnode/jenkins/workspace/swift-PR-osx/branch-master/buildbot_incremental/llvm-macosx-x86_64/lib/libclangLex.a /Users/buildnode/jenkins/workspace/swift-PR-osx/branch-master/buildbot_incremental/llvm-macosx-x86_64/lib/libclangBasic.a lib/sourcekitd.framework/Versions/A/sourcekitd /Users/buildnode/jenkins/workspace/swift-PR-osx/branch-master/buildbot_incremental/llvm-macosx-x86_64/lib/libLLVMCore.a /Users/buildnode/jenkins/workspace/swift-PR-osx/branch-master/buildbot_incremental/llvm-macosx-x86_64/lib/libLLVMRemarks.a /Users/buildnode/jenkins/workspace/swift-PR-osx/branch-master/buildbot_incremental/llvm-macosx-x86_64/lib/libLLVMBitstreamReader.a /Users/buildnode/jenkins/workspace/swift-PR-osx/branch-master/buildbot_incremental/llvm-macosx-x86_64/lib/libLLVMMC.a /Users/buildnode/jenkins/workspace/swift-PR-osx/branch-master/buildbot_incremental/llvm-macosx-x86_64/lib/libLLVMBinaryFormat.a /Users/buildnode/jenkins/workspace/swift-PR-osx/branch-master/buildbot_incremental/llvm-macosx-x86_64/lib/libLLVMDebugInfoCodeView.a /Users/buildnode/jenkins/workspace/swift-PR-osx/branch-master/buildbot_incremental/llvm-macosx-x86_64/lib/libLLVMDebugInfoMSF.a lib/libswiftBasic.a /Users/buildnode/jenkins/workspace/swift-PR-osx/branch-master/buildbot_incremental/llvm-macosx-x86_64/lib/libLLVMSupport.a -lz -lcurses -lm /Users/buildnode/jenkins/workspace/swift-PR-osx/branch-master/buildbot_incremental/llvm-macosx-x86_64/lib/libLLVMDemangle.a lib/libswiftDemangling.a lib/libswiftSyntax.a && :
22:07:59 Undefined symbols for architecture x86_64:
22:07:59   "vtable for llvm::opt::InputArgList", referenced from:
22:07:59       llvm::opt::InputArgList::~InputArgList() in TestOptions.cpp.o
22:07:59   NOTE: a missing vtable usually means the first non-inline virtual member function has no definition.
22:07:59   "llvm::opt::OptTable::OptTable(llvm::ArrayRef<llvm::opt::OptTable::Info>, bool)", referenced from:
22:07:59       sourcekitd_test::TestOptions::parseArgs(llvm::ArrayRef<char const*>) in TestOptions.cpp.o
22:07:59   "llvm::opt::InputArgList::releaseMemory()", referenced from:
22:07:59       llvm::opt::InputArgList::~InputArgList() in TestOptions.cpp.o
22:07:59   "llvm::opt::OptTable::PrintHelp(llvm::raw_ostream&, char const*, char const*, bool, bool) const", referenced from:
22:07:59       sourcekitd_test::TestOptions::parseArgs(llvm::ArrayRef<char const*>) in TestOptions.cpp.o
22:07:59   "llvm::opt::OptTable::ParseArgs(llvm::ArrayRef<char const*>, unsigned int&, unsigned int&, unsigned int, unsigned int) const", referenced from:
22:07:59       sourcekitd_test::TestOptions::parseArgs(llvm::ArrayRef<char const*>) in TestOptions.cpp.o
22:07:59   "llvm::opt::OptTable::~OptTable()", referenced from:
22:07:59       sourcekitd_test::TestOptions::parseArgs(llvm::ArrayRef<char const*>) in TestOptions.cpp.o
22:07:59   "llvm::opt::Arg::getAsString(llvm::opt::ArgList const&) const", referenced from:
22:07:59       sourcekitd_test::TestOptions::parseArgs(llvm::ArrayRef<char const*>) in TestOptions.cpp.o
22:07:59 ld: symbol(s) not found for architecture x86_64

(from https://ci.swift.org/job/swift-PR-osx/18140/console)

Seems that some of those libraries are necessary in macOS.

[finagolfin]

Thanks, looks like it passed on linux, just like it did for me on Android. Should I add option alone back, or do you know if coverage and lto are also needed on macOS?

[finagolfin]

Tracked down when those last two dependencies were added, 8531c5d three years back, apparently for linux. I searched for those two functions he mentioned, but only found the CoverageFilenamesSectionWriter one in this repo back then and now.

Since I don't know how and when those dependencies are pulled in and it's untested by the linux CI, I just added them back in to be safe. @alblue, let us know if anything has changed.

This pull just removes the clearly redundant LLVMCore library now, which seems to already be supplied by the LLVM config elsewhere on all platforms.

[alblue]

They were a necessary inclusion at the time, but if it can be built without them then they can be removed as far as I am concerned. I'm not working on the sourcekit or related functionality any more though.

[drodriguez]

@swift-ci please test

[finagolfin]

Ping, ready to go?