This revision was automatically updated to reflect the committed changes.
Closed by commit rL315462: [Analyzer] Clarify error messages for undefined
result (authored by danielmarjamaki).
Changed prior to commit:
https://reviews.llvm.org/D30295?vs=116865&id=118620#toc
Repository:
rL LLVM
htt
zaks.anna accepted this revision.
zaks.anna added a comment.
Once the comments by @paquette are addressed, LGTM. Thanks!
Comment at: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp:138
+
+OS << " larger or equal to the width of type '"
+ << B->getLHS()->get
paquette added inline comments.
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:201
+ /// comparison)?
+ bool isGreaterOrEqual(const Expr *E, unsigned long long Val);
+
Maybe something like
```
/// Returns true if the value of \p E
danielmarjamaki added a comment.
ping
Repository:
rL LLVM
https://reviews.llvm.org/D30295
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
danielmarjamaki updated this revision to Diff 116865.
danielmarjamaki added a comment.
fixed review comments
Repository:
rL LLVM
https://reviews.llvm.org/D30295
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
lib
zaks.anna added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp:134
+else if (I->isUnsigned())
+ OS << I->getZExtValue() << ", which is";
+else
Please print single quotes around the value.
===
zaks.anna added a comment.
Sorry for the wait!
Repository:
rL LLVM
https://reviews.llvm.org/D30295
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
danielmarjamaki added a comment.
ping
Repository:
rL LLVM
https://reviews.llvm.org/D30295
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
danielmarjamaki added a comment.
ping
Repository:
rL LLVM
https://reviews.llvm.org/D30295
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
danielmarjamaki added a comment.
ping
Repository:
rL LLVM
https://reviews.llvm.org/D30295
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
danielmarjamaki added a comment.
ping
Repository:
rL LLVM
https://reviews.llvm.org/D30295
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
danielmarjamaki added a comment.
ping
Repository:
rL LLVM
https://reviews.llvm.org/D30295
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
xazax.hun accepted this revision.
xazax.hun added a comment.
This revision is now accepted and ready to land.
It looks good to me but let's wait for Anna, NoQ, or Devin for the final word.
Repository:
rL LLVM
https://reviews.llvm.org/D30295
___
c
danielmarjamaki updated this revision to Diff 109590.
danielmarjamaki added a comment.
Cleaned up the patch a little. Thanks Gabor for telling me about
SValBuilder::getKnownValue()
Repository:
rL LLVM
https://reviews.llvm.org/D30295
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/C
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp:126
+ << BinaryOperator::getOpcodeStr(B->getOpcode())
+ << "' expression is undefined due to shift count >= width of type";
+ } else {
danielm
danielmarjamaki marked 4 inline comments as done.
danielmarjamaki added inline comments.
Comment at: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp:126
+ << BinaryOperator::getOpcodeStr(B->getOpcode())
+ << "' expression is undefined due to shift count >=
danielmarjamaki updated this revision to Diff 103585.
danielmarjamaki added a comment.
Fix review comments
Repository:
rL LLVM
https://reviews.llvm.org/D30295
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
lib/S
zaks.anna added a comment.
These are great additions!
Going back to my comment about adding these to CheckerContext, I think we
should be adding helper functions as methods on CheckerContext as it is **the
primary place where checker writers look for helpers**. Two of the three
methods added t
NoQ added a comment.
I have just one comment and i think it'd be good to land.
Comment at: lib/StaticAnalyzer/Core/CheckerHelpers.cpp:104
+ ProgramStateManager &Mgr = State->getStateManager();
+ if (!LHSVal.getAs() && LHSVal.getAs()) {
+LHSVal = Mgr.getStoreManager().getB
danielmarjamaki added a comment.
ping
Repository:
rL LLVM
https://reviews.llvm.org/D30295
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
danielmarjamaki updated this revision to Diff 99022.
danielmarjamaki added a comment.
renamed exprComparesTo to svalComparesTo
Repository:
rL LLVM
https://reviews.llvm.org/D30295
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
lib/StaticAnalyzer/Checkers/Conversio
danielmarjamaki updated this revision to Diff 98970.
danielmarjamaki added a comment.
minor tweak
Repository:
rL LLVM
https://reviews.llvm.org/D30295
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
lib/StaticAnal
xazax.hun added inline comments.
Comment at: lib/StaticAnalyzer/Core/CheckerHelpers.cpp:116
+return false;
+ ConstraintManager &CM = State->getConstraintManager();
+ ProgramStateRef StTrue, StFalse;
danielmarjamaki wrote:
> xazax.hun wrote:
> > Any reason w
danielmarjamaki added inline comments.
Comment at: lib/StaticAnalyzer/Core/CheckerHelpers.cpp:116
+return false;
+ ConstraintManager &CM = State->getConstraintManager();
+ ProgramStateRef StTrue, StFalse;
xazax.hun wrote:
> Any reason why do you get the con
xazax.hun added a comment.
I only found two nits otherwise looks good to me.
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h:46
+bool exprComparesTo(SVal LHSVal, BinaryOperatorKind ComparisonOp, SVal RHSVal,
+ProgramStateRef St
danielmarjamaki marked an inline comment as done.
danielmarjamaki added a comment.
Ping.
Repository:
rL LLVM
https://reviews.llvm.org/D30295
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cf
danielmarjamaki marked 4 inline comments as done.
danielmarjamaki added inline comments.
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h:46
-} // end GR namespace
+bool isExprResultConformsComparisonRule(CheckerContext &C,
+
danielmarjamaki updated this revision to Diff 95740.
danielmarjamaki added a comment.
Fix review comments
- renamed
- reorder function arguments (CheckerContext last)
Repository:
rL LLVM
https://reviews.llvm.org/D30295
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.
zaks.anna added inline comments.
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h:46
-} // end GR namespace
+bool isExprResultConformsComparisonRule(CheckerContext &C,
+BinaryOperatorKind CompRule,
NoQ added a comment.
Hello, sorry for your having to wait on me. The implementation looks good, i'm
only having a couple of public API concerns.
Comment at:
include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h:46-49
+bool isExprResultConformsComparisonRule(Checker
danielmarjamaki added a comment.
Ping
Repository:
rL LLVM
https://reviews.llvm.org/D30295
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
danielmarjamaki updated this revision to Diff 92315.
danielmarjamaki marked an inline comment as done.
danielmarjamaki added a comment.
Fix review comment. Made isShiftOverflow() static.
Repository:
rL LLVM
https://reviews.llvm.org/D30295
Files:
include/clang/StaticAnalyzer/Core/PathSensit
a.sidorin added inline comments.
Comment at: lib/StaticAnalyzer/Core/CheckerHelpers.cpp:99
+bool clang::ento::isExprResultConformsComparisonRule(CheckerContext &C,
+ BinaryOperatorKind BOK,
+
danielmarjamaki marked 2 inline comments as done.
danielmarjamaki added inline comments.
Comment at: lib/StaticAnalyzer/Core/CheckerHelpers.cpp:100
+ BinaryOperatorKind BOK,
+
danielmarjamaki updated this revision to Diff 92150.
danielmarjamaki added a comment.
Fix review comments
Repository:
rL LLVM
https://reviews.llvm.org/D30295
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h
lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
lib/St
a.sidorin added a comment.
Hello Daniel,
Your patch looks mostly good to me. I have only minor naming comments.
Comment at: lib/StaticAnalyzer/Core/CheckerHelpers.cpp:99
+bool clang::ento::isExprResultConformsComparisonRule(CheckerContext &C,
+
danielmarjamaki added a comment.
Ping
Repository:
rL LLVM
https://reviews.llvm.org/D30295
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
danielmarjamaki created this revision.
The error messages are confusing when shift result is undefined because the
shift count is negative or exceeds the bit width. I have seen that users often
draw the conclusion that Clang thinks some operand is uninitialized and
determine that Clang shows a
38 matches
Mail list logo
