[analyzer] Don't specify PLUGIN_TOOL for analyzer plugins

[ArcsinX]

Summary

Analyzer plugins explicitly export clang_registerCheckers and clang_analyzerAPIVersionString symbols, so we don't need to specify a tool to link agains.

Also, without this patch MSVC build fails with cmake flags -DLLVM_ENABLE_PLUGINS=On -DCLANG_PLUGINS_SUPPORT=On -DLLVM_EXPORT_SYMBOLS_FOR_PLUGINS=On

[936/936] Linking CXX shared module bin\SampleAnalyzerPlugin.dll
FAILED: bin/SampleAnalyzerPlugin.dll
cmd.exe /C "cd . && "D:\Program Files\CMake\bin\cmake.exe" -E vs_link_dll --intdir=tools\clang\lib\Analysis\plugins\SampleAnalyzer\CMakeFiles\SampleAnalyzerPlugin.dir --rc=C:\PROGRA~2\WI3CF2~1\10\bin\100183~1.0\x64\rc.exe --mt=C:\PROGRA~2\WI3CF2~1\10\bin\100183~1.0\x64\mt.exe --manifests  -- C:\PROGRA~2\MICROS~4\2019\COMMUN~1\VC\Tools\MSVC\1428~1.299\bin\Hostx64\x64\link.exe /nologo tools\clang\lib\Analysis\plugins\SampleAnalyzer\CMakeFiles\SampleAnalyzerPlugin.dir\MainCallChecker.cpp.obj  /out:bin\SampleAnalyzerPlugin.dll /implib:lib\SampleAnalyzerPlugin.lib /pdb:bin\SampleAnalyzerPlugin.pdb /dll /version:0.0 /machine:x64 /INCREMENTAL:NO  /DEF:"D:/work/llvm-project-original/build-plugins/tools/clang/lib/Analysis/plugins/SampleAnalyzer/SampleAnalyzerPlugin.def"  lib\clang.lib  lib\clangAnalysis.lib  lib\clangAST.lib  lib\clangStaticAnalyzerCore.lib  lib\clangStaticAnalyzerFrontend.lib  lib\clangStaticAnalyzerCheckers.lib  lib\clangStaticAnalyzerCore.lib  lib\clangCrossTU.lib  lib\clangIndex.lib  lib\clangFormat.lib  lib\clangToolingInclusions.lib  lib\clangFrontend.lib  lib\clangDriver.lib  version.lib  lib\clangParse.lib  lib\clangSerialization.lib  lib\clangSema.lib  lib\clangAnalysis.lib  lib\clangEdit.lib  lib\LLVMOption.lib  lib\clangToolingCore.lib  lib\clangRewrite.lib  lib\clangASTMatchers.lib  lib\clangAST.lib  lib\clangLex.lib  lib\clangBasic.lib  lib\LLVMFrontendOpenMP.lib  lib\LLVMScalarOpts.lib  lib\LLVMAggressiveInstCombine.lib  lib\LLVMInstCombine.lib  lib\LLVMTransformUtils.lib  lib\LLVMAnalysis.lib  lib\LLVMProfileData.lib  lib\LLVMDebugInfoDWARF.lib  lib\LLVMObject.lib  lib\LLVMBitReader.lib  lib\LLVMCore.lib  lib\LLVMRemarks.lib  lib\LLVMBitstreamReader.lib  lib\LLVMMCParser.lib  lib\LLVMMC.lib  lib\LLVMDebugInfoCodeView.lib  lib\LLVMTextAPI.lib  lib\LLVMBinaryFormat.lib  lib\LLVMSupport.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:\PROGRA~2\MICROS~4\2019\COMMUN~1\VC\Tools\MSVC\1428~1.299\bin\Hostx64\x64\link.exe /nologo tools\clang\lib\Analysis\plugins\SampleAnalyzer\CMakeFiles\SampleAnalyzerPlugin.dir\MainCallChecker.cpp.obj /out:bin\SampleAnalyzerPlugin.dll /implib:lib\SampleAnalyzerPlugin.lib /pdb:bin\SampleAnalyzerPlugin.pdb /dll /version:0.0 /machine:x64 /INCREMENTAL:NO /DEF:D:/work/llvm-project-original/build-plugins/tools/clang/lib/Analysis/plugins/SampleAnalyzer/SampleAnalyzerPlugin.def lib\clang.lib lib\clangAnalysis.lib lib\clangAST.lib lib\clangStaticAnalyzerCore.lib lib\clangStaticAnalyzerFrontend.lib lib\clangStaticAnalyzerCheckers.lib lib\clangStaticAnalyzerCore.lib lib\clangCrossTU.lib lib\clangIndex.lib lib\clangFormat.lib lib\clangToolingInclusions.lib lib\clangFrontend.lib lib\clangDriver.lib version.lib lib\clangParse.lib lib\clangSerialization.lib lib\clangSema.lib lib\clangAnalysis.lib lib\clangEdit.lib lib\LLVMOption.lib lib\clangToolingCore.lib lib\clangRewrite.lib lib\clangASTMatchers.lib lib\clangAST.lib lib\clangLex.lib lib\clangBasic.lib lib\LLVMFrontendOpenMP.lib lib\LLVMScalarOpts.lib lib\LLVMAggressiveInstCombine.lib lib\LLVMInstCombine.lib lib\LLVMTransformUtils.lib lib\LLVMAnalysis.lib lib\LLVMProfileData.lib lib\LLVMDebugInfoDWARF.lib lib\LLVMObject.lib lib\LLVMBitReader.lib lib\LLVMCore.lib lib\LLVMRemarks.lib lib\LLVMBitstreamReader.lib lib\LLVMMCParser.lib lib\LLVMMC.lib lib\LLVMDebugInfoCodeView.lib lib\LLVMTextAPI.lib lib\LLVMBinaryFormat.lib lib\LLVMSupport.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\SampleAnalyzerPlugin.dll.manifest" failed (exit code 1169) with the following output:
clangStaticAnalyzerCore.lib(BugReporter.cpp.obj) : error LNK2005: "public: __cdecl clang::ento::PathSensitiveBugReport::PathSensitiveBugReport(class clang::ento::BugType const &,class llvm::StringRef,class llvm::StringRef,class clang::ento::ExplodedNode const *,class clang::ento::PathDiagnosticLocation,class clang::Decl const *)" (??0PathSensitiveBugReport@ento@clang@@QEAA@AEBVBugType@12@VStringRef@llvm@@1PEBVExplodedNode@12@VPathDiagnosticLocation@12@PEBVDecl@2@@Z) already defined in clang.lib(clang.exe)
clangStaticAnalyzerCore.lib(BugReporter.cpp.obj) : error LNK2005: "private: virtual void __cdecl clang::ento::BugType::anchor(void)" (?anchor@BugType@ento@clang@@EEAAXXZ) already defined in clang.lib(clang.exe)
clangStaticAnalyzerCore.lib(SVals.cpp.obj) : error LNK2005: "public: class clang::FunctionDecl const * __cdecl clang::ento::SVal::getAsFunctionDecl(void)const " (?getAsFunctionDecl@SVal@ento@clang@@QEBAPEBVFunctionDecl@3@XZ) already defined in clang.lib(clang.exe)
clangStaticAnalyzerCore.lib(ProgramState.cpp.obj) : error LNK2005: "void __cdecl clang::ento::ProgramStateRelease(class clang::ento::ProgramState const *)" (?ProgramStateRelease@ento@clang@@YAXPEBVProgramState@12@@Z) already defined in clang.lib(clang.exe)
clangStaticAnalyzerCore.lib(ProgramState.cpp.obj) : error LNK2005: "void __cdecl clang::ento::ProgramStateRetain(class clang::ento::ProgramState const *)" (?ProgramStateRetain@ento@clang@@YAXPEBVProgramState@12@@Z) already defined in clang.lib(clang.exe)
clangStaticAnalyzerCore.lib(Environment.cpp.obj) : error LNK2005: "public: __cdecl clang::ento::EnvironmentEntry::EnvironmentEntry(class clang::Stmt const *,class clang::LocationContext const *)" (??0EnvironmentEntry@ento@clang@@QEAA@PEBVStmt@2@PEBVLocationContext@2@@Z) already defined in clang.lib(clang.exe)
clangStaticAnalyzerCore.lib(Environment.cpp.obj) : error LNK2005: "public: class clang::ento::SVal __cdecl clang::ento::Environment::getSVal(class clang::ento::EnvironmentEntry const &,class clang::ento::SValBuilder &)const " (?getSVal@Environment@ento@clang@@QEBA?AVSVal@23@AEBVEnvironmentEntry@23@AEAVSValBuilder@23@@Z) already defined in clang.lib(clang.exe)
clangStaticAnalyzerCore.lib(CheckerManager.cpp.obj) : error LNK2005: "public: void __cdecl clang::ento::CheckerManager::_registerForPreStmt(class clang::ento::CheckerFn<void __cdecl(class clang::Stmt const *,class clang::ento::CheckerContext &)>,bool (__cdecl*)(class clang::Stmt const *))" (?_registerForPreStmt@CheckerManager@ento@clang@@QEAAXV?$CheckerFn@$$A6AXPEBVStmt@clang@@AEAVCheckerContext@ento@2@@Z@23@P6A_NPEBVStmt@3@@Z@Z) already defined in clang.lib(clang.exe)
clangStaticAnalyzerCore.lib(CoreEngine.cpp.obj) : error LNK2005: "protected: class clang::ento::ExplodedNode * __cdecl clang::ento::NodeBuilder::generateNodeImpl(class clang::ProgramPoint const &,class llvm::IntrusiveRefCntPtr<class clang::ento::ProgramState const >,class clang::ento::ExplodedNode *,bool)" (?generateNodeImpl@NodeBuilder@ento@clang@@IEAAPEAVExplodedNode@23@AEBVProgramPoint@3@V?$IntrusiveRefCntPtr@$$CBVProgramState@ento@clang@@@llvm@@PEAV423@_N@Z) already defined in clang.lib(clang.exe)
LLVMSupport.lib(SmallVector.cpp.obj) : error LNK2005: "protected: void __cdecl llvm::SmallVectorBase<unsigned int>::grow_pod(void *,unsigned __int64,unsigned __int64)" (?grow_pod@?$SmallVectorBase@I@llvm@@IEAAXPEAX_K1@Z) already defined in clang.lib(clang.exe)
LLVMSupport.lib(FoldingSet.cpp.obj) : error LNK2005: "protected: __cdecl llvm::FoldingSetBase::~FoldingSetBase(void)" (??1FoldingSetBase@llvm@@IEAA@XZ) already defined in clang.lib(clang.exe)
clangAST.lib(ASTImporter.cpp.obj) : error LNK2005: "public: __cdecl clang::ASTImporter::ASTImporter(class clang::ASTContext &,class clang::FileManager &,class clang::ASTContext &,class clang::FileManager &,bool,class std::shared_ptr<class clang::ASTImporterSharedState>)" (??0ASTImporter@clang@@QEAA@AEAVASTContext@1@AEAVFileManager@1@01_NV?$shared_ptr@VASTImporterSharedState@clang@@@std@@@Z) already defined in clang.lib(clang.exe)
clangAST.lib(ASTImporter.cpp.obj) : error LNK2005: "public: class llvm::Expected<class clang::Decl *> __cdecl clang::ASTImporter::Import(class clang::Decl *)" (?Import@ASTImporter@clang@@QEAA?AV?$Expected@PEAVDecl@clang@@@llvm@@PEAVDecl@2@@Z) already defined in clang.lib(clang.exe)
clangAST.lib(ExternalASTSource.cpp.obj) : error LNK2005: "public: virtual __cdecl clang::ExternalASTSource::~ExternalASTSource(void)" (??1ExternalASTSource@clang@@UEAA@XZ) already defined in clang.lib(clang.exe)
clangAST.lib(ExternalASTSource.cpp.obj) : error LNK2005: "public: virtual void __cdecl clang::ExternalASTSource::CompleteRedeclChain(class clang::Decl const *)" (?CompleteRedeclChain@ExternalASTSource@clang@@UEAAXPEBVDecl@2@@Z) already defined in clang.lib(clang.exe)
clangAST.lib(ExternalASTSource.cpp.obj) : error LNK2005: "public: virtual void __cdecl clang::ExternalASTSource::CompleteType(class clang::ObjCInterfaceDecl *)" (?CompleteType@ExternalASTSource@clang@@UEAAXPEAVObjCInterfaceDecl@2@@Z) already defined in clang.lib(clang.exe)
...

