[cfe-dev] clang-tidy / static analysis

[Jonas Toth via cfe-dev]

I think you might miss a `allOf` in your callExpr matcher.

Combining multiple conditions needs the logical operations (allOf, anyOf,
none of, ...).

Am 22.11.2017 3:53 vorm. schrieb "Maurizio Vitale via cfe-dev" <
cfe-dev at lists.llvm.org>:

> 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
>>
>
>
> _______________________________________________
> 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/20171122/fcfe7b4e/attachment.html>