Author: baloghadamsoftware Date: Wed Feb 13 04:25:47 2019 New Revision: 353943
URL: http://llvm.org/viewvc/llvm-project?rev=353943&view=rev Log: [Analyzer] Crash fix for FindLastStoreBRVisitor FindLastStoreBRVisitor tries to find the first node in the exploded graph where the current value was assigned to a region. This node is called the "store site". It is identified by a pair of Pred and Succ nodes where Succ already has the binding for the value while Pred does not have it. However the visitor mistakenly identifies a node pair as the store site where the value is a `LazyCompoundVal` and `Pred` does not have a store yet but `Succ` has it. In this case the `LazyCompoundVal` is different in the `Pred` node because it also contains the store which is different in the two nodes. This error may lead to crashes (a declaration is cast to a parameter declaration without check) or misleading bug path notes. In this patch we fix this problem by checking for unequal `LazyCompoundVals`: if their region is equal, and their store is the same as the store of their nodes we consider them as equal when looking for the "store site". This is an approximation because we do not check for differences of the subvalues (structure members or array elements) in the stores. Differential Revision: https://reviews.llvm.org/D58067 Added: cfe/trunk/test/Analysis/PR40625.cpp Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp cfe/trunk/test/Analysis/uninit-vals.m Modified: cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=353943&r1=353942&r2=353943&view=diff ============================================================================== --- cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original) +++ cfe/trunk/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Wed Feb 13 04:25:47 2019 @@ -153,6 +153,32 @@ const Expr *bugreporter::getDerefExpr(co return E; } +/// Comparing internal representations of symbolic values (via +/// SVal::operator==()) is a valid way to check if the value was updated, +/// unless it's a LazyCompoundVal that may have a different internal +/// representation every time it is loaded from the state. In this function we +/// do an approximate comparison for lazy compound values, checking that they +/// are the immediate snapshots of the tracked region's bindings within the +/// node's respective states but not really checking that these snapshots +/// actually contain the same set of bindings. +bool hasVisibleUpdate(const ExplodedNode *LeftNode, SVal LeftVal, + const ExplodedNode *RightNode, SVal RightVal) { + if (LeftVal == RightVal) + return true; + + const auto LLCV = LeftVal.getAs<nonloc::LazyCompoundVal>(); + if (!LLCV) + return false; + + const auto RLCV = RightVal.getAs<nonloc::LazyCompoundVal>(); + if (!RLCV) + return false; + + return LLCV->getRegion() == RLCV->getRegion() && + LLCV->getStore() == LeftNode->getState()->getStore() && + RLCV->getStore() == RightNode->getState()->getStore(); +} + //===----------------------------------------------------------------------===// // Definitions for bug reporter visitors. //===----------------------------------------------------------------------===// @@ -1177,7 +1203,7 @@ FindLastStoreBRVisitor::VisitNode(const if (Succ->getState()->getSVal(R) != V) return nullptr; - if (Pred->getState()->getSVal(R) == V) { + if (hasVisibleUpdate(Pred, Pred->getState()->getSVal(R), Succ, V)) { Optional<PostStore> PS = Succ->getLocationAs<PostStore>(); if (!PS || PS->getLocationValue() != R) return nullptr; @@ -1198,6 +1224,7 @@ FindLastStoreBRVisitor::VisitNode(const // UndefinedVal.) if (Optional<CallEnter> CE = Succ->getLocationAs<CallEnter>()) { if (const auto *VR = dyn_cast<VarRegion>(R)) { + const auto *Param = cast<ParmVarDecl>(VR->getDecl()); ProgramStateManager &StateMgr = BRC.getStateManager(); Added: cfe/trunk/test/Analysis/PR40625.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/PR40625.cpp?rev=353943&view=auto ============================================================================== --- cfe/trunk/test/Analysis/PR40625.cpp (added) +++ cfe/trunk/test/Analysis/PR40625.cpp Wed Feb 13 04:25:47 2019 @@ -0,0 +1,16 @@ +// RUN: %clang_analyze_cc1 -std=c++11 -analyzer-checker=core,alpha.core.CallAndMessageUnInitRefArg %s -verify + +void f(const int *end); + +void g(const int (&arrr)[10]) { + f(arrr+sizeof(arrr)); // expected-warning{{1st function call argument is a pointer to uninitialized value}} + // FIXME: This is a false positive that should be fixed. Until then this + // tests the crash fix in FindLastStoreBRVisitor (beside + // uninit-vals.m). +} + +void h() { + int arr[10]; + + g(arr); +} Modified: cfe/trunk/test/Analysis/uninit-vals.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/uninit-vals.m?rev=353943&r1=353942&r2=353943&view=diff ============================================================================== --- cfe/trunk/test/Analysis/uninit-vals.m (original) +++ cfe/trunk/test/Analysis/uninit-vals.m Wed Feb 13 04:25:47 2019 @@ -394,11 +394,11 @@ void testSmallStructBitfieldsFirstUnname struct { int : 4; int y : 4; - } a, b, c; + } a, b, c; // expected-note{{'c' initialized here}} a.y = 2; - b = a; // expected-note{{Value assigned to 'c'}} + b = a; clang_analyzer_eval(b.y == 2); // expected-warning{{TRUE}} // expected-note@-1{{TRUE}} @@ -411,11 +411,11 @@ void testSmallStructBitfieldsSecondUnnam struct { int x : 4; int : 4; - } a, b, c; + } a, b, c; // expected-note{{'c' initialized here}} a.x = 1; - b = a; // expected-note{{Value assigned to 'c'}} + b = a; clang_analyzer_eval(b.x == 1); // expected-warning{{TRUE}} // expected-note@-1{{TRUE}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