[aaronpuchert]

This is only observable on Windows, right? Then perhaps @aaron.ballman can comment on this.

The change looks legitimate to me, but I don't work on Windows.

[ArcsinX]

In D116966#3242170, @aaronpuchert wrote:

This is only observable on Windows, right?

Thanks for your reply. Yes, PLUGIN_TOOL argument is used on Windows only, so this problem can't be observed on non-Windows systems and this patch affects only Windows build.

[aaron.ballman]

AFAIK, we don't support plugins on Windows. See: https://reviews.llvm.org/D16761#1441359 I don't think anything has changed in this regard. Based on that, I don't think this patch is necessary, but I'm adding some other folks who may be more familiar with the current state of plugin support on Windows just in case.

[ArcsinX]

In D116966#3243318, @aaron.ballman wrote:

AFAIK, we don't support plugins on Windows. See: https://reviews.llvm.org/D16761#1441359 I don't think anything has changed in this regard.

I can say that I successfully use clang plugins on Windows (and analyzer plugins too, which works in a bit different way). Seems this became possible after https://reviews.llvm.org/D18826

[aaron.ballman]

In D116966#3243346, @ArcsinX wrote:

In D116966#3243318, @aaron.ballman wrote:

AFAIK, we don't support plugins on Windows. See: https://reviews.llvm.org/D16761#1441359 I don't think anything has changed in this regard.

