Merge remote-tracking branch

'origin/GP-7272_caheckman_OverlappingInputVarnodes' (Closes #6259)

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

@@ -1975,6 +1975,15 @@ void ParamActive::registerTrial(const Address &addr,int4 sz)
   slotbase += 1;
 }
 
+/// Copy the memory range of the trial and its boolean properties but assign a new slot.
+/// \param oldTrial is the trial to register
+void ParamActive::reregisterTrial(const ParamTrial &oldTrial)
+
+{
+  trial.emplace_back(oldTrial,slotbase);
+  slotbase += 1;
+}
+
 /// The (index of) the first overlapping trial is returned.
 /// \param addr is the starting address of the given range
 /// \param sz is the number of bytes in the range
@@ -4980,6 +4989,7 @@ int4 FuncCallSpecs::transferLockedInputParam(ProtoParameter *param)
     Address trialend = curtrial.getAddress() + (curtrial.getSize() - 1);
     if (trialend < lastaddr) continue;
     if (curtrial.isDefinitelyNotUsed()) return 0;	// Trial has already been stripped
+    curtrial.markUsed();	// Trial is definitely used, picked up by collectUnlockedTrials
     return curtrial.getSlot();
   }
   if (startaddr.getSpace()->getType() == IPTR_SPACEBASE)
@@ -5069,6 +5079,23 @@ bool FuncCallSpecs::transferLockedOutput(vector<Varnode *> &newoutput,const Func
   return true;
 }
 
+/// \brief Put any trials that might represent a \e varargs parameter in the given container
+///
+/// For a \e locked and \e varargs prototype, collect trials that are not in the locked portion of the prototype.
+/// \param unlockedTrials will hold the collected trials
+void FuncCallSpecs::collectUnlockedTrials(vector<ParamTrial> &unlockedTrials)
+
+{
+  if (!isDotdotdot()) return;
+  for(int4 i=0;i<activeinput.getNumTrials();++i) {
+    const ParamTrial &trial(activeinput.getTrial(i));
+    if (trial.isUsed()) continue;	// Trials for locked slots are marked as used
+    int4 slot = trial.getSlot();
+    if (slot < 1 || slot >= op->numInput()) continue;
+    unlockedTrials.push_back(trial);
+  }
+}
+
 /// \brief Update input Varnodes to \b this CALL to reflect the formal input parameters
 ///
 /// The current input parameters must be locked and are presumably out of date
@@ -5084,6 +5111,8 @@ void FuncCallSpecs::commitNewInputs(Funcdata &data,vector<Varnode *> &newinput)
   if (!isInputLocked()) return;
   Varnode *stackref = getSpacebaseRelative();
   Varnode *placeholder = (Varnode *)0;
+  vector<ParamTrial> unlockedTrials;
+  collectUnlockedTrials(unlockedTrials);
   if (stackPlaceholderSlot>=0)
     placeholder = op->getIn(stackPlaceholderSlot);
   bool noplacehold = true;
@@ -5107,6 +5136,11 @@ void FuncCallSpecs::commitNewInputs(Funcdata &data,vector<Varnode *> &newinput)
       placeholder = (Varnode *)0;	// With a locked stack param, we don't need a placeholder
     }
   }
+  for(int4 i=0;i<unlockedTrials.size();++i) {
+    Varnode *vn = op->getIn(unlockedTrials[i].getSlot());
+    newinput.push_back(vn);
+    activeinput.reregisterTrial(unlockedTrials[i]);
+  }
   if (placeholder != (Varnode *)0) {		// If we still need a placeholder
     newinput.push_back(placeholder);		// Add it at end of parameters
     setStackPlaceholderSlot(newinput.size()-1);

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

@@ -233,6 +233,8 @@ private:
 public:
   /// \brief Construct from components
   ParamTrial(const Address &ad,int4 sz,int4 sl) { addr = ad; size = sz; slot = sl; flags=0; entry=(ParamEntry *)0; offset=-1; fixedPosition = -1; }
+  /// \brief Construct version with new slot
+  ParamTrial(const ParamTrial &op2,int4 sl) { addr = op2.addr; size = op2.size; flags = op2.flags; slot = sl; entry=(ParamEntry *)0; offset=-1; fixedPosition = -1; }
   const Address &getAddress(void) const { return addr; }	///< Get the starting address of \b this trial
   int4 getSize(void) const { return size; }			///< Get the number of bytes in \b this trial
   int4 getSlot(void) const { return slot; }			///< Get the \e slot associated with \b this trial
@@ -296,6 +298,7 @@ public:
   ParamActive(bool recoversub);	///< Construct an empty container
   void clear(void);		///< Reset to an empty container
   void registerTrial(const Address &addr,int4 sz);		///< Add a new trial to the container
+  void reregisterTrial(const ParamTrial &oldTrial);		///< Register a previously existing trial
   int4 getNumTrials(void) const { return trial.size(); }	///< Get the number of trials in \b this container
   ParamTrial &getTrial(int4 i) { return trial[i]; }		///< Get the i-th trial
   const ParamTrial &getTrialForInputVarnode(int4 slot) const;	///< Get trial corresponding to the given input Varnode
@@ -1666,6 +1669,7 @@ class FuncCallSpecs : public FuncProto {
   void transferLockedOutputParam(ProtoParameter *param,vector<Varnode *> &newoutput);
   bool transferLockedInput(vector<Varnode *> &newinput,const FuncProto &source);
   bool transferLockedOutput(vector<Varnode *> &newoutput,const FuncProto &source);
+  void collectUnlockedTrials(vector<ParamTrial> &unlockedTrials);
   void commitNewInputs(Funcdata &data,vector<Varnode *> &newinput);
   void commitNewOutputs(Funcdata &data,vector<Varnode *> &newoutput);
   void collectOutputTrialVarnodes(vector<Varnode *> &trialvn);

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

@@ -539,10 +539,8 @@ void Funcdata::adjustInputVarnodes(const Address &addr,int4 sz)
   // Now that all the intersecting inputs have been pulled out, we can create the new input
   Varnode *invn = newVarnode(sz,addr);
   invn = setInputVarnode(invn);
-  // The new input may cause new heritage and "Heritage AFTER dead removal" errors
-  // So tell heritage to ignore it
-  // FIXME: It would probably be better to insert this directly into heritage's globaldisjoint
-  invn->setWriteMask();
+  // Treat full range as if it has already been heritaged
+  heritage.markRangeHeritaged(invn->getAddr(), invn->getSize());
   // Now change all old inputs to be created as SUBPIECE from the new input
   for(uint4 i=0;i<inlist.size();++i) {
     PcodeOp *op = inlist[i]->getDef();

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

@@ -1962,7 +1962,6 @@ void Heritage::guardInput(const Address &addr,int4 size,vector<Varnode *> &input
   int4 i = 0;
   uintb cur = addr.getOffset();	// Range that needs to be covered
   uintb end = cur + size;
-  //  bool seenunspliced = false;
   Varnode *vn;
   vector<Varnode *> newinput;
 
@@ -1974,11 +1973,8 @@ void Heritage::guardInput(const Address &addr,int4 size,vector<Varnode *> &input
 	int4 sz = vn->getOffset() - cur;
 	vn = fd->newVarnode(sz,Address(addr.getSpace(),cur));
 	vn = fd->setInputVarnode(vn);
-	//	seenunspliced = true;
       }
       else {
-	//	if (vn->hasNoDescend())
-	//	  seenunspliced = true;
 	i += 1;
       }
     }
@@ -1986,7 +1982,6 @@ void Heritage::guardInput(const Address &addr,int4 size,vector<Varnode *> &input
       int4 sz = end-cur;
       vn = fd->newVarnode(sz,Address(addr.getSpace(),cur));
       vn = fd->setInputVarnode(vn);
-      //      seenunspliced = true;
     }
     newinput.push_back(vn);
     cur += vn->getSize();
@@ -1995,17 +1990,6 @@ void Heritage::guardInput(const Address &addr,int4 size,vector<Varnode *> &input
   // Now we need to make sure that all the inputs get linked
   // together into a single input
   if (newinput.size()==1) return; // Will get linked in automatically
-  for(uint4 j=0;j<newinput.size();++j)
-    newinput[j]->setWriteMask();
-//   if (!seenunspliced) {
-//     // Check to see if a concatenation of inputs already exists
-//     // If it existed already it would be defined at fd->getAddress()
-//     // and it would have full size
-//     VarnodeLocSet::const_iterator iter,enditer;
-//     iter = fd->beginLoc(size,addr,fd->getAddress());
-//     enditer = fd->endLoc(size,addr,fd->getAddress());
-//     if (iter != enditer) return; // It already exists
-//   }
   Varnode *newout = fd->newVarnode(size,addr);
   concatPieces(newinput,(PcodeOp *)0,newout)->setActiveHeritage();
 }
@@ -2747,8 +2731,10 @@ void Heritage::heritage(void)
       }
     }
   }
-  placeMultiequals();
-  rename();
+  if (!disjoint.empty()) {
+    placeMultiequals();
+    rename();
+  }
   if (reprocessStackCount > 0)
     reprocessFreeStores(stackSpace, freeStores);
   analyzeNewLoadGuards();
@@ -2851,6 +2837,17 @@ bool Heritage::deadRemovalAllowedSeen(AddrSpace *spc)
   return res;
 }
 
+/// If no heritage has happened yet, do nothing.
+/// \param addr is the start of the range
+/// \param sz is the number of bytes in the range
+void Heritage::markRangeHeritaged(const Address &addr,int4 sz)
+
+{
+  int4 intersect;
+  if (pass > 0)
+    globaldisjoint.add(addr,sz,pass-1,intersect);
+}
+
 /// Reset all analysis as if no heritage passes have yet taken place for the function.
 /// This does not directly affect Varnodes and PcodeOps in the underlying Funcdata.
 void Heritage::clear(void)

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

@@ -90,6 +90,7 @@ public:
   iterator begin(void) { return tasklist.begin(); }			///< Get iterator to beginning of \b this list
   iterator end(void) { return tasklist.end(); }				///< Get iterator to end of \b this list
   void clear(void) { tasklist.clear(); }				///< Clear all ranges in the list
+  bool empty(void) { return tasklist.empty(); }				///< Return \b true if the list is empty
 };
 
 /// \brief Priority queue for the phi-node (MULTIEQUAL) placement algorithm
@@ -329,6 +330,7 @@ public:
   void setDeadCodeDelay(AddrSpace *spc,int4 delay); ///< Set delay for a specific space
   bool deadRemovalAllowed(AddrSpace *spc) const;    ///< Return \b true if it is \e safe to remove dead code
   bool deadRemovalAllowedSeen(AddrSpace *spc);
+  void markRangeHeritaged(const Address &addr,int4 sz);	///< Mark range as heritaged (on previous pass)
   void buildInfoList(void);	                    ///< Initialize information for each space
   void forceRestructure(void) { maxdepth = -1; }    ///< Force regeneration of basic block structures
   void clear(void);				    ///< Reset all analysis of heritage