GP-3429 Intersect tests for stack affecting ops

Ghidra/Features/Decompiler/src/decompile/cpp/cover.cc

@@ -333,6 +333,54 @@ void Cover::intersectList(vector<int4> &listout,const Cover &op2,int4 level) con
   }
 }
 
+/// If any PcodeOp in the set falls inside \b this Cover, a secondary test that the PcodeOp
+/// affects the representative Varnode is performed. If the test returns \b true, this is considered
+/// a full intersection and this method returns \b true.  Otherwise it returns \b false.
+/// \param opSet is the given set of PcodeOps
+/// \param rep is the representative Varnode to use for secondary testing
+/// \return \b true is there is an intersection with \b this
+bool Cover::intersect(const PcodeOpSet &opSet,Varnode *rep) const
+
+{
+  if (opSet.opList.empty()) return false;
+  int4 setBlock = 0;
+  int4 opIndex = opSet.blockStart[setBlock];
+  int4 setIndex = opSet.opList[opIndex]->getParent()->getIndex();
+  map<int4,CoverBlock>::const_iterator coverIter = cover.lower_bound(opSet.opList[0]->getParent()->getIndex());
+  while(coverIter != cover.end()) {
+    int4 coverIndex = (*coverIter).first;
+    if (coverIndex < setIndex) {
+      ++coverIter;
+    }
+    else if (coverIndex > setIndex) {
+      setBlock += 1;
+      if (setBlock >= opSet.blockStart.size()) break;
+      opIndex = opSet.blockStart[setBlock];
+      setIndex = opSet.opList[opIndex]->getParent()->getIndex();
+    }
+    else {
+      const CoverBlock &coverBlock( (*coverIter).second );
+      ++coverIter;
+      int4 opMax = opSet.opList.size();
+      setBlock += 1;
+      if (setBlock < opSet.blockStart.size())
+	opMax = opSet.blockStart[setBlock];
+      do {
+	PcodeOp *op = opSet.opList[opIndex];
+	if (coverBlock.contain(op)) {		// Does range contain the call?
+	  if (coverBlock.boundary(op) == 0) {	// Is the call on the boundary
+	    if (opSet.affectsTest(op, rep))	// Do secondary testing
+	      return true;
+	  }
+	}
+	opIndex += 1;
+      } while(opIndex < opMax);
+      if (setBlock >= opSet.blockStart.size()) break;
+    }
+  }
+  return false;
+}
+
 /// Looking only at the given block, Return
 ///   - 0 if there is no intersection
 ///   - 1 if the only intersection is on a boundary point
@@ -565,4 +613,31 @@ void Cover::print(ostream &s) const
   }
 }
 
+void PcodeOpSet::finalize(void)
+
+{
+  sort(opList.begin(),opList.end(),compareByBlock);
+  int4 blockNum = -1;
+  for(int4 i=0;i<opList.size();++i) {
+    int4 newBlockNum = opList[i]->getParent()->getIndex();
+    if (newBlockNum > blockNum) {
+      blockStart.push_back(i);
+      blockNum = newBlockNum;
+    }
+  }
+  is_pop = true;
+}
+
+/// Compare first by index of the containing basic blocks, then by SeqNum ordering (within the block)
+/// \param a is the first PcodeOp to compare
+/// \param b is the second PcodeOp to compare
+/// \return \b true if the first PcodeOp should be ordered before the second
+bool PcodeOpSet::compareByBlock(const PcodeOp *a,const PcodeOp *b)
+
+{
+  if (a->getParent() != b->getParent())
+    return (a->getParent()->getIndex() < b->getParent()->getIndex());
+  return a->getSeqNum().getOrder() < b->getSeqNum().getOrder();
+}
+
 } // End namespace ghidra

Ghidra/Features/Decompiler/src/decompile/cpp/cover.hh

@@ -26,6 +26,44 @@ class PcodeOp;
 class FlowBlock;
 class Varnode;
 