I can say that I successfully use clang plugins on Windows (and analyzer plugins too, which works in a bit different way). Seems this became possible after https://reviews.llvm.org/D18826

That's good to know, but looking at the dates of the two reviews still confuses me.

2016: https://reviews.llvm.org/D18826
2019: https://reviews.llvm.org/D16761#1441359

It doesn't help that we don't document supported platforms one way or the other in https://clang.llvm.org/docs/ClangPlugins.html.

Does your plugin work against a Clang built with BUILD_SHARED_LIB=0?

[aaron.ballman]

To be clear, I'm trying to figure out whether plugins are *actually* supported on Windows or whether they just so happen to work if the stars line up right for you. If they're supported, then awesome, let's go forward with this. If they're not supported, it's less clear whether we want to make it easier for the stars to line up or not.

[ArcsinX]

In D116966#3243374, @aaron.ballman wrote:

To be clear, I'm trying to figure out whether plugins are *actually* supported on Windows or whether they just so happen to work if the stars line up right for you. If they're supported, then awesome, let's go forward with this. If they're not supported, it's less clear whether we want to make it easier for the stars to line up or not.

That how it work for me (extra cmake flags: -DLLVM_ENABLE_PLUGINS=On -DCLANG_PLUGINS_SUPPORT=On -DLLVM_EXPORT_SYMBOLS_FOR_PLUGINS=On)

