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

Thu Nov 30 12:04:51 PST 2017

Good time of day!

First of all I would like to show cases I've included in current test:

const char *str = "abc";
> bool b = str; // no-warning
> ...
>
return str; // warning
> ...
> return b; // no-warning
>

> bool implStrCast() {
> return "" // warn
> }
>

const char *text() {
> return "some text";
> }
>
> bool implBoolCast() {
> return text(); // warning
> }
>
> bool explBoolCast() {
> bool r = text(); // no-warning
> return r; // no-warning
> }
>

bool implFlootCast() {
> return 1.; // warning
> }
>

bool explBoolCast2() {
> return text() == 0; // no-warning
> }
>

bool boolProxy(int _, bool x, bool y);
> ...
> boolProxy(1, getInteger(), (bool)1); // warn (only for second arg)
>

> I'd really like to know more about the approach you've taken with this
> checker.

Current implementation checks 2 types of statements: CallExpr & ReturnStmt.
First one actually aimed to check unexpected implicit casts of arguments
passed to CallExpr (last example code). ReturnStmt for other presented
cases. I could attach diff if it's appropriate here.

Nowadays we try to make each test file correspond to a single checker.

I'll consider separation of new checker into new one (including dedicated
tests of course).

But i don't think that there is a need for special code in the checker that
> detects testing environment and changes the behavior.
>
I meant detecting & ignoring just special Calls (such as mentioned
*clang_analyzer_eval*) only if we running in test env.

Thanks, Alexey

2017-11-30 19:34 GMT+03:00 Artem Dergachev <noqnoqneo at gmail.com>:

> (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.o
>> rg/potential_checkers.html <https://clang-analyzer.llvm.o
>> rg/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/prof
>> ile/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
>>
>
>

-- 
linkedin.com/profile
<https://www.linkedin.com/profile/view?id=AAMAABn6oKQBDhBteiQnWsYm-S9yxT7wQkfWhSw>

github.com/alexeyknyshev
bitbucket.org/alexeyknyshev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20171130/60ad4f53/attachment.html>