[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width
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
[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width
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/StaticAnalyzer/Checkers/UndefResultChecker.cpp
lib/StaticAnalyzer/Core/CheckerContext.cpp
test/Analysis/bitwise-ops.c
Index: test/Analysis/bitwise-ops.c
===
--- test/Analysis/bitwise-ops.c
+++ test/Analysis/bitwise-ops.c
@@ -22,11 +22,25 @@
case 1:
return 0ULL << 63; // no-warning
case 2:
-return 0ULL << 64; // expected-warning{{The result of the '<<' expression is undefined}}
+return 0ULL << 64; // expected-warning{{The result of the left shift is undefined due to shifting by '64', which is larger or equal to the width of type 'unsigned long long'}}
case 3:
-return 0ULL << 65; // expected-warning{{The result of the '<<' expression is undefined}}
+return 0ULL << 65; // expected-warning{{The result of the left shift is undefined due to shifting by '65', which is larger or equal to the width of type 'unsigned long long'}}
default:
return 0;
}
-}
\ No newline at end of file
+}
+
+int testOverflowShift(int a) {
+ if (a == 323) {
+return 1 << a; // expected-warning{{The result of the left shift is undefined due to shifting by '323', which is larger or equal to the width of type 'int'}}
+ }
+ return 0;
+}
+
+int testNegativeShift(int a) {
+ if (a == -5) {
+return 1 << a; // expected-warning{{The result of the left shift is undefined because the right operand is negative}}
+ }
+ return 0;
+}
Index: lib/StaticAnalyzer/Core/CheckerContext.cpp
===
--- lib/StaticAnalyzer/Core/CheckerContext.cpp
+++ lib/StaticAnalyzer/Core/CheckerContext.cpp
@@ -99,3 +99,35 @@
return Lexer::getSpelling(Loc, buf, getSourceManager(), getLangOpts());
}
+/// Evaluate comparison and return true if it's known that condition is true
+static bool evalComparison(SVal LHSVal, BinaryOperatorKind ComparisonOp,
+ SVal RHSVal, ProgramStateRef State) {
+ if (LHSVal.isUnknownOrUndef())
+return false;
+ ProgramStateManager &Mgr = State->getStateManager();
+ if (!LHSVal.getAs()) {
+LHSVal = Mgr.getStoreManager().getBinding(State->getStore(),
+ LHSVal.castAs());
+if (LHSVal.isUnknownOrUndef() || !LHSVal.getAs())
+ return false;
+ }
+
+ SValBuilder &Bldr = Mgr.getSValBuilder();
+ SVal Eval = Bldr.evalBinOp(State, ComparisonOp, LHSVal, RHSVal,
+ Bldr.getConditionType());
+ if (Eval.isUnknownOrUndef())
+return false;
+ ProgramStateRef StTrue, StFalse;
+ std::tie(StTrue, StFalse) = State->assume(Eval.castAs());
+ return StTrue && !StFalse;
+}
+
+bool CheckerContext::isGreaterOrEqual(const Expr *E, unsigned long long Val) {
+ DefinedSVal V = getSValBuilder().makeIntVal(Val, getASTContext().LongLongTy);
+ return evalComparison(getSVal(E), BO_GE, V, getState());
+}
+
+bool CheckerContext::isNegative(const Expr *E) {
+ DefinedSVal V = getSValBuilder().makeIntVal(0, false);
+ return evalComparison(getSVal(E), BO_LT, V, getState());
+}
Index: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
@@ -59,6 +59,11 @@
return StOutBound && !StInBound;
}
+static bool isShiftOverflow(const BinaryOperator *B, CheckerContext &C) {
+ return C.isGreaterOrEqual(
+ B->getRHS(), C.getASTContext().getIntWidth(B->getLHS()->getType()));
+}
+
void UndefResultChecker::checkPostStmt(const BinaryOperator *B,
CheckerContext &C) const {
ProgramStateRef state = C.getState();
@@ -97,18 +102,46 @@
}
if (Ex) {
- OS << "The " << (isLeft ? "left" : "right")
- << " operand of '"
+ OS << "The " << (isLeft ? "left" : "right") << " operand of '"
<< BinaryOperator::getOpcodeStr(B->getOpcode())
<< "' is a garbage value";
if (isArrayIndexOutOfBounds(C, Ex))
OS << " due to array index out of bounds";
-}
-else {
+} else {
// Neither operand was undefined, but the result is undefined.
- OS << "The result of the '"
- << BinaryOperator::getOpcodeStr(B->getOpcode())
- << "' expression is undefined";
+ if ((B->getOpcode() == BinaryOperatorKind::BO_Shl ||
+ B->getOpcode() == BinaryOperatorKind::BO_Shr) &&
+ C.isNegative(B->getRHS())) {
+OS << "The result of the "
+ << ((B->getOpcode() == BinaryOperatorKind::BO_Shl) ? "left"
+ : "right")
+ << " shi
[PATCH] D28148: [Sema] Suppress warnings for C's zero initializer
This revision was automatically updated to reflect the committed changes.
Closed by commit rL314499: [Sema] Suppress warnings for C's zero initializer
(authored by danielmarjamaki).
Changed prior to commit:
https://reviews.llvm.org/D28148?vs=82849&id=117107#toc
Repository:
rL LLVM
https://reviews.llvm.org/D28148
Files:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/test/Sema/zero-initializer.c
Index: cfe/trunk/lib/AST/Expr.cpp
===
--- cfe/trunk/lib/AST/Expr.cpp
+++ cfe/trunk/lib/AST/Expr.cpp
@@ -1951,6 +1951,17 @@
getInit(0)->getType().getCanonicalType();
}
+bool InitListExpr::isIdiomaticZeroInitializer(const LangOptions &LangOpts) const {
+ assert(isSyntacticForm() && "only test syntactic form as zero initializer");
+
+ if (LangOpts.CPlusPlus || getNumInits() != 1) {
+return false;
+ }
+
+ const IntegerLiteral *Lit = dyn_cast(getInit(0));
+ return Lit && Lit->getValue() == 0;
+}
+
SourceLocation InitListExpr::getLocStart() const {
if (InitListExpr *SyntacticForm = getSyntacticForm())
return SyntacticForm->getLocStart();
Index: cfe/trunk/lib/Sema/SemaInit.cpp
===
--- cfe/trunk/lib/Sema/SemaInit.cpp
+++ cfe/trunk/lib/Sema/SemaInit.cpp
@@ -886,7 +886,8 @@
}
// Complain about missing braces.
-if (T->isArrayType() || T->isRecordType()) {
+if ((T->isArrayType() || T->isRecordType()) &&
+!ParentIList->isIdiomaticZeroInitializer(SemaRef.getLangOpts())) {
SemaRef.Diag(StructuredSubobjectInitList->getLocStart(),
diag::warn_missing_braces)
<< StructuredSubobjectInitList->getSourceRange()
@@ -1833,7 +1834,9 @@
// worthwhile to skip over the rest of the initializer, though.
RecordDecl *RD = DeclType->getAs()->getDecl();
RecordDecl::field_iterator FieldEnd = RD->field_end();
- bool CheckForMissingFields = true;
+ bool CheckForMissingFields =
+!IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts());
+
while (Index < IList->getNumInits()) {
Expr *Init = IList->getInit(Index);
Index: cfe/trunk/include/clang/AST/Expr.h
===
--- cfe/trunk/include/clang/AST/Expr.h
+++ cfe/trunk/include/clang/AST/Expr.h
@@ -4011,6 +4011,10 @@
/// initializer)?
bool isTransparent() const;
+ /// Is this the zero initializer {0} in a language which considers it
+ /// idiomatic?
+ bool isIdiomaticZeroInitializer(const LangOptions &LangOpts) const;
+
SourceLocation getLBraceLoc() const { return LBraceLoc; }
void setLBraceLoc(SourceLocation Loc) { LBraceLoc = Loc; }
SourceLocation getRBraceLoc() const { return RBraceLoc; }
@@ -4020,6 +4024,9 @@
InitListExpr *getSemanticForm() const {
return isSemanticForm() ? nullptr : AltForm.getPointer();
}
+ bool isSyntacticForm() const {
+return !AltForm.getInt() || !AltForm.getPointer();
+ }
InitListExpr *getSyntacticForm() const {
return isSemanticForm() ? AltForm.getPointer() : nullptr;
}
Index: cfe/trunk/test/Sema/zero-initializer.c
===
--- cfe/trunk/test/Sema/zero-initializer.c
+++ cfe/trunk/test/Sema/zero-initializer.c
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -std=c99 -Wmissing-field-initializers -Wmissing-braces -verify %s
+
+// Tests that using {0} in struct initialization or assignment is supported
+struct foo { int x; int y; };
+struct bar { struct foo a; struct foo b; };
+struct A { int a; };
+struct B { struct A a; };
+struct C { struct B b; };
+
+int main(void)
+{
+ struct foo f = { 0 }; // no-warning
+ struct foo g = { 9 }; // expected-warning {{missing field 'y' initializer}}
+ struct foo h = { 9, 9 }; // no-warning
+ struct bar i = { 0 }; // no-warning
+ struct bar j = { 0, 0 }; // expected-warning {{suggest braces around initialization of subobject}} expected-warning {{missing field 'b' initializer}}
+ struct bar k = { { 9, 9 }, { 9, 9 } }; // no-warning
+ struct bar l = { { 9, 9 }, { 0 } }; // no-warning
+ struct bar m = { { 0 }, { 0 } }; // no-warning
+ struct bar n = { { 0 }, { 9, 9 } }; // no-warning
+ struct bar o = { { 9 }, { 9, 9 } }; // expected-warning {{missing field 'y' initializer}}
+ struct C p = { 0 }; // no-warning
+ struct C q = { 9 }; // expected-warning {{suggest braces around initialization of subobject}} expected-warning {{suggest braces around initialization of subobject}}
+ f = (struct foo ) { 0 }; // no-warning
+ g = (struct foo ) { 9 }; // expected-warning {{missing field 'y' initializer}}
+ h = (struct foo ) { 9, 9 }; // no-warning
+ i = (struct bar) { 0 }; // no-warning
+ j = (struct bar) { 0, 0 }; // expected-warning {{suggest braces around initialization of subobject}} expected-warning {{missing field 'b' initializer}}
+ k = (struct bar) { { 9, 9 }, { 9, 9 } };
[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width
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
[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values
danielmarjamaki added a comment. ping Repository: rL LLVM https://reviews.llvm.org/D36471 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written
danielmarjamaki updated this revision to Diff 117956.
danielmarjamaki added a comment.
Herald added a subscriber: szepet.
Fixes according to review comments. Reuse ast matchers in LoopUnrolling.cpp.
Avoid some recursion (however the isChanged() is still recursive but it is very
small and simple).
Repository:
rL LLVM
https://reviews.llvm.org/D37897
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
include/clang/StaticAnalyzer/Core/PathSensitive/LoopUnrolling.h
lib/StaticAnalyzer/Core/ExprEngine.cpp
lib/StaticAnalyzer/Core/LoopUnrolling.cpp
test/Analysis/global-vars.c
Index: test/Analysis/global-vars.c
===
--- test/Analysis/global-vars.c
+++ test/Analysis/global-vars.c
@@ -0,0 +1,18 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha,core -verify %s
+
+// Avoid false positive.
+static char *allv[] = {"rpcgen", "-s", "udp", "-s", "tcp"};
+static int allc = sizeof(allv) / sizeof(allv[0]);
+static void falsePositive1(void) {
+ int i;
+ for (i = 1; i < allc; i++) {
+const char *p = allv[i]; // no-warning
+i++;
+ }
+}
+
+// Detect division by zero.
+static int zero = 0;
+void truePositive1() {
+ int x = 1000 / zero; // expected-warning{{Division by zero}}
+}
Index: lib/StaticAnalyzer/Core/LoopUnrolling.cpp
===
--- lib/StaticAnalyzer/Core/LoopUnrolling.cpp
+++ lib/StaticAnalyzer/Core/LoopUnrolling.cpp
@@ -99,7 +99,10 @@
declRefExpr(to(varDecl(VarNodeMatcher)),
binaryOperator(anyOf(hasOperatorName("="), hasOperatorName("+="),
hasOperatorName("/="), hasOperatorName("*="),
- hasOperatorName("-=")),
+ hasOperatorName("-="), hasOperatorName("%="),
+ hasOperatorName("<<="), hasOperatorName(">>="),
+ hasOperatorName("&="), hasOperatorName("|="),
+ hasOperatorName("^=")),
hasLHS(ignoringParenImpCasts(
declRefExpr(to(varDecl(VarNodeMatcher)));
}
@@ -283,5 +286,16 @@
return false;
return true;
}
+
+bool isVarChanged(const FunctionDecl *FD, const VarDecl *VD) {
+ if (!FD->getBody())
+return false;
+ auto Match = match(
+ stmt(hasDescendant(stmt(anyOf(
+ callByRef(equalsNode(VD)), getAddrTo(equalsNode(VD)),
+ assignedToRef(equalsNode(VD)), changeIntBoundNode(equalsNode(VD)),
+ *FD->getBody(), FD->getASTContext());
+ return !Match.empty();
}
-}
+} // namespace ento
+} // namespace clang
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -103,8 +103,86 @@
// Utility methods.
//===--===//
+static bool isChanged(const Decl *D, const VarDecl *VD) {
+ if (const auto *FD = dyn_cast(D)) {
+return isVarChanged(FD, VD);
+ }
+ if (const auto *Rec = dyn_cast(D)) {
+for (const auto *RecChild : Rec->decls()) {
+ if (isChanged(RecChild, VD))
+return true;
+}
+ }
+ return false;
+}
+
+/** Get initial state for global static variable */
+ProgramStateRef ExprEngine::getInitialStateForGlobalStaticVar(
+const LocationContext *LCtx, ProgramStateRef State, const VarDecl *VD) {
+ // Is variable changed anywhere in TU?
+ for (const Decl *D : AMgr.getASTContext().getTranslationUnitDecl()->decls()) {
+if (isChanged(D, VD))
+ return State;
+ }
+
+ // What is the initialized value?
+ llvm::APSInt InitVal;
+ if (const Expr *I = VD->getInit()) {
+if (!I->EvaluateAsInt(InitVal, getContext()))
+ return State;
+ } else {
+InitVal = 0;
+ }
+
+ const MemRegion *R = State->getRegion(VD, LCtx);
+ if (!R)
+return State;
+ SVal V = State->getSVal(loc::MemRegionVal(R));
+ SVal Constraint_untested =
+ evalBinOp(State, BO_EQ, V, svalBuilder.makeIntVal(InitVal),
+svalBuilder.getConditionType());
+ Optional Constraint =
+ Constraint_untested.getAs();
+ if (!Constraint)
+return State;
+ return State->assume(*Constraint, true);
+}
+
+static void getGlobalStaticVars(const Stmt *FuncBody,
+llvm::SmallSet *Vars) {
+ std::stack Children;
+ Children.push(FuncBody);
+ while (!Children.empty()) {
+const Stmt *Child = Children.top();
+Children.pop();
+if (!Child)
+ continue;
+for (const Stmt *C : Child->children()) {
+ Children.push(C);
+}
+if (const DeclRefExpr *D = dyn_cast(Child)) {
+ const VarDecl *VD = dyn_cast(D->getDecl());
+ if (VD && VD->isDefinedOutsideFunctionOrMethod() &&
+ VD->getType()->isIntegerType() &&
+ VD->getStorageClass() == SC_Static &&
+ !VD->getType()->isPointe
[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written
danielmarjamaki marked an inline comment as done.
danielmarjamaki added inline comments.
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:107
+/** Recursively check if variable is changed in code. */
+static bool isChanged(const Stmt *S, const VarDecl *VD, bool Write) {
+ if (!S)
xazax.hun wrote:
> Usually, we do not like bug recursions since it might eat up the stack.
> Also, do you consider the case when the variable is passed by reference to a
> function in another translation unit?
I rewrote the code to reuse the ast matchers in LoopUnroll.. and that is not
recursive. I rewrote the getGlobalStaticVars (this code is longer and in my
opinion less readable but it's not recursive).
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:169
+ SVal Constraint_untested =
+ evalBinOp(State, BO_EQ, V, svalBuilder.makeIntVal(InitVal),
+svalBuilder.getConditionType());
xazax.hun wrote:
> Does this work for non-integer typed e.g. structs?
Currently static struct objects are unaffected by these changes. There is no
regression.
Reviews are taking too long time. I am not against getting reviews but I am
against waiting for reviews for months, ideally people would only need to wait
for at most a week to get a review. This is why I don't want to handle structs
now -- I am not eager to prolong the review process for this patch.
Repository:
rL LLVM
https://reviews.llvm.org/D37897
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38675: [analyzer] MisusedMovedObjectChecker: Moving the checker out of alpha state
danielmarjamaki added a reviewer: danielmarjamaki. danielmarjamaki added a comment. > However, the checker seems to work with a low false positive rate. (<15 on > the LLVM, 6 effectively different) This does not sound like a low false positive rate to me. Could you describe what the false positives are? Is it possible to fix them? > Is it enough or should I check it on other open source projects? you should check a number of different projects. There might be idioms/usages in other projects that are not seen in LLVM. However I don't know what other open source C++11 projects there are. But I have a script that runs clang on various Debian projects and I can run that and provide you with the results. https://reviews.llvm.org/D38675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38674: [analyzer] MisusedMovedObjectChecker: More precise warning message
danielmarjamaki added a comment. LGTM https://reviews.llvm.org/D38674 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38675: [analyzer] MisusedMovedObjectChecker: Moving the checker out of alpha state
danielmarjamaki added a comment. In https://reviews.llvm.org/D38675#891912, @xazax.hun wrote: > In https://reviews.llvm.org/D38675#891750, @danielmarjamaki wrote: > > > > However, the checker seems to work with a low false positive rate. (<15 > > > on the LLVM, 6 effectively different) > > > > This does not sound like a low false positive rate to me. Could you > > describe what the false positives are? Is it possible to fix them? > > > Note that the unique findings are 6. I think there are non-alpha checks with > more false positives. This does not answer the important questions. What are the false positives, and is it possible to fix them? In https://reviews.llvm.org/D38675#891750, @danielmarjamaki wrote: > >> Is it enough or should I check it on other open source projects? > > you should check a number of different projects. There might be idioms/usages > in other projects that are not seen in LLVM. > > However I don't know what other open source C++11 projects there are. > > But I have a script that runs clang on various Debian projects and I can run > that and provide you with the results. Something didn't go well. I started my script yesterday afternoon. It has checked 426 projects so far. I do not see a single MisusedMovedObject warning. I am thinking that my script doesn't work well in this case. A wild idea is that maybe my script must somehow tell scan-build to use -std=c++11. For information here are the cppcheck results for a similar checker: http://cppcheck.sourceforge.net/cgi-bin/daca2-search.cgi?id=accessMoved Maybe you could pick a few of those projects and check those. That should exercise this checker. Please tell me if you think some of the accessMoved warnings are false positives. These warnings are "inconclusive" which indicates they might not always be correct. https://reviews.llvm.org/D38675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38718: Patch to Bugzilla 20951
danielmarjamaki added a reviewer: danielmarjamaki.
danielmarjamaki added a comment.
I think a test for -Wtautological-pointer-compare should be added that shows
that the bug is fixed.
Comment at: test/Sema/conditional-expr.c:84
+ //char x;
+ return (((x != ((void *) 0)) ? (*x = ((char) 1)) : (void) ((void *) 0)),
(unsigned long) ((void *) 0)); // expected-warning {{C99 forbids conditional
expressions with only one void side}}
}
lebedev.ri wrote:
> Please don't just remove previous tests.
> E.g. does the old test no longer warns?
>
no test is removed. The expected-warning is unchanged.
the problem with the test was that this comparison is always true:
```
(&x) != ((void *)0)
```
the address of x is never 0!
Fixing https://bugs.llvm.org/show_bug.cgi?id=20951 means Clang will warn:
```
warning: comparison of address of 'x' not equal to a null pointer is always
true [-Wtautological-pointer-compare]
```
We changed the test so the -Wtautological-pointer-compare is not reported...
but the original warning is still reported.
https://reviews.llvm.org/D38718
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38718: [Sema] No -Wtautological-pointer-compare warning on variables within parentheses
danielmarjamaki added a comment. LGTM! However I would like to see a review from somebody else also. There are a number of diagnostics that might be affected. The Sema::DiagnoseAlwaysNonNullPointer diagnoses these: diag::warn_this_null_compare diag::warn_this_bool_conversion diag::warn_address_of_reference_null_compare diag::warn_address_of_reference_bool_conversion diag::warn_nonnull_expr_compare diag::warn_cast_nonnull_to_bool diag::warn_null_pointer_compare <-- I think this is the one bug 20951 is about diag::warn_impcast_pointer_to_bool It seems to me that it is an improvement for all these warnings to skip the parentheses. However there is a danger that parentheses should hide some warnings to make it possible for users to hide unwanted warnings. But if that was the design decision then some regression test should complain when we skip the parentheses. Repository: rL LLVM https://reviews.llvm.org/D38718 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values
danielmarjamaki added a comment. ping Repository: rL LLVM https://reviews.llvm.org/D36471 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width
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
https://reviews.llvm.org/D30295
Files:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h
cfe/trunk/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
cfe/trunk/lib/StaticAnalyzer/Core/CheckerContext.cpp
cfe/trunk/test/Analysis/bitwise-ops.c
Index: cfe/trunk/lib/StaticAnalyzer/Core/CheckerContext.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/CheckerContext.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/CheckerContext.cpp
@@ -99,3 +99,35 @@
return Lexer::getSpelling(Loc, buf, getSourceManager(), getLangOpts());
}
+/// Evaluate comparison and return true if it's known that condition is true
+static bool evalComparison(SVal LHSVal, BinaryOperatorKind ComparisonOp,
+ SVal RHSVal, ProgramStateRef State) {
+ if (LHSVal.isUnknownOrUndef())
+return false;
+ ProgramStateManager &Mgr = State->getStateManager();
+ if (!LHSVal.getAs()) {
+LHSVal = Mgr.getStoreManager().getBinding(State->getStore(),
+ LHSVal.castAs());
+if (LHSVal.isUnknownOrUndef() || !LHSVal.getAs())
+ return false;
+ }
+
+ SValBuilder &Bldr = Mgr.getSValBuilder();
+ SVal Eval = Bldr.evalBinOp(State, ComparisonOp, LHSVal, RHSVal,
+ Bldr.getConditionType());
+ if (Eval.isUnknownOrUndef())
+return false;
+ ProgramStateRef StTrue, StFalse;
+ std::tie(StTrue, StFalse) = State->assume(Eval.castAs());
+ return StTrue && !StFalse;
+}
+
+bool CheckerContext::isGreaterOrEqual(const Expr *E, unsigned long long Val) {
+ DefinedSVal V = getSValBuilder().makeIntVal(Val, getASTContext().LongLongTy);
+ return evalComparison(getSVal(E), BO_GE, V, getState());
+}
+
+bool CheckerContext::isNegative(const Expr *E) {
+ DefinedSVal V = getSValBuilder().makeIntVal(0, false);
+ return evalComparison(getSVal(E), BO_LT, V, getState());
+}
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
@@ -123,57 +123,6 @@
C.emitReport(std::move(R));
}
-// Is E value greater or equal than Val?
-static bool isGreaterEqual(CheckerContext &C, const Expr *E,
- unsigned long long Val) {
- ProgramStateRef State = C.getState();
- SVal EVal = C.getSVal(E);
- if (EVal.isUnknownOrUndef())
-return false;
- if (!EVal.getAs() && EVal.getAs()) {
-ProgramStateManager &Mgr = C.getStateManager();
-EVal =
-Mgr.getStoreManager().getBinding(State->getStore(), EVal.castAs());
- }
- if (EVal.isUnknownOrUndef() || !EVal.getAs())
-return false;
-
- SValBuilder &Bldr = C.getSValBuilder();
- DefinedSVal V = Bldr.makeIntVal(Val, C.getASTContext().LongLongTy);
-
- // Is DefinedEVal greater or equal with V?
- SVal GE = Bldr.evalBinOp(State, BO_GE, EVal, V, Bldr.getConditionType());
- if (GE.isUnknownOrUndef())
-return false;
- ConstraintManager &CM = C.getConstraintManager();
- ProgramStateRef StGE, StLT;
- std::tie(StGE, StLT) = CM.assumeDual(State, GE.castAs());
- return StGE && !StLT;
-}
-
-// Is E value negative?
-static bool isNegative(CheckerContext &C, const Expr *E) {
- ProgramStateRef State = C.getState();
- SVal EVal = State->getSVal(E, C.getLocationContext());
- if (EVal.isUnknownOrUndef() || !EVal.getAs())
-return false;
- DefinedSVal DefinedEVal = EVal.castAs();
-
- SValBuilder &Bldr = C.getSValBuilder();
- DefinedSVal V = Bldr.makeIntVal(0, false);
-
- SVal LT =
- Bldr.evalBinOp(State, BO_LT, DefinedEVal, V, Bldr.getConditionType());
-
- // Is E value greater than MaxVal?
- ConstraintManager &CM = C.getConstraintManager();
- ProgramStateRef StNegative, StPositive;
- std::tie(StNegative, StPositive) =
- CM.assumeDual(State, LT.castAs());
-
- return StNegative && !StPositive;
-}
-
bool ConversionChecker::isLossOfPrecision(const ImplicitCastExpr *Cast,
QualType DestType,
CheckerContext &C) const {
@@ -195,18 +144,18 @@
return false;
unsigned long long MaxVal = 1ULL << W;
- return isGreaterEqual(C, Cast->getSubExpr(), MaxVal);
+ return C.isGreaterOrEqual(Cast->getSubExpr(), MaxVal);
}
bool ConversionChecker::isLossOfSign(const ImplicitCastExpr *Cast,
- CheckerContext &C) const {
+ CheckerContext &C) const {
QualType CastType = Cast->getType();
Q
[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written
danielmarjamaki added a comment. In https://reviews.llvm.org/D37897#892667, @dcoughlin wrote: > Apologies for the delay reviewing! As I noted inline, I'm pretty worried > about the performance impact of this. Is it possible to do the analysis in a > single traversal of the translation unit? I agree. I first tried more like that but ran into problems. Don't remember the details. I will try again.. however as far as I see this will mean the LoopUnroller AST matchers can't be reused unless I change them. Repository: rL LLVM https://reviews.llvm.org/D37897 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written
danielmarjamaki added inline comments.
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:123
+ // Is variable changed anywhere in TU?
+ for (const Decl *D : AMgr.getASTContext().getTranslationUnitDecl()->decls())
{
+if (isChanged(D, VD))
dcoughlin wrote:
> Since you are calling `getInitialStateForGlobalStaticVar()` in
> `getInitialState()` for each static variable declaration and
> `getInitialState()` is called for each top-level function, you are doing an
> n^3 operation in the size of the translation unit, which is going to be very,
> very expensive for large translation units.
>
> Have you considered doing the analysis for static variables that are never
> changed during call-graph construction? This should be a linear operation and
> doing it during call-graph construction would avoid an extra walk of the
> entire translation unit.
hmm... could you tell me where the call-graph construction is that I can tweak?
Repository:
rL LLVM
https://reviews.llvm.org/D37897
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written
danielmarjamaki added inline comments.
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:123
+ // Is variable changed anywhere in TU?
+ for (const Decl *D : AMgr.getASTContext().getTranslationUnitDecl()->decls())
{
+if (isChanged(D, VD))
danielmarjamaki wrote:
> dcoughlin wrote:
> > Since you are calling `getInitialStateForGlobalStaticVar()` in
> > `getInitialState()` for each static variable declaration and
> > `getInitialState()` is called for each top-level function, you are doing an
> > n^3 operation in the size of the translation unit, which is going to be
> > very, very expensive for large translation units.
> >
> > Have you considered doing the analysis for static variables that are never
> > changed during call-graph construction? This should be a linear operation
> > and doing it during call-graph construction would avoid an extra walk of
> > the entire translation unit.
> hmm... could you tell me where the call-graph construction is that I can
> tweak?
I think I found it: `clang/lib/Analysis/CallGraph.cpp`
Repository:
rL LLVM
https://reviews.llvm.org/D37897
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written
danielmarjamaki updated this revision to Diff 118947.
danielmarjamaki added a comment.
Track modification of global static variables in CallGraph construction
Repository:
rL LLVM
https://reviews.llvm.org/D37897
Files:
include/clang/AST/Decl.h
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/AST/Decl.cpp
lib/Analysis/CallGraph.cpp
lib/StaticAnalyzer/Core/ExprEngine.cpp
test/Analysis/global-vars.c
Index: test/Analysis/global-vars.c
===
--- test/Analysis/global-vars.c
+++ test/Analysis/global-vars.c
@@ -0,0 +1,18 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha,core -verify %s
+
+// Avoid false positive.
+static char *allv[] = {"rpcgen", "-s", "udp", "-s", "tcp"};
+static int allc = sizeof(allv) / sizeof(allv[0]);
+static void falsePositive1(void) {
+ int i;
+ for (i = 1; i < allc; i++) {
+const char *p = allv[i]; // no-warning
+i++;
+ }
+}
+
+// Detect division by zero.
+static int zero = 0;
+void truePositive1() {
+ int x = 1000 / zero; // expected-warning{{Division by zero}}
+}
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -103,8 +103,71 @@
// Utility methods.
//===--===//
+/** Get initial state for global static variable */
+ProgramStateRef ExprEngine::getInitialStateForGlobalStaticVar(
+const LocationContext *LCtx, ProgramStateRef State, const VarDecl *VD) {
+ // Is variable changed anywhere in TU?
+ if (VD->isModified())
+return State;
+
+ // What is the initialized value?
+ llvm::APSInt InitVal;
+ if (const Expr *I = VD->getInit()) {
+if (!I->EvaluateAsInt(InitVal, getContext()))
+ return State;
+ } else {
+InitVal = 0;
+ }
+
+ const MemRegion *R = State->getRegion(VD, LCtx);
+ if (!R)
+return State;
+ SVal V = State->getSVal(loc::MemRegionVal(R));
+ SVal Constraint_untested =
+ evalBinOp(State, BO_EQ, V, svalBuilder.makeIntVal(InitVal),
+svalBuilder.getConditionType());
+ Optional Constraint =
+ Constraint_untested.getAs();
+ if (!Constraint)
+return State;
+ return State->assume(*Constraint, true);
+}
+
+static void getGlobalStaticVars(const Stmt *FuncBody,
+llvm::SmallSet *Vars) {
+ std::stack Children;
+ Children.push(FuncBody);
+ while (!Children.empty()) {
+const Stmt *Child = Children.top();
+Children.pop();
+if (!Child)
+ continue;
+for (const Stmt *C : Child->children()) {
+ Children.push(C);
+}
+if (const DeclRefExpr *D = dyn_cast(Child)) {
+ const VarDecl *VD = dyn_cast(D->getDecl());
+ if (VD && VD->isDefinedOutsideFunctionOrMethod() &&
+ VD->getType()->isIntegerType() &&
+ VD->getStorageClass() == SC_Static &&
+ !VD->getType()->isPointerType()) {
+Vars->insert(VD);
+ }
+}
+ }
+}
+
ProgramStateRef ExprEngine::getInitialState(const LocationContext *InitLoc) {
ProgramStateRef state = StateMgr.getInitialState(InitLoc);
+ // Get initial states for static global variables.
+ if (const auto *FD = dyn_cast(InitLoc->getDecl())) {
+llvm::SmallSet Vars;
+getGlobalStaticVars(FD->getBody(), &Vars);
+for (const VarDecl *VD : Vars) {
+ state = getInitialStateForGlobalStaticVar(InitLoc, state, VD);
+}
+ }
+
const Decl *D = InitLoc->getDecl();
// Preconditions.
Index: lib/Analysis/CallGraph.cpp
===
--- lib/Analysis/CallGraph.cpp
+++ lib/Analysis/CallGraph.cpp
@@ -33,10 +33,12 @@
CallGraphNode *CallerNode;
public:
- CGBuilder(CallGraph *g, CallGraphNode *N)
-: G(g), CallerNode(N) {}
+ CGBuilder(CallGraph *g, CallGraphNode *N) : G(g), CallerNode(N) {}
- void VisitStmt(Stmt *S) { VisitChildren(S); }
+ void VisitStmt(Stmt *S) {
+markModifiedVars(S);
+VisitChildren(S);
+ }
Decl *getDeclFromCall(CallExpr *CE) {
if (FunctionDecl *CalleeDecl = CE->getDirectCallee())
@@ -63,13 +65,14 @@
if (Decl *D = getDeclFromCall(CE))
addCalledDecl(D);
VisitChildren(CE);
+markModifiedVars(CE);
}
// Adds may-call edges for the ObjC message sends.
void VisitObjCMessageExpr(ObjCMessageExpr *ME) {
if (ObjCInterfaceDecl *IDecl = ME->getReceiverInterface()) {
Selector Sel = ME->getSelector();
-
+
// Find the callee definition within the same translation unit.
Decl *D = nullptr;
if (ME->isInstanceMessage())
@@ -88,6 +91,53 @@
if (SubStmt)
this->Visit(SubStmt);
}
+
+ void modifyVar(Expr *E) {
+auto *D = dyn_cast(E->IgnoreParenCasts());
+if (!D)
+ return;
+VarDecl *VD = dyn_cast(D->getDecl());
+if (VD)
+ VD->setModified();
+ }
+
+ void markModifiedVars(Stmt *
[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written
danielmarjamaki added inline comments.
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:123
+ // Is variable changed anywhere in TU?
+ for (const Decl *D : AMgr.getASTContext().getTranslationUnitDecl()->decls())
{
+if (isChanged(D, VD))
danielmarjamaki wrote:
> danielmarjamaki wrote:
> > dcoughlin wrote:
> > > Since you are calling `getInitialStateForGlobalStaticVar()` in
> > > `getInitialState()` for each static variable declaration and
> > > `getInitialState()` is called for each top-level function, you are doing
> > > an n^3 operation in the size of the translation unit, which is going to
> > > be very, very expensive for large translation units.
> > >
> > > Have you considered doing the analysis for static variables that are
> > > never changed during call-graph construction? This should be a linear
> > > operation and doing it during call-graph construction would avoid an
> > > extra walk of the entire translation unit.
> > hmm... could you tell me where the call-graph construction is that I can
> > tweak?
> I think I found it: `clang/lib/Analysis/CallGraph.cpp`
I now track variable modifications in call-graph construction instead.
Comment at: lib/StaticAnalyzer/Core/ExprEngine.cpp:155
+ Children.push(FuncBody);
+ while (!Children.empty()) {
+const Stmt *Child = Children.top();
szepet wrote:
> I think instead of this logic it would be better to use ConstStmtVisitor. In
> this case it does quite the same thing in a (maybe?) more structured manner.
> What do you think?
As far as I see ConstStmtVisitor is also recursive. Imho let's have readable
code instead..
Repository:
rL LLVM
https://reviews.llvm.org/D37897
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values
danielmarjamaki added a comment. ping Repository: rL LLVM https://reviews.llvm.org/D36471 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38921: [analyzer] LoopUnrolling: update the matched assignment operators
danielmarjamaki added a comment. LGTM.. however I would like approval from somebody else also. https://reviews.llvm.org/D38921 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38801: [analyzer] In getSVal() API, disable auto-detection of void type as char type.
danielmarjamaki added a comment. LGTM https://reviews.llvm.org/D38801 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39049: [analyzer] Fix wrong calculation of offset in ArrayBoundsV2
danielmarjamaki created this revision.
Herald added a subscriber: szepet.
Example code:
void test3_simplified_offset(int x, unsigned long long y) {
int buf[100];
if (x < 0)
x = 0;
for (int i = y - x; i > 0 && i < 100; i++)
buf[i] = 0; // no-warning
}
Without this patch Clang will wrongly report this FP:
File out-of-bounds.c Line 144: Out of bound memory access (accessed memory
precedes memory block)
There is some bug in the getSimplifiedOffsets() calculations. I removed the
wrong calculations and this does not break any existing tests so either no
tests were written in the first place or these calculations got redundant
sometime. If somebody wants to readd the calculations that I remove.. I am not
against that if some tests are added and it does not break my test.
Repository:
rL LLVM
https://reviews.llvm.org/D39049
Files:
lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
test/Analysis/out-of-bounds.c
Index: test/Analysis/out-of-bounds.c
===
--- test/Analysis/out-of-bounds.c
+++ test/Analysis/out-of-bounds.c
@@ -136,6 +136,14 @@
buf[x] = 1; // expected-warning{{Out of bound memory access}}
}
+void test3_simplified_offset(int x, unsigned long long y) {
+ int buf[100];
+ if (x < 0)
+x = 0;
+ for (int i = y - x; i > 0 && i < 100; i++)
+buf[i] = 0; // no-warning
+}
+
void test4(int x) {
int buf[100];
if (x > 99)
Index: lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
===
--- lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -98,10 +98,6 @@
nonloc::SymbolVal(SIE->getLHS()),
svalBuilder.makeIntVal(extent.getValue() / constant),
svalBuilder);
- case BO_Add:
-return getSimplifiedOffsets(
-nonloc::SymbolVal(SIE->getLHS()),
-svalBuilder.makeIntVal(extent.getValue() - constant), svalBuilder);
default:
break;
}
Index: test/Analysis/out-of-bounds.c
===
--- test/Analysis/out-of-bounds.c
+++ test/Analysis/out-of-bounds.c
@@ -136,6 +136,14 @@
buf[x] = 1; // expected-warning{{Out of bound memory access}}
}
+void test3_simplified_offset(int x, unsigned long long y) {
+ int buf[100];
+ if (x < 0)
+x = 0;
+ for (int i = y - x; i > 0 && i < 100; i++)
+buf[i] = 0; // no-warning
+}
+
void test4(int x) {
int buf[100];
if (x > 99)
Index: lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
===
--- lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -98,10 +98,6 @@
nonloc::SymbolVal(SIE->getLHS()),
svalBuilder.makeIntVal(extent.getValue() / constant),
svalBuilder);
- case BO_Add:
-return getSimplifiedOffsets(
-nonloc::SymbolVal(SIE->getLHS()),
-svalBuilder.makeIntVal(extent.getValue() - constant), svalBuilder);
default:
break;
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39031: [Analyzer] Correctly handle parameters passed by reference when bodyfarming std::call_once
danielmarjamaki added a reviewer: danielmarjamaki.
danielmarjamaki added a comment.
Stylistically this looks pretty good to me. Just a minor nit.
Comment at: lib/Analysis/BodyFarm.cpp:389
+ for (unsigned int i = 2; i < D->getNumParams(); i++) {
+
+const ParmVarDecl *PDecl = D->getParamDecl(i);
ehm.. I would remove this blank
https://reviews.llvm.org/D39031
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35109: [Analyzer] SValBuilder Comparison Rearrangement
danielmarjamaki added a comment.
I like this patch overall.. here are some stylistic nits.
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:610
+} else {
+ if (*lInt >= *rInt) {
+newRhsExt = lInt->getExtValue() - rInt->getExtValue();
you can use `else if` here
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:595
+
+ if (origWidth < 128) {
+auto newWidth = std::max(2 * origWidth, (uint32_t) 8);
I would like that "128" is rewritten somehow. Using expression instead.
Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:596
+ if (origWidth < 128) {
+auto newWidth = std::max(2 * origWidth, (uint32_t) 8);
+auto newAPSIntType = APSIntType(newWidth, false);
Is `origWidth < 4` possible?
I wonder about "8". Is that CHAR_BIT hardcoded?
https://reviews.llvm.org/D35109
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38986: [Analyzer] Better unreachable message in enumeration
danielmarjamaki added a comment. > I think it is much better when the assert failure tells the developer _what_ > value is failing, rather than saying "oops we are dead". yes of course, more informative assert messages is better. https://reviews.llvm.org/D38986 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39015: [Analyzer] Always use non-reference types when creating expressions in BodyFarm, removes std::call_once crash
danielmarjamaki added inline comments.
Comment at: cfe/trunk/lib/Analysis/BodyFarm.cpp:139
-DeclRefExpr *ASTMaker::makeDeclRefExpr(const VarDecl *D,
- bool RefersToEnclosingVariableOrCapture,
- bool GetNonReferenceType) {
- auto Type = D->getType();
- if (GetNonReferenceType)
-Type = Type.getNonReferenceType();
+DeclRefExpr *ASTMaker::makeDeclRefExpr(
+const VarDecl *D,
this looks strange, did clang-format do this?
Repository:
rL LLVM
https://reviews.llvm.org/D39015
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30489: [analyzer] catch out of bounds for VLA
danielmarjamaki updated this revision to Diff 119590.
danielmarjamaki added a comment.
Herald added a subscriber: szepet.
As suggested, use a ProgramState trait to detect VLA overflows.
I did not yet manage to get a SubRegion from the DeclStmt that matches the
location SubRegion. Therefore I am using VariableArrayType in the trait for now.
Repository:
rL LLVM
https://reviews.llvm.org/D30489
Files:
lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
test/Analysis/out-of-bounds.c
Index: test/Analysis/out-of-bounds.c
===
--- test/Analysis/out-of-bounds.c
+++ test/Analysis/out-of-bounds.c
@@ -174,3 +174,7 @@
clang_analyzer_eval(x <= 99); // expected-warning{{TRUE}}
}
+void vla(int X) {
+ char buf[X];
+ buf[X] = 0; // expected-warning {{Out of bound memory access (access exceeds upper limit of memory block)}}
+}
Index: lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
===
--- lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
+++ lib/StaticAnalyzer/Checkers/ArrayBoundCheckerV2.cpp
@@ -27,17 +27,18 @@
using namespace ento;
namespace {
-class ArrayBoundCheckerV2 :
-public Checker {
+class ArrayBoundCheckerV2
+: public Checker> {
mutable std::unique_ptr BT;
enum OOB_Kind { OOB_Precedes, OOB_Excedes, OOB_Tainted };
void reportOOB(CheckerContext &C, ProgramStateRef errorState,
OOB_Kind kind) const;
public:
- void checkLocation(SVal l, bool isLoad, const Stmt*S,
+ void checkPreStmt(const DeclStmt *DS, CheckerContext &C) const;
+ void checkLocation(SVal l, bool isLoad, const Stmt *S,
CheckerContext &C) const;
};
@@ -64,7 +65,10 @@
void dump() const;
void dumpToStream(raw_ostream &os) const;
};
-}
+} // namespace
+
+REGISTER_MAP_WITH_PROGRAMSTATE(VariableLengthArrayExtent,
+ const VariableArrayType *, NonLoc);
static SVal computeExtentBegin(SValBuilder &svalBuilder,
const MemRegion *region) {
@@ -111,8 +115,46 @@
return std::pair(offset, extent);
}
+void ArrayBoundCheckerV2::checkPreStmt(const DeclStmt *DS,
+ CheckerContext &checkerContext) const {
+ const VarDecl *VD = dyn_cast(DS->getSingleDecl());
+ if (!VD)
+return;
+
+ ASTContext &Ctx = checkerContext.getASTContext();
+ const VariableArrayType *VLA = Ctx.getAsVariableArrayType(VD->getType());
+ if (!VLA)
+return;
+
+ SVal VLASize = checkerContext.getSVal(VLA->getSizeExpr());
+
+ Optional Sz = VLASize.getAs();
+ if (!Sz)
+return;
+
+ ProgramStateRef state = checkerContext.getState();
+ checkerContext.addTransition(
+ state->set(VLA, Sz.getValue()));
+}
+
+static const VariableArrayType *getVLAFromExtent(DefinedOrUnknownSVal extentVal,
+ ASTContext &Ctx) {
+ if (extentVal.getSubKind() != nonloc::SymbolValKind)
+return nullptr;
+
+ SymbolRef SR = extentVal.castAs().getSymbol();
+ const SymbolExtent *SE = dyn_cast(SR);
+ const MemRegion *SEMR = SE->getRegion();
+ if (SEMR->getKind() != MemRegion::VarRegionKind)
+return nullptr;
+
+ const VarRegion *VR = cast(SEMR);
+ QualType T = VR->getDecl()->getType();
+ return Ctx.getAsVariableArrayType(T);
+}
+
void ArrayBoundCheckerV2::checkLocation(SVal location, bool isLoad,
-const Stmt* LoadS,
+const Stmt *LoadS,
CheckerContext &checkerContext) const {
// NOTE: Instead of using ProgramState::assumeInBound(), we are prototyping
@@ -175,13 +217,21 @@
}
do {
+
// CHECK UPPER BOUND: Is byteOffset >= extent(baseRegion)? If so,
// we are doing a load/store after the last valid offset.
DefinedOrUnknownSVal extentVal =
- rawOffset.getRegion()->getExtent(svalBuilder);
+rawOffset.getRegion()->getExtent(svalBuilder);
if (!extentVal.getAs())
break;
+if (const VariableArrayType *VLA =
+getVLAFromExtent(extentVal, checkerContext.getASTContext())) {
+ const NonLoc *V = state->get(VLA);
+ if (V)
+extentVal = *V;
+}
+
if (extentVal.getAs()) {
std::pair simplifiedOffsets =
getSimplifiedOffsets(rawOffset.getByteOffset(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width
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
[PATCH] D31029: [analyzer] Fix logical not for pointers with different bit width
danielmarjamaki added a comment. ping Repository: rL LLVM https://reviews.llvm.org/D31029 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31029: [analyzer] Fix logical not for pointers with different bit width
This revision was automatically updated to reflect the committed changes.
danielmarjamaki marked an inline comment as done.
Closed by commit rL305669: [analyzer] Fix logical not for pointers with
different bit width (authored by danielmarjamaki).
Changed prior to commit:
https://reviews.llvm.org/D31029?vs=99114&id=102999#toc
Repository:
rL LLVM
https://reviews.llvm.org/D31029
Files:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -980,10 +980,9 @@
//transfer functions as "0 == E".
SVal Result;
if (Optional LV = V.getAs()) {
-Loc X = svalBuilder.makeNull();
+Loc X = svalBuilder.makeNullWithType(Ex->getType());
Result = evalBinOp(state, BO_EQ, *LV, X, U->getType());
- }
- else if (Ex->getType()->isFloatingType()) {
+ } else if (Ex->getType()->isFloatingType()) {
// FIXME: handle floating point types.
Result = UnknownVal();
} else {
Index:
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
===
---
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
+++
cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
@@ -180,6 +180,11 @@
return getValue(X);
}
+ inline const llvm::APSInt& getZeroWithTypeSize(QualType T) {
+assert(T->isScalarType());
+return getValue(0, Ctx.getTypeSize(T), true);
+ }
+
inline const llvm::APSInt& getZeroWithPtrWidth(bool isUnsigned = true) {
return getValue(0, Ctx.getTypeSize(Ctx.VoidPtrTy), isUnsigned);
}
Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -315,6 +315,13 @@
return nonloc::ConcreteInt(BasicVals.getTruthValue(b));
}
+ /// Create NULL pointer, with proper pointer bit-width for given address
+ /// space.
+ /// \param type pointer type.
+ Loc makeNullWithType(QualType type) {
+return loc::ConcreteInt(BasicVals.getZeroWithTypeSize(type));
+ }
+
Loc makeNull() {
return loc::ConcreteInt(BasicVals.getZeroWithPtrWidth());
}
Index: cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -980,10 +980,9 @@
//transfer functions as "0 == E".
SVal Result;
if (Optional LV = V.getAs()) {
-Loc X = svalBuilder.makeNull();
+Loc X = svalBuilder.makeNullWithType(Ex->getType());
Result = evalBinOp(state, BO_EQ, *LV, X, U->getType());
- }
- else if (Ex->getType()->isFloatingType()) {
+ } else if (Ex->getType()->isFloatingType()) {
// FIXME: handle floating point types.
Result = UnknownVal();
} else {
Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
@@ -180,6 +180,11 @@
return getValue(X);
}
+ inline const llvm::APSInt& getZeroWithTypeSize(QualType T) {
+assert(T->isScalarType());
+return getValue(0, Ctx.getTypeSize(T), true);
+ }
+
inline const llvm::APSInt& getZeroWithPtrWidth(bool isUnsigned = true) {
return getValue(0, Ctx.getTypeSize(Ctx.VoidPtrTy), isUnsigned);
}
Index: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -315,6 +315,13 @@
return nonloc::ConcreteInt(BasicVals.getTruthValue(b));
}
+ /// Create NULL pointer, with proper pointer bit-width for given address
+ /// space.
+ /// \param type pointer type.
+ Loc makeNullWithType(QualType type) {
+return loc::ConcreteInt(BasicVals.getZeroWithTypeSize(type));
+ }
+
Loc makeNull() {
return loc::ConcreteInt(BasicVals.getZeroWithPtrWidth());
}
___
cfe
[PATCH] D32346: [clang-tidy] New readability check for strlen argument
danielmarjamaki abandoned this revision. danielmarjamaki added a comment. I will not continue working on this checker Repository: rL LLVM https://reviews.llvm.org/D32346 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width
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/StaticAnalyzer/Checkers/UndefResultChecker.cpp
lib/StaticAnalyzer/Core/CheckerContext.cpp
test/Analysis/bitwise-ops.c
Index: test/Analysis/bitwise-ops.c
===
--- test/Analysis/bitwise-ops.c
+++ test/Analysis/bitwise-ops.c
@@ -22,11 +22,25 @@
case 1:
return 0ULL << 63; // no-warning
case 2:
-return 0ULL << 64; // expected-warning{{The result of the '<<' expression is undefined}}
+return 0ULL << 64; // expected-warning{{The result of the left shift is undefined due to shifting by 64, which is larger than the width of type 'unsigned long long'}}
case 3:
-return 0ULL << 65; // expected-warning{{The result of the '<<' expression is undefined}}
+return 0ULL << 65; // expected-warning{{The result of the left shift is undefined due to shifting by 65, which is larger than the width of type 'unsigned long long'}}
default:
return 0;
}
-}
\ No newline at end of file
+}
+
+int testOverflowShift(int a) {
+ if (a == 323) {
+return 1 << a; // expected-warning{{The result of the left shift is undefined due to shifting by 323, which is larger than the width of type 'int'}}
+ }
+ return 0;
+}
+
+int testNegativeShift(int a) {
+ if (a == -5) {
+return 1 << a; // expected-warning{{The result of the left shift is undefined because the right operand is negative}}
+ }
+ return 0;
+}
Index: lib/StaticAnalyzer/Core/CheckerContext.cpp
===
--- lib/StaticAnalyzer/Core/CheckerContext.cpp
+++ lib/StaticAnalyzer/Core/CheckerContext.cpp
@@ -99,3 +99,35 @@
return Lexer::getSpelling(Loc, buf, getSourceManager(), getLangOpts());
}
+/// Evaluate comparison and return true if it's known that condition is true
+static bool evalComparison(SVal LHSVal, BinaryOperatorKind ComparisonOp,
+ SVal RHSVal, ProgramStateRef State) {
+ if (LHSVal.isUnknownOrUndef())
+return false;
+ ProgramStateManager &Mgr = State->getStateManager();
+ if (!LHSVal.getAs()) {
+LHSVal = Mgr.getStoreManager().getBinding(State->getStore(),
+ LHSVal.castAs());
+if (LHSVal.isUnknownOrUndef() || !LHSVal.getAs())
+ return false;
+ }
+
+ SValBuilder &Bldr = Mgr.getSValBuilder();
+ SVal Eval = Bldr.evalBinOp(State, ComparisonOp, LHSVal, RHSVal,
+ Bldr.getConditionType());
+ if (Eval.isUnknownOrUndef())
+return false;
+ ProgramStateRef StTrue, StFalse;
+ std::tie(StTrue, StFalse) = State->assume(Eval.castAs());
+ return StTrue && !StFalse;
+}
+
+bool CheckerContext::isGreaterOrEqual(const Expr *E, unsigned long long Val) {
+ DefinedSVal V = getSValBuilder().makeIntVal(Val, getASTContext().LongLongTy);
+ return evalComparison(getSVal(E), BO_GE, V, getState());
+}
+
+bool CheckerContext::isNegative(const Expr *E) {
+ DefinedSVal V = getSValBuilder().makeIntVal(0, false);
+ return evalComparison(getSVal(E), BO_LT, V, getState());
+}
Index: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
@@ -59,6 +59,26 @@
return StOutBound && !StInBound;
}
+static bool isShiftOverflow(const BinaryOperator *B, CheckerContext &C) {
+ return C.isGreaterOrEqual(
+ B->getRHS(), C.getASTContext().getIntWidth(B->getLHS()->getType()));
+}
+
+static const llvm::APSInt *getConcreteValue(const Expr *E, CheckerContext &C) {
+ SVal V = C.getSVal(E);
+ if (!V.getAs()) {
+ProgramStateRef State = C.getState();
+ProgramStateManager &Mgr = State->getStateManager();
+V = Mgr.getStoreManager().getBinding(State->getStore(), V.castAs());
+ }
+ if (V.getSubKind() == nonloc::ConcreteIntKind) {
+const auto &CI = V.castAs().castAs();
+const llvm::APSInt &I = CI.getValue();
+return &I;
+ }
+ return nullptr;
+}
+
void UndefResultChecker::checkPostStmt(const BinaryOperator *B,
CheckerContext &C) const {
ProgramStateRef state = C.getState();
@@ -97,18 +117,44 @@
}
if (Ex) {
- OS << "The " << (isLeft ? "left" : "right")
- << " operand of '"
+ OS << "The " << (isLeft ? "left" : "right") << " operand of '"
<< BinaryOperator::getOpcodeStr(B->getOpcode())
<< "' is a garbage value";
if (isArrayIndexOutOfBounds(C, Ex))
OS << " due to array index out of bounds";
-}
-else {
+} else {
// Neither operand was undefined, but the result is undefined.
- OS << "The result of the '"
[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width
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 >= width of type";
+ } else {
zaks.anna wrote:
> It's best not to use ">=" in diagnostic messages.
> Suggestions: "due to shift count >= width of type" ->
> - "due to shifting by a value larger than the width of type"
> - "due to shifting by 5, which is larger than the width of type 'int'" //
> Providing the exact value and the type would be very useful and this
> information is readily available to us. Note that the users might not see the
> type or the value because of macros and such.
I used "due to shifting by 5, which is larger than the width of type 'int'"
However I did not see an easy way to show the exact value. So I added
getConcreteValue(). Maybe you have a better suggestion. If it's a ConcreteInt I
show the exact value, but if it's some range etc then I write "due to shifting
by a value that is larger..." instead.
The message "due to shifting by 64, which is larger than the width of type
'unsigned long long'" is a bit weird imho. Because 64 is not larger than the
width. Not sure how this can be rephrazed better though.
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
[PATCH] D36670: misc-misplaced-widening-cast: fix assertion
This revision was automatically updated to reflect the committed changes.
Closed by commit rL311984: [clang-tidy] Fix 'misc-misplaced-widening-cast'
assertion error. (authored by danielmarjamaki).
Changed prior to commit:
https://reviews.llvm.org/D36670?vs=110940&id=113026#toc
Repository:
rL LLVM
https://reviews.llvm.org/D36670
Files:
clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp
Index:
clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp
===
---
clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp
+++
clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp
@@ -56,3 +56,9 @@
return (long)a * 1000;
}
}
+
+// Shall not generate an assert. https://bugs.llvm.org/show_bug.cgi?id=33660
+template class A {
+ enum Type {};
+ static char *m_fn1() { char p = (Type)(&p - m_fn1()); }
+};
Index: clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.cpp
@@ -192,6 +192,10 @@
if (Calc->getLocStart().isMacroID())
return;
+ if (Cast->isTypeDependent() || Cast->isValueDependent() ||
+ Calc->isTypeDependent() || Calc->isValueDependent())
+return;
+
ASTContext &Context = *Result.Context;
QualType CastType = Cast->getType();
Index: clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp
@@ -56,3 +56,9 @@
return (long)a * 1000;
}
}
+
+// Shall not generate an assert. https://bugs.llvm.org/show_bug.cgi?id=33660
+template class A {
+ enum Type {};
+ static char *m_fn1() { char p = (Type)(&p - m_fn1()); }
+};
Index: clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/misc/MisplacedWideningCastCheck.cpp
@@ -192,6 +192,10 @@
if (Calc->getLocStart().isMacroID())
return;
+ if (Cast->isTypeDependent() || Cast->isValueDependent() ||
+ Calc->isTypeDependent() || Calc->isValueDependent())
+return;
+
ASTContext &Context = *Result.Context;
QualType CastType = Cast->getType();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width
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
[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis
danielmarjamaki added inline comments. Comment at: tools/scan-build-py/libscanbuild/analyze.py:165 +with open(filename, 'r') as in_file: +for line in in_file: +yield line I believe you can write: for line in open(filename, 'r'): Comment at: tools/scan-build-py/libscanbuild/analyze.py:172 +extern_fns_map_file = os.path.join(ctudir, CTU_FUNCTION_MAP_FILENAME) +with open(extern_fns_map_file, 'w') as out_file: +for mangled_name, ast_file in mangled_ast_pairs: this 'with' seems redundant. I suggest an assignment and then less indentation will be needed below Comment at: tools/scan-build-py/libscanbuild/analyze.py:223 +ctu_config = get_ctu_config(args) +if ctu_config.collect: +shutil.rmtree(ctu_config.dir, ignore_errors=True) not a big deal but I would use early exits in this function Comment at: tools/scan-build-py/libscanbuild/clang.py:177 +arch = "" +i = 0 +while i < len(cmd) and cmd[i] != "-triple": I am guessing that you can use cmd.find() instead of the loop https://reviews.llvm.org/D30691 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D34512: Add preliminary Cross Translation Unit support library
danielmarjamaki added a comment.
small nits
Comment at: include/clang/CrossTU/CrossTranslationUnit.h:58
+
+/// \brief This function can parse an index file that determines which
+///translation unit contains which definition.
I suggest that "can" is removed.
Comment at: include/clang/CrossTU/CrossTranslationUnit.h:62
+/// The index file format is the following:
+/// each line consists of an USR separated by a filepath.
+llvm::Expected>
there is no \return or \returns here.
Comment at: include/clang/CrossTU/CrossTranslationUnit.h:62
+/// The index file format is the following:
+/// each line consists of an USR separated by a filepath.
+llvm::Expected>
danielmarjamaki wrote:
> there is no \return or \returns here.
maybe: each line consists of an USR and a filepath separated by a space
Comment at: include/clang/CrossTU/CrossTranslationUnit.h:68
+
+/// \brief This class can be used for tools that requires cross translation
+///unit capability.
Maybe /can be/is/ unless you envision that tools that require cross translation
unit capability might use some other classes.
Comment at: include/clang/CrossTU/CrossTranslationUnit.h:92
+ /// definition of the function will be merged into the original AST using
+ /// the AST Importer. The declaration with the definition will be returned.
+ /// If no suitable definition is found in the index file, null will be
you should split out some of this information to a \return or \returns
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:60
+
+char IndexError::ID = 0;
+
redundant assignment
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:79
+ std::string Line;
+ unsigned LineNo = 0;
+ while (std::getline(ExternalFnMapFile, Line)) {
I suggest that LineNo is 1 on the first line.
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:81
+ while (std::getline(ExternalFnMapFile, Line)) {
+size_t Pos = Line.find(" ");
+StringRef LineRef{Line};
Pos can be const
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:82
+size_t Pos = Line.find(" ");
+StringRef LineRef{Line};
+if (Pos > 0 && Pos != std::string::npos) {
LineRef can be const
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:84
+if (Pos > 0 && Pos != std::string::npos) {
+ FunctionName = LineRef.substr(0, Pos);
+ if (Result.count(FunctionName))
FunctionName and FileName can be moved here and it is possible to make these
variables const.
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:85
+ FunctionName = LineRef.substr(0, Pos);
+ if (Result.count(FunctionName))
+return llvm::make_error(
I would like to see some FunctionName validation. For instance "123" should not
be a valid function name.
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:89
+ FileName = LineRef.substr(Pos + 1);
+ SmallString<256> FilePath = CrossTUDir;
+ llvm::sys::path::append(FilePath, FileName);
Stupid question .. how will this work if the path is longer than 256 bytes?
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:102
+createCrossTUIndexString(const llvm::StringMap &Index) {
+ std::stringstream Result;
+ for (const auto &E : Index) {
how about std::ostringstream , imho that is cleaner
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:121
+
+/// Recursively visits the funtion decls of a DeclContext, and looks up a
+/// function based on USRs.
/funtion/function/
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:125
+CrossTranslationUnitContext::findFunctionInDeclContext(const DeclContext *DC,
+ StringRef LookupFnName)
{
+ assert(DC && "Declaration Context must not be null");
LookupFnName could be const right?
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:148
+
+ std::string LookupFnName = getLookupName(FD);
+ if (LookupFnName.empty())
I believe LookupFnName can be const
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:189
+ return nullptr; // No definition found even in some other build unit.
+ASTFileName = It->second;
+auto ASTCacheEntry = FileASTUnitMap.find(ASTFileName);
It is possible to make ASTFileName const
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:223
+assert(ToDecl->hasBody());
+assert(FD->hasBody() && "Functions already imported should have body.");
+return ToDecl;
sorry I am probably missing something here.. you first assert !FD->hasBod
[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis
danielmarjamaki accepted this revision.
danielmarjamaki added inline comments.
This revision is now accepted and ready to land.
Comment at: tools/scan-build-py/libscanbuild/analyze.py:165
+with open(filename, 'r') as in_file:
+for line in in_file:
+yield line
whisperity wrote:
> danielmarjamaki wrote:
> > I believe you can write:
> >
> > for line in open(filename, 'r'):
> Do we want to rely on the interpreter implementation on when the file is
> closed.
>
> If
>
> ```
> for line in open(filename, 'r'):
> something()
> ```
>
> is used, the file handle will be closed based on garbage collection rules.
> Having this handle disposed after the iteration is true for the stock CPython
> implementation, but it is still nontheless an implementation specific
> approach.
>
> Whereas using `with` will explicitly close the file handle on the spot, no
> matter what.
ok I did not know that. feel free to ignore my comment.
Comment at: tools/scan-build-py/libscanbuild/analyze.py:172
+extern_fns_map_file = os.path.join(ctudir, CTU_FUNCTION_MAP_FILENAME)
+with open(extern_fns_map_file, 'w') as out_file:
+for mangled_name, ast_file in mangled_ast_pairs:
whisperity wrote:
> danielmarjamaki wrote:
> > this 'with' seems redundant. I suggest an assignment and then less
> > indentation will be needed below
> I don't seem to understand what do you want to assign to what.
I did not consider the garbage collection. I assumed that out_file would Always
be closed when it Went out of scope and then this would require less
indentation:
out_file = open(extern_fns_map_file, 'w')
for mangled_name, ast_file in mangled_ast_pairs:
out_file.write('%s %s\n' % (mangled_name, ast_file))
Comment at: tools/scan-build-py/libscanbuild/analyze.py:223
+ctu_config = get_ctu_config(args)
+if ctu_config.collect:
+shutil.rmtree(ctu_config.dir, ignore_errors=True)
danielmarjamaki wrote:
> not a big deal but I would use early exits in this function
with "not a big deal" I mean; feel free to ignore my comment if you want to
have it this way.
https://reviews.llvm.org/D30691
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width
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
[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values
danielmarjamaki updated this revision to Diff 113969.
danielmarjamaki added a comment.
minor code cleanup
Repository:
rL LLVM
https://reviews.llvm.org/D36471
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
lib/StaticAnalyzer/Core/ExprEngineC.cpp
lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
Index: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -304,6 +304,8 @@
void print(ProgramStateRef State, raw_ostream &Out, const char *nl,
const char *sep) override;
+ ProgramStateRef evalRangeOp(ProgramStateRef state, SVal V) override;
+
//===--===//
// Implementation for interface from RangedConstraintManager.
//===--===//
@@ -741,3 +743,56 @@
}
Out << nl;
}
+
+ProgramStateRef RangeConstraintManager::evalRangeOp(ProgramStateRef St,
+SVal V) {
+ const SymExpr *SE = V.getAsSymExpr();
+ if (!SE)
+return nullptr;
+
+ const SymIntExpr *SIE = dyn_cast(SE);
+ if (!SIE)
+return nullptr;
+
+ const clang::BinaryOperatorKind Opc = SIE->getOpcode();
+
+ if (Opc != BO_Add && Opc != BO_Sub && Opc != BO_Div)
+return nullptr;
+
+ const SymExpr *LHS = SIE->getLHS();
+ const llvm::APSInt &RHS = SIE->getRHS();
+
+ ConstraintRangeTy Ranges = St->get();
+ for (ConstraintRangeTy::iterator I = Ranges.begin(), E = Ranges.end(); I != E;
+ ++I) {
+if (LHS != I.getKey())
+ continue;
+const auto D = I.getData();
+for (auto I = D.begin(); I != D.end(); ++I) {
+ if (I->From().isUnsigned() != RHS.isUnsigned())
+// TODO: Handle sign conversions.
+return nullptr;
+ if (I->From().getBitWidth() != RHS.getBitWidth())
+// TODO: Promote values.
+return nullptr;
+ if (I->From().isNegative())
+// TODO: Handle negative range values
+return nullptr;
+
+ BasicValueFactory &BVF = getBasicVals();
+ const llvm::APSInt *Lower = BVF.evalAPSInt(Opc, I->From(), RHS);
+ if (!Lower)
+return nullptr;
+ const llvm::APSInt *Upper = BVF.evalAPSInt(Opc, I->To(), RHS);
+ if (!Upper)
+return nullptr;
+
+ SymbolRef Sym = V.getAsSymbol();
+ RangeSet RS =
+ getRange(St, Sym).Intersect(getBasicVals(), F, *Lower, *Upper);
+ // TODO: This only evaluates the first range. Evaluate all ranges.
+ return RS.isEmpty() ? nullptr : St->set(Sym, RS);
+}
+ }
+ return nullptr;
+}
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -98,7 +98,9 @@
}
state = state->BindExpr(B, LCtx, Result);
- Bldr.generateNode(B, *it, state);
+ ProgramStateRef state2 =
+ getConstraintManager().evalRangeOp(state, Result);
+ Bldr.generateNode(B, *it, state2 ? state2 : state);
continue;
}
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
@@ -142,13 +142,15 @@
/// Scan all symbols referenced by the constraints. If the symbol is not
/// alive, remove it.
virtual ProgramStateRef removeDeadBindings(ProgramStateRef state,
- SymbolReaper& SymReaper) = 0;
+ SymbolReaper &SymReaper) = 0;
- virtual void print(ProgramStateRef state,
- raw_ostream &Out,
- const char* nl,
+ virtual void print(ProgramStateRef state, raw_ostream &Out, const char *nl,
const char *sep) = 0;
+ virtual ProgramStateRef evalRangeOp(ProgramStateRef state, SVal V) {
+return nullptr;
+ }
+
virtual void EndPath(ProgramStateRef state) {}
/// Convenience method to query the state to see if a symbol is null or
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28148: [Sema] Suppress warnings for C's zero initializer
danielmarjamaki added a comment. This is not committed as far as I see.. do you have write permission or do you want that I commit it? https://reviews.llvm.org/D28148 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width
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
[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values
danielmarjamaki added a comment. ping Repository: rL LLVM https://reviews.llvm.org/D36471 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written
danielmarjamaki created this revision.
I saw a false positive where the analyzer made wrong conclusions about a static
variable.
Static variables that are not written have known values (initialized values).
This is the (simplified) code that motivated me to create this patch:
static char *allv[] = {
"rpcgen", "-s", "udp", "-s", "tcp",
};
static int allc = sizeof(allv) / sizeof(allv[0]);
static void f(void) {
int i;
for (i = 1; i < allc; i++) {
const char *p = allv[i]; // <- line 28
i++;
}
}
Clang output:
array-fp3.c:28:19: warning: Access out-of-bound array element (buffer
overflow)
const char *p = allv[i];
^~~
I added testcases that shows this patch solves both false positives and false
negatives
Repository:
rL LLVM
https://reviews.llvm.org/D37897
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/ExprEngine.cpp
test/Analysis/global-vars.c
Index: test/Analysis/global-vars.c
===
--- test/Analysis/global-vars.c
+++ test/Analysis/global-vars.c
@@ -0,0 +1,18 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha,core -verify %s
+
+// Avoid false positive.
+static char *allv[] = {"rpcgen", "-s", "udp", "-s", "tcp"};
+static int allc = sizeof(allv) / sizeof(allv[0]);
+static void falsePositive1(void) {
+ int i;
+ for (i = 1; i < allc; i++) {
+const char *p = allv[i]; // no-warning
+i++;
+ }
+}
+
+// Detect division by zero.
+static int zero = 0;
+void truePositive1() {
+ int x = 1000 / zero; // expected-warning{{Division by zero}}
+}
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -103,8 +103,106 @@
// Utility methods.
//===--===//
+static bool isChanged(const Stmt *S, const VarDecl *VD, bool W) {
+ if (!S)
+return false;
+ if (const BinaryOperator *B = dyn_cast(S)) {
+if (B->isAssignmentOp())
+ return isChanged(B->getLHS(), VD, true) || isChanged(B->getRHS(), VD, W);
+ } else if (const UnaryOperator *U = dyn_cast(S)) {
+if (U->getOpcode() == UO_AddrOf || U->getOpcode() == UO_PreInc ||
+U->getOpcode() == UO_PreDec || U->getOpcode() == UO_PostInc ||
+U->getOpcode() == UO_PostDec)
+ return isChanged(U->getSubExpr(), VD, true);
+ } else if (const DeclRefExpr *D = dyn_cast(S)) {
+return W && D->getDecl() == VD;
+ }
+
+ for (const Stmt *Child : S->children()) {
+if (isChanged(Child, VD, W))
+ return true;
+ }
+ return false;
+}
+
+static bool isChanged(const Decl *D, const VarDecl *VD) {
+ if (isa(D))
+return isChanged(D->getBody(), VD, false);
+ if (const auto *Rec = dyn_cast(D)) {
+for (const auto *RecChild : Rec->decls()) {
+ if (isChanged(RecChild, VD))
+return true;
+}
+ }
+ return false;
+}
+
+ProgramStateRef ExprEngine::getInitialState(const LocationContext *LCtx,
+ProgramStateRef State,
+const VarDecl *VD) {
+ // Is variable used in location context?
+ const FunctionDecl *FD = dyn_cast(LCtx->getDecl());
+ if (!FD)
+return State;
+
+ // Is variable changed anywhere in TU?
+ for (const Decl *D : AMgr.getASTContext().getTranslationUnitDecl()->decls()) {
+if (isChanged(D, VD))
+ return State;
+ }
+
+ // What is the initialized value?
+ llvm::APSInt InitVal;
+ if (const Expr *I = VD->getInit()) {
+if (!I->EvaluateAsInt(InitVal, getContext()))
+ return State;
+ } else {
+InitVal = 0;
+ }
+
+ const MemRegion *R = State->getRegion(VD, LCtx);
+ if (!R)
+return State;
+ SVal V = State->getSVal(loc::MemRegionVal(R));
+ SVal Constraint_untested =
+ evalBinOp(State, BO_EQ, V, svalBuilder.makeIntVal(InitVal),
+svalBuilder.getConditionType());
+ Optional Constraint =
+ Constraint_untested.getAs();
+ if (!Constraint)
+return State;
+ return State->assume(*Constraint, true);
+}
+
+static void getStaticVars(const Stmt *S,
+ llvm::SmallSet *Vars) {
+ if (!S)
+return;
+ if (const DeclRefExpr *D = dyn_cast(S)) {
+const VarDecl *VD = dyn_cast(D->getDecl());
+if (VD && VD->isDefinedOutsideFunctionOrMethod() &&
+VD->getType()->isIntegerType() && VD->getStorageClass() == SC_Static &&
+!VD->getType()->isPointerType()) {
+ Vars->insert(VD);
+}
+ }
+ for (const Stmt *Child : S->children()) {
+getStaticVars(Child, Vars);
+ }
+}
+
ProgramStateRef ExprEngine::getInitialState(const LocationContext *InitLoc) {
ProgramStateRef state = StateMgr.getInitialState(InitLoc);
+
+ // Get initial states for static global variab
[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written
danielmarjamaki updated this revision to Diff 115385.
danielmarjamaki added a comment.
Minor cleanups. Changed names. Updated comments.
Repository:
rL LLVM
https://reviews.llvm.org/D37897
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
lib/StaticAnalyzer/Core/ExprEngine.cpp
test/Analysis/global-vars.c
Index: test/Analysis/global-vars.c
===
--- test/Analysis/global-vars.c
+++ test/Analysis/global-vars.c
@@ -0,0 +1,18 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=alpha,core -verify %s
+
+// Avoid false positive.
+static char *allv[] = {"rpcgen", "-s", "udp", "-s", "tcp"};
+static int allc = sizeof(allv) / sizeof(allv[0]);
+static void falsePositive1(void) {
+ int i;
+ for (i = 1; i < allc; i++) {
+const char *p = allv[i]; // no-warning
+i++;
+ }
+}
+
+// Detect division by zero.
+static int zero = 0;
+void truePositive1() {
+ int x = 1000 / zero; // expected-warning{{Division by zero}}
+}
Index: lib/StaticAnalyzer/Core/ExprEngine.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngine.cpp
+++ lib/StaticAnalyzer/Core/ExprEngine.cpp
@@ -103,8 +103,107 @@
// Utility methods.
//===--===//
+/** Recursively check if variable is changed in code. */
+static bool isChanged(const Stmt *S, const VarDecl *VD, bool Write) {
+ if (!S)
+return false;
+ if (const BinaryOperator *B = dyn_cast(S)) {
+if (B->isAssignmentOp())
+ return isChanged(B->getLHS(), VD, true) ||
+ isChanged(B->getRHS(), VD, Write);
+ } else if (const UnaryOperator *U = dyn_cast(S)) {
+// Operators that mean operand is written.
+// AddrOf is here because the address might be used to write the operand
+// later indirectly.
+if (U->getOpcode() == UO_AddrOf || U->getOpcode() == UO_PreInc ||
+U->getOpcode() == UO_PreDec || U->getOpcode() == UO_PostInc ||
+U->getOpcode() == UO_PostDec)
+ return isChanged(U->getSubExpr(), VD, true);
+ } else if (const DeclRefExpr *D = dyn_cast(S)) {
+return Write && D->getDecl() == VD;
+ }
+
+ for (const Stmt *Child : S->children()) {
+if (isChanged(Child, VD, Write))
+ return true;
+ }
+ return false;
+}
+
+/** Is variable changed in function or method? */
+static bool isChanged(const Decl *D, const VarDecl *VD) {
+ if (isa(D))
+return isChanged(D->getBody(), VD, false);
+ if (const auto *Rec = dyn_cast(D)) {
+for (const auto *RecChild : Rec->decls()) {
+ if (isChanged(RecChild, VD))
+return true;
+}
+ }
+ return false;
+}
+
+/** Get initial state for global static variable */
+ProgramStateRef ExprEngine::getInitialStateForGlobalStaticVar(
+const LocationContext *LCtx, ProgramStateRef State, const VarDecl *VD) {
+ // Is variable changed anywhere in TU?
+ for (const Decl *D : AMgr.getASTContext().getTranslationUnitDecl()->decls()) {
+if (isChanged(D, VD))
+ return State;
+ }
+
+ // What is the initialized value?
+ llvm::APSInt InitVal;
+ if (const Expr *I = VD->getInit()) {
+if (!I->EvaluateAsInt(InitVal, getContext()))
+ return State;
+ } else {
+InitVal = 0;
+ }
+
+ const MemRegion *R = State->getRegion(VD, LCtx);
+ if (!R)
+return State;
+ SVal V = State->getSVal(loc::MemRegionVal(R));
+ SVal Constraint_untested =
+ evalBinOp(State, BO_EQ, V, svalBuilder.makeIntVal(InitVal),
+svalBuilder.getConditionType());
+ Optional Constraint =
+ Constraint_untested.getAs();
+ if (!Constraint)
+return State;
+ return State->assume(*Constraint, true);
+}
+
+static void getGlobalStaticVars(const Stmt *S,
+llvm::SmallSet *Vars) {
+ if (!S)
+return;
+ if (const DeclRefExpr *D = dyn_cast(S)) {
+const VarDecl *VD = dyn_cast(D->getDecl());
+if (VD && VD->isDefinedOutsideFunctionOrMethod() &&
+VD->getType()->isIntegerType() && VD->getStorageClass() == SC_Static &&
+!VD->getType()->isPointerType()) {
+ Vars->insert(VD);
+}
+ }
+ for (const Stmt *Child : S->children()) {
+getStaticVars(Child, Vars);
+ }
+}
+
ProgramStateRef ExprEngine::getInitialState(const LocationContext *InitLoc) {
ProgramStateRef state = StateMgr.getInitialState(InitLoc);
+
+ // Get initial states for static global variables.
+ if (const auto *FD = dyn_cast(InitLoc->getDecl())) {
+llvm::SmallSet Vars;
+getGlobalStaticVars(FD->getBody(), &Vars);
+for (const VarDecl *VD : Vars) {
+ state = getInitialState(InitLoc, state, VD);
+}
+ }
+
const Decl *D = InitLoc->getDecl();
// Preconditions.
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine
[PATCH] D37897: [StaticAnalyzer] Fix ProgramState for static variables that are not written
danielmarjamaki added a comment. In https://reviews.llvm.org/D37897#871858, @xazax.hun wrote: > Out of curiosity, does the false positive disappear after making the static > variables const? Yes that fixes the false positive Repository: rL LLVM https://reviews.llvm.org/D37897 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36141: [StaticAnalyzer] Fix FP in UnreachableCodeChecker
danielmarjamaki created this revision.
Herald added a subscriber: whisperity.
This fixes a FP. Without the fix, the checker says that "static int x;" is
unreachable.
Repository:
rL LLVM
https://reviews.llvm.org/D36141
Files:
lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
test/Analysis/unreachable-code-path.c
Index: test/Analysis/unreachable-code-path.c
===
--- test/Analysis/unreachable-code-path.c
+++ test/Analysis/unreachable-code-path.c
@@ -213,3 +213,13 @@
RETURN(1); // no-warning
}
+// Avoid FP when macro argument is known
+void writeSomething(int *x);
+#define MACRO(C)\
+ if (!C) { \
+static int x; \
+writeSomething(&x); \
+ }
+void macro2(void) {
+ MACRO(1);
+}
Index: lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
@@ -112,7 +112,7 @@
continue;
// Check for false positives
-if (CB->size() > 0 && isInvalidPath(CB, *PM))
+if (isInvalidPath(CB, *PM))
continue;
// It is good practice to always have a "default" label in a "switch", even
Index: test/Analysis/unreachable-code-path.c
===
--- test/Analysis/unreachable-code-path.c
+++ test/Analysis/unreachable-code-path.c
@@ -213,3 +213,13 @@
RETURN(1); // no-warning
}
+// Avoid FP when macro argument is known
+void writeSomething(int *x);
+#define MACRO(C)\
+ if (!C) { \
+static int x; \
+writeSomething(&x); \
+ }
+void macro2(void) {
+ MACRO(1);
+}
Index: lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
@@ -112,7 +112,7 @@
continue;
// Check for false positives
-if (CB->size() > 0 && isInvalidPath(CB, *PM))
+if (isInvalidPath(CB, *PM))
continue;
// It is good practice to always have a "default" label in a "switch", even
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36141: [StaticAnalyzer] Fix FP in UnreachableCodeChecker
This revision was automatically updated to reflect the committed changes.
Closed by commit rL309799: [StaticAnalyzer] Fix false positives for unreachable
code in macros. (authored by danielmarjamaki).
Changed prior to commit:
https://reviews.llvm.org/D36141?vs=109086&id=109294#toc
Repository:
rL LLVM
https://reviews.llvm.org/D36141
Files:
cfe/trunk/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
cfe/trunk/test/Analysis/unreachable-code-path.c
Index: cfe/trunk/test/Analysis/unreachable-code-path.c
===
--- cfe/trunk/test/Analysis/unreachable-code-path.c
+++ cfe/trunk/test/Analysis/unreachable-code-path.c
@@ -213,3 +213,13 @@
RETURN(1); // no-warning
}
+// Avoid FP when macro argument is known
+void writeSomething(int *x);
+#define MACRO(C)\
+ if (!C) { \
+static int x; \
+writeSomething(&x); \
+ }
+void macro2(void) {
+ MACRO(1);
+}
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
@@ -112,7 +112,7 @@
continue;
// Check for false positives
-if (CB->size() > 0 && isInvalidPath(CB, *PM))
+if (isInvalidPath(CB, *PM))
continue;
// It is good practice to always have a "default" label in a "switch", even
Index: cfe/trunk/test/Analysis/unreachable-code-path.c
===
--- cfe/trunk/test/Analysis/unreachable-code-path.c
+++ cfe/trunk/test/Analysis/unreachable-code-path.c
@@ -213,3 +213,13 @@
RETURN(1); // no-warning
}
+// Avoid FP when macro argument is known
+void writeSomething(int *x);
+#define MACRO(C)\
+ if (!C) { \
+static int x; \
+writeSomething(&x); \
+ }
+void macro2(void) {
+ MACRO(1);
+}
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/UnreachableCodeChecker.cpp
@@ -112,7 +112,7 @@
continue;
// Check for false positives
-if (CB->size() > 0 && isInvalidPath(CB, *PM))
+if (isInvalidPath(CB, *PM))
continue;
// It is good practice to always have a "default" label in a "switch", even
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width
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/CheckerContext.h
lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
lib/StaticAnalyzer/Core/CheckerContext.cpp
test/Analysis/bitwise-ops.c
Index: test/Analysis/bitwise-ops.c
===
--- test/Analysis/bitwise-ops.c
+++ test/Analysis/bitwise-ops.c
@@ -22,11 +22,25 @@
case 1:
return 0ULL << 63; // no-warning
case 2:
-return 0ULL << 64; // expected-warning{{The result of the '<<' expression is undefined}}
+return 0ULL << 64; // expected-warning{{The result of the left shift is undefined due to shifting by 64, which is larger or equal with the width of type 'unsigned long long'}}
case 3:
-return 0ULL << 65; // expected-warning{{The result of the '<<' expression is undefined}}
+return 0ULL << 65; // expected-warning{{The result of the left shift is undefined due to shifting by 65, which is larger or equal with the width of type 'unsigned long long'}}
default:
return 0;
}
-}
\ No newline at end of file
+}
+
+int testOverflowShift(int a) {
+ if (a == 323) {
+return 1 << a; // expected-warning{{The result of the left shift is undefined due to shifting by 323, which is larger or equal with the width of type 'int'}}
+ }
+ return 0;
+}
+
+int testNegativeShift(int a) {
+ if (a == -5) {
+return 1 << a; // expected-warning{{The result of the left shift is undefined because the right operand is negative}}
+ }
+ return 0;
+}
Index: lib/StaticAnalyzer/Core/CheckerContext.cpp
===
--- lib/StaticAnalyzer/Core/CheckerContext.cpp
+++ lib/StaticAnalyzer/Core/CheckerContext.cpp
@@ -99,3 +99,35 @@
return Lexer::getSpelling(Loc, buf, getSourceManager(), getLangOpts());
}
+/// Evaluate comparison and return true if it's known that condition is true
+static bool evalComparison(SVal LHSVal, BinaryOperatorKind ComparisonOp,
+ SVal RHSVal, ProgramStateRef State) {
+ if (LHSVal.isUnknownOrUndef())
+return false;
+ ProgramStateManager &Mgr = State->getStateManager();
+ if (!LHSVal.getAs()) {
+LHSVal = Mgr.getStoreManager().getBinding(State->getStore(),
+ LHSVal.castAs());
+if (LHSVal.isUnknownOrUndef() || !LHSVal.getAs())
+ return false;
+ }
+
+ SValBuilder &Bldr = Mgr.getSValBuilder();
+ SVal Eval = Bldr.evalBinOp(State, ComparisonOp, LHSVal, RHSVal,
+ Bldr.getConditionType());
+ if (Eval.isUnknownOrUndef())
+return false;
+ ProgramStateRef StTrue, StFalse;
+ std::tie(StTrue, StFalse) = State->assume(Eval.castAs());
+ return StTrue && !StFalse;
+}
+
+bool CheckerContext::isGreaterOrEqual(const Expr *E, unsigned long long Val) {
+ DefinedSVal V = getSValBuilder().makeIntVal(Val, getASTContext().LongLongTy);
+ return evalComparison(getSVal(E), BO_GE, V, getState());
+}
+
+bool CheckerContext::isNegative(const Expr *E) {
+ DefinedSVal V = getSValBuilder().makeIntVal(0, false);
+ return evalComparison(getSVal(E), BO_LT, V, getState());
+}
Index: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
@@ -59,6 +59,11 @@
return StOutBound && !StInBound;
}
+static bool isShiftOverflow(const BinaryOperator *B, CheckerContext &C) {
+ return C.isGreaterOrEqual(
+ B->getRHS(), C.getASTContext().getIntWidth(B->getLHS()->getType()));
+}
+
void UndefResultChecker::checkPostStmt(const BinaryOperator *B,
CheckerContext &C) const {
ProgramStateRef state = C.getState();
@@ -97,18 +102,46 @@
}
if (Ex) {
- OS << "The " << (isLeft ? "left" : "right")
- << " operand of '"
+ OS << "The " << (isLeft ? "left" : "right") << " operand of '"
<< BinaryOperator::getOpcodeStr(B->getOpcode())
<< "' is a garbage value";
if (isArrayIndexOutOfBounds(C, Ex))
OS << " due to array index out of bounds";
-}
-else {
+} else {
// Neither operand was undefined, but the result is undefined.
- OS << "The result of the '"
- << BinaryOperator::getOpcodeStr(B->getOpcode())
- << "' expression is undefined";
+ if ((B->getOpcode() == BinaryOperatorKind::BO_Shl ||
+ B->getOpcode() == BinaryOperatorKind::BO_Shr) &&
+ C.isNegative(B->getRHS())) {
+OS << "The result of the "
+ << ((B->getOpcode() == BinaryOperatorKind::BO_Shl) ? "left"
+
[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values
danielmarjamaki created this revision.
In the code below the division result should be a value between 5 and 25.
if (a >= 10 && a <= 50) {
int b = a / 2;
}
This patch will calculate results for additions, subtractions and divisions.
I intentionally do not try to handle all possible cases that can be handled. I
want to know if my approach is ok.
Repository:
rL LLVM
https://reviews.llvm.org/D36471
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
lib/StaticAnalyzer/Core/ExprEngineC.cpp
lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
test/Analysis/eval-range.c
Index: test/Analysis/eval-range.c
===
--- test/Analysis/eval-range.c
+++ test/Analysis/eval-range.c
@@ -0,0 +1,18 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
+
+void test1(int a) {
+ if (a >= 10 && a <= 50) {
+int b;
+
+b = a + 2;
+clang_analyzer_eval(b >= 12 && b <= 52); // expected-warning{{TRUE}}
+
+b = a - 2;
+clang_analyzer_eval(b >= 8 && b <= 48); // expected-warning{{TRUE}}
+
+b = a / 2;
+clang_analyzer_eval(b >= 5 && b <= 25); // expected-warning{{TRUE}}
+ }
+}
Index: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -304,6 +304,8 @@
void print(ProgramStateRef State, raw_ostream &Out, const char *nl,
const char *sep) override;
+ ProgramStateRef evalRangeOp(ProgramStateRef state, SVal V) override;
+
//===--===//
// Implementation for interface from RangedConstraintManager.
//===--===//
@@ -741,3 +743,65 @@
}
Out << nl;
}
+
+ProgramStateRef RangeConstraintManager::evalRangeOp(ProgramStateRef St,
+SVal V) {
+ const SymExpr *SE = V.getAsSymExpr();
+ if (!SE)
+return nullptr;
+
+ const SymIntExpr *SIE = dyn_cast(SE);
+ if (!SIE)
+return nullptr;
+
+ const clang::BinaryOperatorKind Opc = SIE->getOpcode();
+
+ if (Opc != BO_Add && Opc != BO_Sub && Opc != BO_Div)
+return nullptr;
+
+ const SymExpr *LHS = SIE->getLHS();
+ const llvm::APSInt &RHS = SIE->getRHS();
+
+ if (RHS.isNegative())
+// TODO: Handle negative values.
+return nullptr;
+
+ ConstraintRangeTy Ranges = St->get();
+ for (ConstraintRangeTy::iterator I = Ranges.begin(), E = Ranges.end(); I != E;
+ ++I) {
+if (LHS == I.getKey()) {
+ const auto D = I.getData();
+ for (auto I = D.begin(); I != D.end(); ++I) {
+if (I->From().isUnsigned() != RHS.isUnsigned())
+ // TODO: Handle sign conversions.
+ return nullptr;
+if (I->From().getBitWidth() != RHS.getBitWidth())
+ // TODO: Promote values.
+ return nullptr;
+if (I->From().isNegative())
+ // TODO: Handle negative range values
+ return nullptr;
+llvm::APSInt Lower;
+llvm::APSInt Upper;
+if (Opc == BO_Add) {
+ Lower = I->From() + RHS;
+ Upper = I->To() + RHS;
+} else if (Opc == BO_Sub) {
+ if (RHS > I->From())
+return nullptr;
+ Lower = I->From() - RHS;
+ Upper = I->To() - RHS;
+} else if (Opc == BO_Div) {
+ Lower = I->From() / RHS;
+ Upper = I->To() / RHS;
+}
+SymbolRef Sym = V.getAsSymbol();
+RangeSet RS =
+getRange(St, Sym).Intersect(getBasicVals(), F, Lower, Upper);
+// TODO: This only evaluates the first range. Evaluate all ranges.
+return RS.isEmpty() ? nullptr : St->set(Sym, RS);
+ }
+}
+ }
+ return nullptr;
+}
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -98,6 +98,14 @@
}
state = state->BindExpr(B, LCtx, Result);
+
+ {
+ProgramStateRef St2 = getConstraintManager().evalRangeOp(state, Result);
+if (St2) {
+ Bldr.generateNode(B, *it, St2);
+ continue;
+}
+ }
Bldr.generateNode(B, *it, state);
continue;
}
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
@@ -142,13 +142,15 @@
/// Scan all symbols referenced by the constraints. If the symbol is not
/// alive, remove it.
virtual ProgramStateRef removeDeadBindings(ProgramStateRef state,
-
[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values
danielmarjamaki updated this revision to Diff 110220.
danielmarjamaki added a comment.
A minor code cleanup. No functional change.
Repository:
rL LLVM
https://reviews.llvm.org/D36471
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
lib/StaticAnalyzer/Core/ExprEngineC.cpp
lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
test/Analysis/eval-range.c
Index: test/Analysis/eval-range.c
===
--- test/Analysis/eval-range.c
+++ test/Analysis/eval-range.c
@@ -0,0 +1,18 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
+
+void test1(int a) {
+ if (a >= 10 && a <= 50) {
+int b;
+
+b = a + 2;
+clang_analyzer_eval(b >= 12 && b <= 52); // expected-warning{{TRUE}}
+
+b = a - 2;
+clang_analyzer_eval(b >= 8 && b <= 48); // expected-warning{{TRUE}}
+
+b = a / 2;
+clang_analyzer_eval(b >= 5 && b <= 25); // expected-warning{{TRUE}}
+ }
+}
Index: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -304,6 +304,8 @@
void print(ProgramStateRef State, raw_ostream &Out, const char *nl,
const char *sep) override;
+ ProgramStateRef evalRangeOp(ProgramStateRef state, SVal V) override;
+
//===--===//
// Implementation for interface from RangedConstraintManager.
//===--===//
@@ -741,3 +743,65 @@
}
Out << nl;
}
+
+ProgramStateRef RangeConstraintManager::evalRangeOp(ProgramStateRef St,
+SVal V) {
+ const SymExpr *SE = V.getAsSymExpr();
+ if (!SE)
+return nullptr;
+
+ const SymIntExpr *SIE = dyn_cast(SE);
+ if (!SIE)
+return nullptr;
+
+ const clang::BinaryOperatorKind Opc = SIE->getOpcode();
+
+ if (Opc != BO_Add && Opc != BO_Sub && Opc != BO_Div)
+return nullptr;
+
+ const SymExpr *LHS = SIE->getLHS();
+ const llvm::APSInt &RHS = SIE->getRHS();
+
+ if (RHS.isNegative())
+// TODO: Handle negative values.
+return nullptr;
+
+ ConstraintRangeTy Ranges = St->get();
+ for (ConstraintRangeTy::iterator I = Ranges.begin(), E = Ranges.end(); I != E;
+ ++I) {
+if (LHS == I.getKey()) {
+ const auto D = I.getData();
+ for (auto I = D.begin(); I != D.end(); ++I) {
+if (I->From().isUnsigned() != RHS.isUnsigned())
+ // TODO: Handle sign conversions.
+ return nullptr;
+if (I->From().getBitWidth() != RHS.getBitWidth())
+ // TODO: Promote values.
+ return nullptr;
+if (I->From().isNegative())
+ // TODO: Handle negative range values
+ return nullptr;
+llvm::APSInt Lower;
+llvm::APSInt Upper;
+if (Opc == BO_Add) {
+ Lower = I->From() + RHS;
+ Upper = I->To() + RHS;
+} else if (Opc == BO_Sub) {
+ if (RHS > I->From())
+return nullptr;
+ Lower = I->From() - RHS;
+ Upper = I->To() - RHS;
+} else if (Opc == BO_Div) {
+ Lower = I->From() / RHS;
+ Upper = I->To() / RHS;
+}
+SymbolRef Sym = V.getAsSymbol();
+RangeSet RS =
+getRange(St, Sym).Intersect(getBasicVals(), F, Lower, Upper);
+// TODO: This only evaluates the first range. Evaluate all ranges.
+return RS.isEmpty() ? nullptr : St->set(Sym, RS);
+ }
+}
+ }
+ return nullptr;
+}
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -98,7 +98,9 @@
}
state = state->BindExpr(B, LCtx, Result);
- Bldr.generateNode(B, *it, state);
+ ProgramStateRef state2 =
+ getConstraintManager().evalRangeOp(state, Result);
+ Bldr.generateNode(B, *it, state2 ? state2 : state);
continue;
}
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
@@ -142,13 +142,15 @@
/// Scan all symbols referenced by the constraints. If the symbol is not
/// alive, remove it.
virtual ProgramStateRef removeDeadBindings(ProgramStateRef state,
- SymbolReaper& SymReaper) = 0;
+ SymbolReaper &SymReaper) = 0;
- virtual void print(ProgramStateRef state,
- raw_ostream &Out,
- const char* nl,
+ vir
[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values
danielmarjamaki added a comment. In https://reviews.llvm.org/D36471#835410, @xazax.hun wrote: > Can't you reuse somehow some machinery already available to evaluate the > arithmetic operators? Those should already handle most of your TODOs and > overflows. Sounds good.. I have not seen that machinery.. I will look around. To me it seems it would be nice if this machinery was builtin in APSInt so I could calculate (x+y) even if x and y did not have the same signedness and that the result would be unsigned. Repository: rL LLVM https://reviews.llvm.org/D36471 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values
danielmarjamaki updated this revision to Diff 110378.
danielmarjamaki added a comment.
Refactoring, use BasicValueFactory::evalAPSInt
Repository:
rL LLVM
https://reviews.llvm.org/D36471
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
lib/StaticAnalyzer/Core/ExprEngineC.cpp
lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
test/Analysis/eval-range.c
Index: test/Analysis/eval-range.c
===
--- test/Analysis/eval-range.c
+++ test/Analysis/eval-range.c
@@ -0,0 +1,18 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
+
+void test1(int a) {
+ if (a >= 10 && a <= 50) {
+int b;
+
+b = a + 2;
+clang_analyzer_eval(b >= 12 && b <= 52); // expected-warning{{TRUE}}
+
+b = a - 2;
+clang_analyzer_eval(b >= 8 && b <= 48); // expected-warning{{TRUE}}
+
+b = a / 2;
+clang_analyzer_eval(b >= 5 && b <= 25); // expected-warning{{TRUE}}
+ }
+}
Index: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -304,6 +304,8 @@
void print(ProgramStateRef State, raw_ostream &Out, const char *nl,
const char *sep) override;
+ ProgramStateRef evalRangeOp(ProgramStateRef state, SVal V) override;
+
//===--===//
// Implementation for interface from RangedConstraintManager.
//===--===//
@@ -741,3 +743,56 @@
}
Out << nl;
}
+
+ProgramStateRef RangeConstraintManager::evalRangeOp(ProgramStateRef St,
+SVal V) {
+ const SymExpr *SE = V.getAsSymExpr();
+ if (!SE)
+return nullptr;
+
+ const SymIntExpr *SIE = dyn_cast(SE);
+ if (!SIE)
+return nullptr;
+
+ const clang::BinaryOperatorKind Opc = SIE->getOpcode();
+
+ if (Opc != BO_Add && Opc != BO_Sub && Opc != BO_Div)
+return nullptr;
+
+ const SymExpr *LHS = SIE->getLHS();
+ const llvm::APSInt &RHS = SIE->getRHS();
+
+ ConstraintRangeTy Ranges = St->get();
+ for (ConstraintRangeTy::iterator I = Ranges.begin(), E = Ranges.end(); I != E;
+ ++I) {
+if (LHS == I.getKey()) {
+ const auto D = I.getData();
+ for (auto I = D.begin(); I != D.end(); ++I) {
+if (I->From().isUnsigned() != RHS.isUnsigned())
+ // TODO: Handle sign conversions.
+ return nullptr;
+if (I->From().getBitWidth() != RHS.getBitWidth())
+ // TODO: Promote values.
+ return nullptr;
+if (I->From().isNegative())
+ // TODO: Handle negative range values
+ return nullptr;
+
+BasicValueFactory &BVF = getBasicVals();
+const llvm::APSInt *Lower = BVF.evalAPSInt(Opc, I->From(), RHS);
+if (!Lower)
+ return nullptr;
+const llvm::APSInt *Upper = BVF.evalAPSInt(Opc, I->To(), RHS);
+if (!Upper)
+ return nullptr;
+
+SymbolRef Sym = V.getAsSymbol();
+RangeSet RS =
+getRange(St, Sym).Intersect(getBasicVals(), F, *Lower, *Upper);
+// TODO: This only evaluates the first range. Evaluate all ranges.
+return RS.isEmpty() ? nullptr : St->set(Sym, RS);
+ }
+}
+ }
+ return nullptr;
+}
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -98,7 +98,9 @@
}
state = state->BindExpr(B, LCtx, Result);
- Bldr.generateNode(B, *it, state);
+ ProgramStateRef state2 =
+ getConstraintManager().evalRangeOp(state, Result);
+ Bldr.generateNode(B, *it, state2 ? state2 : state);
continue;
}
Index: include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
@@ -142,13 +142,15 @@
/// Scan all symbols referenced by the constraints. If the symbol is not
/// alive, remove it.
virtual ProgramStateRef removeDeadBindings(ProgramStateRef state,
- SymbolReaper& SymReaper) = 0;
+ SymbolReaper &SymReaper) = 0;
- virtual void print(ProgramStateRef state,
- raw_ostream &Out,
- const char* nl,
+ virtual void print(ProgramStateRef state, raw_ostream &Out, const char *nl,
const char *sep) = 0;
+ virtual ProgramStateRef evalRangeOp(ProgramStateRef state, SVal V) {
+return nullptr;
+ }
+
virtual
[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values
danielmarjamaki added a comment. Should evalAPSInt() have machinery to do standard sign/type promotions? I suggest that I add one more argument `bool promote = false`, do you think that sounds good? Repository: rL LLVM https://reviews.llvm.org/D36471 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36670: misc-misplaced-widening-cast: fix assertion
danielmarjamaki added a comment. LGTM. I let others approve this. Repository: rL LLVM https://reviews.llvm.org/D36670 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36672: [clang-tidy] readability-non-const-parameter: fixit on all function declarations
danielmarjamaki added a comment. LGTM. But others should approve. Repository: rL LLVM https://reviews.llvm.org/D36672 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width
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
[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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[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 revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Diagnose signed integer overflows. The core.UndefResultChecker will warn.
This is a bit unfinished.. I wonder if you like the approach. It only checks
addition right now. and not underflow.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D92634
Files:
clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
clang/test/Analysis/integer-overflow.c
Index: clang/test/Analysis/integer-overflow.c
===
--- /dev/null
+++ clang/test/Analysis/integer-overflow.c
@@ -0,0 +1,7 @@
+// RUN: %clang_analyze_cc1 -analyze -analyzer-checker core.UndefinedBinaryOperatorResult -verify %s
+
+void f(int x) {
+if (x > 0x7f00 && x + 32 < 0x7fff){}
+if (x > 0x7ff0 && x + 32 < 0x7fff){} // expected-warning {{The result of the '+' expression is undefined}}
+}
+
Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -42,6 +42,8 @@
SVal evalBinOpLN(ProgramStateRef state, BinaryOperator::Opcode op,
Loc lhs, NonLoc rhs, QualType resultTy) override;
+ bool isGreater(ProgramStateRef State, const SymExpr *LHS, int64_t RHS);
+
/// getKnownValue - evaluates a given SVal. If the SVal has only one possible
/// (integer) value, that value is returned. Otherwise, returns NULL.
const llvm::APSInt *getKnownValue(ProgramStateRef state, SVal V) override;
@@ -51,7 +53,8 @@
SVal simplifySVal(ProgramStateRef State, SVal V) override;
SVal MakeSymIntVal(const SymExpr *LHS, BinaryOperator::Opcode op,
- const llvm::APSInt &RHS, QualType resultTy);
+ const llvm::APSInt &RHS, QualType resultTy,
+ ProgramStateRef State);
};
} // end anonymous namespace
@@ -210,14 +213,29 @@
}
}
+bool SimpleSValBuilder::isGreater(ProgramStateRef State, const SymExpr *LHS,
+ int64_t RHS) {
+ ProgramStateManager &Mgr = State->getStateManager();
+ SValBuilder &Bldr = Mgr.getSValBuilder();
+ SVal Eval =
+ Bldr.evalBinOp(State, BO_GT, nonloc::SymbolVal(LHS),
+ makeIntVal(RHS, LHS->getType()), Bldr.getConditionType());
+ if (Eval.isUnknownOrUndef())
+return false;
+ ProgramStateRef StTrue, StFalse;
+ std::tie(StTrue, StFalse) = State->assume(Eval.castAs());
+ return (StTrue && !StFalse);
+}
+
//===--===//
// Transfer function for binary operators.
//===--===//
SVal SimpleSValBuilder::MakeSymIntVal(const SymExpr *LHS,
-BinaryOperator::Opcode op,
-const llvm::APSInt &RHS,
-QualType resultTy) {
+ BinaryOperator::Opcode op,
+ const llvm::APSInt &RHS,
+ QualType resultTy,
+ ProgramStateRef State) {
bool isIdempotent = false;
// Check for a few special cases with known reductions first.
@@ -281,6 +299,15 @@
if (isIdempotent)
return evalCastFromNonLoc(nonloc::SymbolVal(LHS), resultTy);
+ // Is there a signed integer overflow
+ if (LHS->getType()->isSignedIntegerType() && op == BO_Add) {
+int64_t ResultTyMaxVal =
+(1ULL << (getContext().getTypeSize(resultTy) - 1)) - 1;
+if (RHS > 0 && RHS < ResultTyMaxVal &&
+isGreater(State, LHS, ResultTyMaxVal - RHS.getExtValue()))
+ return UndefinedVal();
+ }
+
// If we reach this point, the expression cannot be simplified.
// Make a SymbolVal for the entire expression, after converting the RHS.
const llvm::APSInt *ConvertedRHS = &RHS;
@@ -748,7 +775,7 @@
}
// Otherwise, make a SymIntExpr out of the expression.
- return MakeSymIntVal(symIntExpr, op, *RHSValue, resultTy);
+ return MakeSymIntVal(symIntExpr, op, *RHSValue, resultTy, state);
}
}
@@ -764,7 +791,7 @@
// Is the RHS a constant?
if (const llvm::APSInt *RHSValue = getKnownValue(state, rhs))
-return MakeSymIntVal(Sym, op, *RHSValue, resultTy);
+return MakeSymIntVal(Sym, op, *RHSValue, resultTy, state);
if (Optional V = tryRearrange(state, op, lhs, rhs, resultTy))
return *V;
@@ -938,7 +965,7 @@
[PATCH] D39049: [analyzer] Fix wrong calculation of offset in ArrayBoundsV2
danielmarjamaki added a comment.
> Do you mind writing some tests with multidimensional arrays to check what do
> we lose if we remove that code?
I have spent a few hours trying to write a test case that shows there is false
negatives caused by this change. And fail.
I see lots of false negatives for multidimensional arrays with or without this
code.
For instance:
void f(int x) {
int buf[2][3];
if (x < 4 || x>10)
return;
buf[1][x] = 0;
}
Repository:
rL LLVM
https://reviews.llvm.org/D39049
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36471: [StaticAnalyzer] Try to calculate arithmetic result when operand has a range of possible values
danielmarjamaki updated this revision to Diff 121726.
danielmarjamaki added a comment.
Herald added a subscriber: szepet.
I have updated the patch so it uses evalBinOpNN. This seems to work properly.
I have a number of TODOs in the test cases that should be fixed. Truncations
are not handled properly.
Here is a short example code:
void f(unsigned char X) {
if (X >= 10 && X <= 50) {
unsigned char Y = X + 0x100; // truncation
clang_analyzer_eval(Y >= 10 && Y <= 50); // expected-warning{{FALSE}}
}
}
The expected-warning should be TRUE but currently FALSE is written.
When the "Y >= 10" condition is evaluated the ProgramState is:
Store (direct and default bindings), 0x222ab0fe5f8 :
(Y,0,direct) : (unsigned char) ((reg_$0) + 256)
Expressions:
(0x222a96d6050,0x222ab0eb930) X + 256 : (unsigned char) ((reg_$0) + 256)
(0x222a96d6050,0x222ab0eb960) clang_analyzer_eval :
&code{clang_analyzer_eval}
(0x222a96d6050,0x222ab0eb988) Y : &Y
(0x222a96d6050,0x222ab0eb9d8) Y : (unsigned char) ((reg_$0)
+ 256)
(0x222a96d6050,0x222ab0eb9f0) Y : (unsigned char) ((reg_$0)
+ 256)
(0x222a96d6050,0x222ab0eba08) Y >= 10 : ((unsigned char) ((reg_$0) + 256)) >= 10
(0x222a96d6050,0x222ab0ebb28) clang_analyzer_eval :
&code{clang_analyzer_eval}
Ranges of symbol values:
reg_$0 : { [10, 50] }
(reg_$0) + 256 : { [10, 50] }
It seems to me that the symbol initialization does not handle the range
properly. Imho there is nothing wrong with the calculation. What you think
about adding a range like below?
(unsigned char) ((reg_$0) + 256) : { [10, 50] }
Repository:
rL LLVM
https://reviews.llvm.org/D36471
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
include/clang/StaticAnalyzer/Core/PathSensitive/SimpleConstraintManager.h
lib/StaticAnalyzer/Core/ExprEngineC.cpp
lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
test/Analysis/range_calc.c
Index: test/Analysis/range_calc.c
===
--- test/Analysis/range_calc.c
+++ test/Analysis/range_calc.c
@@ -0,0 +1,143 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection -verify %s
+
+void clang_analyzer_eval(int);
+
+#define INT_MAX((signed int)((~0U)>>1))
+#define INT_MIN((signed int)(~((~0U)>>1)))
+
+void addInts(int X)
+{
+ if (X >= 10 && X <= 50) {
+int Y = X + 2;
+clang_analyzer_eval(Y >= 12 && Y <= 52); // expected-warning{{TRUE}}
+ }
+
+ if (X < 5) {
+int Y = X + 1;
+clang_analyzer_eval(Y < 6); // expected-warning{{TRUE}}
+ }
+
+ if (X >= 1000) {
+int Y = X + 1; // might overflow
+clang_analyzer_eval(Y >= 1001); // expected-warning{{UNKNOWN}}
+clang_analyzer_eval(Y == INT_MIN); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(Y == INT_MIN || Y >= 1001); // expected-warning{{TRUE}}
+ }
+}
+
+void addU8(unsigned char X)
+{
+ if (X >= 10 && X <= 50) {
+unsigned char Y = X + 2;
+clang_analyzer_eval(Y >= 12 && Y <= 52); // expected-warning{{TRUE}}
+ }
+
+ if (X < 5) {
+unsigned char Y = X + 1;
+clang_analyzer_eval(Y < 6); // expected-warning{{TRUE}}
+ }
+
+ // TODO
+ if (X >= 10 && X <= 50) {
+unsigned char Y = X + (-256); // truncation
+clang_analyzer_eval(Y >= 10 && Y <= 50); // expected-warning{{FALSE}}
+ }
+
+ // TODO
+ if (X >= 10 && X <= 50) {
+unsigned char Y = X + 256; // truncation
+clang_analyzer_eval(Y >= 10 && Y <= 50); // expected-warning{{FALSE}} expected-warning{{UNKNOWN}}
+ }
+
+ // TODO
+ if (X >= 100) {
+unsigned char Y = X + 1; // truncation
+ clang_analyzer_eval(Y == 0); // expected-warning{{FALSE
+ clang_analyzer_eval(Y >= 101); // expected-warning{{TRUE}}
+clang_analyzer_eval(Y == 0 || Y >= 101); // expected-warning{{TRUE}}
+ }
+
+ if (X >= 100) {
+unsigned short Y = X + 1;
+clang_analyzer_eval(Y >= 101 && Y <= 256); // expected-warning{{TRUE}}
+ }
+}
+
+void sub1(int X)
+{
+ if (X >= 10 && X <= 50) {
+int Y = X - 2;
+clang_analyzer_eval(Y >= 8 && Y <= 48); // expected-warning{{TRUE}}
+ }
+
+ if (X >= 10 && X <= 50) {
+unsigned char Y = (unsigned int)X - 20; // truncation
+clang_analyzer_eval(Y <= 30 || Y >= 246); // expected-warning{{TRUE}}
+ }
+
+ // TODO
+ if (X >= 10 && X <= 50) {
+unsigned char Y = (unsigned int)X - 256; // truncation
+clang_analyzer_eval(Y >= 10 && Y <= 50); // expected-warning{{FALSE}} expected-warning{{UNKNOWN}}
+ }
+
+ if (X < 5) {
+int Y = X - 1; // might overflow
+clang_analyzer_eval(Y < 4); // expected-warning{{UNKNOWN}}
+clang_analyzer_eval(Y == INT_MAX); // expected-warning{{UNKNOWN}}
+ clang_analyzer_eval(Y == INT_MAX || Y < 4); // expected-warning{{TRUE}}
+ }
+
+ if (X >= 1000) {
+int Y = X - 1;
+clang_analyzer_eval(Y >= 999); // expected-warning{{TRUE}}
+ }
+}
+
+void subU8(unsigned char X)
+{
+ if (X >= 10 && X <= 50) {
+unsigned char Y = X - 2;
+clang_analyzer_eval(Y >=
[PATCH] D39049: [analyzer] Fix wrong calculation of offset in ArrayBoundsV2
danielmarjamaki added a comment.
> Could you do a similar analysis that I did above to check why does this not
> work for the multidimensional case? (I.e.: checking what constraints are
> generated and what the analyzer does with them.)
the "location.dump()" will just say "x".
the ProgramState is:
Expressions:
(0x2acd12a78a0,0x2acd1478b80) buf : &buf
(0x2acd12a78a0,0x2acd1478bf8) buf : &element{buf,0 S64b,int [3]}
(0x2acd12a78a0,0x2acd1478c10) buf[1] : &element{buf,1 S64b,int [3]}
(0x2acd12a78a0,0x2acd1478c38) x : &x
(0x2acd12a78a0,0x2acd1478c88) buf[1] : &element{element{buf,1 S64b,int [3]},0
S64b,int}
Ranges of symbol values:
reg_$0 : { [4, 10] }
rawOffsetVal => 0
extentBegin => 0
For getSimplifiedOffset() , the offset is not a SymIntExpr it will just return
0.
Repository:
rL LLVM
https://reviews.llvm.org/D39049
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39049: [analyzer] Fix wrong calculation of offset in ArrayBoundsV2
danielmarjamaki added a comment. > So what are the arguments that are passed to getSimplifiedOffset() in that > case? 0? That does not seem to be correct. yes. so the conclusion is: - this code does not work - this code is untested - this code is not even used in the use cases it was intended for because of bugs elsewhere therefore it should be removed. Repository: rL LLVM https://reviews.llvm.org/D39049 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28148: [Sema] Suppress warnings for C's zero initializer
danielmarjamaki added a comment.
Thanks for working on these. imo these are false positives.
Comment at: lib/AST/Expr.cpp:1893
+
+ const IntegerLiteral *lit = dyn_cast(getInit(0));
+ if (!lit) {
I would recommend capital first letter for this variable
Comment at: lib/AST/Expr.cpp:1894
+ const IntegerLiteral *lit = dyn_cast(getInit(0));
+ if (!lit) {
+return false;
I suggest a single line:
```
return Lit && Lit->getValue() == 0;
```
https://reviews.llvm.org/D28148
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28278: [StaticAnalyzer] dont show wrong 'garbage value' warning when there is array index out of bounds
danielmarjamaki created this revision.
danielmarjamaki added reviewers: zaks.anna, dcoughlin.
danielmarjamaki added a subscriber: cfe-commits.
danielmarjamaki set the repository for this revision to rL LLVM.
Example code:
void f1(int x) {
int a[20] = {0};
if (x==25) {}
if (a[x] == 123) {} // <- Warning
}
If I don't enable alpha, only core, then Clang writes this misleading FP:
undef.c:5:12: warning: The left operand of '==' is a garbage value
I say it's a FP because the message is wrong. If the message correctly said
"array index out of bounds" and pointed out a[x] directly, then it would be TP.
This message goes away if alpha is enabled and I believe that is by intention.
Since there is a array-index-out-of-bounds check in alpha I am guessing that
the UndefinedBinaryOperatorResult should not report "array index out of
bounds". Therefore I remove this warning from this check.
This patch is a experimental work in progress. I would like to know if you
think I should modifiy the UndefinedBinaryOperatorResult check or if I should
do something in the ExprEngine? Maybe array index out of bounds should not lead
to Undef SVal?
With this patch, all the existing tests succeed.
Repository:
rL LLVM
https://reviews.llvm.org/D28278
Files:
lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
Index: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
@@ -61,7 +61,7 @@
SmallString<256> sbuf;
llvm::raw_svector_ostream OS(sbuf);
const Expr *Ex = nullptr;
-bool isLeft = true;
+bool isLeft;
if (state->getSVal(B->getLHS(), LCtx).isUndef()) {
Ex = B->getLHS()->IgnoreParenCasts();
@@ -73,6 +73,24 @@
}
if (Ex) {
+ if (isa(Ex)) {
+SVal Loc = state->getSVal(Ex,LCtx);
+if (Loc.isValid()) {
+ const MemRegion *MR = Loc.castAs().getRegion();
+ if (const ElementRegion *ER = dyn_cast(MR)) {
+DefinedOrUnknownSVal Idx =
ER->getIndex().castAs();
+DefinedOrUnknownSVal NumElements
+ = C.getStoreManager().getSizeInElements(state,
ER->getSuperRegion(),
+ER->getValueType());
+ProgramStateRef StInBound = state->assumeInBound(Idx, NumElements,
true);
+ProgramStateRef StOutBound = state->assumeInBound(Idx,
NumElements, false);
+if (StOutBound && !StInBound) {
+ return;
+}
+ }
+}
+ }
+
OS << "The " << (isLeft ? "left" : "right")
<< " operand of '"
<< BinaryOperator::getOpcodeStr(B->getOpcode())
Index: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
@@ -61,7 +61,7 @@
SmallString<256> sbuf;
llvm::raw_svector_ostream OS(sbuf);
const Expr *Ex = nullptr;
-bool isLeft = true;
+bool isLeft;
if (state->getSVal(B->getLHS(), LCtx).isUndef()) {
Ex = B->getLHS()->IgnoreParenCasts();
@@ -73,6 +73,24 @@
}
if (Ex) {
+ if (isa(Ex)) {
+SVal Loc = state->getSVal(Ex,LCtx);
+if (Loc.isValid()) {
+ const MemRegion *MR = Loc.castAs().getRegion();
+ if (const ElementRegion *ER = dyn_cast(MR)) {
+DefinedOrUnknownSVal Idx = ER->getIndex().castAs();
+DefinedOrUnknownSVal NumElements
+ = C.getStoreManager().getSizeInElements(state, ER->getSuperRegion(),
+ER->getValueType());
+ProgramStateRef StInBound = state->assumeInBound(Idx, NumElements, true);
+ProgramStateRef StOutBound = state->assumeInBound(Idx, NumElements, false);
+if (StOutBound && !StInBound) {
+ return;
+}
+ }
+}
+ }
+
OS << "The " << (isLeft ? "left" : "right")
<< " operand of '"
<< BinaryOperator::getOpcodeStr(B->getOpcode())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28297: [StaticAnalyzer] Fix crash in CastToStructChecker
danielmarjamaki created this revision.
danielmarjamaki added a reviewer: NoQ.
danielmarjamaki added a subscriber: cfe-commits.
danielmarjamaki set the repository for this revision to rL LLVM.
This fix the crash reported in https://llvm.org/bugs/show_bug.cgi?id=31173
Repository:
rL LLVM
https://reviews.llvm.org/D28297
Files:
lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp
test/Analysis/cast-to-struct.cpp
Index: test/Analysis/cast-to-struct.cpp
===
--- test/Analysis/cast-to-struct.cpp
+++ test/Analysis/cast-to-struct.cpp
@@ -65,3 +65,8 @@
void *VP = P;
Abc = (struct ABC *)VP;
}
+
+// https://llvm.org/bugs/show_bug.cgi?id=31173
+void dontCrash(struct AB X) {
+ struct UndefS *S = (struct UndefS *)&X;
+}
Index: lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp
@@ -55,6 +55,12 @@
if (!ToPointeeTy->isStructureOrClassType())
return true;
+ if (const RecordType *RD = dyn_cast(ToPointeeTy.getTypePtr())) {
+if (!RD->getDecl()->getDefinition()) {
+ return true;
+}
+ }
+
// We allow cast from void*.
if (OrigPointeeTy->isVoidType())
return true;
Index: test/Analysis/cast-to-struct.cpp
===
--- test/Analysis/cast-to-struct.cpp
+++ test/Analysis/cast-to-struct.cpp
@@ -65,3 +65,8 @@
void *VP = P;
Abc = (struct ABC *)VP;
}
+
+// https://llvm.org/bugs/show_bug.cgi?id=31173
+void dontCrash(struct AB X) {
+ struct UndefS *S = (struct UndefS *)&X;
+}
Index: lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp
+++ lib/StaticAnalyzer/Checkers/CastToStructChecker.cpp
@@ -55,6 +55,12 @@
if (!ToPointeeTy->isStructureOrClassType())
return true;
+ if (const RecordType *RD = dyn_cast(ToPointeeTy.getTypePtr())) {
+if (!RD->getDecl()->getDefinition()) {
+ return true;
+}
+ }
+
// We allow cast from void*.
if (OrigPointeeTy->isVoidType())
return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28278: [StaticAnalyzer] dont show wrong 'garbage value' warning when there is array index out of bounds
danielmarjamaki added a comment. In https://reviews.llvm.org/D28278#639710, @xazax.hun wrote: > Did you experience any problems with the array out of bounds check lately? In > case it was stable on large code-bases and did not give too many false > positives, I think it might be worth to move that check out of alpha at the > same time, so users who do not turn on alpha checks will not lose any > functionality. What do you think? I don't have precise statistics. But these array-index-out-of-bounds messages are often false positives. Fixes are needed in the ExprEngine. Repository: rL LLVM https://reviews.llvm.org/D28278 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D28148: [Sema] Suppress warnings for C's zero initializer
danielmarjamaki accepted this revision. danielmarjamaki added a reviewer: danielmarjamaki. danielmarjamaki added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D28148 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width
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
[PATCH] D31029: [analyzer] Fix logical not for pointers with different bit width
danielmarjamaki created this revision.
The Static Analyzer assumed that all pointers had the same bit width. Now pass
the type to the 'makeNull' method, to construct a null
pointer of the appropiate bit width.
Example code that does not work well:
int main(void) {
__cm void *cm_p = 0;
if (cm_p == 0)
(void)cm_p;
}
Unfortunately there is no proper testcase here. The problem is seen with a
custom target.
Repository:
rL LLVM
https://reviews.llvm.org/D31029
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
lib/StaticAnalyzer/Core/ExprEngineC.cpp
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -980,10 +980,9 @@
//transfer functions as "0 == E".
SVal Result;
if (Optional LV = V.getAs()) {
-Loc X = svalBuilder.makeNull();
+Loc X = svalBuilder.makeNullWithType(Ex->getType());
Result = evalBinOp(state, BO_EQ, *LV, X, U->getType());
- }
- else if (Ex->getType()->isFloatingType()) {
+ } else if (Ex->getType()->isFloatingType()) {
// FIXME: handle floating point types.
Result = UnknownVal();
} else {
Index: include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -310,10 +310,14 @@
return nonloc::ConcreteInt(BasicVals.getTruthValue(b));
}
- Loc makeNull() {
-return loc::ConcreteInt(BasicVals.getZeroWithPtrWidth());
+ // Pass type to accomodate for different pointer bit-witdths of different
+ // address spaces.
+ Loc makeNullWithType(QualType type) {
+return loc::ConcreteInt(BasicVals.getZeroWithTypeSize(type));
}
+ Loc makeNull() { return loc::ConcreteInt(BasicVals.getZeroWithPtrWidth()); }
+
Loc makeLoc(SymbolRef sym) {
return loc::MemRegionVal(MemMgr.getSymbolicRegion(sym));
}
Index: include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
@@ -176,6 +176,11 @@
return getValue(X);
}
+ inline const llvm::APSInt &getZeroWithTypeSize(QualType T,
+ bool isUnsigned = true) {
+return getValue(0, Ctx.getTypeSize(T), isUnsigned);
+ }
+
inline const llvm::APSInt& getZeroWithPtrWidth(bool isUnsigned = true) {
return getValue(0, Ctx.getTypeSize(Ctx.VoidPtrTy), isUnsigned);
}
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -980,10 +980,9 @@
//transfer functions as "0 == E".
SVal Result;
if (Optional LV = V.getAs()) {
-Loc X = svalBuilder.makeNull();
+Loc X = svalBuilder.makeNullWithType(Ex->getType());
Result = evalBinOp(state, BO_EQ, *LV, X, U->getType());
- }
- else if (Ex->getType()->isFloatingType()) {
+ } else if (Ex->getType()->isFloatingType()) {
// FIXME: handle floating point types.
Result = UnknownVal();
} else {
Index: include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -310,10 +310,14 @@
return nonloc::ConcreteInt(BasicVals.getTruthValue(b));
}
- Loc makeNull() {
-return loc::ConcreteInt(BasicVals.getZeroWithPtrWidth());
+ // Pass type to accomodate for different pointer bit-witdths of different
+ // address spaces.
+ Loc makeNullWithType(QualType type) {
+return loc::ConcreteInt(BasicVals.getZeroWithTypeSize(type));
}
+ Loc makeNull() { return loc::ConcreteInt(BasicVals.getZeroWithPtrWidth()); }
+
Loc makeLoc(SymbolRef sym) {
return loc::MemRegionVal(MemMgr.getSymbolicRegion(sym));
}
Index: include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
@@ -176,6 +176,11 @@
return getValue(X);
}
+ inline const llvm::APSInt &getZeroWithTypeSize(QualType T,
+ boo
[PATCH] D31029: [analyzer] Fix logical not for pointers with different bit width
danielmarjamaki added a comment. I am not sure where to look. I heard somebody say OpenCL has different pointer widths. Repository: rL LLVM https://reviews.llvm.org/D31029 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width
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/StaticAnalyzer/Checkers/UndefResultChecker.cpp
lib/StaticAnalyzer/Core/CheckerHelpers.cpp
test/Analysis/bitwise-ops.c
Index: test/Analysis/bitwise-ops.c
===
--- test/Analysis/bitwise-ops.c
+++ test/Analysis/bitwise-ops.c
@@ -29,4 +29,18 @@
default:
return 0;
}
-}
\ No newline at end of file
+}
+
+int testOverflowShift(int a) {
+ if (a == 323) {
+return 1 << a; // expected-warning{{The result of the '<<' expression is undefined due to shift count >= width of type}}
+ }
+ return 0;
+}
+
+int testNegativeShift(int a) {
+ if (a == -5) {
+return 1 << a; // expected-warning{{The result of the '<<' expression is undefined due to negative value on the right side of operand}}
+ }
+ return 0;
+}
Index: lib/StaticAnalyzer/Core/CheckerHelpers.cpp
===
--- lib/StaticAnalyzer/Core/CheckerHelpers.cpp
+++ lib/StaticAnalyzer/Core/CheckerHelpers.cpp
@@ -94,3 +94,39 @@
return std::make_pair(VD, RHS);
}
+
+bool clang::ento::isExprResultConformsCompRule(CheckerContext &C,
+ BinaryOperatorKind BOK,
+ const Expr *LHSExpr,
+ const SVal RHSVal) {
+ ProgramStateRef State = C.getState();
+
+ SVal LHSVal = C.getSVal(LHSExpr);
+ if (LHSVal.isUnknownOrUndef() || !LHSVal.getAs())
+return false;
+
+ SValBuilder &Bldr = C.getSValBuilder();
+ SVal Eval =
+ Bldr.evalBinOp(State, BOK, LHSVal, RHSVal, Bldr.getConditionType());
+ if (Eval.isUnknownOrUndef())
+return false;
+
+ ConstraintManager &CM = C.getConstraintManager();
+ ProgramStateRef StTrue, StFalse;
+ std::tie(StTrue, StFalse) = CM.assumeDual(State, Eval.castAs());
+ return StTrue && !StFalse;
+}
+
+// Is E value greater or equal than Val?
+bool clang::ento::isGreaterEqual(CheckerContext &C, const Expr *E,
+ unsigned long long Val) {
+ DefinedSVal V =
+ C.getSValBuilder().makeIntVal(Val, C.getASTContext().LongLongTy);
+ return isExprResultConformsCompRule(C, BO_GE, E, V);
+}
+
+// Is E value negative?
+bool clang::ento::isNegative(CheckerContext &C, const Expr *E) {
+ DefinedSVal V = C.getSValBuilder().makeIntVal(0, false);
+ return isExprResultConformsCompRule(C, BO_LT, E, V);
+}
Index: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
@@ -17,6 +17,7 @@
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/Support/raw_ostream.h"
@@ -59,6 +60,11 @@
return StOutBound && !StInBound;
}
+bool isShiftOverflow(CheckerContext &C, const BinaryOperator *B) {
+ return isGreaterEqual(C, B->getRHS(),
+C.getASTContext().getIntWidth(B->getLHS()->getType()));
+}
+
void UndefResultChecker::checkPostStmt(const BinaryOperator *B,
CheckerContext &C) const {
ProgramStateRef state = C.getState();
@@ -106,9 +112,24 @@
}
else {
// Neither operand was undefined, but the result is undefined.
- OS << "The result of the '"
- << BinaryOperator::getOpcodeStr(B->getOpcode())
- << "' expression is undefined";
+ if ((B->getOpcode() == BinaryOperatorKind::BO_Shl ||
+ B->getOpcode() == BinaryOperatorKind::BO_Shr) &&
+ isNegative(C, B->getRHS())) {
+OS << "The result of the '"
+ << BinaryOperator::getOpcodeStr(B->getOpcode())
+ << "' expression is undefined due to negative value on the right "
+ "side of operand";
+ } else if ((B->getOpcode() == BinaryOperatorKind::BO_Shl ||
+ B->getOpcode() == BinaryOperatorKind::BO_Shr) &&
+ isShiftOverflow(C, B)) {
+OS << "The result of the '"
+ << BinaryOperator::getOpcodeStr(B->getOpcode())
+ << "' expression is undefined due to shift count >= width of type";
+ } else {
+OS << "The result of the '"
+ << BinaryOperator::getOpcodeStr(B->getOpcode())
+ << "' expression is undefined";
+ }
}
auto report = llvm::make_unique(*BT, OS.str(), N);
if (Ex) {
Index: lib/StaticAn
[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width
danielmarjamaki marked 2 inline comments as done.
danielmarjamaki added inline comments.
Comment at: lib/StaticAnalyzer/Core/CheckerHelpers.cpp:100
+ BinaryOperatorKind BOK,
+ const Expr *LExpr,
+ const SVal RVal) {
a.sidorin wrote:
> I think we should rename these variables to LHSExpr, RHSVal, LHSVal. I don't
> like LVal/RVal because they may be associated with rvalue/lvalue types which
> is not what we want.
I agree. Good point.
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
[PATCH] D31097: [clang-tidy] don't warn about implicit widening casts in function calls
danielmarjamaki created this revision.
Herald added a subscriber: JDevlieghere.
This patch fixes https://bugs.llvm.org/show_bug.cgi?id=32246
To avoid spurious warnings, clang-tidy should not warn about misplaced widening
casts for implicit casts in function calls.
Repository:
rL LLVM
https://reviews.llvm.org/D31097
Files:
clang-tidy/misc/MisplacedWideningCastCheck.cpp
test/clang-tidy/misc-misplaced-widening-cast.cpp
Index: test/clang-tidy/misc-misplaced-widening-cast.cpp
===
--- test/clang-tidy/misc-misplaced-widening-cast.cpp
+++ test/clang-tidy/misc-misplaced-widening-cast.cpp
@@ -45,8 +45,8 @@
}
void call(unsigned int n) {
+ // Don't warn about implicit casts. See
https://bugs.llvm.org/show_bug.cgi?id=32246
func(n << 8);
- // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: either cast from 'unsigned int'
to 'long'
func((long)(n << 8));
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: either cast from 'unsigned int'
to 'long'
func((long)n << 8);
Index: clang-tidy/misc/MisplacedWideningCastCheck.cpp
===
--- clang-tidy/misc/MisplacedWideningCastCheck.cpp
+++ clang-tidy/misc/MisplacedWideningCastCheck.cpp
@@ -46,7 +46,7 @@
Finder->addMatcher(varDecl(hasInitializer(Cast)), this);
Finder->addMatcher(returnStmt(hasReturnValue(Cast)), this);
- Finder->addMatcher(callExpr(hasAnyArgument(Cast)), this);
+ Finder->addMatcher(callExpr(hasAnyArgument(ExplicitCast.bind("Cast"))),
this);
Finder->addMatcher(binaryOperator(hasOperatorName("="), hasRHS(Cast)), this);
Finder->addMatcher(
binaryOperator(matchers::isComparisonOperator(), hasEitherOperand(Cast)),
Index: test/clang-tidy/misc-misplaced-widening-cast.cpp
===
--- test/clang-tidy/misc-misplaced-widening-cast.cpp
+++ test/clang-tidy/misc-misplaced-widening-cast.cpp
@@ -45,8 +45,8 @@
}
void call(unsigned int n) {
+ // Don't warn about implicit casts. See https://bugs.llvm.org/show_bug.cgi?id=32246
func(n << 8);
- // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: either cast from 'unsigned int' to 'long'
func((long)(n << 8));
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: either cast from 'unsigned int' to 'long'
func((long)n << 8);
Index: clang-tidy/misc/MisplacedWideningCastCheck.cpp
===
--- clang-tidy/misc/MisplacedWideningCastCheck.cpp
+++ clang-tidy/misc/MisplacedWideningCastCheck.cpp
@@ -46,7 +46,7 @@
Finder->addMatcher(varDecl(hasInitializer(Cast)), this);
Finder->addMatcher(returnStmt(hasReturnValue(Cast)), this);
- Finder->addMatcher(callExpr(hasAnyArgument(Cast)), this);
+ Finder->addMatcher(callExpr(hasAnyArgument(ExplicitCast.bind("Cast"))), this);
Finder->addMatcher(binaryOperator(hasOperatorName("="), hasRHS(Cast)), this);
Finder->addMatcher(
binaryOperator(matchers::isComparisonOperator(), hasEitherOperand(Cast)),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31097: [clang-tidy] don't warn about implicit widening casts in function calls
danielmarjamaki added a comment. In my opinion, we should stop warning about all implicit casts. Take for instance: long l1; if (condition) l1 = n << 8; // <- implicit cast else l1 = ~0L; That is fine. Nothing suspicious. Just because the destination variable is long doesn't have to mean the result is long. If we want to warn I would say that valueflow analysis should be used to see if there is truncation. The original idea was that we would warn if the user tried to cast the result but did that wrong. I don't feel that this is the idea of this checker anymore. Repository: rL LLVM https://reviews.llvm.org/D31097 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31097: [clang-tidy] don't warn about implicit widening casts in function calls
danielmarjamaki added a comment. In https://reviews.llvm.org/D31097#704628, @alexfh wrote: > In https://reviews.llvm.org/D31097#704626, @alexfh wrote: > > > In https://reviews.llvm.org/D31097#704621, @xazax.hun wrote: > > > > > I wonder whether warning on implicit casts still makes sense for example > > > in mission critical code. So maybe it is worth to have a configuration > > > option with the default setting being less strict and chatty. What do you > > > think? > > > > > > But it's not about "misplaced casts", it's about implicit conversions and > > -Wconversion diagnostic can take care of this. > > > Actually, the diagnostics about implicit casts here might be useful (but > maybe in a separate check). I have to look again at > https://reviews.llvm.org/D17987. there can definitely be bugs when there are such implicit casts. but this checking has no precision at all. I am against that we don't care about precision. adding casts to silence such warnings are dangerous too. I have seen for instance in Clang repo when there is "loss of sign" warning and the developer fix that by casting for instance a "size_t" to "int" and then there is logically loss of precision. In https://reviews.llvm.org/D31097#704626, @alexfh wrote: > In https://reviews.llvm.org/D31097#704621, @xazax.hun wrote: > > > I wonder whether warning on implicit casts still makes sense for example in > > mission critical code. So maybe it is worth to have a configuration option > > with the default setting being less strict and chatty. What do you think? > > > But it's not about "misplaced casts", it's about implicit conversions and > -Wconversion diagnostic can take care of this. I agree.. Just want to advertise the analyzer ConversionChecker also in case you didn't know about it. That is supposed to be a precise checker for loss of precision and loss of sign. It does not detect this loss of precision in implicit casts but I would like that is taken care of. Repository: rL LLVM https://reviews.llvm.org/D31097 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width
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/PathSensitive/CheckerHelpers.h
lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
lib/StaticAnalyzer/Core/CheckerHelpers.cpp
test/Analysis/bitwise-ops.c
Index: test/Analysis/bitwise-ops.c
===
--- test/Analysis/bitwise-ops.c
+++ test/Analysis/bitwise-ops.c
@@ -29,4 +29,18 @@
default:
return 0;
}
-}
\ No newline at end of file
+}
+
+int testOverflowShift(int a) {
+ if (a == 323) {
+return 1 << a; // expected-warning{{The result of the '<<' expression is undefined due to shift count >= width of type}}
+ }
+ return 0;
+}
+
+int testNegativeShift(int a) {
+ if (a == -5) {
+return 1 << a; // expected-warning{{The result of the '<<' expression is undefined due to negative value on the right side of operand}}
+ }
+ return 0;
+}
Index: lib/StaticAnalyzer/Core/CheckerHelpers.cpp
===
--- lib/StaticAnalyzer/Core/CheckerHelpers.cpp
+++ lib/StaticAnalyzer/Core/CheckerHelpers.cpp
@@ -94,3 +94,40 @@
return std::make_pair(VD, RHS);
}
+
+bool clang::ento::isExprResultConformsComparisonRule(
+CheckerContext &C, BinaryOperatorKind CompRule, const Expr *LHSExpr,
+const SVal RHSVal) {
+ ProgramStateRef State = C.getState();
+
+ SVal LHSVal = C.getSVal(LHSExpr);
+ if (LHSVal.isUnknownOrUndef() || !LHSVal.getAs())
+return false;
+
+ SValBuilder &Bldr = C.getSValBuilder();
+ SVal Eval =
+ Bldr.evalBinOp(State, CompRule, LHSVal, RHSVal, Bldr.getConditionType());
+ if (Eval.isUnknownOrUndef())
+return false;
+ ConstraintManager &CM = C.getConstraintManager();
+ ProgramStateRef StTrue, StFalse;
+ std::tie(StTrue, StFalse) = CM.assumeDual(State, Eval.castAs());
+ return StTrue && !StFalse;
+}
+
+// Is E value greater or equal than Val?
+bool clang::ento::isGreaterEqual(CheckerContext &C, const Expr *E,
+ unsigned long long Val) {
+ llvm::APSInt In;
+ E->EvaluateAsInt(In, C.getASTContext());
+
+ DefinedSVal V =
+ C.getSValBuilder().makeIntVal(Val, C.getASTContext().LongLongTy);
+ return isExprResultConformsComparisonRule(C, BO_GE, E, V);
+}
+
+// Is E value negative?
+bool clang::ento::isNegative(CheckerContext &C, const Expr *E) {
+ DefinedSVal V = C.getSValBuilder().makeIntVal(0, false);
+ return isExprResultConformsComparisonRule(C, BO_LT, E, V);
+}
Index: lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UndefResultChecker.cpp
@@ -17,6 +17,7 @@
#include "clang/StaticAnalyzer/Core/Checker.h"
#include "clang/StaticAnalyzer/Core/CheckerManager.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
+#include "clang/StaticAnalyzer/Core/PathSensitive/CheckerHelpers.h"
#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/Support/raw_ostream.h"
@@ -59,6 +60,11 @@
return StOutBound && !StInBound;
}
+static bool isShiftOverflow(CheckerContext &C, const BinaryOperator *B) {
+ return isGreaterEqual(C, B->getRHS(),
+C.getASTContext().getIntWidth(B->getLHS()->getType()));
+}
+
void UndefResultChecker::checkPostStmt(const BinaryOperator *B,
CheckerContext &C) const {
ProgramStateRef state = C.getState();
@@ -97,18 +103,31 @@
}
if (Ex) {
- OS << "The " << (isLeft ? "left" : "right")
- << " operand of '"
+ OS << "The " << (isLeft ? "left" : "right") << " operand of '"
<< BinaryOperator::getOpcodeStr(B->getOpcode())
<< "' is a garbage value";
if (isArrayIndexOutOfBounds(C, Ex))
OS << " due to array index out of bounds";
-}
-else {
+} else {
// Neither operand was undefined, but the result is undefined.
- OS << "The result of the '"
- << BinaryOperator::getOpcodeStr(B->getOpcode())
- << "' expression is undefined";
+ if ((B->getOpcode() == BinaryOperatorKind::BO_Shl ||
+ B->getOpcode() == BinaryOperatorKind::BO_Shr) &&
+ isNegative(C, B->getRHS())) {
+OS << "The result of the '"
+ << BinaryOperator::getOpcodeStr(B->getOpcode())
+ << "' expression is undefined due to negative value on the right "
+ "side of operand";
+ } else if ((B->getOpcode() == BinaryOperatorKind::BO_Shl ||
+ B->getOpcode() == BinaryOperatorKind::BO_Shr) &&
+ isShiftOverflow(C, B)) {
+OS <<
[PATCH] D31097: [clang-tidy] don't warn about implicit widening casts in function calls
danielmarjamaki updated this revision to Diff 92322.
danielmarjamaki added a comment.
Remove warnings for implicit casts.
Repository:
rL LLVM
https://reviews.llvm.org/D31097
Files:
clang-tidy/misc/MisplacedWideningCastCheck.cpp
clang-tidy/misc/MisplacedWideningCastCheck.h
docs/clang-tidy/checks/misc-misplaced-widening-cast.rst
test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp
test/clang-tidy/misc-misplaced-widening-cast.cpp
Index: test/clang-tidy/misc-misplaced-widening-cast.cpp
===
--- test/clang-tidy/misc-misplaced-widening-cast.cpp
+++ test/clang-tidy/misc-misplaced-widening-cast.cpp
@@ -6,13 +6,11 @@
long l;
l = a * b;
- // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' is ineffective, or there is loss of precision before the conversion [misc-misplaced-widening-cast]
l = (long)(a * b);
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
l = (long)a * b;
l = a << 8;
- // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
l = (long)(a << 8);
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
l = (long)b << 8;
@@ -25,9 +23,7 @@
bool l;
l = a * b == c;
- // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
l = c == a * b;
- // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long'
l = (long)(a * b) == c;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
l = c == (long)(a * b);
@@ -38,24 +34,21 @@
void init(unsigned int n) {
long l1 = n << 8;
- // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: either cast from 'unsigned int' to 'long'
long l2 = (long)(n << 8);
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: either cast from 'unsigned int' to 'long'
long l3 = (long)n << 8;
}
void call(unsigned int n) {
func(n << 8);
- // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: either cast from 'unsigned int' to 'long'
func((long)(n << 8));
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: either cast from 'unsigned int' to 'long'
func((long)n << 8);
}
long ret(int a) {
if (a < 0) {
return a * 1000;
-// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long'
} else if (a > 0) {
return (long)(a * 1000);
// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long'
Index: test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp
===
--- test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp
+++ test/clang-tidy/misc-misplaced-widening-cast-explicit-only.cpp
@@ -1,58 +0,0 @@
-// RUN: %check_clang_tidy %s misc-misplaced-widening-cast %t -- -config="{CheckOptions: [{key: misc-misplaced-widening-cast.CheckImplicitCasts, value: 0}]}" --
-
-void func(long arg) {}
-
-void assign(int a, int b) {
- long l;
-
- l = a * b;
- l = (long)(a * b);
- // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long' is ineffective, or there is loss of precision before the conversion [misc-misplaced-widening-cast]
- l = (long)a * b;
-
- l = a << 8;
- l = (long)(a << 8);
- // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
- l = (long)b << 8;
-
- l = static_cast(a * b);
- // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
-}
-
-void compare(int a, int b, long c) {
- bool l;
-
- l = a * b == c;
- l = c == a * b;
- l = (long)(a * b) == c;
- // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: either cast from 'int' to 'long'
- l = c == (long)(a * b);
- // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long'
- l = (long)a * b == c;
- l = c == (long)a * b;
-}
-
-void init(unsigned int n) {
- long l1 = n << 8;
- long l2 = (long)(n << 8);
- // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: either cast from 'unsigned int' to 'long'
- long l3 = (long)n << 8;
-}
-
-void call(unsigned int n) {
- func(n << 8);
- func((long)(n << 8));
- // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: either cast from 'unsigned int' to 'long'
- func((long)n << 8);
-}
-
-long ret(int a) {
- if (a < 0) {
-return a * 1000;
- } else if (a > 0) {
-return (long)(a * 1000);
-// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: either cast from 'int' to 'long'
- } else {
-return (long)a * 1000;
- }
-}
Index: docs/clang-tidy/checks/misc-misplaced-widening-cast.rst
===
--- docs/clang-tidy/checks/misc-misplaced-widening-cast.rst
+++ docs/clang-tidy/checks/misc-misplaced-widening-cast.rst
@@ -32,18 +32,7 @@
return (long)x * 1000;
}
-Implicit casts
---
-
-Forgetting to place the cast at all is at least as dangerous and at least as
-common as misplacing it. If :option:`CheckImplicitCasts` is enabled the check
-also detect
[PATCH] D31097: [clang-tidy] don't warn about implicit widening casts in function calls
danielmarjamaki added a comment.
I believe there is pointless code in relativeIntSizes etc. If there is for
instance "a+b" then the result can't be a char type.
static int relativeIntSizes(BuiltinType::Kind Kind) {
switch (Kind) {
case BuiltinType::UChar:
return 1;
case BuiltinType::SChar:
return 1;
case BuiltinType::Char_U:
return 1;
case BuiltinType::Char_S:
return 1;
case BuiltinType::UShort:
return 2;
case BuiltinType::Short:
return 2;
case BuiltinType::UInt:
return 3;
case BuiltinType::Int:
return 3;
case BuiltinType::ULong:
return 4;
case BuiltinType::Long:
return 4;
case BuiltinType::ULongLong:
return 5;
case BuiltinType::LongLong:
return 5;
case BuiltinType::UInt128:
return 6;
case BuiltinType::Int128:
return 6;
default:
return 0;
}
}
Repository:
rL LLVM
https://reviews.llvm.org/D31097
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31097: [clang-tidy] don't warn about implicit widening casts in function calls
danielmarjamaki added a comment.
In https://reviews.llvm.org/D31097#707385, @baloghadamsoftware wrote:
> Hi!
>
> There is an option to disable the checking of widening casts. It is enabled
> by default. You can disable it any time. Or, if you find too much false
> positives, we can discuss about setting this option to disabled as default.
>
> I am convinced that checking implicit widening casts are also necessary. We
> should probably change the error message in the implicit case from
> "misplaced" to "missing", and maybe also rename the checker itself.
> Separating it to two different checkers, which are almost copy of each other
> is huge code duplication.
It would help to disable it by default and changing the message. But also I
believe it's philosophically different to the original checker.
I would say that your logic is more philosophically similar to Wconversion.
Could it be added there instead?
Did you try this check on real code? Do you think there is a problem that
should be fixed here?
void foo(unsigned int N) {
long L;
if (N<10)
L = N << 8;
...
I am assuming that such code is not uncommon. If you don't think there is a
problem in that code then I would personally suggest that you update the
ConversionChecker in the analyzer instead.
I do believe that a warning about that code is a false positive.
The idea with the misc-misplaced-widening-cast is that if the developer writes
such code:
void foo(unsigned int N) {
long L;
if (N<10)
L = (long)(N << 8);
...
Then there is a message "either cast is misplaced or there is truncation". In
this case the cast is misplaced and it can be removed.
Repository:
rL LLVM
https://reviews.llvm.org/D31097
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31097: [clang-tidy] don't warn about implicit widening casts in function calls
danielmarjamaki added a comment. Well.. feel free to provide an alternative fix. If the message is more specific and it must be enabled explicitly by an option then maybe it's good enough for me. Repository: rL LLVM https://reviews.llvm.org/D31097 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25596: alpha.core.Conversion - Fix false positive for 'U32 += S16; ' expression, that is not unsafe
danielmarjamaki updated this revision to Diff 92634.
danielmarjamaki added a comment.
I added more testcases. There are several undetected "TODO: loss of precision"
right now in the tests that I would like to fix. If this patch to fix FP is
accepted I will commit it and continue working on the TODO tests. If it's not
accepted I will continue investigating the TODO tests anyway..
Repository:
rL LLVM
https://reviews.llvm.org/D25596
Files:
lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
test/Analysis/conversion.c
Index: test/Analysis/conversion.c
===
--- test/Analysis/conversion.c
+++ test/Analysis/conversion.c
@@ -14,6 +14,64 @@
S8 = U;
}
+void addAssign() {
+ unsigned long L = 1000;
+ int I = -100;
+ U8 += L; // TODO: Loss of precision
+ L += I;
+}
+
+void subAssign() {
+ unsigned long L = 1000;
+ int I = -100;
+ U8 -= L; // TODO: Loss of precision
+ L -= I;
+}
+
+void mulAssign() {
+ unsigned long L = 1000;
+ int I = -1;
+ U8 *= L; // TODO: Loss of precision
+ L *= I; // expected-warning {{Loss of sign in implicit conversion}}
+ I = 10;
+ L *= I;
+}
+
+void divAssign() {
+ unsigned long L = 1000;
+ int I = -1;
+ U8 /= L;
+ L /= I; // expected-warning {{Loss of sign in implicit conversion}}
+}
+
+void remAssign() {
+ unsigned long L = 1000;
+ int I = -1;
+ U8 %= L;
+ L %= I; // expected-warning {{Loss of sign in implicit conversion}}
+}
+
+void andAssign() {
+ unsigned long L = 1000;
+ int I = -1;
+ U8 &= L;
+ L &= I; // expected-warning {{Loss of sign in implicit conversion}}
+}
+
+void orAssign() {
+ unsigned long L = 1000;
+ int I = -1;
+ U8 |= L; // TODO: Loss of precision
+ L |= I; // expected-warning {{Loss of sign in implicit conversion}}
+}
+
+void xorAssign() {
+ unsigned long L = 1000;
+ int I = -1;
+ U8 ^= L; // TODO: Loss of precision
+ L ^= I; // expected-warning {{Loss of sign in implicit conversion}}
+}
+
void init1() {
long long A = 1LL << 60;
short X = A; // expected-warning {{Loss of precision in implicit conversion}}
Index: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
+++ lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
@@ -73,8 +73,22 @@
// Loss of sign/precision in binary operation.
if (const auto *B = dyn_cast(Parent)) {
BinaryOperator::Opcode Opc = B->getOpcode();
-if (Opc == BO_Assign || Opc == BO_AddAssign || Opc == BO_SubAssign ||
-Opc == BO_MulAssign) {
+if (Opc == BO_Assign) {
+ LossOfSign = isLossOfSign(Cast, C);
+ LossOfPrecision = isLossOfPrecision(Cast, C);
+} else if (Opc == BO_AddAssign || Opc == BO_SubAssign) {
+ // No loss of sign.
+ LossOfPrecision = isLossOfPrecision(Cast, C);
+} else if (Opc == BO_MulAssign) {
+ LossOfSign = isLossOfSign(Cast, C);
+ LossOfPrecision = isLossOfPrecision(Cast, C);
+} else if (Opc == BO_DivAssign || Opc == BO_RemAssign) {
+ LossOfSign = isLossOfSign(Cast, C);
+ // No loss of precision.
+} else if (Opc == BO_AndAssign) {
+ LossOfSign = isLossOfSign(Cast, C);
+ // No loss of precision.
+} else if (Opc == BO_OrAssign || Opc == BO_XorAssign) {
LossOfSign = isLossOfSign(Cast, C);
LossOfPrecision = isLossOfPrecision(Cast, C);
} else if (B->isRelationalOp() || B->isMultiplicativeOp()) {
@@ -153,7 +167,7 @@
}
bool ConversionChecker::isLossOfPrecision(const ImplicitCastExpr *Cast,
-CheckerContext &C) const {
+ CheckerContext &C) const {
// Don't warn about explicit loss of precision.
if (Cast->isEvaluatable(C.getASTContext()))
return false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31029: [analyzer] Fix logical not for pointers with different bit width
danielmarjamaki updated this revision to Diff 92773.
danielmarjamaki added a comment.
Added a testcase that will crash without the fix. Used the amdgcn target as
that happens to use different pointer bit widths for different address spaces.
Updated the comment. I am not good at english so I hope the grammar is ok.
Repository:
rL LLVM
https://reviews.llvm.org/D31029
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
lib/StaticAnalyzer/Core/ExprEngineC.cpp
test/Analysis/ptr.cl
Index: test/Analysis/ptr.cl
===
--- test/Analysis/ptr.cl
+++ test/Analysis/ptr.cl
@@ -0,0 +1,10 @@
+// RUN: %clang_analyze_cc1 -triple amdgcn-unknown-unknown -target-cpu verde
-analyze -analyzer-checker=core %s
+
+#define __cm __attribute__((address_space(256)))
+
+// Don't crash when pointer bit-widths are different for different address
spaces
+void dontCrash(void) {
+ __cm void *cm_p = 0;
+ if (!cm_p)
+(void)cm_p;
+}
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -980,10 +980,9 @@
//transfer functions as "0 == E".
SVal Result;
if (Optional LV = V.getAs()) {
-Loc X = svalBuilder.makeNull();
+Loc X = svalBuilder.makeNullWithType(Ex->getType());
Result = evalBinOp(state, BO_EQ, *LV, X, U->getType());
- }
- else if (Ex->getType()->isFloatingType()) {
+ } else if (Ex->getType()->isFloatingType()) {
// FIXME: handle floating point types.
Result = UnknownVal();
} else {
Index: include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -310,10 +310,15 @@
return nonloc::ConcreteInt(BasicVals.getTruthValue(b));
}
- Loc makeNull() {
-return loc::ConcreteInt(BasicVals.getZeroWithPtrWidth());
+ /// Create NULL pointer, with proper pointer bit-width for given address
+ /// space.
+ /// \param type pointer type.
+ Loc makeNullWithType(QualType type) {
+return loc::ConcreteInt(BasicVals.getZeroWithTypeSize(type));
}
+ Loc makeNull() { return loc::ConcreteInt(BasicVals.getZeroWithPtrWidth()); }
+
Loc makeLoc(SymbolRef sym) {
return loc::MemRegionVal(MemMgr.getSymbolicRegion(sym));
}
Index: include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/BasicValueFactory.h
@@ -176,7 +176,12 @@
return getValue(X);
}
- inline const llvm::APSInt& getZeroWithPtrWidth(bool isUnsigned = true) {
+ inline const llvm::APSInt &getZeroWithTypeSize(QualType T,
+ bool isUnsigned = true) {
+return getValue(0, Ctx.getTypeSize(T), isUnsigned);
+ }
+
+ inline const llvm::APSInt &getZeroWithPtrWidth(bool isUnsigned = true) {
return getValue(0, Ctx.getTypeSize(Ctx.VoidPtrTy), isUnsigned);
}
Index: test/Analysis/ptr.cl
===
--- test/Analysis/ptr.cl
+++ test/Analysis/ptr.cl
@@ -0,0 +1,10 @@
+// RUN: %clang_analyze_cc1 -triple amdgcn-unknown-unknown -target-cpu verde -analyze -analyzer-checker=core %s
+
+#define __cm __attribute__((address_space(256)))
+
+// Don't crash when pointer bit-widths are different for different address spaces
+void dontCrash(void) {
+ __cm void *cm_p = 0;
+ if (!cm_p)
+(void)cm_p;
+}
Index: lib/StaticAnalyzer/Core/ExprEngineC.cpp
===
--- lib/StaticAnalyzer/Core/ExprEngineC.cpp
+++ lib/StaticAnalyzer/Core/ExprEngineC.cpp
@@ -980,10 +980,9 @@
//transfer functions as "0 == E".
SVal Result;
if (Optional LV = V.getAs()) {
-Loc X = svalBuilder.makeNull();
+Loc X = svalBuilder.makeNullWithType(Ex->getType());
Result = evalBinOp(state, BO_EQ, *LV, X, U->getType());
- }
- else if (Ex->getType()->isFloatingType()) {
+ } else if (Ex->getType()->isFloatingType()) {
// FIXME: handle floating point types.
Result = UnknownVal();
} else {
Index: include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
===
--- include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
+++ include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h
@@ -310,10 +310,15
[PATCH] D31029: [analyzer] Fix logical not for pointers with different bit width
danielmarjamaki added a comment. In https://reviews.llvm.org/D31029#703428, @zaks.anna wrote: > Are there other cases where makeNull would need to be replaced? There might be. As I understand it, this is the only known case at the moment. Repository: rL LLVM https://reviews.llvm.org/D31029 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25596: alpha.core.Conversion - Fix false positive for 'U32 += S16; ' expression, that is not unsafe
danielmarjamaki updated this revision to Diff 92810.
danielmarjamaki added a comment.
Updated the patch so all the loss of precision are detected also
Repository:
rL LLVM
https://reviews.llvm.org/D25596
Files:
lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
test/Analysis/conversion.c
Index: test/Analysis/conversion.c
===
--- test/Analysis/conversion.c
+++ test/Analysis/conversion.c
@@ -14,6 +14,64 @@
S8 = U;
}
+void addAssign() {
+ unsigned long L = 1000;
+ int I = -100;
+ U8 += L; // expected-warning {{Loss of precision in implicit conversion}}
+ L += I;
+}
+
+void subAssign() {
+ unsigned long L = 1000;
+ int I = -100;
+ U8 -= L; // expected-warning {{Loss of precision in implicit conversion}}
+ L -= I;
+}
+
+void mulAssign() {
+ unsigned long L = 1000;
+ int I = -1;
+ U8 *= L; // expected-warning {{Loss of precision in implicit conversion}}
+ L *= I; // expected-warning {{Loss of sign in implicit conversion}}
+ I = 10;
+ L *= I;
+}
+
+void divAssign() {
+ unsigned long L = 1000;
+ int I = -1;
+ U8 /= L;
+ L /= I; // expected-warning {{Loss of sign in implicit conversion}}
+}
+
+void remAssign() {
+ unsigned long L = 1000;
+ int I = -1;
+ U8 %= L;
+ L %= I; // expected-warning {{Loss of sign in implicit conversion}}
+}
+
+void andAssign() {
+ unsigned long L = 1000;
+ int I = -1;
+ U8 &= L;
+ L &= I; // expected-warning {{Loss of sign in implicit conversion}}
+}
+
+void orAssign() {
+ unsigned long L = 1000;
+ int I = -1;
+ U8 |= L; // expected-warning {{Loss of precision in implicit conversion}}
+ L |= I; // expected-warning {{Loss of sign in implicit conversion}}
+}
+
+void xorAssign() {
+ unsigned long L = 1000;
+ int I = -1;
+ U8 ^= L; // expected-warning {{Loss of precision in implicit conversion}}
+ L ^= I; // expected-warning {{Loss of sign in implicit conversion}}
+}
+
void init1() {
long long A = 1LL << 60;
short X = A; // expected-warning {{Loss of precision in implicit conversion}}
Index: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
+++ lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
@@ -41,7 +41,8 @@
mutable std::unique_ptr BT;
// Is there loss of precision
- bool isLossOfPrecision(const ImplicitCastExpr *Cast, CheckerContext &C) const;
+ bool isLossOfPrecision(const ImplicitCastExpr *Cast, QualType DestType,
+ CheckerContext &C) const;
// Is there loss of sign
bool isLossOfSign(const ImplicitCastExpr *Cast, CheckerContext &C) const;
@@ -73,16 +74,30 @@
// Loss of sign/precision in binary operation.
if (const auto *B = dyn_cast(Parent)) {
BinaryOperator::Opcode Opc = B->getOpcode();
-if (Opc == BO_Assign || Opc == BO_AddAssign || Opc == BO_SubAssign ||
-Opc == BO_MulAssign) {
+if (Opc == BO_Assign) {
LossOfSign = isLossOfSign(Cast, C);
- LossOfPrecision = isLossOfPrecision(Cast, C);
+ LossOfPrecision = isLossOfPrecision(Cast, Cast->getType(), C);
+} else if (Opc == BO_AddAssign || Opc == BO_SubAssign) {
+ // No loss of sign.
+ LossOfPrecision = isLossOfPrecision(Cast, B->getLHS()->getType(), C);
+} else if (Opc == BO_MulAssign) {
+ LossOfSign = isLossOfSign(Cast, C);
+ LossOfPrecision = isLossOfPrecision(Cast, B->getLHS()->getType(), C);
+} else if (Opc == BO_DivAssign || Opc == BO_RemAssign) {
+ LossOfSign = isLossOfSign(Cast, C);
+ // No loss of precision.
+} else if (Opc == BO_AndAssign) {
+ LossOfSign = isLossOfSign(Cast, C);
+ // No loss of precision.
+} else if (Opc == BO_OrAssign || Opc == BO_XorAssign) {
+ LossOfSign = isLossOfSign(Cast, C);
+ LossOfPrecision = isLossOfPrecision(Cast, B->getLHS()->getType(), C);
} else if (B->isRelationalOp() || B->isMultiplicativeOp()) {
LossOfSign = isLossOfSign(Cast, C);
}
} else if (isa(Parent)) {
LossOfSign = isLossOfSign(Cast, C);
-LossOfPrecision = isLossOfPrecision(Cast, C);
+LossOfPrecision = isLossOfPrecision(Cast, Cast->getType(), C);
}
if (LossOfSign || LossOfPrecision) {
@@ -113,6 +128,13 @@
unsigned long long Val) {
ProgramStateRef State = C.getState();
SVal EVal = C.getSVal(E);
+ if (EVal.isUnknownOrUndef())
+return false;
+ if (!EVal.getAs() && EVal.getAs()) {
+ProgramStateManager &Mgr = C.getStateManager();
+EVal =
+Mgr.getStoreManager().getBinding(State->getStore(), EVal.castAs());
+ }
if (EVal.isUnknownOrUndef() || !EVal.getAs())
return false;
@@ -153,22 +175,22 @@
}
bool ConversionChecker::isLossOfPrecision(const ImplicitCastExpr *Cast,
-CheckerContext &C) const {
+ QualType DestType,
+ CheckerC
[PATCH] D31029: [analyzer] Fix logical not for pointers with different bit width
danielmarjamaki marked an inline comment as done. danielmarjamaki added a comment. In https://reviews.llvm.org/D31029#708567, @danielmarjamaki wrote: > In https://reviews.llvm.org/D31029#703428, @zaks.anna wrote: > > > Are there other cases where makeNull would need to be replaced? > > > There might be. As I understand it, this is the only known case at the moment. To clarify. The static analyser runs fine on plenty of code. I ran CSA using this target on a 1000's of C-files project. I think it works well.. but I can't guarantee there won't be any more issues. Repository: rL LLVM https://reviews.llvm.org/D31029 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis
danielmarjamaki added inline comments.
Comment at: include/clang/AST/ASTContext.h:42
#include "clang/Basic/Specifiers.h"
+#include "clang/Basic/VersionTuple.h"
#include "llvm/ADT/APSInt.h"
I don't see why this is included here.
Comment at: include/clang/AST/Mangle.h:55
const ManglerKind Kind;
+ // Used for cross tranlsation unit analysis.
+ // To reduce the risk of function name collision in C projects, we force
tranlsation => translation
Comment at: lib/AST/ASTContext.cpp:1457
+return nullptr;
+ for (Decl *D : DC->decls()) {
+const auto *SubDC = dyn_cast(D);
could add a "const" perhaps
Comment at: lib/AST/ASTContext.cpp:1491
+ std::string MangledFnName = getMangledName(FD, MangleCtx.get());
+ std::string ExternalFunctionMap = (XTUDir + "/externalFnMap.txt").str();
+ ASTUnit *Unit = nullptr;
as far as I see ExternalFunctionMap can be moved into the subscope.
Comment at: lib/AST/ASTContext.cpp:1500
+ std::string FunctionName, FileName;
+ while (ExternalFnMapFile >> FunctionName >> FileName)
+FunctionFileMap[FunctionName] = (XTUDir + "/" + FileName).str();
I would recommend that a parser is added that make sure the file data is
correct/sane. If there is some load/save mismatch now or in the future or if
the user choose to tweak the file for some reason, the behaviour could be wrong.
Comment at: lib/AST/ASTContext.cpp:1502
+FunctionFileMap[FunctionName] = (XTUDir + "/" + FileName).str();
+ ExternalFnMapFile.close();
+}
well.. I believe it's redundant to close this explicitly since the
std::ifstream destructor should do that. But it doesn't hurt doing it.
Comment at: lib/Basic/SourceManager.cpp:2028
/// \brief Determines the order of 2 source locations in the translation unit.
+/// FIXME: It also works when two locations are from different translation
unit.
+///In that case it will return *some* order.
I am not good at english. But I believe unit=>units.
Comment at: lib/Basic/SourceManager.cpp:2126
}
- llvm_unreachable("Unsortable locations found");
+ // FIXME: Source locations from different translation unit.
+ return LOffs.first < ROffs.first;
I am not good at english. But I believe unit=>units.
Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:48
+#include
+#include
+#include
I believe sys/file.h and unistd.h are posix includes.
Comment at: lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp:433
+return;
+ int fd = open(fileName.c_str(), O_CREAT|O_WRONLY|O_APPEND, 0666);
+ flock(fd, LOCK_EX);
this is posix file-I/O
Comment at: tools/clang-cmdline-arch-extractor/ClangCmdlineArchExtractor.cpp:56
+ if (Sources.empty())
+return 1;
+
Maybe use the EXIT_FAILURE instead
Comment at: tools/clang-cmdline-arch-extractor/ClangCmdlineArchExtractor.cpp:66
+ for (StringRef SourceFile : Sources) {
+char *Path = realpath(SourceFile.data(), nullptr);
+if (Path)
This is posix function as far as I know.
Comment at: tools/clang-cmdline-arch-extractor/ClangCmdlineArchExtractor.cpp:72
+
+ return 0;
+}
EXIT_SUCCESS is also possible however I guess that is 0 on all implementations.
Comment at: tools/clang-func-mapping/ClangFnMapGen.cpp:32
+#include
+#include
+#include
posix includes
Comment at: tools/clang-func-mapping/ClangFnMapGen.cpp:153
+
+ if (!FileName.empty())
+switch (FD->getLinkageInternal()) {
I do not see how FileName could be empty. It always starts with "/ast/" right?
Comment at: tools/clang-func-mapping/ClangFnMapGen.cpp:193
+ lockedWrite(BuildDir + "/definedFns.txt", DefinedFuncsStr.str());
+ std::stringstream CFGStr;
+ for (auto &Entry : CG) {
if a STL stream will be used I recommend the std::ostringstream instead
Comment at: tools/clang-func-mapping/ClangFnMapGen.cpp:244
+errs() << "Exactly one XTU dir should be provided\n";
+return 1;
+ }
maybe use EXIT_FAILURE
Comment at: tools/clang-func-mapping/ClangFnMapGen.cpp:257
+ Tool.run(newFrontendActionFactory().get());
+}
no return.
Comment at: tools/xtu-analysis/xtu-analyze.py:29
+
+threading_factor = int(multiprocessing.cpu_count() * 1.5)
+analyser_output_formats = ['plist-multi-file', 'plist', 'plist-html',
does this mean that if there are 4 cores this script will by default use 6
threads? isn't that too aggressive?
Repository:
rL LLVM
https://reviews.llvm.
[PATCH] D31029: [analyzer] Fix logical not for pointers with different bit width
danielmarjamaki added a comment. Ping Repository: rL LLVM https://reviews.llvm.org/D31029 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30295: [analyzer] clarify undef shift result when shift count is negative or exceeds the bit width
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
[PATCH] D25596: alpha.core.Conversion - Fix false positive for 'U32 += S16; ' expression, that is not unsafe
danielmarjamaki added a comment. Ping Repository: rL LLVM https://reviews.llvm.org/D25596 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30691: [analyzer] Support for naive cross translational unit analysis
danielmarjamaki added inline comments. Comment at: tools/xtu-analysis/xtu-analyze.py:29 + +threading_factor = int(multiprocessing.cpu_count() * 1.5) +analyser_output_formats = ['plist-multi-file', 'plist', 'plist-html', gerazo wrote: > danielmarjamaki wrote: > > does this mean that if there are 4 cores this script will by default use 6 > > threads? isn't that too aggressive? > Yes, it does mean that. You are right, it is aggressive. To be honest, the > xtu-build step is more io intensive where it really makes sense. In the > xtu-analyze step, it is marginal when big files are compiled (more cpu, less > io). We will put this one back to 1.0 instead. I see. Feel free to use such ratio. https://reviews.llvm.org/D30691 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25596: alpha.core.Conversion - Fix false positive for 'U32 += S16; ' expression, that is not unsafe
This revision was automatically updated to reflect the committed changes.
Closed by commit rL299523: [analyzer] alpha.core.Conversion - Fix false
positive for 'U32 += S16;'… (authored by danielmarjamaki).
Changed prior to commit:
https://reviews.llvm.org/D25596?vs=92810&id=94176#toc
Repository:
rL LLVM
https://reviews.llvm.org/D25596
Files:
cfe/trunk/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
cfe/trunk/test/Analysis/conversion.c
Index: cfe/trunk/test/Analysis/conversion.c
===
--- cfe/trunk/test/Analysis/conversion.c
+++ cfe/trunk/test/Analysis/conversion.c
@@ -9,9 +9,67 @@
if (U > 300)
S8 = U; // expected-warning {{Loss of precision in implicit conversion}}
if (S > 10)
-U8 = S;
+U8 = S; // no-warning
if (U < 200)
-S8 = U;
+S8 = U; // no-warning
+}
+
+void addAssign() {
+ unsigned long L = 1000;
+ int I = -100;
+ U8 += L; // expected-warning {{Loss of precision in implicit conversion}}
+ L += I; // no-warning
+}
+
+void subAssign() {
+ unsigned long L = 1000;
+ int I = -100;
+ U8 -= L; // expected-warning {{Loss of precision in implicit conversion}}
+ L -= I; // no-warning
+}
+
+void mulAssign() {
+ unsigned long L = 1000;
+ int I = -1;
+ U8 *= L; // expected-warning {{Loss of precision in implicit conversion}}
+ L *= I; // expected-warning {{Loss of sign in implicit conversion}}
+ I = 10;
+ L *= I; // no-warning
+}
+
+void divAssign() {
+ unsigned long L = 1000;
+ int I = -1;
+ U8 /= L; // no-warning
+ L /= I; // expected-warning {{Loss of sign in implicit conversion}}
+}
+
+void remAssign() {
+ unsigned long L = 1000;
+ int I = -1;
+ U8 %= L; // no-warning
+ L %= I; // expected-warning {{Loss of sign in implicit conversion}}
+}
+
+void andAssign() {
+ unsigned long L = 1000;
+ int I = -1;
+ U8 &= L; // no-warning
+ L &= I; // expected-warning {{Loss of sign in implicit conversion}}
+}
+
+void orAssign() {
+ unsigned long L = 1000;
+ int I = -1;
+ U8 |= L; // expected-warning {{Loss of precision in implicit conversion}}
+ L |= I; // expected-warning {{Loss of sign in implicit conversion}}
+}
+
+void xorAssign() {
+ unsigned long L = 1000;
+ int I = -1;
+ U8 ^= L; // expected-warning {{Loss of precision in implicit conversion}}
+ L ^= I; // expected-warning {{Loss of sign in implicit conversion}}
}
void init1() {
@@ -21,7 +79,7 @@
void relational(unsigned U, signed S) {
if (S > 10) {
-if (U < S) {
+if (U < S) { // no-warning
}
}
if (S < -10) {
@@ -32,14 +90,14 @@
void multiplication(unsigned U, signed S) {
if (S > 5)
-S = U * S;
+S = U * S; // no-warning
if (S < -10)
S = U * S; // expected-warning {{Loss of sign}}
}
void division(unsigned U, signed S) {
if (S > 5)
-S = U / S;
+S = U / S; // no-warning
if (S < -10)
S = U / S; // expected-warning {{Loss of sign}}
}
Index: cfe/trunk/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/ConversionChecker.cpp
@@ -41,7 +41,8 @@
mutable std::unique_ptr BT;
// Is there loss of precision
- bool isLossOfPrecision(const ImplicitCastExpr *Cast, CheckerContext &C) const;
+ bool isLossOfPrecision(const ImplicitCastExpr *Cast, QualType DestType,
+ CheckerContext &C) const;
// Is there loss of sign
bool isLossOfSign(const ImplicitCastExpr *Cast, CheckerContext &C) const;
@@ -73,16 +74,30 @@
// Loss of sign/precision in binary operation.
if (const auto *B = dyn_cast(Parent)) {
BinaryOperator::Opcode Opc = B->getOpcode();
-if (Opc == BO_Assign || Opc == BO_AddAssign || Opc == BO_SubAssign ||
-Opc == BO_MulAssign) {
+if (Opc == BO_Assign) {
LossOfSign = isLossOfSign(Cast, C);
- LossOfPrecision = isLossOfPrecision(Cast, C);
+ LossOfPrecision = isLossOfPrecision(Cast, Cast->getType(), C);
+} else if (Opc == BO_AddAssign || Opc == BO_SubAssign) {
+ // No loss of sign.
+ LossOfPrecision = isLossOfPrecision(Cast, B->getLHS()->getType(), C);
+} else if (Opc == BO_MulAssign) {
+ LossOfSign = isLossOfSign(Cast, C);
+ LossOfPrecision = isLossOfPrecision(Cast, B->getLHS()->getType(), C);
+} else if (Opc == BO_DivAssign || Opc == BO_RemAssign) {
+ LossOfSign = isLossOfSign(Cast, C);
+ // No loss of precision.
+} else if (Opc == BO_AndAssign) {
+ LossOfSign = isLossOfSign(Cast, C);
+ // No loss of precision.
+} else if (Opc == BO_OrAssign || Opc == BO_XorAssign) {
+ LossOfSign = isLossOfSign(Cast, C);
+ LossOfPrecision = isLossOfPrecision(Cast, B->getLHS()->getType(), C);
} else if (B->isRelationalOp() || B->isMultiplicativeOp()) {
LossOfSign = isLossOfSign(Cast, C);
}
} else if (isa(Parent)) {
LossOfSign = isLossOfSign(
[PATCH] D31650: [Analyzer] Detect when function pointer is freed
danielmarjamaki added a comment.
In https://reviews.llvm.org/D31650#717691, @NoQ wrote:
> Is freeing function pointers always undefined?
I guess not.. however I don't personally see why it would be useful to allocate
function pointers with malloc.
> I wonder what happens if we take some JIT-enabled javascript engine, maybe
> with some on-stack replacement of theirs, it may `malloc()` a memory and use
> it as a function, and then eventually it'd need to free it by design.
> However, because we're analyzing a small part of the program, we may fail to
> see in the analyzer that the symbolic pointer originally comes from
> `malloc()`. Would such rare but important users be able to avoid/suppress the
> warning?
Maybe when writing JIT there is some usecase, I don't know. The code could be
rewritten like:
void *malloc(unsigned long);
void free(void*);
typedef void (*fnptr)(int);
void allocatedFunctionPointer() {
void *p = malloc(sizeof(fnptr));
fnptr p2 = (fnptr)p;
free(p);
}
no warning is written about this code.
Repository:
rL LLVM
https://reviews.llvm.org/D31650
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31650: [Analyzer] Detect when function pointer is freed
danielmarjamaki added a comment. > void *p = malloc(sizeof(fnptr)); sorry ... I guess that should be something like "void *p = malloc(100);" Repository: rL LLVM https://reviews.llvm.org/D31650 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30489: [analyzer] catch out of bounds for VLA
danielmarjamaki updated this revision to Diff 94509.
danielmarjamaki added a comment.
This is just work in progress!!
With these changes Clang static analyzer will detect overflow in this sample
code:
void foo(int X) {
char *Data = new char[X];
Data[X] = 0; // <- error
delete[] Data;
}
I updated SimpleSValBuilder so evalEQ can calculate a SVal when both lhs and
rhs are symbols. Source code that used to have problems:
DefinedOrUnknownSVal extentMatchesSize = svalBuilder.evalEQ(
State, Extent, SizeInBytes.castAs());
Inputs: Extent is "extent_$3{SymRegion{conj_$1{char *}}}" and SizeInBytes is
"reg_$0".
Before my quick fix the return SVal is "Unknown".
With my quick fix it will return a SVal "(extent_$3{SymRegion{conj_$1{char
*}}}) == (reg_$0)".
I also made a simple fix for the ConstraintManager. If the state says X==Y then
the ConstraintManager should be able to evaluate Y>=X. My evalUgly loops
through the constraints and matches them manually.
Do you have some feedback? Do you think my SimpleSValBuilder approach is fine
to commit if I polish it? It was just a quick hack so I guess it might make
some tests fail etc.
About the ConstraintManager fix. Is it a good idea to handle simple SymSymExpr
constraints? Or should this be handled by Z3 instead?
Repository:
rL LLVM
https://reviews.llvm.org/D30489
Files:
include/clang/StaticAnalyzer/Core/PathSensitive/ConstraintManager.h
lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
Index: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -547,6 +547,13 @@
if (const llvm::APSInt *RHSValue = getKnownValue(state, rhs))
return MakeSymIntVal(Sym, op, *RHSValue, resultTy);
+ if (!state->isTainted(rhs) && !state->isTainted(lhs)) {
+const SymExpr *lhse = lhs.getAsSymExpr();
+const SymExpr *rhse = rhs.getAsSymExpr();
+return nonloc::SymbolVal(
+SymMgr.getSymSymExpr(lhse, op, rhse, resultTy));
+ }
+
// Give up -- this is not a symbolic expression we can handle.
return makeSymExprValNN(state, op, InputLHS, InputRHS, resultTy);
}
Index: lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/SimpleConstraintManager.cpp
@@ -53,6 +53,19 @@
ProgramStateRef SimpleConstraintManager::assumeAux(ProgramStateRef State,
NonLoc Cond,
bool Assumption) {
+ if (1) {
+Optional SymVal = Cond.getAs();
+if (SymVal && SymVal->isExpression()) {
+ const SymExpr *SE = SymVal->getSymbol();
+ if (const SymSymExpr *SSE = dyn_cast(SE)) {
+if (SSE->getOpcode() == BO_GE) {
+ SymSymExpr SSE2(SSE->getRHS(), BO_EQ, SSE->getLHS(), SSE->getType());
+ if (State->getConstraintManager().uglyEval(&SSE2, State))
+return Assumption ? State : nullptr;
+}
+ }
+}
+ }
// We cannot reason about SymSymExprs, and can only reason about some
// SymIntExprs.
Index: lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -287,6 +287,8 @@
RangeConstraintManager(SubEngine *SE, SValBuilder &SVB)
: RangedConstraintManager(SE, SVB) {}
+ virtual bool uglyEval(const SymSymExpr *SSE, ProgramStateRef state);
+
//===--===//
// Implementation for interface from ConstraintManager.
//===--===//
@@ -723,6 +725,25 @@
// Pretty-printing.
//======/
+bool RangeConstraintManager::uglyEval(const SymSymExpr *SSE,
+ ProgramStateRef State) {
+ ConstraintRangeTy Ranges = State->get();
+ for (ConstraintRangeTy::iterator I = Ranges.begin(), E = Ranges.end(); I !=
E;
+ ++I) {
+SymbolRef SR = I.getKey();
+if (const SymSymExpr *SSE2 = dyn_cast(SR)) {
+ if (SSE->getOpcode() != SSE2->getOpcode())
+continue;
+ if (SSE->getLHS() != SSE2->getLHS())
+continue;
+ if (SSE->getRHS() != SSE2->getRHS())
+continue;
+ return true;
+}
+ }
+ return false;
+}
+
void RangeConstraintManager::print(ProgramStateRef St, raw_ostream &Out,
const char *nl, const char *sep) {
Index: include/clang/StaticAnalyzer/Core/Pat
[PATCH] D31886: [analyzer] Simplify values in binary operations more aggressively
danielmarjamaki added inline comments. Comment at: lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:49 + /// Recursively descends into symbolic expressions and replaces symbols + /// with thier known values (in the sense of the getKnownValue() method). + SVal simplifySVal(ProgramStateRef State, SVal V) override; thier => their https://reviews.llvm.org/D31886 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31868: [analyzer] Check NULL pointer dereference issue for memset function
danielmarjamaki added a comment.
Thanks! Looks like a valueable addition.
Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2004
+void CStringChecker::evalMemset(CheckerContext &C, const CallExpr *CE) const {
+ if (CE->getNumArgs() < 3)
+return;
even better: != 3
Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2009
+
+ const Expr *S = CE->getArg(0);
+ const Expr *Size = CE->getArg(2);
The name "S" does not tell me much.. how about something like Data / DataArg /
PtrArg / ..?
Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2011
+ const Expr *Size = CE->getArg(2);
+ ProgramStateRef state = C.getState();
+
Variables should start with capital.. State, SizeVal, SizeTy, ...
Comment at: lib/StaticAnalyzer/Checkers/CStringChecker.cpp:2034
+ // If the size can be nonzero, we have to check the other arguments.
+ if (stateNonZeroSize) {
+state = stateNonZeroSize;
use early return:
if (!stateNonZeroSize)
return;
Repository:
rL LLVM
https://reviews.llvm.org/D31868
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D30909: [Analyzer] Finish taint propagation to derived symbols of tainted regions
danielmarjamaki added inline comments.
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/TaintManager.h:45
+: public ProgramStatePartialTrait {
+ static void *GDMIndex() { static int index = 0; return &index; }
+};
Nit: =0 is redundant
https://reviews.llvm.org/D30909
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D31029: [analyzer] Fix logical not for pointers with different bit width
danielmarjamaki added a comment. Ping Repository: rL LLVM https://reviews.llvm.org/D31029 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
