Page MenuHomePhabricator

libclang: Make shared object symbol exporting by default
ClosedPublic

Authored by cristian.adam on Feb 20 2020, 8:59 AM.

Details

Summary

https://reviews.llvm.org/D74564 enabled static building for libclang, and for non CMake consumers they had to set the CMAKE_EXPORTS define when consuming libclang.

This commit makes the non CMake users of the static building have to define CMAKE_NO_EXPORTS.

Diff Detail

Event Timeline

cristian.adam created this revision.Feb 20 2020, 8:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 20 2020, 8:59 AM
thakis accepted this revision.Feb 20 2020, 9:06 AM

Thanks! Does this make it so that libclang.dll is built again with -DLLVM_ENABLE_PIC=NO on Window? From what I understand, it doesn't. Is that correct? If so, could you reinstantiate that too?

clang/include/clang-c/Platform.h
33

Why is this change necessary?

This revision is now accepted and ready to land.Feb 20 2020, 9:06 AM
cristian.adam marked an inline comment as done.Feb 20 2020, 9:09 AM
cristian.adam added inline comments.
clang/include/clang-c/Platform.h
33

I thought it would be nice to match the line 44, which deals with __attribute__((deprecated))

Thanks! Does this make it so that libclang.dll is built again with -DLLVM_ENABLE_PIC=NO on Window? From what I understand, it doesn't. Is that correct? If so, could you reinstantiate that too?

It should work with -DLLVM_ENABLE_PIC=NO because now only the static targets get the CINDEX_NO_EXPORTS define.

https://reviews.llvm.org/D74564 only adds CINDEX_EXPORTS to libclang and not obj.libclang.

I do not have commit rights, I need help to submit this patch.

Thanks! Does this make it so that libclang.dll is built again with -DLLVM_ENABLE_PIC=NO on Window? From what I understand, it doesn't. Is that correct? If so, could you reinstantiate that too?

It should work with -DLLVM_ENABLE_PIC=NO because now only the static targets get the CINDEX_NO_EXPORTS define.

https://reviews.llvm.org/D74564 only adds CINDEX_EXPORTS to libclang and not obj.libclang.

