[PATCH] D41816: [analyzer] Model and check unrepresentable left shifts

[Reka Kovacs via Phabricator via cfe-commits]

rnkovacs added inline comments.


================
Comment at: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp:150
+            SB.getKnownValue(state, C.getSVal(B->getRHS()));
+        if ((unsigned) RHS->getZExtValue() > LHS->countLeadingZeros()) {
+          OS << "The result of the left shift is undefined due to shifting \'"
----------------
dcoughlin wrote:
> This inner 'if' looks fishy to me because if the 'else' branch is ever taken then OS will be empty.
> 
> If the else branch can't be taken then you should turn it into an assert(). If it can be taken, then you should make sure that the fall through goes through the "default" else case at the bottom. One way to do this is to pull out the "is representable logic" into a helper function and call that in the containing 'else if'.
> 
> Something like:
> 
> ```
> if (B->getOpcode() == BinaryOperatorKind::BO_Shl && isLeftShiftResultRepresentable(LHS, RHS)) {
>   OS << "The result of the left shift ..."
> }
> ```
I overlooked this issue, thanks for pointing out. I pulled the logic out into a helper function.


https://reviews.llvm.org/D41816