From 2a73bca9ac2c74b8c75a4fb929d0efabd07aee03 Mon Sep 17 00:00:00 2001
From: caheckman <48068198+caheckman@users.noreply.github.com>
Date: Mon, 17 Jun 2019 23:47:05 -0400
Subject: [PATCH] overflow checks, take into account LOAD size

---
 .../Decompiler/src/decompile/cpp/heritage.cc  |  4 ++-
 .../Decompiler/src/decompile/cpp/rangeutil.cc | 18 +++++++++++++
 .../Decompiler/src/decompile/cpp/rangeutil.hh |  3 +++
 .../Decompiler/src/decompile/cpp/varmap.cc    | 25 +++++++++++--------
 4 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/heritage.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/heritage.cc
index 38a6b211a4..7307e4e124 100644
--- a/Ghidra/Features/Decompiler/src/decompile/cpp/heritage.cc
+++ b/Ghidra/Features/Decompiler/src/decompile/cpp/heritage.cc
@@ -642,8 +642,10 @@ void LoadGuard::finalizeRange(const ValueSetRead &valueSet)
       step = range.getStep();
     minimumOffset = range.getMin();
     maximumOffset = (range.getEnd() - 1) & range.getMask();	// NOTE: Don't subtract a whole step
-    if (maximumOffset < minimumOffset)	// Values extend into what is usually stack parameters
+    if (maximumOffset < minimumOffset) {	// Values extend into what is usually stack parameters
       maximumOffset = spc->getHighest();
+      analysisState = 1;	// Remove the lock as we have likely overflowed
+    }
   }
   if (minimumOffset > spc->getHighest())
     minimumOffset = spc->getHighest();
diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/rangeutil.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/rangeutil.cc
index cf86fb98eb..04f6024380 100644
--- a/Ghidra/Features/Decompiler/src/decompile/cpp/rangeutil.cc
+++ b/Ghidra/Features/Decompiler/src/decompile/cpp/rangeutil.cc
@@ -2565,3 +2565,21 @@ void ValueSetSolver::solve(int4 max,Widener &widener)
   for(riter=readNodes.begin();riter!=readNodes.end();++riter)
     (*riter).second.compute();				// Calculate any follow-on value sets
 }
+
+#ifdef CPUI_DEBUG
+void ValueSetSolver::dumpValueSets(ostream &s) const
+
+{
+  list<ValueSet>::const_iterator iter;
+  for(iter=valueNodes.begin();iter!=valueNodes.end();++iter) {
+    (*iter).printRaw(s);
+    s << endl;
+  }
+  map<SeqNum,ValueSetRead>::const_iterator riter;
+  for(riter=readNodes.begin();riter!=readNodes.end();++riter) {
+    (*riter).second.printRaw(s);
+    s << endl;
+  }
+}
+
+#endif
diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/rangeutil.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/rangeutil.hh
index 6f91bbff38..e4f37b0639 100644
--- a/Ghidra/Features/Decompiler/src/decompile/cpp/rangeutil.hh
+++ b/Ghidra/Features/Decompiler/src/decompile/cpp/rangeutil.hh
@@ -319,6 +319,9 @@ public:
   map<SeqNum,ValueSetRead>::const_iterator beginValueSetReads(void) const { return readNodes.begin(); }	///< Start of ValueSetReads
   map<SeqNum,ValueSetRead>::const_iterator endValueSetReads(void) const { return readNodes.end(); }	///< End of ValueSetReads
   const ValueSetRead &getValueSetRead(const SeqNum &seq) { return (*readNodes.find(seq)).second; }	///< Get ValueSetRead by SeqNum
+#ifdef CPUI_DEBUG
+  void dumpValueSets(ostream &s) const;
+#endif
 };
 
 /// \param op2 is the range to compare \b this to
diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/varmap.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/varmap.cc
index a52ac0c98d..b81bdb9058 100644
--- a/Ghidra/Features/Decompiler/src/decompile/cpp/varmap.cc
+++ b/Ghidra/Features/Decompiler/src/decompile/cpp/varmap.cc
@@ -258,10 +258,14 @@ bool RangeHint::merge(RangeHint *b,AddrSpace *space,TypeFactory *typeFactory)
       return overlapProblems;		// Discard b entirely in favor of a
     }
     // Concede confusion about types, set unknown type rather than a or b's type
+    rangeType = RangeHint::fixed;
     size = space->wrapOffset(end-start);
+    if (size != 1 && size != 2 && size != 4 && size != 8) {
+      size = 1;
+      rangeType = RangeHint::open;
+    }
     type = typeFactory->getBase(size,TYPE_UNKNOWN);
     flags = 0;
-    rangeType = RangeHint::fixed;
     highind = -1;
     return overlapProblems;
   }
@@ -929,16 +933,17 @@ void MapState::gatherOpen(const Funcdata &fd)
       ct = ((TypePointer *) ct)->getPtrTo();
       while (ct->getMetatype() == TYPE_ARRAY)
 	ct = ((TypeArray *) ct)->getBase();
-      if (ct->getSize() != step) {
-	// Datatype doesn't match step:  field in array of structures or something more unusual
-	if (ct->getSize() > step || (step % ct->getSize()) != 0)
-	  continue;
-	// Since ct's size divides the step and we want to preserve the arrayness
-	// we pretend we have an array of ct's size
-	step = ct->getSize();
-      }
     }
-    else {
+    int4 outSize = guard.getOp()->getOut()->getSize();
+    if (outSize != step) {
+      // LOAD size doesn't match step:  field in array of structures or something more unusual
+      if (outSize > step || (step % outSize) != 0)
+	continue;
+      // Since the LOAD size divides the step and we want to preserve the arrayness
+      // we pretend we have an array of LOAD's size
+      step = outSize;
+    }
+    if (ct->getSize() != step) {	// Make sure data-type matches our step size
       if (step > 8)
 	continue;		// Don't manufacture primitives bigger than 8-bytes
       ct = fd.getArch()->types->getBase(step, TYPE_UNKNOWN);