Skip to content

Commit

Permalink
[analyzer] Fix constraint being dropped when analyzing a program with…
Browse files Browse the repository at this point in the history 
…out taint tracking enabled

Summary:
This patch removes the constraint dropping when taint tracking is disabled.

It also voids the crash reported in D28953 by treating a SymSymExpr with non pointer symbols as an opaque expression.

Updated the regressions and verifying the big projects now; I'll update here when they're done.

Based on the discussion on the mailing list and the patches by @ddcc.

Reviewers: george.karpenkov, NoQ, ddcc, baloghadamsoftware

Reviewed By: george.karpenkov

Subscribers: delcypher, llvm-commits, rnkovacs, xazax.hun, szepet, a.sidorin, ddcc

Differential Revision: https://reviews.llvm.org/D48650

llvm-svn: 337167
  • Loading branch information
@mikhailramalho
mikhailramalho committed Jul 16, 2018
1 parent de50663 commit e254b0f
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 39 deletions.
2 changes: 1 addition & 1 deletion clang/lib/StaticAnalyzer/Core/AnalyzerOptions.cpp
View file
Expand Up @@ -390,7 +390,7 @@ unsigned AnalyzerOptions::getGraphTrimInterval() {

unsigned AnalyzerOptions::getMaxSymbolComplexity() {
if (!MaxSymbolComplexity.hasValue())
MaxSymbolComplexity = getOptionAsInteger("max-symbol-complexity", 10000);
MaxSymbolComplexity = getOptionAsInteger("max-symbol-complexity", 25);
return MaxSymbolComplexity.getValue();
}

Expand Down
23 changes: 12 additions & 11 deletions clang/lib/StaticAnalyzer/Core/RangedConstraintManager.cpp
View file
Expand Up @@ -52,17 +52,18 @@ ProgramStateRef RangedConstraintManager::assumeSym(ProgramStateRef State,
assert(BinaryOperator::isComparisonOp(Op));

// For now, we only support comparing pointers.
assert(Loc::isLocType(SSE->getLHS()->getType()));
assert(Loc::isLocType(SSE->getRHS()->getType()));
QualType DiffTy = SymMgr.getContext().getPointerDiffType();
SymbolRef Subtraction =
SymMgr.getSymSymExpr(SSE->getRHS(), BO_Sub, SSE->getLHS(), DiffTy);

const llvm::APSInt &Zero = getBasicVals().getValue(0, DiffTy);
Op = BinaryOperator::reverseComparisonOp(Op);
if (!Assumption)
Op = BinaryOperator::negateComparisonOp(Op);
return assumeSymRel(State, Subtraction, Op, Zero);
if (Loc::isLocType(SSE->getLHS()->getType()) &&
Loc::isLocType(SSE->getRHS()->getType())) {
QualType DiffTy = SymMgr.getContext().getPointerDiffType();
SymbolRef Subtraction =
SymMgr.getSymSymExpr(SSE->getRHS(), BO_Sub, SSE->getLHS(), DiffTy);

const llvm::APSInt &Zero = getBasicVals().getValue(0, DiffTy);
Op = BinaryOperator::reverseComparisonOp(Op);
if (!Assumption)
Op = BinaryOperator::negateComparisonOp(Op);
return assumeSymRel(State, Subtraction, Op, Zero);
}
}

// If we get here, there's nothing else we can do but treat the symbol as
Expand Down
4 changes: 1 addition & 3 deletions clang/lib/StaticAnalyzer/Core/SValBuilder.cpp
View file
Expand Up @@ -379,11 +379,9 @@ SVal SValBuilder::makeSymExprValNN(ProgramStateRef State,
BinaryOperator::Opcode Op,
NonLoc LHS, NonLoc RHS,
QualType ResultTy) {
if (!State->isTainted(RHS) && !State->isTainted(LHS))
return UnknownVal();

const SymExpr *symLHS = LHS.getAsSymExpr();
const SymExpr *symRHS = RHS.getAsSymExpr();

// TODO: When the Max Complexity is reached, we should conjure a symbol
// instead of generating an Unknown value and propagate the taint info to it.
const unsigned MaxComp = StateMgr.getOwningEngine()
Expand Down
2 changes: 1 addition & 1 deletion clang/test/Analysis/PR37855.c
View file
Expand Up @@ -20,5 +20,5 @@ void k(l, node) {
nodep = n;
}
if (nodep) // expected-warning {{Branch condition evaluates to a garbage value}}
n[1].node->s; // expected-warning {{Dereference of undefined pointer value}}
n[1].node->s;
}
5 changes: 2 additions & 3 deletions clang/test/Analysis/bitwise-ops.c
View file
Expand Up @@ -8,9 +8,8 @@ void testPersistentConstraints(int x, int y) {
CHECK(x); // expected-warning{{TRUE}}
CHECK(x & 1); // expected-warning{{TRUE}}

// False positives due to SValBuilder giving up on certain kinds of exprs.
CHECK(1 - x); // expected-warning{{UNKNOWN}}
CHECK(x & y); // expected-warning{{UNKNOWN}}
CHECK(1 - x); // expected-warning{{TRUE}}
CHECK(x & y); // expected-warning{{TRUE}}
}

int testConstantShifts_PR18073(int which) {
Expand Down
3 changes: 1 addition & 2 deletions clang/test/Analysis/std-c-library-functions.c
View file
Expand Up @@ -57,8 +57,7 @@ void test_fread_fwrite(FILE *fp, int *buf) {
size_t y = fread(buf, sizeof(int), 10, fp);
clang_analyzer_eval(y <= 10); // expected-warning{{TRUE}}
size_t z = fwrite(buf, sizeof(int), y, fp);
// FIXME: should be TRUE once symbol-symbol constraint support is improved.
clang_analyzer_eval(z <= y); // expected-warning{{UNKNOWN}}
clang_analyzer_eval(z <= y); // expected-warning{{TRUE}}
}

ssize_t getline(char **, size_t *, FILE *);
Expand Down
36 changes: 18 additions & 18 deletions clang/test/Analysis/svalbuilder-rearrange-comparisons.c
View file
Expand Up @@ -560,7 +560,7 @@ void compare_same_symbol_plus_left_int_equal_unsigned() {
clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 1}}
clang_analyzer_dump(y); // expected-warning{{conj_$2{int}}}
clang_analyzer_dump(x == y);
// expected-warning@-1{{Unknown}} // FIXME: Can this be simplified?
// expected-warning@-1{{((conj_$2{int}) + 1U) == (conj_$2{int})}}
}

void compare_same_symbol_minus_left_int_equal_unsigned() {
Expand All @@ -569,23 +569,23 @@ void compare_same_symbol_minus_left_int_equal_unsigned() {
clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 1}}
clang_analyzer_dump(y); // expected-warning{{conj_$2{int}}}
clang_analyzer_dump(x == y);
// expected-warning@-1{{Unknown}} // FIXME: Can this be simplified?
// expected-warning@-1{{((conj_$2{int}) - 1U) == (conj_$2{int})}}
}

void compare_same_symbol_plus_right_int_equal_unsigned() {
unsigned x = f(), y = x+1;
clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
clang_analyzer_dump(y); // expected-warning{{(conj_$2{int}) + 1}}
clang_analyzer_dump(x == y);
// expected-warning@-1{{Unknown}} // FIXME: Can this be simplified?
// expected-warning@-1{{(conj_$2{int}) == ((conj_$2{int}) + 1U)}}
}

void compare_same_symbol_minus_right_int_equal_unsigned() {
unsigned x = f(), y = x-1;
clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
clang_analyzer_dump(y); // expected-warning{{(conj_$2{int}) - 1}}
clang_analyzer_dump(x == y);
// expected-warning@-1{{Unknown}} // FIXME: Can this be simplified?
// expected-warning@-1{{(conj_$2{int}) == ((conj_$2{int}) - 1U)}}
}