+/// \brief A set of PcodeOps that can be tested for Cover intersections
+///
+/// This is a set of PcodeOp objects, designed for quick intersection tests with a Cover.  The set is
+/// lazily constructed via its populate() method at the time the first intersection test is needed.
+/// Once an intersection has been established between a PcodeOp in \b this set and a Varnode Cover,
+/// affectsTest() can do secondary testing to determine if the intersection should prevent merging.
+class PcodeOpSet {
+  friend class Cover;
+  vector<PcodeOp *> opList;		// Ops in this set, sorted on block index, then SeqNum::order
+  vector<int4> blockStart;		// Index of first op in each non-empty block
+  bool is_pop;				// Has the populate() method been called
+protected:
+  void addOp(PcodeOp *op) { opList.push_back(op); }	///< Add a PcodeOp into the set
+  void finalize(void);			// Sort ops in the set into blocks
+public:
+  PcodeOpSet(void) { is_pop = false; }
+  bool isPopulated(void) const { return is_pop; }	/// Return \b true if \b this set is populated
+  virtual ~PcodeOpSet(void) {}
+
+  /// \brief Populate the PcodeOp object in \b this set
+  ///
+  /// Call-back to the owner to lazily add PcodeOps to \b this set.  The override method calls addOp() for
+  /// each PcodeOp it wants to add, then calls finalize() to make \b this set ready for intersection tests.
+  virtual void populate(void)=0;
+
+  /// \brief (Secondary) test that the given PcodeOp affects the Varnode
+  ///
+  /// This method is called after an intersection of a PcodeOp in \b this set with a Varnode Cover has been
+  /// determined.  This allows the owner to make a final determination if merging should be prevented.
+  /// \param op is the PcodeOp that intersects with the Varnode Cover
+  /// \param vn is the Varnode whose Cover is intersected
+  /// \return \b true if merging should be prevented
+  virtual bool affectsTest(PcodeOp *op,Varnode *vn) const=0;
+
+  void clear(void) { is_pop = false; opList.clear(); blockStart.clear(); }	///< Clear all PcodeOps in \b this
+  static bool compareByBlock(const PcodeOp *a,const PcodeOp *b);	///< Compare PcodeOps for \b this set
+};
+
 /// \brief The topological scope of a variable within a basic block
 ///
 /// Within a basic block, the topological scope of a variable can be considered
@@ -78,6 +116,7 @@ public:
   int4 intersect(const Cover &op2) const;	///< Characterize the intersection between \b this and another Cover.
   int4 intersectByBlock(int4 blk,const Cover &op2) const;	///< Characterize the intersection on a specific block
   void intersectList(vector<int4> &listout,const Cover &op2,int4 level) const;
+  bool intersect(const PcodeOpSet &opSet,Varnode *rep) const;	///< Does \b this cover any PcodeOp in the given PcodeOpSet
   bool contain(const PcodeOp *op,int4 max) const;
   int4 containVarnodeDef(const Varnode *vn) const;
   void merge(const Cover &op2);			///< Merge \b this with another Cover block by block

Ghidra/Features/Decompiler/src/decompile/cpp/merge.cc

@@ -60,6 +60,34 @@ int4 BlockVarnode::findFront(int4 blocknum,const vector<BlockVarnode> &list)
   return min;
 }
 
+void StackAffectingOps::populate(void)
+
+{
+  for(int4 i=0;i<data.numCalls();++i) {
+    PcodeOp *op = data.getCallSpecs(i)->getOp();
+    addOp(op);
+  }
+  const list<LoadGuard> &storeGuard( data.getStoreGuards() );
+  for(list <LoadGuard>::const_iterator iter=storeGuard.begin();iter!=storeGuard.end();++iter) {
+    if ((*iter).isValid(CPUI_STORE))
+      addOp((*iter).getOp());
+  }
+  finalize();
+}
+
+bool StackAffectingOps::affectsTest(PcodeOp *op,Varnode *vn) const
+
+{
+  if (op->code() == CPUI_STORE) {
+    const LoadGuard *loadGuard = data.getStoreGuard(op);
+    if (loadGuard == (const LoadGuard *)0)
+      return true;
+    return loadGuard->isGuarded(vn->getAddr());
+  }
+  // We could conceivably do secondary testing of CALL ops here
+  return true;
+}
+
 /// \brief Required tests to merge HighVariables that are not Cover related
 ///
 /// This is designed to short circuit merge tests, when we know properties of the
