[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

danielmarjamaki added a comment. In D92634#2484404 , @steakhal wrote: > Here is a link for our results on a few more projects. It might be useful for > you. > https://codechecker-demo.eastus.cloudapp.azure.com/Default/runs?run=D92634&items-per-page=50&sor

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

danielmarjamaki added a comment. > Typically in such cases bug visitors should be added/improved until it is > clear from the user-facing report why does the analyzer think so. They'd > highlight the important events, prevent path pruning, and potentially > suppress reports if the reason is dis

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

steakhal added a comment. Here is a link for our results on a few more projects. It might be useful for you. https://codechecker-demo.eastus.cloudapp.azure.com/Default/runs?run=D92634&items-per-page=50&sort-by=name&sort-desc=false Note: use the diff to filter only for the new reports. Repositor

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

NoQ added a comment. > My code seems to think that a variable print_count has the value INT_MAX for > some reason Typically in such cases bug visitors should be added/improved until it is clear from the user-facing report why does the analyzer think so. They'd highlight the important events, p

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

danielmarjamaki added a comment. I have run clang static analysis on random open source projects. The very first finding that I look at seems (to me) to be a false positive. :-( My code seems to think that a variable `print_count` has the value INT_MAX for some reason and to me that seems impos

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

danielmarjamaki added a comment. > I also agree with @NoQ's D92634#2478703 > comment. well.. maybe it's better that I stop programming then and take this code I had and test it out. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

steakhal added a comment. In D92634#2478503 , @OikawaKirie wrote: > Besides, as far as I am thinking, the compiler optimizes the expression as it > is literally inferable, i.e. the result should be determinable literally > during compilation. Otherwise,

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

danielmarjamaki added a comment. > BTW, I cannot optimize function f to returning zero directly with GCC-10.2.1 > and Clang-10.0.1 under -O3. Should I add any other flags? Or it is version > specific? Yeah I can't reproduce that with latest gcc/clang neither. Try gcc 4.x and gcc 5.x. > But fo

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

NoQ added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:219 + ProgramStateManager &Mgr = State->getStateManager(); + SValBuilder &Bldr = Mgr.getSValBuilder(); + SVal Eval = `Bldr` is always equal to `*this` here, there's no n

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

NoQ added a comment. This patch adds a new core checker. I wouldn't be comfortable enabling it by default without a much more careful evaluation (than a single lit test). In particular, i'm worried about false positives due to constraint solving issues and our incorrect cast representation - it

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

OikawaKirie added a comment. In D92634#2477342 , @danielmarjamaki wrote: >> However, the mainstream compilers like GCC and Clang implement this as the >> overflowed value, and some programmers also use this feature to do some >> tricky things. > > hmm..

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

danielmarjamaki added a comment. > However, the mainstream compilers like GCC and Clang implement this as the > overflowed value, and some programmers also use this feature to do some > tricky things. hmm.. you mean if some -fwrapv flag is used right. yes I should disable this checking then.

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

OikawaKirie added a comment. In D92634#2476161 , @danielmarjamaki wrote: >> Besides, the return value should be the exact value computed from the two >> integers, even unknown, rather than undefined. As the developers may >> overflow an integer on purpo

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

danielmarjamaki added a comment. > Besides, the return value should be the exact value computed from the two > integers, even unknown, rather than undefined. As the developers may overflow > an integer on purpose. I am not sure what you mean. If there is undefined behavior then the value shoul

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

danielmarjamaki reclaimed this revision. danielmarjamaki added a comment. ok.. thanks for the reviews. I will see if I can make some new check. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92634/new/ https://reviews.llvm.org/D92634 __

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

martong added a comment. In D92634#2453822 , @OikawaKirie wrote: > I think it could be better to implement this check with a checker on > `PreStmt` and so on. And IMO, checkers have enough > functionalities to report these problems. > > Besides, the retu

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

martong added a comment. In D92634#2451646 , @danielmarjamaki wrote: > No reviews => I will not contribute. Hi, thanks for the patch! I am terribly sorry that we could not pick this up earlier. You can always ping the community with a simple 'Ping' mess

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

OikawaKirie added a comment. I think it could be better to implement this check with a checker on `PreStmt` and so on. And IMO, checkers have enough functionalities to report these problems. Besides, the return value should be the exact value computed from the two integers, even unknown, rathe

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

steakhal added a comment. This would be a serious change. Especially if we internally already depend on modulo arithmetic. For example, the `ArrayBoundV2` might exploit this fact - when it rearranges the inequality. I might be wrong on this though. Besides that checker, I can't mention any other

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

Charusso added a reviewer: vsavchenko. Charusso added a comment. Hey! We are somewhat slow in reviews, please understand that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92634/new/ https://reviews.llvm.org/D92634 ___

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

danielmarjamaki abandoned this revision. danielmarjamaki added a comment. No reviews => I will not contribute. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D92634/new/ https://reviews.llvm.org/D92634 ___

[PATCH] D92634: [Analyzer] Diagnose signed integer overflow

danielmarjamaki created this revision. danielmarjamaki added reviewers: NoQ, xazax.hun. Herald added subscribers: steakhal, ASDenysPetrov, martong, Charusso, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware. danielmarjamaki requested review of this r