The problem is (I believe) that add_clang_library(libclang is passed ${ENABLE_SHARED} and that used to be true on Windows but no longer is after your change (due to the change in line 80 in your original change), so libclang.dll isn't built at all any longer. If it was still built, the _CINDEX_LIB_ logic around line 128 wouldn't be passed for the same reason.

-DLLVM_ENABLE_PIC=OFF on Windows if LIBCLANG_BUILD_STATIC is not set to ON will generate a SHARED libclang.dll

This comment was removed by cristian.adam.

The problem is (I believe) that add_clang_library(libclang is passed ${ENABLE_SHARED} and that used to be true on Windows but no longer is after your change (due to the change in line 80 in your original change), so libclang.dll isn't built at all any longer. If it was still built, the _CINDEX_LIB_ logic around line 128 wouldn't be passed for the same reason.

Right, I've uploaded a new patch which should take care of it.

thakis accepted this revision.Feb 20 2020, 11:24 AM

Thanks! Let me try landing this and see what happens :)

This revision was automatically updated to reflect the committed changes.

Thank you!

A build with -DLIBCLANG_BUILD_STATIC=ON and -DLLVM_ENABLE_PIC=OFF is green at:
https://github.com/cristianadam/llvm-project/actions/runs/42545217

The default build with -DLIBCLANG_BUILD_STATIC=OFF and -DLLVM_ENABLE_PIC=ON is green at:
https://github.com/cristianadam/llvm-project/actions/runs/42516378

Looks like things are still broken in several ways even with this in:

[2600/2950] Linking CXX executable bin\c-index-test.exe
 FAILED: bin/c-index-test.exe 
 cmd.exe /C "cd . && C:\b\s\w\ir\cache\builder\src\third_party\llvm-build-tools\cmake-3.12.1-win32-x86\bin\cmake.exe -E vs_link_exe --intdir=tools\clang\tools\c-index-test\CMakeFiles\c-index-test.dir --manifests  -- C:\b\s\w\ir\cache\builder\src\third_party\depot_tools\win_toolchain\vs_files\9ff60e43ba91947baca460d0ca3b1b980c3a2c23\VC\Tools\MSVC\14.23.28105\bin\Hostx64\x64\link.exe /nologo tools\clang\tools\c-index-test\CMakeFiles\c-index-test.dir\c-index-test.c.obj tools\clang\tools\c-index-test\CMakeFiles\c-index-test.dir\core_main.cpp.obj tools\clang\tools\c-index-test\CMakeFiles\c-index-test.dir\C_\b\s\w\ir\cache\builder\src\third_party\llvm\llvm\resources\windows_version_resource.rc.res  /out:bin\c-index-test.exe /implib:lib\c-index-test.lib /pdb:bin\c-index-test.pdb /version:0.0  /machine:x64 /STACK:10000000 /INCREMENTAL:NO /subsystem:console  lib\LLVMSupport.lib lib\libclang.lib lib\clangAST.lib lib\clangBasic.lib lib\clangCodeGen.lib lib\clangFrontend.lib lib\clangIndex.lib lib\clangSerialization.lib lib\LLVMCoverage.lib lib\LLVMLTO.lib lib\LLVMObjCARCOpts.lib lib\LLVMPasses.lib lib\LLVMCoroutines.lib lib\LLVMipo.lib lib\LLVMIRReader.lib lib\LLVMAsmParser.lib lib\LLVMLinker.lib lib\LLVMInstrumentation.lib lib\LLVMVectorize.lib lib\LLVMCodeGen.lib lib\LLVMBitWriter.lib lib\LLVMScalarOpts.lib lib\LLVMAggressiveInstCombine.lib lib\LLVMInstCombine.lib lib\LLVMTarget.lib lib\clangFrontend.lib lib\clangDriver.lib version.lib lib\clangParse.lib lib\LLVMOption.lib lib\clangSerialization.lib lib\clangSema.lib lib\clangAnalysis.lib lib\clangASTMatchers.lib lib\clangEdit.lib lib\clangFormat.lib lib\clangToolingInclusions.lib lib\clangToolingCore.lib lib\clangAST.lib lib\LLVMFrontendOpenMP.lib lib\LLVMTransformUtils.lib lib\LLVMAnalysis.lib lib\LLVMObject.lib lib\LLVMBitReader.lib lib\LLVMMCParser.lib lib\LLVMTextAPI.lib lib\LLVMProfileData.lib lib\clangRewrite.lib lib\clangLex.lib lib\clangBasic.lib lib\LLVMCore.lib lib\LLVMRemarks.lib lib\LLVMBitstreamReader.lib lib\LLVMMC.lib lib\LLVMBinaryFormat.lib lib\LLVMDebugInfoCodeView.lib lib\LLVMDebugInfoMSF.lib lib\LLVMSupport.lib C:\b\s\w\ir\cache\builder\src\third_party\llvm-build-tools\zlib-1.2.11\zlib.lib psapi.lib shell32.lib ole32.lib uuid.lib advapi32.lib delayimp.lib -delayload:shell32.dll -delayload:ole32.dll lib\LLVMDemangle.lib kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib && cd ."
 LINK: command "C:\b\s\w\ir\cache\builder\src\third_party\depot_tools\win_toolchain\vs_files\9ff60e43ba91947baca460d0ca3b1b980c3a2c23\VC\Tools\MSVC\14.23.28105\bin\Hostx64\x64\link.exe /nologo tools\clang\tools\c-index-test\CMakeFiles\c-index-test.dir\c-index-test.c.obj tools\clang\tools\c-index-test\CMakeFiles\c-index-test.dir\core_main.cpp.obj tools\clang\tools\c-index-test\CMakeFiles\c-index-test.dir\C_\b\s\w\ir\cache\builder\src\third_party\llvm\llvm\resources\windows_version_resource.rc.res /out:bin\c-index-test.exe /implib:lib\c-index-test.lib /pdb:bin\c-index-test.pdb /version:0.0 /machine:x64 /STACK:10000000 /INCREMENTAL:NO /subsystem:console lib\LLVMSupport.lib lib\libclang.lib lib\clangAST.lib lib\clangBasic.lib lib\clangCodeGen.lib lib\clangFrontend.lib lib\clangIndex.lib lib\clangSerialization.lib lib\LLVMCoverage.lib lib\LLVMLTO.lib lib\LLVMObjCARCOpts.lib lib\LLVMPasses.lib lib\LLVMCoroutines.lib lib\LLVMipo.lib lib\LLVMIRReader.lib lib\LLVMAsmParser.lib lib\LLVMLinker.lib lib\LLVMInstrumentation.lib lib\LLVMVectorize.lib lib\LLVMCodeGen.lib lib\LLVMBitWriter.lib lib\LLVMScalarOpts.lib lib\LLVMAggressiveInstCombine.lib lib\LLVMInstCombine.lib lib\LLVMTarget.lib lib\clangFrontend.lib lib\clangDriver.lib version.lib lib\clangParse.lib lib\LLVMOption.lib lib\clangSerialization.lib lib\clangSema.lib lib\clangAnalysis.lib lib\clangASTMatchers.lib lib\clangEdit.lib lib\clangFormat.lib lib\clangToolingInclusions.lib lib\clangToolingCore.lib lib\clangAST.lib lib\LLVMFrontendOpenMP.lib lib\LLVMTransformUtils.lib lib\LLVMAnalysis.lib lib\LLVMObject.lib lib\LLVMBitReader.lib lib\LLVMMCParser.lib lib\LLVMTextAPI.lib lib\LLVMProfileData.lib lib\clangRewrite.lib lib\clangLex.lib lib\clangBasic.lib lib\LLVMCore.lib lib\LLVMRemarks.lib lib\LLVMBitstreamReader.lib lib\LLVMMC.lib lib\LLVMBinaryFormat.lib lib\LLVMDebugInfoCodeView.lib lib\LLVMDebugInfoMSF.lib lib\LLVMSupport.lib C:\b\s\w\ir\cache\builder\src\third_party\llvm-build-tools\zlib-1.2.11\zlib.lib psapi.lib shell32.lib ole32.lib uuid.lib advapi32.lib delayimp.lib -delayload:shell32.dll -delayload:ole32.dll lib\LLVMDemangle.lib kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib /MANIFEST /MANIFESTFILE:bin\c-index-test.exe.manifest" failed (exit code 1120) with the following output:
 c-index-test.c.obj : error LNK2019: unresolved external symbol clang_getCString referenced in function index_diagnostic
(and many other symbols)

and also

ninja: warning: multiple rules generate lib/libclang.lib. builds involving this target will not be correct; continuing anyway [-w dupbuild=warn]

(which is a hard error with newer ninjas).

I'll revert for now to unbreak things. Happy to test new attempts before they go in :)