void compare_same_symbol_plus_left_plus_right_int_equal_unsigned() {
Expand All @@ -603,7 +603,7 @@ void compare_same_symbol_plus_left_minus_right_int_equal_unsigned() {
clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 1}}
clang_analyzer_dump(y); // expected-warning{{(conj_$2{int}) - 1}}
clang_analyzer_dump(x == y);
// expected-warning@-1{{Unknown}} // FIXME: Can this be simplified?
// expected-warning@-1{{((conj_$2{int}) + 1U) == ((conj_$2{int}) - 1U)}}
}

void compare_same_symbol_minus_left_plus_right_int_equal_unsigned() {
Expand All @@ -612,7 +612,7 @@ void compare_same_symbol_minus_left_plus_right_int_equal_unsigned() {
clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 1}}
clang_analyzer_dump(y); // expected-warning{{(conj_$2{int}) + 1}}
clang_analyzer_dump(x == y);
// expected-warning@-1{{Unknown}} // FIXME: Can this be simplified?
// expected-warning@-1{{((conj_$2{int}) - 1U) == ((conj_$2{int}) + 1U)}}
}

void compare_same_symbol_minus_left_minus_right_int_equal_unsigned() {
Expand Down Expand Up @@ -710,7 +710,7 @@ void compare_same_symbol_plus_left_int_less_or_equal_unsigned() {
clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 1}}
clang_analyzer_dump(y); // expected-warning{{conj_$2{int}}}
clang_analyzer_dump(x <= y);
// expected-warning@-1{{Unknown}} // FIXME: Can this be simplified?
// expected-warning@-1{{((conj_$2{int}) + 1U) <= (conj_$2{int})}}
}

void compare_same_symbol_minus_left_int_less_or_equal_unsigned() {
Expand All @@ -719,23 +719,23 @@ void compare_same_symbol_minus_left_int_less_or_equal_unsigned() {
clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 1}}
clang_analyzer_dump(y); // expected-warning{{conj_$2{int}}}
clang_analyzer_dump(x <= y);
// expected-warning@-1{{Unknown}} // FIXME: Can this be simplified?
// expected-warning@-1{{((conj_$2{int}) - 1U) <= (conj_$2{int})}}
}

void compare_same_symbol_plus_right_int_less_or_equal_unsigned() {
unsigned x = f(), y = x+1;
clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
clang_analyzer_dump(y); // expected-warning{{(conj_$2{int}) + 1}}
clang_analyzer_dump(x <= y);
// expected-warning@-1{{Unknown}} // FIXME: Can this be simplified?
// expected-warning@-1{{(conj_$2{int}) <= ((conj_$2{int}) + 1U)}}
}

void compare_same_symbol_minus_right_int_less_or_equal_unsigned() {
unsigned x = f(), y = x-1;
clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
clang_analyzer_dump(y); // expected-warning{{(conj_$2{int}) - 1}}
clang_analyzer_dump(x <= y);
// expected-warning@-1{{Unknown}} // FIXME: Can this be simplified?
// expected-warning@-1{{(conj_$2{int}) <= ((conj_$2{int}) - 1U)}}
}

void compare_same_symbol_plus_left_plus_right_int_less_or_equal_unsigned() {
Expand All @@ -753,7 +753,7 @@ void compare_same_symbol_plus_left_minus_right_int_less_or_equal_unsigned() {
clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 1}}
clang_analyzer_dump(y); // expected-warning{{(conj_$2{int}) - 1}}
clang_analyzer_dump(x <= y);
// expected-warning@-1{{Unknown}} // FIXME: Can this be simplified?
// expected-warning@-1{{((conj_$2{int}) + 1U) <= ((conj_$2{int}) - 1U)}}
}