d:\work\llvm-project\build>bin\clang.exe -Xclang -plugin -Xclang print-fns d:\work\1.cpp -c
error: unable to find plugin 'print-fns'

d:\work\llvm-project\build>bin\clang.exe -fplugin=bin\PrintFunctionNames.dll -Xclang -plugin -Xclang print-fns d:\work\1.cpp -c
top-level-decl: "main"
top-level-decl: "a"

d:\work\llvm-project\build>cat CMakeCache.txt | grep BUILD_SHARED_LIB
BUILD_SHARED_LIBS:BOOL=OFF

[ArcsinX]

Friendly ping.

I am not sure that this patch is clear, so I will try to explain it again:

• clang static analyzer plugins are *NOT* a typical clang plugins: instead of llvm::Registry<>::Add usage, these plugins provides сlang_registerCheckers and clang_analyzerAPIVersionString functions which are called from static analyzer. So, these plugins work ok without anything special on Windows (but typical clang plugins are not and D18826 was a try to fix it).
• PLUGIN_TOOL together with LLVM_EXPORT_SYMBOLS_FOR_PLUGINS (introduced in D18826) is a try to add support for a typical clang plugins as a DLL on Windows. Thus, PLUGIN_TOOL does nothing on non-Windows OS.
• PLUGIN_TOOL does nothing without LLVM_EXPORT_SYMBOLS_FOR_PLUGINS=On, but in clang static analyzer plugins LLVM_EXPORTED_SYMBOL_FILE file is used to specify symbols we want to export (сlang_registerCheckers and clang_analyzerAPIVersionString)

