[llvm-branch-commits] [cfe-branch] r354130 - Merging r353943:

Hans Wennborg via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Fri Feb 15 06:00:26 PST 2019


Author: hans
Date: Fri Feb 15 06:00:25 2019
New Revision: 354130

URL: http://llvm.org/viewvc/llvm-project?rev=354130&view=rev
Log:
Merging r353943:
------------------------------------------------------------------------
r353943 | baloghadamsoftware | 2019-02-13 13:25:47 +0100 (Wed, 13 Feb 2019) | 22 lines

[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/branches/release_80/test/Analysis/PR40625.cpp
      - copied unchanged from r353943, cfe/trunk/test/Analysis/PR40625.cpp
Modified:
    cfe/branches/release_80/   (props changed)
    cfe/branches/release_80/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
    cfe/branches/release_80/test/Analysis/uninit-vals.m

Propchange: cfe/branches/release_80/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Fri Feb 15 06:00:25 2019
@@ -1,4 +1,4 @@
 /cfe/branches/type-system-rewrite:134693-134817
-/cfe/trunk:351334,351340,351344,351360,351457,351459,351531,351579-351580,352040,352079,352099,352102,352105,352156,352221-352222,352229,352307,352323,352463,352539,352610,352672,352822,353142,353393,353402,353411,353431,353493,353495,353656,353976,354074
+/cfe/trunk:351334,351340,351344,351360,351457,351459,351531,351579-351580,352040,352079,352099,352102,352105,352156,352221-352222,352229,352307,352323,352463,352539,352610,352672,352822,353142,353393,353402,353411,353431,353493,353495,353656,353943,353976,354074
 /cfe/trunk/test:170344
 /cfe/trunk/test/SemaTemplate:126920

Modified: cfe/branches/release_80/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/release_80/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp?rev=354130&r1=354129&r2=354130&view=diff
==============================================================================
--- cfe/branches/release_80/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp (original)
+++ cfe/branches/release_80/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp Fri Feb 15 06:00:25 2019
@@ -154,6 +154,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.
 //===----------------------------------------------------------------------===//
@@ -1188,7 +1214,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;
@@ -1209,6 +1235,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();

Modified: cfe/branches/release_80/test/Analysis/uninit-vals.m
URL: http://llvm.org/viewvc/llvm-project/cfe/branches/release_80/test/Analysis/uninit-vals.m?rev=354130&r1=354129&r2=354130&view=diff
==============================================================================
--- cfe/branches/release_80/test/Analysis/uninit-vals.m (original)
+++ cfe/branches/release_80/test/Analysis/uninit-vals.m Fri Feb 15 06:00:25 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}}
 




More information about the llvm-branch-commits mailing list