Merge remote-tracking branch 'origin/GP-3429_VariablesCrossingCalls'

(Closes #5237)

Ghidra/Features/Decompiler/certification.manifest

@@ -61,6 +61,7 @@ src/decompile/datatests/switchind.xml||GHIDRA||||END|
 src/decompile/datatests/threedim.xml||GHIDRA||||END|
 src/decompile/datatests/twodim.xml||GHIDRA||||END|
 src/decompile/datatests/union_datatype.xml||GHIDRA||||END|
+src/decompile/datatests/varcross.xml||GHIDRA||||END|
 src/decompile/datatests/wayoffarray.xml||GHIDRA||||END|
 src/main/doc/commonprofile.xsl||GHIDRA||||END|
 src/main/doc/cspec.xml||GHIDRA||||END|

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);

Ghidra/Features/Decompiler/src/decompile/datatests/varcross.xml

@@ -0,0 +1,62 @@
+<decompilertest>
+<binaryimage arch="x86:LE:64:default:gcc">
+<!--
+    Examples where the decompiler is tempted to (but shouldn't) merge address tied varnodes
+    with other varnodes that cross a potential access through an alias.
+-->
+<bytechunk space="ram" offset="0x10002b" readonly="true">
+                      4883ec6831
+c083ff07b918000000b8480000000f45
+c84889e0488d542450c7000000000048
+83c0044839d075f1894c24284889e7e8
+9cffffff4883c468c3
+</bytechunk>
+<bytechunk space="ram" offset="0x100a29" readonly="true">
+                  f30f1efa5383ff
+07bb28000000b8580000000f45d8e8bd
+ffffff891db70f00005bc3
+</bytechunk>
+<bytechunk space="ram" offset="0x100000d80" readonly="true">
+4881ec800000004c8d7c244089f383c3
+07498bf766e87800895c244066e8b000
+03442440c3
+</bytechunk>
+<symbol space="ram" offset="0x100000" name="use_array"/>
+<symbol space="ram" offset="0x10002b" name="store_cross"/>
+<symbol space="ram" offset="0x100a00" name="read_glob"/>
+<symbol space="ram" offset="0x100a29" name="global_cross"/>
+<symbol space="ram" offset="0x100000d80" name="local_cross"/>
+<symbol space="ram" offset="0x100000e10" name="othercall"/>
+<symbol space="ram" offset="0x100000e50" name="retval"/>
+</binaryimage>
+<script>
+  <com>map addr r0x101a00 uint4 glob1</com>
+  <com>lo fu store_cross</com>
+  <com>map addr s0xffffffffffffff98 uint4 local_array[20]</com>
+  <com>decompile</com>
+  <com>print C</com>
+  <com>lo fu global_cross</com>
+  <com>decompile</com>
+  <com>print C</com>
+  <com>parse line extern int4 local_cross(uint8 param_1,int4 param_2);</com>
+  <com>lo fu local_cross</com>
+  <com>map addr s0xffffffffffffffc0 int4 local_int[16]</com>
+  <com>decompile</com>
+  <com>print C</com>
+  <com>quit</com>
+</script>
+<stringmatch name="Store cross #1" min="0" max="0">local_array\[10\] = 0x18;</stringmatch>
+<stringmatch name="Store cross #2" min="0" max="0">local_array\[10\] = 0x48;</stringmatch>
+<stringmatch name="Store cross #3" min="1" max="1">uVar. = 0x18;</stringmatch>
+<stringmatch name="Store cross #4" min="1" max="1">uVar. = 0x48;</stringmatch>
+<stringmatch name="Store cross #5" min="1" max="1">local_array\[10\] = uVar.;
+                                                   use_array\(local_array\);</stringmatch>
+<stringmatch name="Global cross #1" min="1" max="1">uVar. = 0x28;</stringmatch>
+<stringmatch name="Global cross #2" min="1" max="1">uVar. = 0x58;</stringmatch>
+<stringmatch name="Global cross #3" min="1" max="1">read_glob\(\);
+                                                   glob1 = uVar.;</stringmatch>
+<stringmatch name="Local cross #1" min="1" max="1">othercall\(param_1,local_int\);
+                                                   local_int\[0\] = param_2 \+ 7;</stringmatch>
+<stringmatch name="Local cross #2" min="1" max="1">iVar. = retval\(\);
+						   return iVar. \+ local_int\[0\];</stringmatch>
+</decompilertest>

Ghidra/Features/Decompiler/src/decompile/unittests/testcirclerange.cc

@@ -66,10 +66,12 @@ class CircleRangeTest {
   int4 bytes;
   bool getStartStopStep(uintb &start,uintb &stop,int4 &step);
 public:
+  CircleRangeTest(int4 b) { bytes = b; mask = calc_mask(b); }
   CircleRangeTest(const CircleRange &range);
   void set_intersect(CircleRangeTest &op2);
   void set_union(CircleRangeTest &op2);
   void pushUnary(OpCode opcode,int4 outsize);
+  void pushBinary(OpCode opcode,int4 outsize,CircleRangeTest &in1,CircleRangeTest &in2);
   void pullbackUnary(OpCode opcode,int4 insize);
   void pullbackBinary(OpCode opcode,int4 slot,uintb val);
   bool testEqual(bool valid,const CircleRange &range);
@@ -78,6 +80,8 @@ public:
   static bool testPullbackUnary(uintb start,uintb stop,int4 step,int4 bytes,OpCode opcode,int4 insize);
   static bool testPullbackBinary(uintb start,uintb stop,int4 step,int4 bytes,OpCode opcode,int4 slot,uintb val);
   static bool testPushUnary(uintb start,uintb stop,int4 step,int4 bytes,OpCode opcode,int4 outsize);
+  static bool testPushBinary(uintb start1,uintb stop1,int4 step1,uintb start2,uintb stop2,int4 step2,
+			     int4 bytes,OpCode opcode,int4 outsize);
 };
 
 bool CircleRangeTest::testPullbackUnary(uintb start,uintb stop,int4 step,int4 bytes,OpCode opcode,int4 insize)
@@ -111,6 +115,20 @@ bool CircleRangeTest::testPushUnary(uintb start,uintb stop,int4 step,int4 bytes,
   return testrange.testEqual(valid,res);
 }
 
+bool CircleRangeTest::testPushBinary(uintb start1,uintb stop1,int4 step1,uintb start2,uintb stop2,int4 step2,
+				     int4 bytes,OpCode opcode,int4 outsize)
+{
+  CircleRange range1(start1,stop1,bytes,step1);
+  CircleRange range2(start2,stop2,bytes,step2);
+  CircleRangeTest testrange1(range1);
+  CircleRangeTest testrange2(range2);
+  CircleRange res;
+  bool valid = res.pushForwardBinary(opcode, range1, range2, bytes, outsize, 32);
+  CircleRangeTest testres(outsize);
+  testres.pushBinary(opcode, outsize, testrange1, testrange2);
+  return testres.testEqual(valid,res);
+}
+
 bool CircleRangeTest::testIntersect(uintb start1,uintb stop1,uintb start2,uintb stop2,int4 step,int4 bytes)
 
 {
@@ -217,6 +235,23 @@ void CircleRangeTest::pushUnary(OpCode opcode,int4 outsize)
   }
 }
 
+void CircleRangeTest::pushBinary(OpCode opcode,int4 outsize,CircleRangeTest &in1,CircleRangeTest &in2)
+
+{
+  CircleRangeTestEnvironment::build();
+  OpBehavior *behave = glb->inst[opcode]->getBehavior();
+  elements.clear();
+  for(int4 i=0;i<in1.elements.size();++i) {
+    for(int4 j=0;j<in2.elements.size();++j) {
+      elements.push_back(behave->evaluateBinary(outsize, in1.bytes, in1.elements[i], in2.elements[j]));
+    }
+  }
+  if (outsize != bytes) {
+    bytes = outsize;
+    mask = calc_mask(outsize);
+  }
+}
+
 void CircleRangeTest::pullbackUnary(OpCode opcode,int4 insize)
 
 {
@@ -263,12 +298,37 @@ bool CircleRangeTest::getStartStopStep(uintb &start,uintb &stop,int4 &step)
     return true;
   }
   sort(elements.begin(),elements.end());
-  int4 bigpos = -1;
-  uintb biggest1 = 0;
-  uintb biggest2 = 0;
+  vector<uintb> dedup;
+  uintb lastel = elements[0];
+  dedup.push_back(lastel);
+  for(int4 i=1;i<elements.size();++i) {	// Dedeuplicate the values
+    uintb curel = elements[i];
+    if (curel == lastel) continue;
+    dedup.push_back(curel);
+    lastel = curel;
+  }
+  elements.swap(dedup);
 
   if (elements.back() > mask) return false;
 
