[cfe-dev] Question about custom Clang static analyzer checker registration in clang-tidy without using plugin

Alexander Kornienko via cfe-dev cfe-dev at lists.llvm.org
Tue Apr 17 03:53:00 PDT 2018


We've talked with George offline and we seem to have found a maintainable
solution. I've sent https://reviews.llvm.org/D45718 for review.

On Sat, Mar 17, 2018 at 3:03 AM Artem Dergachev <noqnoqneo at gmail.com> wrote:

> Hmm, okay, i see, well, i mean, in approach #1 i was thinking about
> moving the global registry to clang-tidy and keeping the clang binary
> free from it, because clang-tidy is a smaller tool and less things could
> go wrong.
>
> I personally have no immediate objections against having a global
> registry of statically-linked-in (is it even a word?) checkers in the
> analyzer, with a clear explanation of why this is necessary and what to
> do with it if the global becomes a problem, examples, ideally tests of
> course (if it would be easy enough with our build system; we don't seem
> to have tests for our plugins, not sure why). After all, i doubt we'd
> ever (or any time soon) have to restart the analysis with a different
> list of statically-linked-in checkers without restarting the clang
> process. So i'll accept patches in this direction as well.
>
> I guess llvm::ManagedStatic<> would be handy.
>
> On 16/03/2018 2:31 PM, likan_999.student at sina.com wrote:
> > Friendly ping on this.
> >
> > ----- Original Message -----
> > From: <likan_999.student at sina.com>
> > To: "Artem Dergachev" <noqnoqneo at gmail.com>, "Alexander Kornienko" <
> alexfh at google.com>,
> > Cc: "George Karpenkov" <ekarpenkov at apple.com>, "clang developer list" <
> cfe-dev at lists.llvm.org>,
> > Subject: Re: Re: [cfe-dev] Question about custom Clang static analyzer
> checker registration in clang-tidy without using plugin
> > Date: 2018-03-15 05:24
> >
> > For the George's question on how we did it for clang-tidy. We make use
> of ClangTidyModuleRegistry:
> https://github.com/llvm-mirror/clang-tools-extra/blob/master/clang-tidy/ClangTidyModuleRegistry.h
> > Basically we have a separate static library that registers our internal
> clang-tidy checkers with ClangTidyModuleRegistry (which essentially creates
> a global variable whose constructor does the registration work). In this
> way we can simply modify our build rules to link clang-tidy with this
> library, without modifying the clang source tree.
> > For Artem's remarks, since there are a bunch of places in Clang that
> make use the register mechanism, can we say #2 might not be super against
> Clang's coding style/conventions, if we use the Clang builtin registration
> mechanism? I feel this is the best solution from our side. We can use the
> same mechanism that is used for clang-tidy to register checkers for CSA,
> without modifying the Clang source tree.
> > For approach #1, there are two things that needs to be solved before we
> can go with this approach. First is how to pass in additional checkers (or
> checker factories) without breaking existing API. By saying existing API I
> mean ento::CreateAnalysisConsumer, ClangTidyASTConsumerFactory,
> runClangTidy, etc.  Second, even if we come up with some API that allows
> passing in checkers, we can't do it without modifying Clang source tree. We
> still need to modify clang-tidy source files to actually pass in our custom
> checkers.
> > For approach #3, I agree this is not the best approach and it doesn't
> work on all platforms.
> > What do you think?
> > ----- Original Message -----
> > From: Artem Dergachev <noqnoqneo at gmail.com>
> > To: Alexander Kornienko <alexfh at google.com>, likan_999.student at sina.com
> > Cc: George Karpenkov <ekarpenkov at apple.com>, clang developer list <
> cfe-dev at lists.llvm.org>
> > Subject: Re: [cfe-dev] Question about custom Clang static analyzer
> checker registration in clang-tidy without using plugin
> > Date: 2018-03-15 03:32
> > All right, so approach #1 by Kan with passing arguments through
> > AnalysisConsumer seems to have no problems at all, as long as there's an
> > example somewhere in the /examples/ directory that anybody could use and
> > see if it works. As far as i understand, it doesn't cause any globals to
> > be introduced whatsoever, and keeps all the logic on your side or in
> > clang-tidy. Not sure if this boils down to moving the globals to
> > clang-tidy. I also understand that it'd be quite some coding, but
> > hopefully it'd be good code :)
> > Approach #3 with weak functions may also work, and you can totally come
> > up with an infrastructure on your private library side to make it
> > possible to add checkers from multiple places within the library. At the
> > same time it would prevent multiple private libraries from being used
> > simultaneously (weak function being the global thingy) and i'm not sure
> > if it works on all OSes.
> > Approach #2 with a global sort-of-vector of checkers to check out when
> > filling the checker registry is indeed the scariest - it might "just
> > work" now, but nobody knows what happens when the number of singletons
> > starts growing, so i kinda understand why they don't like to see this
> > sort of stuff in clang, even if it's purely religious i guess we'd
> > rather not.
> > On 14/03/2018 1:19 AM, Alexander Kornienko wrote:
> >> Let me try to resolve the confusion I might have caused. Kan has come
> >> up with a static analyzer checker that addresses incorrect use of an
> >> internal API (so it doesn't make sense upstream). We can't use
> >> dynamically loaded plugins for a number of reasons, so we'd like to
> >> have a way to use statically-linked static analyzer checkers with
> >> minimal modification to Clang sources (ideally, one-two lines or just
> >> linking in a library that would register the checker in a constructor
> >> of a global object). I wasn't aware of something like that being
> >> possible in the static analyzer, so I've suggested that Kan asks this
> >> here.
> >>
> >> On Tue, Mar 13, 2018 at 9:46 PM via cfe-dev <cfe-dev at lists.llvm.org
> >> <mailto:cfe-dev at lists.llvm.org>> wrote:
> >>
> >>      Thanks Artem for explaining.
> >>
> >>      What happened is, I got it work using the plugin system. On the
> >>      other hand, the clang-tidy team in our company (not Clang's
> >>      clang-tidy team) wants me to integrate into clang-tidy, and they
> >>      are not happy about the plugin approach (maybe based on reasons
> >>      George listed). They want a mechanism registering checkers that
> >>      are out of Clang source tree, but still statically linked, instead
> >>      of being dynamically loaded as plugin. I can try asking them to
> >>      reply to this thread.
> >>
> >>      ----- Original Message -----
> >>      From: Artem Dergachev <noqnoqneo at gmail.com
> >>      <mailto:noqnoqneo at gmail.com>>
> >>      To: likan_999.student at sina.com
> >>      <mailto:likan_999.student at sina.com>, George Karpenkov
> >>      <ekarpenkov at apple.com <mailto:ekarpenkov at apple.com>>
> >>      Cc: cfe-dev <cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org
> >>
> >>      Subject: Re: [cfe-dev] Question about custom Clang static analyzer
> >>      checker registration in clang-tidy without using plugin
> >>      Date: 2018-03-14 03:07
> >>
> >>
> >>      I still don't understand your approach. There's the current plugin
> >>      system that auto-registers checkers (the trick with "-cc1 -load"
> >>      demonstrated by examples/analyzer-plugin/MainCallChecker.cpp) and
> >>      there's the normal way of writing a checker in clang source tree
> >>      (like
> >>      all default checkers are written) that semi-automatically (through
> >>      the
> >>      registerChecker<>() call) puts the checker into the checker
> >>      registry and
> >>      makes it automatically accessible to clang-tidy when it links to
> the
> >>      updated checkers library.
> >>      Because none of these approaches seem to have any problems with
> >>      checker
> >>      registration, could you describe what exactly are you doing that
> >>      causes
> >>      problems with checker registration?
> >>      On 13/03/2018 11:11 AM, via cfe-dev wrote:
> >>      > Hi George,
> >>      >
> >>      > Thanks for the fast response. I totally agree with your point of
> >>      the disadvantages of using plugins. My question was exactly how to
> >>      avoid using plugins. Maybe I had some misunderstanding, does the
> >>      "plugin" you talked about not only include dynamic libraries, but
> >>      also include registering checkers in programs that uses clang as a
> >>      library?
> >>      >
> >>      > For the 2 solutions you propsed, they are good suggestions, but
> >>      unfortunately they won't work in our specific situation. The
> >>      checker I am adding is very company specific, and involves
> >>      specific class in company's code base; I believe it is impossible
> >>      to make it public, because it will reveal company's internal
> >>      codebase; also it is useless to public because the situation it
> >>      tries to analyze is very ad-hoc, company specific, thus won't
> >>      apply to other people's code at all.
> >>      >
> >>      > As for rebasing against upstream, that's not feasible as well
> >>      because this involves maintenance cost. I am at a large company
> >>      where I don't have control or any influence on the process it sync
> >>      third-party libraries with upstream, neither does the clang-tidy
> >>      team of our company. That's why they suggest me doing it upstream.
> >>      >
> >>      > The 3 approaches in my original question was in fact about
> >>      modifying upstream to expose the checker registration process, so
> >>      that not only us, but other people could also benefit from it. If
> >>      there is such an API, then in a program that uses clang as a
> >>      library, they can call the exposed API to register their own
> >>      checker, before using AnalysisConsumer to do a static analysis.
> >>      >
> >>      > Neither of the 3 approaches I proposed uses plugin architecture.
> >>      It is just caller program who instantiates checker (or some form
> >>      of checker factory), then hands it over to Clang, therefore there
> >>      is no need to worry about how Clang can find the libraries.
> >>      >
> >>      > I am more then happy if there are other ideas and I can
> >>      contribute my manpower to coming up with some patches.
> >>      >
> >>      > Best,
> >>      > Kan Li
> >>      >
> >>      > ----- Original Message -----
> >>      > From: George Karpenkov <ekarpenkov at apple.com
> >>      <mailto:ekarpenkov at apple.com>>
> >>      > To: Li Kan <likan_999.student at sina.com
> >>      <mailto:likan_999.student at sina.com>>
> >>      > Cc: cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
> >>      > Subject: Re: [cfe-dev] Question about custom Clang static
> >>      analyzer checker registration in clang-tidy without using plugin
> >>      > Date: 2018-03-14 01:05
> >>      >
> >>      >
> >>      > Hi Li,
> >>      > I haven’t used plugins myself, but I can see two solutions
> >>      side-stepping the problem.
> >>      > 1) Could you upstream your checker and put it into “alpha”, and
> >>      then turn it on via flag?
> >>      > 2) If (1) is impossible [though upstreaming into “alpha” should
> >>      not be too hard], you could fork clang and add your changes there,
> >>      > and then (hopefully) regularly rebase vs. trunk.
> >>      > The problem with the plugin infrastructure is that Clang does
> >>      not guarantee a stable API.
> >>      > Even though things would probably work, there’s no guarantee
> >>      they won’t break with a new release,
> >>      > and if you use plugins you would get all the associated
> >>      disadvantages (no straightforward injection, have to care about
> >>      library being found, etc)
> >>      > without the usual advantages (plugin might unexpectedly stop
> >>      working).
> >>      > Though if you absolutely must use plugins, I think it would be
> >>      possible to come up with an API for doing so.
> >>      > George
> >>      >> On Mar 12, 2018, at 6:20 PM, Li Kan via cfe-dev
> >>      <cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>> wrote:
> >>      >>
> >>      >> Hi,
> >>      >>
> >>      >> I have a question about how to register custom checkers without
> >>      using a plugin. The background is, I have been working on a
> >>      checker and I was able to come up with a solution to integrate
> >>      into our code review system by using AnalysisConsumer and load
> >>      custom checkers using plugins. However, the team of our company
> >>      who works on clang-tidy integration requests me to do that in
> >>      clang-tidy, instead of a separate set of things.
> >>      >>
> >>      >> The way clang-tidy is used in the code review system is to
> >>      create ClangTidyASTConsumerFactory and then create a
> >>      ClangTidyASTConsumer, which is later used to parse the codebase.
> >>      Therefore the goal here is to find a way to let the users of clang
> >>      libraries, especially ClangTidyASTConsumer{,Factory}, to let
> >>      AnalysisConsumer pickup custom checkers statically linked into the
> >>      program;
> >>      >>
> >>      >> Currently the checker registration is very deep in the stack,
> >>      the only places it looks for checkers are a set of builtin
> >>      checkers and plugins. There are a few directions I can think of
> >>      >> 1) Add some arguments to AnalysisConsumer constructor and pass
> >>      it down to createCheckerManager, and add logic there to look for
> >>      checkers passed in; however, this is problematic because all the
> >>      users of AnalysisConsumer, such as ClangTidyASTConsumer, needs to
> >>      expose the arguments as well;
> >>      >> 2) Have some static variable that stores the list of checkers
> >>      factories, and expose a function that allows user to add checkers
> >>      factories to it. Later the ClangCheckerRegistry picks up the
> >>      checkers in the list. This is the ordinary way, but the existence
> >>      of a static variable looks ugly and not thread-safe.
> >>      >> 3) Like registerBuiltinCheckers, add a weak function
> >>      registerCustomCheckers, initially empty, and client code is
> >>      allowed to override it, filled in with their custom checkers. In
> >>      ClangCheckerRegistry it invokes the function, just after it
> >>      invokes the registerBuiltinCheckers. This is the least intrusive
> >>      way but the drawback is if there are multiple places in the system
> >>      that needs to add a checker, it is not very convenient.
> >>      >>
> >>      >> What's the best approach, or is there other better approach? I
> >>      am more than happy to help and send out some patch to add some
> >>      mechanism. I want to hear the opinions from Clang developers and
> >>      get blessed by the direction, before I actually start on it, to
> >>      avoid any arguments.
> >>      >>
> >>      >>
> >>      >> _______________________________________________
> >>      >> cfe-dev mailing list
> >>      >> cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
> >>      >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
> >>      > _______________________________________________
> >>      > cfe-dev mailing list
> >>      > cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
> >>      > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
> >>      _______________________________________________
> >>      cfe-dev mailing list
> >>      cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
> >>      http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
> > >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20180417/6431bd85/attachment-0001.html>


More information about the cfe-dev mailing list