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

Daniel Marjamäki via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Jan 5 03:08:51 PST 2021


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 for the example you mentioned, I prefer implementing this check with a coding style checker for Clang-Tidy, although the integer overflow checker in the CSA is also necessary.

Yes if that was all we wanted to detect. I'd rather also flag more complex examples.. whenever the optimizer can see that there is overflow and has an opportunity to destroy the code. Here is a more complex example: https://stackoverflow.com/questions/57650026/optimized-clang-handling-overflow

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

I agree 1000%. The patch I provided so far was only a proof of concept. Thanks for all comments I feel I will remake this.

> Not necessarily a new checker but bug reports from this checker are already lacking a lot of information. In this new check both operands should be tracked with visitors. Also in other checks there's a bailout path that produces generic diagnostics ("the result... is undefined" without explaining why); we should probably silence those instead because it's clear that we can't explain what's going on.

Yes I really agree about that. The current warning message is unacceptable and it must be clarified.

Sorry I don't make much progress right now.. I hope to have some new code to show soon..


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92634/new/

https://reviews.llvm.org/D92634



More information about the cfe-commits mailing list