GP-4541 Apply nolocalalias to STORE indirects

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

@@ -2930,12 +2930,9 @@ int4 RuleIndirectCollapse::applyOp(PcodeOp *op,Funcdata &data)
 	return 0;		// Partial overlap, not sure what to do
       }
     }
-    else if (indop->isCall()) {
+    else if (op->getOut()->hasNoLocalAlias()) {
       if (op->isIndirectCreation() || op->noIndirectCollapse())
 	return 0;
-      // If there are no aliases to a local variable, collapse
-      if (!op->getOut()->hasNoLocalAlias())
-	return 0;
     }
     else if (indop->usesSpacebasePtr()) {
       if (indop->code() == CPUI_STORE) {
@@ -5644,6 +5641,7 @@ void AddTreeState::clear(void)
 {
   multsum = 0;
   nonmultsum = 0;
+  biggestNonMultCoeff = 0;
   if (pRelType != (const TypePointerRel *)0) {
     nonmultsum = ((TypePointerRel *)ct)->getPointerOffset();
     nonmultsum &= ptrmask;
@@ -5687,6 +5685,7 @@ AddTreeState::AddTreeState(Funcdata &d,PcodeOp *op,int4 slot)
 {
   baseOp = op;
   baseSlot = slot;
+  biggestNonMultCoeff = 0;
   ptr = op->getIn(slot);
   ct = (const TypePointer *)ptr->getTypeReadFacing(op);
   ptrsize = ptr->getSize();
@@ -5716,36 +5715,6 @@ AddTreeState::AddTreeState(Funcdata &d,PcodeOp *op,int4 slot)
   isDegenerate = (baseType->getAlignSize() <= unitsize && baseType->getAlignSize() > 0);
 }
 
-/// Even if the current base data-type is not an array, the pointer expression may incorporate
-/// an array access for a sub component.  This manifests as a non-constant non-multiple terms in
-/// the tree.  If this term is itself defined by a CPUI_INT_MULT with a constant, the constant
-/// indicates a likely element size. Return a non-zero value, the likely element size, if there
-/// is evidence of a non-constant non-multiple term. Return zero otherwise.
-/// \return a non-zero value indicating likely element size, or zero
-uint4 AddTreeState::findArrayHint(void) const
-
-{
-  uint4 res = 0;
-  for(int4 i=0;i<nonmult.size();++i) {
-    Varnode *vn = nonmult[i];
-    if (vn->isConstant()) continue;
-    uint4 vncoeff = 1;
-    if (vn->isWritten()) {
-      PcodeOp *op = vn->getDef();
-      if (op->code() == CPUI_INT_MULT) {
-	Varnode *vnconst = op->getIn(1);
-	if (vnconst->isConstant()) {
-	  intb sval = sign_extend(vnconst->getOffset(),vnconst->getSize()*8-1);
-	  vncoeff = (sval < 0) ? (uint4)-sval : (uint4)sval;
-	}
-      }
-    }
-    if (vncoeff > res)
-      res = vncoeff;
-  }
-  return res;
-}
-
 /// \brief Given an offset into the base data-type and array hints find sub-component being referenced
 ///
 /// An explicit offset should target a specific sub data-type,
@@ -5838,6 +5807,9 @@ bool AddTreeState::checkMultTerm(Varnode *vn,PcodeOp *op,uint8 treeCoeff)
 	  return spanAddTree(vnterm->getDef(), val);
 	}
       }
+      uint4 vncoeff = (sval < 0) ? (uint4)-sval : (uint4)sval;
+      if (vncoeff > biggestNonMultCoeff)
+	biggestNonMultCoeff = vncoeff;
       return true;
     }
     else {
@@ -5848,6 +5820,8 @@ bool AddTreeState::checkMultTerm(Varnode *vn,PcodeOp *op,uint8 treeCoeff)
       return false;
     }
   }
+  if (treeCoeff > biggestNonMultCoeff)
+    biggestNonMultCoeff = treeCoeff;
   return true;
 }
 
@@ -5898,6 +5872,8 @@ bool AddTreeState::checkTerm(Varnode *vn,uint8 treeCoeff)
     valid = false;
     return false;
   }