@@ -1571,6 +1599,7 @@ void Merge::clear(void)
   testCache.clear();
   copyTrims.clear();
   protoPartial.clear();
+  stackAffectingOps.clear();
 }
 
 /// \brief Inflate the Cover of a given Varnode with a HighVariable

Ghidra/Features/Decompiler/src/decompile/cpp/merge.hh

@@ -41,6 +41,21 @@ public:
 
 class Funcdata;
 
+/// \brief The set of CALL and STORE ops that might indirectly affect stack variables
+///
+/// Intersect tests between local address tied and non-address tied Varnodes need to check for
+/// possible uses of aliases to the address tied Varnode.  This object is populated with the set of
+/// PcodeOps through which any stack Varnode might be modified through an alias.  Given an intersection
+/// of the Cover of an address tied Varnode and a PcodeOp in this set, affectsTest() can do
+/// secondary testing of whether the Varnode is actually modified by the PcodeOp.
+class StackAffectingOps : public PcodeOpSet {
+  Funcdata &data;
+public:
+  StackAffectingOps(Funcdata &fd) : data(fd) {}
+  virtual void populate(void);
+  virtual bool affectsTest(PcodeOp *op,Varnode *vn) const;
+};
+
 /// \brief Class for merging low-level Varnodes into high-level HighVariables
 ///
 /// As a node in Single Static Assignment (SSA) form, a Varnode has at most one defining
