Adjustments to onlyOpUse

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

@@ -27,7 +27,7 @@
 vector<ArchitectureCapability *> ArchitectureCapability::thelist;
 
 const uint4 ArchitectureCapability::majorversion = 4;
-const uint4 ArchitectureCapability::minorversion = 0;
+const uint4 ArchitectureCapability::minorversion = 1;
 
 /// This builds a list of just the ArchitectureCapability extensions
 void ArchitectureCapability::initialize(void)

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

@@ -1852,7 +1852,7 @@ int4 ActionReturnRecovery::apply(Funcdata &data)
 	int4 slot = trial.getSlot();
 	vn = op->getIn(slot);
 	if (ancestorReal.execute(op,slot,&trial,false))
-	  if (data.ancestorOpUse(maxancestor,vn,op,trial))
+	  if (data.ancestorOpUse(maxancestor,vn,op,trial,0))
 	    trial.markActive(); // This varnode sees active use as a parameter
 	count += 1;
       }

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

@@ -4744,7 +4744,7 @@ void FuncCallSpecs::checkInputTrialUse(Funcdata &data,AliasChecker &aliascheck)
 	  trial.markNoUse();
       }
       else if (ancestorReal.execute(op,slot,&trial,false)) {
-	if (data.ancestorOpUse(maxancestor,vn,op,trial))
+	if (data.ancestorOpUse(maxancestor,vn,op,trial,0))
 	  trial.markActive();
 	else
 	  trial.markInactive();
@@ -4754,7 +4754,7 @@ void FuncCallSpecs::checkInputTrialUse(Funcdata &data,AliasChecker &aliascheck)
     }
     else {
       if (ancestorReal.execute(op,slot,&trial,true)) {
-	if (data.ancestorOpUse(maxancestor,vn,op,trial)) {
+	if (data.ancestorOpUse(maxancestor,vn,op,trial,0)) {
 	  trial.markActive();
 	  if (trial.hasCondExeEffect())
 	    activeinput.markNeedsFinalCheck();

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

@@ -58,6 +58,11 @@ class Funcdata {
     baddata_present = 0x800,	///< Set if function flowed into bad data
     double_precis_on = 0x1000	///< Set if we are performing double precision recovery
   };
+  enum {
+    traverse_actionalt = 1,	///< Alternate path traverses a solid action or \e non-incidental COPY
+    traverse_indirect = 2,	///< Main path traverses an INDIRECT
+    traverse_indirectalt = 4	///< Alternate path traverses an INDIRECT
+  };
   uint4 flags;			///< Boolean properties associated with \b this function
   uint4 clean_up_index;		///< Creation index of first Varnode created after start of cleanup
   uint4 high_level_index;	///< Creation index of first Varnode created after HighVariables are created
@@ -116,6 +121,7 @@ class Funcdata {
   void nodeSplitCloneVarnode(PcodeOp *op,PcodeOp *newop);
   void nodeSplitRawDuplicate(BlockBasic *b,BlockBasic *bprime);
   void nodeSplitInputPatch(BlockBasic *b,BlockBasic *bprime,int4 inedge);
+  static bool isAlternatePathValid(const Varnode *vn,uint4 flags);
   static bool descendantsOutside(Varnode *vn);
   static void saveVarnodeXml(ostream &s,VarnodeLocSet::const_iterator iter,VarnodeLocSet::const_iterator enditer);
   static bool checkIndirectUse(Varnode *vn);
@@ -363,9 +369,9 @@ public:
 
   HighVariable *findHigh(const string &name) const;	///< Find a high-level variable by name
   void mapGlobals(void);			///< Make sure there is a Symbol entry for all global Varnodes
-  bool checkCallDoubleUse(const PcodeOp *opmatch,const PcodeOp *op,const Varnode *vn,const ParamTrial &trial) const;
+  bool checkCallDoubleUse(const PcodeOp *opmatch,const PcodeOp *op,const Varnode *vn,uint4 flags,const ParamTrial &trial) const;
-  bool onlyOpUse(const Varnode *invn,const PcodeOp *opmatch,const ParamTrial &trial) const;
+  bool onlyOpUse(const Varnode *invn,const PcodeOp *opmatch,const ParamTrial &trial,uint4 mainFlags) const;
-  bool ancestorOpUse(int4 maxlevel,const Varnode *invn,const PcodeOp *op,ParamTrial &trial) const;
+  bool ancestorOpUse(int4 maxlevel,const Varnode *invn,const PcodeOp *op,ParamTrial &trial,uint4 mainFlags) const;
   bool syncVarnodesWithSymbols(const ScopeLocal *lm,bool typesyes);
   void transferVarnodeProperties(Varnode *vn,Varnode *newVn,int4 lsbOffset);
   bool fillinReadOnly(Varnode *vn);		///< Replace the given Varnode with its (constant) value in the load image

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

@@ -1472,6 +1472,30 @@ void Funcdata::mapGlobals(void)
     warningHeader("Globals starting with '_' overlap smaller symbols at the same address");
 }
 
+/// \brief Return \b true if the alternate path looks more valid than the main path.
+///
+/// Two different paths from a common Varnode each terminate at a CALL, CALLIND, or RETURN.
+/// Evaluate which path most likely represents actual parameter/return value passing,
+/// based on traversal information about each path.
+/// \param vn is the Varnode terminating the \e alternate path
+/// \param flags indicates traversals for both paths
+/// \return \b true if the alternate path is preferred
+bool Funcdata::isAlternatePathValid(const Varnode *vn,uint4 flags)
+
+{
+  if ((flags & (traverse_indirect | traverse_indirectalt)) == traverse_indirect)
+    // If main path traversed an INDIRECT but the alternate did not
+    return true;	// Main path traversed INDIRECT, alternate did not
+  if ((flags & (traverse_indirect | traverse_indirectalt)) == traverse_indirectalt)
+    return false;	// Alternate path traversed INDIRECT, main did not
+  if ((flags & traverse_actionalt) != 0)
+    return true;	// Alternate path traversed a dedicated COPY
+  if (vn->loneDescend() == (PcodeOp*)0) return false;
+  const PcodeOp *op = vn->getDef();
+  if (op == (PcodeOp*)0) return true;
+  return !op->isMarker();	// MULTIEQUAL or INDIRECT indicates multiple values
+}
+
 /// \brief Test for legitimate double use of a parameter trial
 ///
 /// The given trial is a \e putative input to first CALL, but can also trace its data-flow
@@ -1480,9 +1504,10 @@ void Funcdata::mapGlobals(void)
 /// \param opmatch is the first CALL linked to the trial
 /// \param op is the second CALL
 /// \param vn is the Varnode parameter for the second CALL
+/// \param flags indicates what p-code ops were crossed to reach \e vn
 /// \param trial is the given parameter trial
 /// \return \b true for a legitimate double use
-bool Funcdata::checkCallDoubleUse(const PcodeOp *opmatch,const PcodeOp *op,const Varnode *vn,const ParamTrial &trial) const
+bool Funcdata::checkCallDoubleUse(const PcodeOp *opmatch,const PcodeOp *op,const Varnode *vn,uint4 flags,const ParamTrial &trial) const
 
 {
   int4 j = op->getSlot(vn);
@@ -1511,7 +1536,13 @@ bool Funcdata::checkCallDoubleUse(const PcodeOp *opmatch,const PcodeOp *op,const
 
   if (fc->isInputActive()) {
     const ParamTrial &curtrial( fc->getActiveInput()->getTrialForInputVarnode(j) );
-    if ((!curtrial.isChecked())||(!curtrial.isActive())) return true;
+    if (curtrial.isChecked()) {
+      if (curtrial.isActive())
+	return false;
+    }
+    else if (isAlternatePathValid(vn,flags))
+      return false;
+    return true;
   }
   return false;
 }
@@ -1523,28 +1554,31 @@ bool Funcdata::checkCallDoubleUse(const PcodeOp *opmatch,const PcodeOp *op,const
 /// \param invn is the given Varnode
 /// \param opmatch is the putative CALL op using the Varnode for parameter passing
 /// \param trial is the parameter trial object associated with the Varnode
+/// \param mainFlags are flags describing traversals along the \e main path, from \e invn to \e opmatch
 /// \return \b true if the Varnode seems only to be used as parameter to \b opmatch
-bool Funcdata::onlyOpUse(const Varnode *invn,const PcodeOp *opmatch,const ParamTrial &trial) const
+bool Funcdata::onlyOpUse(const Varnode *invn,const PcodeOp *opmatch,const ParamTrial &trial,uint4 mainFlags) const
 
 {
-  vector<const Varnode *> varlist;
+  vector<TraverseNode> varlist;
   list<PcodeOp *>::const_iterator iter;
   const Varnode *vn,*subvn;
   const PcodeOp *op;
   int4 i;
   bool res = true;
 
+  varlist.reserve(64);
   invn->setMark();		// Marks prevent infinite loops
-  varlist.push_back(invn);
+  varlist.emplace_back(invn,mainFlags);
 
-  i = 0;
+  for(i=0;i < varlist.size();++i) {
-  while(i < varlist.size()) {
+    vn = varlist[i].vn;
-    vn = varlist[i++];
+    uint4 baseFlags = varlist[i].flags;
     for(iter=vn->descend.begin();iter!=vn->descend.end();++iter) {
       op = *iter;
       if (op == opmatch) {
 	if (op->getIn(trial.getSlot())==vn) continue;
       }
+      uint4 curFlags = baseFlags;
       switch(op->code()) {
       case CPUI_BRANCH:		// These ops define a USE of a variable
       case CPUI_CBRANCH:
@@ -1555,17 +1589,39 @@ bool Funcdata::onlyOpUse(const Varnode *invn,const PcodeOp *opmatch,const ParamT
 	break;
       case CPUI_CALL:
       case CPUI_CALLIND:
-	if (checkCallDoubleUse(opmatch,op,vn,trial)) continue;
+	if (checkCallDoubleUse(opmatch,op,vn,curFlags,trial)) continue;
 	res = false;
 	break;
+      case CPUI_INDIRECT:
+	curFlags |= Funcdata::traverse_indirectalt;
+	break;
+      case CPUI_COPY:
+	if ((op->getOut()->getSpace()->getType()!=IPTR_INTERNAL)&&!op->isIncidentalCopy()&&!vn->isIncidentalCopy()) {
+	  curFlags |= Funcdata::traverse_actionalt;
+	}
+	break;
       case CPUI_RETURN:
 	if (opmatch->code()==CPUI_RETURN) { // Are we in a different return
 	  if (op->getIn(trial.getSlot())==vn) // But at the same slot
 	    continue;
 	}
+	else if (activeoutput != (ParamActive *)0) {	// Are we in the middle of analyzing returns
+	  if (op->getIn(0) != vn) {		// Unless we hold actual return value
+	    if (!isAlternatePathValid(vn,curFlags))
+	      continue;				// Don't consider this a "use"
+	  }
+	}
 	res = false;
 	break;
+      case CPUI_MULTIEQUAL:
+      case CPUI_PIECE:
+      case CPUI_SUBPIECE:
+      case CPUI_INT_SEXT:
+      case CPUI_INT_ZEXT:
+      case CPUI_CAST:
+	break;
       default:
+	curFlags |= Funcdata::traverse_actionalt;
 	break;
       }
       if (!res) break;
@@ -1576,7 +1632,7 @@ bool Funcdata::onlyOpUse(const Varnode *invn,const PcodeOp *opmatch,const ParamT
 	  break;
 	}
 	if (!subvn->isMark()) {
-	  varlist.push_back(subvn);
+	  varlist.emplace_back(subvn,curFlags);
 	  subvn->setMark();
 	}
       }
@@ -1584,7 +1640,7 @@ bool Funcdata::onlyOpUse(const Varnode *invn,const PcodeOp *opmatch,const ParamT
     if (!res) break;
   }
   for(i=0;i<varlist.size();++i)
-    varlist[i]->clearMark();
+    varlist[i].vn->clearMark();
   return res;
 }
 
@@ -1596,9 +1652,10 @@ bool Funcdata::onlyOpUse(const Varnode *invn,const PcodeOp *opmatch,const ParamT
 /// \param invn is the given trial Varnode to test
 /// \param op is the given CALL or RETURN
 /// \param trial is the associated parameter trial object
+/// \param mainFlags describes traversals along the path from \e invn to \e op
 /// \return \b true if the Varnode is only used for the CALL/RETURN
 bool Funcdata::ancestorOpUse(int4 maxlevel,const Varnode *invn,
-			     const PcodeOp *op,ParamTrial &trial) const
+			     const PcodeOp *op,ParamTrial &trial,uint4 mainFlags) const
 
 {
   int4 i;
@@ -1610,7 +1667,7 @@ bool Funcdata::ancestorOpUse(int4 maxlevel,const Varnode *invn,
     if (!invn->isTypeLock()) return false;
 				// If the input is typelocked
 				// this is as good as being written
-    return onlyOpUse(invn,op,trial); // Test if varnode is only used in op
+    return onlyOpUse(invn,op,trial,mainFlags); // Test if varnode is only used in op
   }
 
   const PcodeOp *def = invn->getDef();
@@ -1620,7 +1677,7 @@ bool Funcdata::ancestorOpUse(int4 maxlevel,const Varnode *invn,
     // as an "only use"
     if (def->isIndirectCreation())
       return false;
-    return ancestorOpUse(maxlevel-1,def->getIn(0),op,trial);
+    return ancestorOpUse(maxlevel-1,def->getIn(0),op,trial,mainFlags | Funcdata::traverse_indirect);
   case CPUI_MULTIEQUAL:
 				// Check if there is any ancestor whose only
 				// use is in this op
@@ -1628,7 +1685,7 @@ bool Funcdata::ancestorOpUse(int4 maxlevel,const Varnode *invn,
     def->setMark();		// Mark that this MULTIEQUAL is on the path
 				// Note: onlyOpUse is using Varnode::setMark
     for(i=0;i<def->numInput();++i) {
-      if (ancestorOpUse(maxlevel-1,def->getIn(i),op,trial)) {
+      if (ancestorOpUse(maxlevel-1,def->getIn(i),op,trial, mainFlags)) {
 	def->clearMark();
 	return true;
       }
@@ -1637,13 +1694,12 @@ bool Funcdata::ancestorOpUse(int4 maxlevel,const Varnode *invn,
     return false;
   case CPUI_COPY:
     if ((invn->getSpace()->getType()==IPTR_INTERNAL)||def->isIncidentalCopy()||def->getIn(0)->isIncidentalCopy()) {
-      if (!ancestorOpUse(maxlevel-1,def->getIn(0),op,trial)) return false;
+      return ancestorOpUse(maxlevel-1,def->getIn(0),op,trial,mainFlags);
-      return true;
     }
     break;
   case CPUI_PIECE:
     // Concatenation tends to be artificial, so recurse through the least significant part
-    return ancestorOpUse(maxlevel-1,def->getIn(1),op,trial);
+    return ancestorOpUse(maxlevel-1,def->getIn(1),op,trial,mainFlags);
   case CPUI_SUBPIECE:
     // This is a rather kludgy way to get around where a DIV (or other similar) instruction
     // causes a register that looks like the high precision piece of the function return
@@ -1664,7 +1720,7 @@ bool Funcdata::ancestorOpUse(int4 maxlevel,const Varnode *invn,
     break;
   }
 				// This varnode must be top ancestor at this point
-  return onlyOpUse(invn,op,trial); // Test if varnode is only used in op
+  return onlyOpUse(invn,op,trial,mainFlags); // Test if varnode is only used in op
 }
 
 /// \return \b true if there are two input flows, one of which is a normal \e solid flow

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

@@ -376,6 +376,13 @@ public:
 #endif
 };
 
+/// \brief Node for a forward traversal of a Varnode expression
+struct TraverseNode {
+  const Varnode *vn;		///< Varnode at the point of traversal
+  uint4 flags;			///< Flags associated with the node
+  TraverseNode(const Varnode *v,uint4 f) { vn = v; flags = f; }
+};
+
 bool contiguous_test(Varnode *vn1,Varnode *vn2);	///< Test if Varnodes are pieces of a whole
 Varnode *findContiguousWhole(Funcdata &data,Varnode *vn1,
 				  Varnode *vn2);	///< Retrieve the whole Varnode given pieces