[cfe-dev] Clang Static Analyzer (different.ConversionToBool check)

[Artem Dergachev via cfe-dev]

(forgot to add cfe-dev on reply, re-replying, sry)

Hello,

I'd really like to know more about the approach you've taken with this 
checker. What kinds of conversions does it flag? Eg.:

bool foo() {
   return ""; // warn.
   // or...
   char *str1 = "";
   return str1; // warn?
   // or...
   char *null_str = 0;
   return null_str; // nope.
}

It's not clear to me why would it warn on converting something to an 
int. In C, there doesn't seem to be an easy way to discriminate between 
int and boolean types. You might eventually want to hardcode a list of 
common library functions that treat their int arguments as boolean 
flags. And even if we have platform/library-specific typedefs such as 
ObjC BOOL, they are usually quite distinct from plain ints (eg. by 
typedef sugar).

Then, ideally new checkers shouldn't be enabled on existing tests. 
Nowadays we try to make each test file correspond to a single checker. 
So you're running into a problem that arises because we didn't take this 
approach before, so we have some tests that have the whole alpha.core 
enabled (which is gross).

If you're running into existing test file with a lot of eg. 
clang_analyzer_eval(BOOL) calls, and many of those suddenly get flagged, 
i think these are true positives. The checker works as intended, and we 
could just add expected-warnings on these lines. If there are too many 
such lines, i'd probably suggest to disable the checker for this file, 
i.e. adding -analyzer-disable-checker=alpha.core.YourChecker to the 
run-line at the top of the file. Separate files for the checker should 
be enough.

But i don't think that there is a need for special code in the checker 
that detects testing environment and changes the behavior. After all, 
that defeats the purpose of testing.

On 11/29/17 12:20 PM, Alexey Knyshev via cfe-dev wrote:
> Hello everyone!
>
> I have implemented a prototype of "different.ConversionToBool(C, C++)".
> According to list of potential checkers 
> (https://clang-analyzer.llvm.org/potential_checkers.html 
> <https://clang-analyzer.llvm.org/potential_checkers.html>) I've 
> implemented it as the part of existing alpha.core.BoolAssignment 
> checker. There is one problem I've faced while testing changes via 
> llvm-lit. There are many FnChecks calls like /clang_analyzer_eval/ in 
> Analysis/casts.[c/m] which are called with integer arguments instead 
> of bool according declaration. Obviously it leads to false positives 
> in implemented checker (implicit cast to boolean). Actually, the 
> question is: What is the best way to avoid such false-positives? I 
> came up with the following possible solutions:
>
> 1. Is there way to distinguish FnCheck CallExpr from common CallExpr 
> (any flag or whatever). If so, it's quite simple to ignore them in the 
> checker.
> 2. Is there way to determine if checker is running in testing env? And 
> afterwards filter any calls to FnChecks like/clang_analyzer_eval /by 
> name. As it was implemented in /CStringChecker:evalCall/.
> 3. Add special FnCheck like /clang_analyser_eval_implicit_cast(bool)/ 
> which is supposed to replace /clang_analyzer_eval /in cast tests.
>
> Any suggestions?
>
> Thanks, Alexey K
>
> -- 
> linkedin.com/profile 
> <https://www.linkedin.com/profile/view?id=AAMAABn6oKQBDhBteiQnWsYm-S9yxT7wQkfWhSw>
>
> github.com/alexeyknyshev <http://github.com/alexeyknyshev>
> bitbucket.org/alexeyknyshev <https://bitbucket.org/alexeyknyshev/>
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev