steakhal accepted this revision.
steakhal added a comment.
In D156312#4588406 <https://reviews.llvm.org/D156312#4588406>, @donat.nagy
wrote:
> After investigating this issue, I added the testcases
> `signed_aritmetic_{good,bad}` which document the current sub-optimal state.
> The root cause of this problem is a high-level property of the engine (that
> it assumes that signed overflows are always possible and acceptable) and I
> don't see a local workaround that would silence or fix these incorrect error
> messages.
>
> @steakhal @NoQ What do you know about these signed overflow issues (I presume
> that analogous issues also appear in other checkers)? How should we handle
> this limitation of this checker?
I don't think most checkers depend on value ranges explicitly, except for a
handful of checkers maybe. So, I don't think it's such a huge deal, but I agree
that it's a problem.
I would bet that the StdLibraryFunctionsChecker would suffer from the same
issue, and the OOB checker(s) along with it.
I don't know of heuristics mitigating the damage.
I don't think we should do anything about it unless it's frequent enough.
Try to come up with a heuristic to be able to measure how often this happens,
if you really care.
Once you have a semi-working heuristic for determining if that bugpath suffers
from this, you can as well use it for marking the bugreport invalid if that's
the case to lean towards suppressing those FPs.
I don't think it's completely necessary to fix those FPs to land this. I think
of that as an improvement, on top of this one.
I hope I clarified my standpoint.
================
Comment at: clang/test/Analysis/bitwise-shift-sanity-checks.c:70-74
+ if (right < 0)
+ return 0;
+ return left << (right + 32);
+ // expected - warning@-1 {{Left shift overflows the capacity of 'int'}}
+ // expected-warning@-2 {{Right operand is negative in left shift}}
----------------
Let's be explicit about the actual assumed value range, and use
`clang_analyzer_value()`.
I also recommend using an explicit FIXME for calling out what should be there,
instead of abusing a non-matching `expected` pattern. I know I used that in the
past, and probably seen it from me. I feel ashamed that I did that. I know I
did that to have cleaner diffs, once fixed, but I honestly think it does more
harm than good.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D156312/new/
https://reviews.llvm.org/D156312
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