void compare_same_symbol_minus_left_plus_right_int_less_or_equal_unsigned() {
Expand All @@ -762,7 +762,7 @@ void compare_same_symbol_minus_left_plus_right_int_less_or_equal_unsigned() {
clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 1}}
clang_analyzer_dump(y); // expected-warning{{(conj_$2{int}) + 1}}
clang_analyzer_dump(x <= y);
// expected-warning@-1{{Unknown}} // FIXME: Can this be simplified?
// expected-warning@-1{{((conj_$2{int}) - 1U) <= ((conj_$2{int}) + 1U)}}
}

void compare_same_symbol_minus_left_minus_right_int_less_or_equal_unsigned() {
Expand Down Expand Up @@ -860,7 +860,7 @@ void compare_same_symbol_plus_left_int_less_unsigned() {
clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 1}}
clang_analyzer_dump(y); // expected-warning{{conj_$2{int}}}
clang_analyzer_dump(x < y);
// expected-warning@-1{{Unknown}} // FIXME: Can this be simplified?
// expected-warning@-1{{((conj_$2{int}) + 1U) < (conj_$2{int})}}
}

void compare_same_symbol_minus_left_int_less_unsigned() {
Expand All @@ -869,23 +869,23 @@ void compare_same_symbol_minus_left_int_less_unsigned() {
clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 1}}
clang_analyzer_dump(y); // expected-warning{{conj_$2{int}}}
clang_analyzer_dump(x < y);
// expected-warning@-1{{Unknown}} // FIXME: Can this be simplified?
// expected-warning@-1{{((conj_$2{int}) - 1U) < (conj_$2{int})}}
}

void compare_same_symbol_plus_right_int_less_unsigned() {
unsigned x = f(), y = x+1;
clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
clang_analyzer_dump(y); // expected-warning{{(conj_$2{int}) + 1}}
clang_analyzer_dump(x < y);
// expected-warning@-1{{Unknown}} // FIXME: Can this be simplified?
// expected-warning@-1{{(conj_$2{int}) < ((conj_$2{int}) + 1U)}}
}

void compare_same_symbol_minus_right_int_less_unsigned() {
unsigned x = f(), y = x-1;
clang_analyzer_dump(x); // expected-warning{{conj_$2{int}}}
clang_analyzer_dump(y); // expected-warning{{(conj_$2{int}) - 1}}
clang_analyzer_dump(x < y);
// expected-warning@-1{{Unknown}} // FIXME: Can this be simplified?
// expected-warning@-1{{(conj_$2{int}) < ((conj_$2{int}) - 1U)}}
}

void compare_same_symbol_plus_left_plus_right_int_less_unsigned() {
Expand All @@ -903,7 +903,7 @@ void compare_same_symbol_plus_left_minus_right_int_less_unsigned() {
clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) + 1}}
clang_analyzer_dump(y); // expected-warning{{(conj_$2{int}) - 1}}
clang_analyzer_dump(x < y);
// expected-warning@-1{{Unknown}} // FIXME: Can this be simplified?
// expected-warning@-1{{((conj_$2{int}) + 1U) < ((conj_$2{int}) - 1U)}}
}

void compare_same_symbol_minus_left_plus_right_int_less_unsigned() {
Expand All @@ -912,7 +912,7 @@ void compare_same_symbol_minus_left_plus_right_int_less_unsigned() {
clang_analyzer_dump(x); // expected-warning{{(conj_$2{int}) - 1}}
clang_analyzer_dump(y); // expected-warning{{(conj_$2{int}) + 1}}
clang_analyzer_dump(x < y);
// expected-warning@-1{{Unknown}} // FIXME: Can this be simplified?
// expected-warning@-1{{((conj_$2{int}) - 1U) < ((conj_$2{int}) + 1U)}}
}

void compare_same_symbol_minus_left_minus_right_int_less_unsigned() {
Expand Down

0 comments on commit e254b0f

Please sign in to comment.