@@ -66,6 +81,7 @@ class Funcdata;
 ///   - Merging Varnodes that hold the same data-type
 class Merge {
   Funcdata &data;		///< The function containing the Varnodes to be merged
+  StackAffectingOps stackAffectingOps;		///< Set of CALL and STORE ops indirectly affecting stack variables
   HighIntersectTest testCache;	///< Cached intersection tests
   vector<PcodeOp *> copyTrims;	///< COPY ops inserted to facilitate merges
   vector<PcodeOp *> protoPartial;	///< Roots of unmapped CONCAT trees
@@ -101,7 +117,7 @@ class Merge {
   void processHighRedundantCopy(HighVariable *high);
   void groupPartialRoot(Varnode *vn);
 public:
-  Merge(Funcdata &fd) : data(fd) {} ///< Construct given a specific function
+  Merge(Funcdata &fd) : data(fd), stackAffectingOps(fd), testCache(stackAffectingOps) {} ///< Construct given a specific function
   void clear(void);
   bool inflateTest(Varnode *a,HighVariable *high);
   void inflate(Varnode *a,HighVariable *high);

Ghidra/Features/Decompiler/src/decompile/cpp/rangeutil.cc

@@ -1239,8 +1239,8 @@ bool CircleRange::pushForwardBinary(OpCode opc,const CircleRange &in1,const Circ
       }
       int4 wholeSize = 8*sizeof(uintb) - count_leading_zeros(mask);
       if (in1.getMaxInfo() + in2.getMaxInfo() > wholeSize) {
-	left = in1.left;	// Covered everything
-	right = in1.left;
+	left = (in1.left * in2.left) % step;
+	right = left;				// Covered everything
 	normalize();
 	return true;
       }
@@ -1264,7 +1264,7 @@ bool CircleRange::pushForwardBinary(OpCode opc,const CircleRange &in1,const Circ
       uint4 tmp = sa;
       while(step < maxStep && tmp > 0) {
 	step <<= 1;
-	sa -= 1;
+	tmp -= 1;
       }
       left = (in1.left << sa)&mask;
       right = (in1.right << sa)&mask;
@@ -1283,13 +1283,14 @@ bool CircleRange::pushForwardBinary(OpCode opc,const CircleRange &in1,const Circ
       int4 sa = (int4)in2.left * 8;
       mask = calc_mask(outSize);
       step = (sa == 0) ? in1.step : 1;
+      uintb range = (in1.left < in1.right) ? in1.right-in1.left : in1.left - in1.right;
 
-      left = (in1.left >> sa)&mask;
-      right = (in1.right >> sa)&mask;
-      if ((left& ~mask) != (right & ~mask)) {	// Truncated part is different
+      if (range == 0 || ((range >> sa) > mask )) {
 	left = right = 0;	// We cover everything
       }
       else {
+	left = in1.left >> sa;
+	right = ((in1.right - in1.step) >> sa) + step;
 	left &= mask;
 	right &= mask;
 	normalize();
@@ -1331,7 +1332,7 @@ bool CircleRange::pushForwardBinary(OpCode opc,const CircleRange &in1,const Circ
 	valLeft = sign_extend(valLeft,bitPos);
       }
       left = (valLeft >> sa) & mask;
-      right = (valRight >> sa) & mask;
+      right = (((valRight - in1.step) >> sa) + 1) & mask;
       if (left == right)	// Don't truncate accidentally to everything
 	right = (left + 1)&mask;
       break;

Ghidra/Features/Decompiler/src/decompile/cpp/testfunction.cc

@@ -21,13 +21,25 @@ void FunctionTestProperty::startTest(void) const
 
 {
   count = 0;
+  patnum = 0;
 }
 
 void FunctionTestProperty::processLine(const string &line) const
 
 {
-  if (std::regex_search(line,pattern))
-    count += 1;
+  if (std::regex_search(line,pattern[patnum])) {
+    patnum += 1;
+    if (patnum >= pattern.size()) {
+      count += 1;		// Full pattern has matched.  Count it.
+      patnum = 0;
+    }
+  }
+  else if (patnum > 0) {
+    patnum = 0;		// Abort current multi-line match, restart trying to match first line
+    if (std::regex_search(line,pattern[patnum])) {
+      patnum += 1;
+    }
+  }
 }
 
 bool FunctionTestProperty::endTest(void) const
@@ -44,7 +56,24 @@ void FunctionTestProperty::restoreXml(const Element *el)
   s1 >> minimumMatch;
   istringstream s2(el->getAttributeValue("max"));
   s2 >> maximumMatch;
-  pattern = std::regex(el->getContent());
+  string::size_type pos = 0;
+  const string &line(el->getContent());
+  do {
+    while(pos < line.size() && (line[pos] == ' ' || line[pos] == '\t'))	// Remove whitespace at front of pattern
+      pos += 1;
+    if (pos >= line.size())
+      break;
+   string::size_type nextpos = line.find('\n',pos);	// A newline in the pattern indicates a multi-line regex
+   string::size_type n;
+   if (nextpos == string::npos)
+     n = string::npos;			// If no (additional) newlines, take all remaining chars
+   else {
+     n = nextpos - pos;			// Create a line regex upto newline char
+     nextpos += 1;			// Skip newline when creating next line regex
+   }
+   pattern.emplace_back(line.substr(pos, n));	// Add a regex to list of lines to match
+   pos = nextpos;
+  } while(pos != string::npos);
 }
 
 void ConsoleCommands::readLine(string &line)

Ghidra/Features/Decompiler/src/decompile/cpp/testfunction.hh

@@ -36,7 +36,8 @@ class FunctionTestProperty {
   int4 minimumMatch;		///< Minimum number of times property is expected to match
   int4 maximumMatch;		///< Maximum number of times property is expected to match
   string name;			///< Name of the test, to be printed in test summaries
-  std::regex pattern;		///< Regular expression to match against a line of output
+  vector<std::regex> pattern;	///< Regular expression(s) to match against a line(s) of output
+  mutable uint4 patnum;	///< Index of current pattern to match against
   mutable uint4 count;		///< Number of times regular expression has been seen
 public:
   string getName(void) const { return name; }	///< Get the name of the property

Ghidra/Features/Decompiler/src/decompile/cpp/variable.cc

@@ -1048,6 +1048,29 @@ void HighIntersectTest::purgeHigh(HighVariable *high)
   highedgemap.erase(iterfirst,iterlast);
 }
 
+/// \brief Test if a given HighVariable might intersect an address tied HighVariable during a call
+///
+/// If an address tied Varnode has aliases, we need to consider it as \e in \e scope during
+/// calls, even if the value is never read after the call.  In particular, another Varnode
+/// that \e crosses the call is considered to be intersecting with the address tied Varnode.
+/// This method tests whether the address tied HighVariable has aliases, then if so,
+/// it tests if the given HighVariable intersects a call site.
+/// \param tied is the address tied HighVariable
+/// \param untied is the given HighVariable to consider for intersection
+/// \return \b true if we consider the HighVariables to be intersecting
+bool HighIntersectTest::testUntiedCallIntersection(HighVariable *tied,HighVariable *untied)
+
+{
+  // If the address tied part is global, we do not need to test for crossings, as the
+  // address forcing mechanism should act as a placeholder across calls
+  if (tied->isPersist()) return false;
+  Varnode *vn = tied->getTiedVarnode();
+  if (vn->hasNoLocalAlias()) return false;	// A local variable is only in scope if it has aliases
+  if (!affectingOps.isPopulated())
+    affectingOps.populate();
+  return untied->getCover().intersect(affectingOps,vn);
+}
+
 /// \brief Translate any intersection tests for \e high2 into tests for \e high1
 ///
 /// The two variables will be merged and \e high2, as an object, will be freed.
@@ -1151,6 +1174,16 @@ bool HighIntersectTest::intersection(HighVariable *a,HighVariable *b)
       break;
     }
   }
+  if (!res) {
+    bool aTied = a->isAddrTied();
+    bool bTied = b->isAddrTied();
+    if (aTied != bTied) {			// If one variable is address tied and the other isn't
+      if (aTied)
+	res = testUntiedCallIntersection(a,b);		// Test if the non-tied variable crosses any calls
+      else
+	res = testUntiedCallIntersection(b,a);
+    }
+  }
   highedgemap[ HighEdge(a,b) ] = res; // Cache the result
   highedgemap[ HighEdge(b,a) ] = res;
   return res;

Ghidra/Features/Decompiler/src/decompile/cpp/variable.hh

@@ -253,12 +253,15 @@ public:
 /// and still keeping the cached tests accurate, by calling the updateHigh() method.  If two HighVariables
 /// to be merged, the cached tests can be updated by calling moveIntersectTest() before merging.
 class HighIntersectTest {
+  PcodeOpSet &affectingOps;		///< PcodeOps that may indirectly affect the intersection test
   map<HighEdge,bool> highedgemap; ///< A cache of intersection tests, sorted by HighVariable pair
   static void gatherBlockVarnodes(HighVariable *a,int4 blk,const Cover &cover,vector<Varnode *> &res);
   static bool testBlockIntersection(HighVariable *a,int4 blk,const Cover &cover,int4 relOff,const vector<Varnode *> &blist);
   bool blockIntersection(HighVariable *a,HighVariable *b,int4 blk);
   void purgeHigh(HighVariable *high); ///< Remove cached intersection tests for a given HighVariable
+  bool testUntiedCallIntersection(HighVariable *tied,HighVariable *untied);
 public:
+  HighIntersectTest(PcodeOpSet &cCover) : affectingOps(cCover) {}
   void moveIntersectTests(HighVariable *high1,HighVariable *high2);
   bool updateHigh(HighVariable *a); ///< Make sure given HighVariable's Cover is up-to-date
   bool intersection(HighVariable *a,HighVariable *b);