r353943 - [Analyzer] Crash fix for FindLastStoreBRVisitor
Hans Wennborg via cfe-commits
cfe-commits at lists.llvm.org
Fri Feb 15 06:00:14 PST 2019
Merged to release_80 in r354130. Please let me know if there are any follow-ups.
On Wed, Feb 13, 2019 at 1:25 PM Adam Balogh via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
>
> 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 at -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 at -1{{TRUE}}
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list