rnkovacs created this revision. rnkovacs added reviewers: NoQ, dcoughlin, xazax.hun. Herald added subscribers: a.sidorin, szepet, baloghadamsoftware, whisperity.
Left shifting a signed positive value is undefined if the result is not representable in the unsigned version of the return type. The analyzer now returns an UndefVal in this case and UndefResultChecker is updated to warn about it. https://reviews.llvm.org/D41816 Files: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp lib/StaticAnalyzer/Core/BasicValueFactory.cpp test/Analysis/bitwise-ops.c Index: test/Analysis/bitwise-ops.c =================================================================== --- test/Analysis/bitwise-ops.c +++ test/Analysis/bitwise-ops.c @@ -51,3 +51,9 @@ } return 0; } + +int testUnrepresentableLeftShift(int a) { + if (a == 8) + return a << 30; // expected-warning{{The result of the left shift is undefined because it is not representable in the return type}} + return 0; +} Index: lib/StaticAnalyzer/Core/BasicValueFactory.cpp =================================================================== --- lib/StaticAnalyzer/Core/BasicValueFactory.cpp +++ lib/StaticAnalyzer/Core/BasicValueFactory.cpp @@ -224,7 +224,6 @@ // FIXME: This logic should probably go higher up, where we can // test these conditions symbolically. - // FIXME: Expand these checks to include all undefined behavior. if (V1.isSigned() && V1.isNegative()) return nullptr; @@ -236,16 +235,17 @@ if (Amt >= V1.getBitWidth()) return nullptr; + if (V1.isSigned() && (unsigned) Amt > V1.countLeadingZeros()) + return nullptr; + return &getValue( V1.operator<<( (unsigned) Amt )); } case BO_Shr: { // FIXME: This logic should probably go higher up, where we can // test these conditions symbolically. - // FIXME: Expand these checks to include all undefined behavior. - if (V2.isSigned() && V2.isNegative()) return nullptr; Index: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp +++ lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp @@ -141,6 +141,15 @@ C.isNegative(B->getLHS())) { OS << "The result of the left shift is undefined because the left " "operand is negative"; + } else if (B->getOpcode() == BinaryOperatorKind::BO_Shl) { + SValBuilder &SB = C.getSValBuilder(); + const llvm::APSInt *LHS = + SB.getKnownValue(state, C.getSVal(B->getLHS())); + const llvm::APSInt *RHS = + SB.getKnownValue(state, C.getSVal(B->getRHS())); + if ((unsigned) RHS->getZExtValue() > LHS->countLeadingZeros()) + OS << "The result of the left shift is undefined because it is not " + "representable in the return type"; } else { OS << "The result of the '" << BinaryOperator::getOpcodeStr(B->getOpcode())
Index: test/Analysis/bitwise-ops.c =================================================================== --- test/Analysis/bitwise-ops.c +++ test/Analysis/bitwise-ops.c @@ -51,3 +51,9 @@ } return 0; } + +int testUnrepresentableLeftShift(int a) { + if (a == 8) + return a << 30; // expected-warning{{The result of the left shift is undefined because it is not representable in the return type}} + return 0; +} Index: lib/StaticAnalyzer/Core/BasicValueFactory.cpp =================================================================== --- lib/StaticAnalyzer/Core/BasicValueFactory.cpp +++ lib/StaticAnalyzer/Core/BasicValueFactory.cpp @@ -224,7 +224,6 @@ // FIXME: This logic should probably go higher up, where we can // test these conditions symbolically. - // FIXME: Expand these checks to include all undefined behavior. if (V1.isSigned() && V1.isNegative()) return nullptr; @@ -236,16 +235,17 @@ if (Amt >= V1.getBitWidth()) return nullptr; + if (V1.isSigned() && (unsigned) Amt > V1.countLeadingZeros()) + return nullptr; + return &getValue( V1.operator<<( (unsigned) Amt )); } case BO_Shr: { // FIXME: This logic should probably go higher up, where we can // test these conditions symbolically. - // FIXME: Expand these checks to include all undefined behavior. - if (V2.isSigned() && V2.isNegative()) return nullptr; Index: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp =================================================================== --- lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp +++ lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp @@ -141,6 +141,15 @@ C.isNegative(B->getLHS())) { OS << "The result of the left shift is undefined because the left " "operand is negative"; + } else if (B->getOpcode() == BinaryOperatorKind::BO_Shl) { + SValBuilder &SB = C.getSValBuilder(); + const llvm::APSInt *LHS = + SB.getKnownValue(state, C.getSVal(B->getLHS())); + const llvm::APSInt *RHS = + SB.getKnownValue(state, C.getSVal(B->getRHS())); + if ((unsigned) RHS->getZExtValue() > LHS->countLeadingZeros()) + OS << "The result of the left shift is undefined because it is not " + "representable in the return type"; } else { OS << "The result of the '" << BinaryOperator::getOpcodeStr(B->getOpcode())
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits