clang-tidy / static analysis

hello clang clan,

I’m getting started with checkers and I have one in mind that I don’t see a good way to implement.

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).

Is there a way to use the fixit infrastructure together with the static analyzer?
If so, is it possible to make multiple changes part of the same fixit?
What would be the recommended way of proceeding in this case? any similar checks/analysis I can take a look at?

Thanks a lot,

Maurizio

Hi,

clang-tidy seems to be the correct point to start. It is made to rewrite source code and is easy to begin with.

Take a look into the clang-tools-extra repository and browse the code, there are a lot of examples. There is a dedicated tutorial in the docs to get started as well.

Thanks. I didn’t realize that some flow information is available in clang tidy. All the example I saw were local matches of the AST.
But I see now that checks like https://clang.llvm.org/extra/clang-tidy/checks/misc-use-after-move.html do require analysis not dissimilar from what I need, so I now have something to study.

[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@mail.gmail.com>,
    Maurizio Vitale via cfe-dev <cfe-dev@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 :slight_smile:

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.

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/clang-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

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

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

Thanks, but I don’t think that’s the reason.

The documentation says “For convenience, all node matchers take an arbitrary number of arguments and implicitly act as allOf matchers.” and I already had multiple matchers working fine.
But it seems like stringLiteral doesn’t match there, even though it does in clang-query and expr() does work in its stead. Dumping the AST shows an implicitCastExpr there, but even adding it around stringLiteral doesn’t work.

I’m not sure how to debug, but I guess it is time to learn how to unleash gdb on clang-tidy.

[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 <CAAeLbQLgNGRUwaxE4rwRSE74dm6X-KmM6GtZB2q2V+iacep1=w@mail.gmail.com>,
    Maurizio Vitale via cfe-dev <cfe-dev@lists.llvm.org> writes:

[...] Dumping the AST shows an
implicitCastExpr there, but even adding it around stringLiteral doesn't
work.

I don't see anything wrong with your approach; it's the one I've used
successfully in the past. However, I haven't attempted any clang-tidy
hacking in quite some time so something in the infrastructure might
have changed in the meantime.

I've generally gone through cycles of using clang-query, dumping the
parsed AST and trying things in clang-tidy.

If that works do it :slight_smile:

Maybe a ignoreImplicitCasts is worth a try.
I usually brute force matchers until they work. If someone knows better ways please teach me :slight_smile: