Page MenuHomePhabricator

[analyzer] Conditionnaly include clang Analysis examples with cmake.
AbandonedPublic

Authored by Jiboo on Sep 21 2019, 9:21 AM.

Details

Summary

Without this patch, the clang Analysis plugin examples (in clang/lib/Analysis/plugins) are always built and installed even if CLANG_BUILD_EXAMPLES is OFF.

When using llvm-toolchain-bionic in http://apt.llvm.org/bionic/ the package llvm-10-dev includes a LLVMExports.cmake which includes thus samples. See /data/user/lib/llvm-10/lib/cmake/llvm/LLVMExports.cmake in http://apt.llvm.org/bionic/pool/main/l/llvm-toolchain-snapshot/llvm-10-dev_10~svn372305-1~exp1%2b0~20190919071908.1206~1.gbp6f40e3_amd64.deb

When trying to use llvm-10-dev using CMake from travis (with this config https://github.com/Jiboo/wembed/blob/54e9664d615313f39e1a4325c1e7f028ce2b72db/.travis.yml), I get the error:

CMake Error at /usr/lib/llvm-10/lib/cmake/llvm/LLVMExports.cmake:1341 (message):
The imported target "SampleAnalyzerPlugin" references the file
   "/usr/lib/llvm-10/lib/SampleAnalyzerPlugin.so"
but this file does not exist.  Possible reasons include:
* The file was deleted, renamed, or moved to another location.
* An install or uninstall procedure did not complete successfully.
* The installation package was faulty and contained
   "/usr/lib/llvm-10/lib/cmake/llvm/LLVMExports.cmake"
but not all the files it references.

I couldn't find SampleAnalyzerPlugin.so in any other packages (clang-10-examples only has sources), so I tried to replicate the problem locally, CLANG_BUILD_EXAMPLES was OFF, although the sample plugins were built and installed. Deleting /usr/local/lib/SampleAnalyzerPlugin.so replicated the travis error.

This fix looks ok to me, although I'm no CMake expert, particularly in LLVM context, and I might cure a symptom and not the cause, but with this patch if CLANG_BUILD_EXAMPLES is OFF, they don't appear in LLVMExports.cmake are not build/installed, and vice-versa if it's ON.

Diff Detail

Repository
rC Clang

Event Timeline

Jiboo created this revision.Sep 21 2019, 9:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 21 2019, 9:21 AM
Jiboo edited the summary of this revision. (Show Details)Sep 21 2019, 9:22 AM

Please upload all patches with full context (-U99999)

clang/lib/Analysis/plugins/CMakeLists.txt
1–2

Sure it isn't just the SampleAnalyzer that should be wrapped into CLANG_BUILD_EXAMPLES?

clang/test/CMakeLists.txt
130

Likewise

Jiboo updated this revision to Diff 221181.EditedSep 21 2019, 9:31 AM
Jiboo marked 3 inline comments as done.

Updated to add -U999999.

Jiboo added inline comments.Sep 21 2019, 9:33 AM
clang/lib/Analysis/plugins/CMakeLists.txt
1–2

Dunno, I though they were all examples, due to their contents:

CheckerDependencyHandling: registry.addDependency("example.DependendentChecker", "example.Dependency");
CheckerOptionHandling: registry.addChecker(registerMyChecker, shouldRegisterMyChecker, "example.MyChecker", "Example Description",

Jiboo edited the summary of this revision. (Show Details)Sep 21 2019, 9:36 AM
Szelethus retitled this revision from Conditionnaly include clang Analysis examples with cmake. to [analyzer] Conditionnaly include clang Analysis examples with cmake..Sep 21 2019, 9:40 AM
Szelethus added a reviewer: Szelethus.

Apologies for the intrusion, but these plugins were added by me, and test an important case for our internal plugins. Could you please give me just a day or so the check whether the tests work for us correctly?

Jiboo added a comment.EditedSep 21 2019, 10:00 AM

@Szelethus take your time, I find it strange to be the first with the problem (couldn't find reference on google or llvm bug tracker), I wouldn't be surprised if I fucked up somewhere and that this patch isn't really necessary.

Jiboo changed the repository for this revision from rC Clang to rG LLVM Github Monorepo.Sep 21 2019, 11:04 AM

Actually, uh oh. @Jiboo can you please file this as a bug?
This really should go into next patch release. CC @tstellar

@Szelethus this breaks LLVMExports.cmake as compared with LLVM-8, so it's actually a critical bug.

Jiboo edited the summary of this revision. (Show Details)Sep 24 2019, 6:20 AM
Szelethus added a comment.EditedSep 24 2019, 7:24 AM

I have no objections to this patch. I know that a variety patches played with these plugins and it moved all over the place, so I see that these are kind of touchy :/

I didn't know about this patch and published D68172: adding BUILDTREE_ONLY still builds the examples, but doesn't install them. It's also used by other test plugins like llvm/lib/Transforms/Hello.
Thanks to @lebedev.ri for informing my about this patch.

Jiboo added a comment.Sep 28 2019, 5:04 AM

@aaronpuchert thanks, that looks like a better fix than mine for PR43430, although I'm not sure D68172 obsoletes this patch.
@Szelethus could you confirm that thus examples shouldn't be built when CLANG_BUILD_EXAMPLES is OFF, and that this patch is still valid?

@Szelethus could you confirm that thus examples shouldn't be built when CLANG_BUILD_EXAMPLES is OFF, and that this patch is still valid?

The problem is that CLANG_BUILD_EXAMPLES is off by default. Maybe we want to build these plugins by default though to make sure there are no compiler errors. Since their compilation shouldn't take a lot of time, I think that is a reasonable position to assume.

Jiboo abandoned this revision.Sep 28 2019, 10:10 AM

Superseded by D68172.