+  if (elements.size() == 1) {
+    start = elements[0];
+    stop = (start + 1) & mask;
+    step = 1;
+    return true;
+  }
+  if (elements.size() == 2) {
+    uintb diff = (elements[0] - elements[1]) & mask;
+    if (diff == 1 || diff == 2 || diff == 4 || diff == 8) {
+      start = elements[1];
+      stop = (start + diff + diff) & mask;
+      step = diff;
+      return true;
+    }
+  }
+  int4 bigpos = -1;
+  uintb biggest1 = 0;
+  uintb biggest2 = 0;
   for(int4 i=1;i<elements.size();++i) {
     uintb diff = elements[i] - elements[i-1];
     if (diff >= biggest1) {
@@ -797,4 +857,72 @@ TEST(circlerange_pushsext7) {
   ASSERT(CircleRangeTest::testPushUnary(0,0,4,1, CPUI_INT_SEXT, 2));
 }
 
+TEST(circlerange_pushadd1) {
+  ASSERT(CircleRangeTest::testPushBinary(10, 15, 1, 30, 35, 1, 1, CPUI_INT_ADD, 1));
+}
+
+TEST(circlerange_pushadd2) {
+  ASSERT(CircleRangeTest::testPushBinary(1,10,1,0xfffffffe,5,1,4,CPUI_INT_ADD, 4));
+}
+
+TEST(circlerange_pushadd3) {
+  ASSERT(CircleRangeTest::testPushBinary(0,20,4,0xfff0,6,2,2,CPUI_INT_ADD,2));
+}
+
+TEST(circlerange_pushadd4) {
+  ASSERT(CircleRangeTest::testPushBinary(1,250,1,20,30,1,1,CPUI_INT_ADD,1));
+}
+
+TEST(circlerange_pushmult1) {
+  ASSERT(CircleRangeTest::testPushBinary(0x1000,0x1010,1,2,3,1,4,CPUI_INT_MULT,4));
+}
+
+TEST(circlerange_pushmult2) {
+  ASSERT(CircleRangeTest::testPushBinary(0xfffc,8,2,4,5,1,2,CPUI_INT_MULT,2));
+}
+
+TEST(circlerange_pushmult3) {
+  ASSERT(CircleRangeTest::testPushBinary(5,133,1,2,3,1,1,CPUI_INT_MULT,1));
+}
+
+TEST(circlerange_pushleft1) {
+  ASSERT(CircleRangeTest::testPushBinary(1,5,1,1,2,1,4,CPUI_INT_LEFT,4));
+}
+
+TEST(circlerange_pushleft2) {
+  ASSERT(CircleRangeTest::testPushBinary(8,72,4,2,3,1,1,CPUI_INT_LEFT,1));
+}
+
+TEST(circlerange_pushsubpiece1) {
+  ASSERT(CircleRangeTest::testPushBinary(0xfffe,0x10005,1,0,1,1,4,CPUI_SUBPIECE,1));
+}
+
+TEST(circlerange_pushsubpiece2) {
+  ASSERT(CircleRangeTest::testPushBinary(0xfffe,0x10005,1,1,2,1,4,CPUI_SUBPIECE,1));
+}
+
+TEST(circlerange_pushsubpiece3) {
+  ASSERT(CircleRangeTest::testPushBinary(0x10f0,0x1200,1,0,1,1,4,CPUI_SUBPIECE,1));
+}
+
+TEST(circlerange_pushright1) {
+  ASSERT(CircleRangeTest::testPushBinary(0x30a6,0x30c0,2,4,5,1,2,CPUI_INT_RIGHT,2));
+}
+
+TEST(circlerange_pushright2) {
+  ASSERT(CircleRangeTest::testPushBinary(0xfe00,0xffc0,0x20,9,10,1,2,CPUI_INT_RIGHT,2));
+}
+
+TEST(circlerange_pushright3) {
+  ASSERT(CircleRangeTest::testPushBinary(7,10,1,4,5,1,4,CPUI_INT_RIGHT,4));
+}
+
+TEST(circlerange_pushsright1) {
+  ASSERT(CircleRangeTest::testPushBinary(0x3000,0x3064,4,3,4,1,2,CPUI_INT_SRIGHT,2));
+}
+
+TEST(circlerange_pushsright2) {
+  ASSERT(CircleRangeTest::testPushBinary(0xfff0,0x24,4,3,4,1,2,CPUI_INT_SRIGHT,2));
+}
+
 } // End namespace ghidra