[cfe-dev] clang-tidy / static analysis

[Maurizio Vitale via cfe-dev]

Hello again,

I'm working on putenv->setenv rewrite as a way to get my feet wet with
clang-tidy

I managed to get detection working fine. Now I'm starting working on
recommending fixes and I'd like to reduce the matches to what I'm able to
rewrite.
The first should capture a simple putenv("VAR=VALUE"). The following works
in clang-query:

clang-query> let p callee(functionDecl(hasName("::putenv")))
clang-query> match callExpr(p, argumentCountIs(1), hasArgument(0,
stringLiteral().bind("arg")))

Match #1:

/tmp/mav_clang/build/../third_party/clang-tools-extra/test/c
lang-tidy/misc-putenv.cpp:7:10: note: "arg" binds here
  putenv("VAR=VALUE");
         ^~~~~~~~~~~
/tmp/mav_clang/build/../third_party/clang-tools-extra/test/clang-tidy/misc-putenv.cpp:7:3:
note: "root" binds here
  putenv("VAR=VALUE");
  ^~~~~~~~~~~~~~~~~~~
1 match.


But when I convert it to code in a clang-tidy check, as in:

  auto putenvCallee = callee(functionDecl(hasName("::putenv")));

  auto putenvConstantCallMatcher =
      callExpr(putenvCallee, argumentCountIs(1),
                   hasArgument(0, stringLiteral().bind("putenv_arg")))
          .bind("putenv_call");

  Finder->addMatcher(putenvConstantCallMatcher, this);

the match fails.

I'm a bit puzzled by clang-query behaving differently from the compiled
checker.

If I replace the stringLiteral() matcher with expr(), then it does match,
but it is too loose of a match.

I'm probably missing something obvious and I would appreciate any help.
[feel free to throw away my matchers and show me the proper way to match
::putenv("a string") and not ::putenv(a_char_ptr_var)]

Thanks a lot,

   Maurizio


On Mon, Nov 13, 2017 at 11:51 AM, Richard via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

> [Please reply *only* to the list and do not include my email directly
> in the To: or Cc: of your reply; otherwise I will not see your reply.
> Thanks.]
>
> In article <CAAeLbQKtKAHE5RAweH2a0n+Dq8EwOKmjy8n2n1bMVYrDCdhQOQ at mail.gm
> ail.com>,
>     Maurizio Vitale via cfe-dev <cfe-dev at lists.llvm.org> writes:
>
> > The idea is to find all uses of putenv and replace them w/ setenv. This
> > requires analyzing the argument to discover constant parts in it (setenv
> > require a separate variable name). It also require to check that the
> > argument is not modified (or such modifications need also to be replaces
> w/
> > setenvs).
>
> In addition to what's already been mentioned, I would recommend
> implementing your check in stages:
>
> 1. Constant string arguments
>
>     putenv("FOO=bar");
>
>     =>
>
>     setenv("FOO", "bar", 1);
>
> 2. Constant environment variable names
>
>     char buff[256];
>     sprintf(buff, "FOO=%d", value);
>     putenv(buff);
>
>     =>
>
>     char buff[256];
>     sprintf(buff, "%d", value);
>     setenv("FOO", buff, 1);
>
> 3. Varying environment variable names
>
>     .... you get the idea :)
>
> Basically start with the simplest case and get that working and then
> enhance for subsequent cases.  You can even submit incremental work
> for review and incorporation into clang-tidy this way.  It is better
> to have a working check that handles simple cases and does no harm on
> complex cases than to wait for a check that covers 100% of everything.
> --
> "The Direct3D Graphics Pipeline" free book <http://tinyurl.com/d3d-pipeli
> ne>
>             The Terminals Wiki <http://terminals-wiki.org>
>      The Computer Graphics Museum <http://ComputerGraphicsMuseum.org>
>   Legalize Adulthood! (my blog) <http://LegalizeAdulthood.wordpress.com>
> _______________________________________________
> cfe-dev mailing list
> 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/20171121/0d2a8763/attachment.html>