Thus, for now the only thing PLUGIN_TOOL specification for clang static analyzer plugins does is breaking MSVC build when LLVM_EXPORT_SYMBOLS_FOR_PLUGINS=On (and plugins support enabled with -DCLANG_PLUGINS_SUPPORT=On -DLLVM_ENABLE_PLUGINS=On).

[aaron.ballman]

In D116966#3260571, @ArcsinX wrote:

Friendly ping.

I am not sure that this patch is clear, so I will try to explain it again:

• clang static analyzer plugins are *NOT* a typical clang plugins: instead of llvm::Registry<>::Add usage, these plugins provides сlang_registerCheckers and clang_analyzerAPIVersionString functions which are called from static analyzer. So, these plugins work ok without anything special on Windows (but typical clang plugins are not and D18826 was a try to fix it).
• PLUGIN_TOOL together with LLVM_EXPORT_SYMBOLS_FOR_PLUGINS (introduced in D18826) is a try to add support for a typical clang plugins as a DLL on Windows. Thus, PLUGIN_TOOL does nothing on non-Windows OS.
• PLUGIN_TOOL does nothing without LLVM_EXPORT_SYMBOLS_FOR_PLUGINS=On, but in clang static analyzer plugins LLVM_EXPORTED_SYMBOL_FILE file is used to specify symbols we want to export (сlang_registerCheckers and clang_analyzerAPIVersionString)

Thus, for now the only thing PLUGIN_TOOL specification for clang static analyzer plugins does is breaking MSVC build when LLVM_EXPORT_SYMBOLS_FOR_PLUGINS=On (and plugins support enabled with -DCLANG_PLUGINS_SUPPORT=On -DLLVM_ENABLE_PLUGINS=On).

Thanks for this! FWIW, I've been spending some time trying to figure out whether support is intended or not, and I've confirmed that we don't support plugins in general, but we do have some base level of support for them on Windows, depending on what the plugin does (it sounds like if you only call shared functions, you're likely okay, but once you try touching shared data objects, you're likely not okay). It sounds like you're in one of the situations that works well enough because it's only calling through function interfaces (and C interfaces at that). In light of that, this patch seems reasonable to me as it's another step towards better plugin support.

However, the changes you've made don't look to be specific to building on Windows; this removes PLUGIN_TOOL for all targets. I presume it's still needed for non-Windows targets, isn't it?

[ArcsinX]

In D116966#3261168, @aaron.ballman wrote:

However, the changes you've made don't look to be specific to building on Windows; this removes PLUGIN_TOOL for all targets. I presume it's still needed for non-Windows targets, isn't it?

As I can see, PLUGIN_TOOL is unused for non-Windows OS (at least for now): https://github.com/llvm/llvm-project/blob/main/llvm/cmake/modules/AddLLVM.cmake#L648

[aaron.ballman]

In D116966#3261202, @ArcsinX wrote:

In D116966#3261168, @aaron.ballman wrote:

However, the changes you've made don't look to be specific to building on Windows; this removes PLUGIN_TOOL for all targets. I presume it's still needed for non-Windows targets, isn't it?

As I can see, PLUGIN_TOOL is unused for non-Windows OS (at least for now): https://github.com/llvm/llvm-project/blob/main/llvm/cmake/modules/AddLLVM.cmake#L648

Yeah, that seems to be the case. LGTM!

[beanz]

Have you looked at the impact on binary size? PLUGIN_TOOL _should_ cause the plugins to link against the copy of LLVM & Clang in the tool target binary which reduces the binary size of the plugin and avoids duplicate global initializations.

[ArcsinX]

In D116966#3261479, @beanz wrote:

Have you looked at the impact on binary size? PLUGIN_TOOL _should_ cause the plugins to link against the copy of LLVM & Clang in the tool target binary which reduces the binary size of the plugin and avoids duplicate global initializations.

To see this difference we need to build this plugin with and without PLUGIN_TOOL (and with -DLLVM_EXPORT_SYMBOLS_FOR_PLUGINS=On)
But with PLUGIN_TOOL and -DLLVM_EXPORT_SYMBOLS_FOR_PLUGINS=On SampleAnalyzerPlugin can't be built using MSVC because of libraries conflict: we link DLL agains clang tool and also link libraries which this tool already contains (clangAnalysis, clangAST, clangStaticAnalyzerCore, clangStaticAnalyzerFrontend). You can check build log in the description.

clang/lib/Analysis/plugins/CheckerDependencyHandling/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Support
   )
 
 set(LLVM_EXPORTED_SYMBOL_FILE ${CMAKE_CURRENT_SOURCE_DIR}/CheckerDependencyHandlingAnalyzerPlugin.exports)
-add_llvm_library(CheckerDependencyHandlingAnalyzerPlugin MODULE BUILDTREE_ONLY CheckerDependencyHandling.cpp PLUGIN_TOOL clang)
+add_llvm_library(CheckerDependencyHandlingAnalyzerPlugin MODULE BUILDTREE_ONLY CheckerDependencyHandling.cpp)
 
 clang_target_link_libraries(CheckerDependencyHandlingAnalyzerPlugin PRIVATE
   clangAnalysis
   clangAST
   clangStaticAnalyzerCore
   clangStaticAnalyzerFrontend
   )

clang/lib/Analysis/plugins/CheckerOptionHandling/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Support
   )
 
 set(LLVM_EXPORTED_SYMBOL_FILE ${CMAKE_CURRENT_SOURCE_DIR}/CheckerOptionHandlingAnalyzerPlugin.exports)
-add_llvm_library(CheckerOptionHandlingAnalyzerPlugin MODULE BUILDTREE_ONLY CheckerOptionHandling.cpp PLUGIN_TOOL clang)
+add_llvm_library(CheckerOptionHandlingAnalyzerPlugin MODULE BUILDTREE_ONLY CheckerOptionHandling.cpp)
 
 clang_target_link_libraries(CheckerOptionHandlingAnalyzerPlugin PRIVATE
   clangAnalysis
   clangAST
   clangStaticAnalyzerCore
   clangStaticAnalyzerFrontend
   )

clang/lib/Analysis/plugins/SampleAnalyzer/CMakeLists.txt

 set(LLVM_LINK_COMPONENTS
   Support
   )
 
 set(LLVM_EXPORTED_SYMBOL_FILE ${CMAKE_CURRENT_SOURCE_DIR}/SampleAnalyzerPlugin.exports)
-add_llvm_library(SampleAnalyzerPlugin MODULE BUILDTREE_ONLY MainCallChecker.cpp PLUGIN_TOOL clang)
+add_llvm_library(SampleAnalyzerPlugin MODULE BUILDTREE_ONLY MainCallChecker.cpp)
 
 clang_target_link_libraries(SampleAnalyzerPlugin PRIVATE
   clangAnalysis
   clangAST
   clangStaticAnalyzerCore
   clangStaticAnalyzerFrontend
   )