+  if (treeCoeff > biggestNonMultCoeff)
+    biggestNonMultCoeff = treeCoeff;
   return true;
 }
 
@@ -5956,7 +5932,7 @@ void AddTreeState::calcSubtype(void)
     else {
       // For a negative sum, if the baseType is a structure and there is array hints,
       // we assume the sum is an array index at a lower level
-      if (baseType->getMetatype() == TYPE_STRUCT && findArrayHint() != 0)
+      if (baseType->getMetatype() == TYPE_STRUCT && biggestNonMultCoeff != 0)
 	offset = nonmultsum;
       else
 	offset = (uint8)(snonmult + size);
@@ -5975,9 +5951,8 @@ void AddTreeState::calcSubtype(void)
   else if (baseType->getMetatype() == TYPE_SPACEBASE) {
     int8 nonmultbytes = AddrSpace::addressToByteInt(nonmultsum,ct->getWordSize()); // Convert to bytes
     int8 extra;
-    uint4 arrayHint = findArrayHint();
     // Get offset into mapped variable
-    if (!hasMatchingSubType(nonmultbytes, arrayHint, &extra)) {
+    if (!hasMatchingSubType(nonmultbytes, biggestNonMultCoeff, &extra)) {
       valid = false;		// Cannot find mapped variable but nonmult is non-empty
       return;
     }
@@ -5989,9 +5964,8 @@ void AddTreeState::calcSubtype(void)
     intb snonmult = sign_extend(nonmultsum,ptrsize*8-1);
     int8 nonmultbytes = AddrSpace::addressToByteInt(snonmult,ct->getWordSize()); // Convert to bytes
     int8 extra;
-    uint4 arrayHint = findArrayHint();
     // Get offset into field in structure
-    if (!hasMatchingSubType(nonmultbytes, arrayHint, &extra)) {
+    if (!hasMatchingSubType(nonmultbytes, biggestNonMultCoeff, &extra)) {
       if (nonmultbytes < 0 || nonmultbytes >= baseType->getSize()) {	// Compare as bytes! not address units
 	valid = false; // Out of structure's bounds
 	return;

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

@@ -51,6 +51,7 @@ class AddTreeState {
   int4 ptrsize;			///< Size of the pointer
   int4 size;			///< Size of data-type being pointed to (in address units) or 0 for open ended pointer
   int4 baseSlot;		///< Slot of the ADD tree base that is holding the pointer
+  uint4 biggestNonMultCoeff;	///< Biggest coefficient that is not a multiple
   uint8 ptrmask;		///< Mask for modulo calculations in ptr space
   uint8 offset;			///< Number of bytes we dig into the base data-type
   uint8 correct;		///< Number of bytes being double counted
@@ -65,7 +66,6 @@ class AddTreeState {
   bool isSubtype;		///< Is there a sub-type (using CPUI_PTRSUB)
   bool valid;			///< Set to \b true if the whole expression can be transformed
   bool isDegenerate;		///< Set to \b true if pointer to unitsize or smaller
-  uint4 findArrayHint(void) const;	///< Look for evidence of an array in a sub-component
   bool hasMatchingSubType(int8 off,uint4 arrayHint,int8 *newoff) const;
   bool checkMultTerm(Varnode *vn,PcodeOp *op,uint8 treeCoeff);	///< Accumulate details of INT_MULT term and continue traversal if appropriate
   bool checkTerm(Varnode *vn,uint8 treeCoeff);			///< Accumulate details of given term and continue tree traversal

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

@@ -432,7 +432,7 @@ bool ScopeLocal::isUnmappedUnaliased(Varnode *vn) const
 
 {
   if (vn->getSpace() != space) return false;	// Must be in mapped local (stack) space
-  if (maxParamOffset < minParamOffset) return false;	// If no min/max, then we have no know stack parameters
+  if (maxParamOffset < minParamOffset) return true;	// If no min/max, then we have no know stack parameters
   if (vn->getOffset() < minParamOffset || vn->getOffset() > maxParamOffset)
     return true;
   return false;