Merge remote-tracking branch 'origin/GP-3682_SnipIndirectPieces'

(Closes #5588)

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

@@ -771,103 +771,71 @@ void Merge::mergeOp(PcodeOp *op)
   }
 }
 
-/// \brief Collect all instances of the given HighVariable whose Cover intersects a p-code op
+/// \brief Collect Varnode instances or pieces from a specific HighVariable that are inputs to a given PcodeOp
 ///
-/// Efficiently test if each instance Varnodes contains the specific p-code op in its Cover
+/// A Varnode is considered an input if it is a \e direct input to the PcodeOp or if it is
-/// and return a list of the instances that do.
+/// \e indirectly affected by the PcodeOp.  The specific \e read of the Varnode is passed back as
-/// \param vlist will hold the resulting list of intersecting instances
+/// a PcodeOp and slot pair (PcodeOpNode).  The passed back PcodeOp will either be the given PcodeOp or
-/// \param high is the given HighVariable
+/// an INDIRECT caused by the given PcodeOp.
-/// \param op is the specific PcodeOp to test intersection with
+/// \param high is the specific HighVariable through which to search for input instances
-void Merge::collectCovering(vector<Varnode *> &vlist,HighVariable *high,PcodeOp *op)
+/// \param oplist will hold the PcodeOpNodes being passed back
+/// \param op is the given PcodeOp
+void Merge::collectInputs(HighVariable *high,vector<PcodeOpNode> &oplist,PcodeOp *op)
 
 {
-  int4 blk = op->getParent()->getIndex();
+  VariableGroup *group = (VariableGroup *)0;
-  for(int4 i=0;i<high->numInstances();++i) {
+  if (high->piece != (VariablePiece *)0)
-    Varnode *vn = high->getInstance(i);
+    group = high->piece->getGroup();
-    if (vn->getCover()->getCoverBlock(blk).contain(op))
+  for(;;) {
-      vlist.push_back(vn);
+    for(int4 i=0;i<op->numInput();++i) {
-  }
+      Varnode *vn = op->getIn(i);
-}
+      if (vn->isAnnotation()) continue;
-
+      HighVariable *testHigh = vn->getHigh();
-/// \brief Check for for p-code op intersections that are correctable
+      if (testHigh == high || (testHigh->piece != (VariablePiece *)0 && testHigh->piece->getGroup() == group)) {
-///
+	oplist.emplace_back(op, i);
-/// Given a list of Varnodes that intersect a specific PcodeOp, check that each intersection is
-/// on the boundary, and if so, pass back the \e read op(s) that cause the intersection.
-/// \param vlist is the given list of intersecting Varnodes
-/// \param oplist will hold the boundary intersecting \e read ops
-/// \param slotlist will hold the corresponding input slots of the instance
-/// \param op is the specific intersecting PcodeOp
-/// \return \b false if any instance in the list intersects the PcodeOp on the interior
-bool Merge::collectCorrectable(const vector<Varnode *> &vlist,list<PcodeOp *> &oplist,
-			       vector<int4> &slotlist,PcodeOp *op)
-{
-  int4 blk = op->getParent()->getIndex();
-  vector<Varnode *>::const_iterator viter;
-  list<PcodeOp *>::const_iterator oiter;
-  Varnode *vn;
-  PcodeOp *edgeop;
-  int4 slot,bound;
-  uintm opuindex = CoverBlock::getUIndex(op);
-
-  for(viter=vlist.begin();viter!=vlist.end();++viter) {
-    vn = *viter;
-    bound = vn->getCover()->getCoverBlock(blk).boundary(op);
-    if (bound == 0) return false;
-    if (bound == 2) continue;	// Not defined before op (intersects with write op)
-    for(oiter=vn->beginDescend();oiter!=vn->endDescend();++oiter) {
-      edgeop = *oiter;
-      if (CoverBlock::getUIndex(edgeop) == opuindex) { // Correctable
-	oplist.push_back(edgeop);
-	slot = edgeop->getSlot(vn);
-	slotlist.push_back(slot);
       }
     }
+    op = op->previousOp();
+    if (op == (PcodeOp *)0 || op->code() != CPUI_INDIRECT)
+      break;
   }
-  return true;
 }
 
-/// \brief Snip instances of the input of an INDIRECT op that interfere with its output
+/// \brief Snip instances of the output of an INDIRECT that are also inputs to to the underlying PcodeOp
 ///
-/// Examine the input and output HighVariable for the given INDIRECT op.
+/// Examine the output HighVariable for the given INDIRECT op. Varnode instances (or pieces) that are also
-/// Varnode instances of the input that intersect the output Cover are snipped by creating
+/// inputs to the underlying PcodeOp causing the INDIRECT are snipped by creating a new COPY op from the
-/// a new COPY op from the input to a new temporary and then replacing the Varnode reads
+/// Varnode to a new temporary and then replacing the \e read with the temporary.
-/// with the temporary.
 /// \param indop is the given INDIRECT op
-void Merge::snipIndirect(PcodeOp *indop)
+/// \return \b true if specific instances are snipped
+bool Merge::snipOutputInterference(PcodeOp *indop)
 
 {
   PcodeOp *op = PcodeOp::getOpFromConst(indop->getIn(1)->getAddr()); // Indirect effect op
-  vector<Varnode *> problemvn;
-  list<PcodeOp *> correctable;
-  vector<int4> correctslot;
 				// Collect instances of output->high that are defined
 				// before (and right up to) op. These need to be snipped.
-  collectCovering(problemvn,indop->getOut()->getHigh(),op);
+  vector<PcodeOpNode> correctable;
-  if (problemvn.empty()) return;
+  collectInputs(indop->getOut()->getHigh(), correctable, op);
-				// Collect vn reads where the snip needs to be.
+  if (correctable.empty())
-				// If cover properly contains op, report an error.
+    return false;
-				// This should not be possible as that vn would have
-				// to intersect with indop->output, which it is merged with.
-  if (!collectCorrectable(problemvn,correctable,correctslot,op))
-    throw LowlevelError("Unable to force indirect merge");
 
-  if (correctable.empty()) return;
+  sort(correctable.begin(),correctable.end(),PcodeOpNode::compareByHigh);
-  Varnode *refvn = correctable.front()->getIn(correctslot[0]);
+  PcodeOp *snipop = (PcodeOp *)0;
-  PcodeOp *snipop,*insertop;
+  HighVariable *curHigh = (HighVariable *)0;
-
+  for(int4 i=0;i<correctable.size();++i) {
-				// NOTE: the covers for any input to op which is
+    PcodeOp *insertop = correctable[i].op;
-				// an instance of the output high must
+    int4 slot = correctable[i].slot;
-				// all intersect so the varnodes must all be
+    Varnode *vn = insertop->getIn(slot);
-				// traceable via COPY to the same root
+    if (vn->getHigh() != curHigh) {
-  snipop = allocateCopyTrim(refvn, op->getAddr(), correctable.front());
+	// NOTE: the covers for any input to op which is an instance of the output high must
-  data.opInsertBefore(snipop,op);
+	// all intersect so the varnodes must all be traceable via COPY to the same root
-  list<PcodeOp *>::iterator oiter;
+      snipop = allocateCopyTrim(vn, insertop->getAddr(), insertop);
-  int4 i,slot;
+      data.opInsertBefore(snipop,insertop);
-  for(oiter=correctable.begin(),i=0;i<correctslot.size();++oiter,++i) {
+      curHigh = vn->getHigh();
-    insertop = *oiter;
+    }
-    slot = correctslot[i];
     data.opSetInput(insertop,snipop->getOut(),slot);
   }
+  return true;
 }
 
 /// \brief Force the merge of all input and output Varnodes to a given INDIRECT op
@@ -879,24 +847,28 @@ void Merge::mergeIndirect(PcodeOp *indop)
 
 {
   Varnode *outvn = indop->getOut();
-  Varnode *invn0 = indop->getIn(0);
   if (!outvn->isAddrForce()) {	// If the output is NOT address forced
     mergeOp(indop);		// We can merge in the same way as a MULTIEQUAL
     return;
   }
 
+  Varnode *invn0 = indop->getIn(0);
   if (mergeTestRequired(outvn->getHigh(),invn0->getHigh())) {
     if (merge(invn0->getHigh(),outvn->getHigh(),false))
       return;
   }
-  snipIndirect(indop);		// If we cannot merge, the only thing that can go
+  // If we cannot merge, the only thing that can go wrong with an input trim, is if the output of
-				// wrong with an input trim, is if the output of
+  // indop is involved in the input to the op causing the indirect effect. So test for this.
-				// indop is involved in the input to the op causing
+  if (snipOutputInterference(indop)) {
-				// the indirect effect. So fix this
+    // If we found (and snipped) something related to the output, try merging again before snipping the INDIRECT
+    if (mergeTestRequired(outvn->getHigh(), invn0->getHigh())) {
+      if (merge(invn0->getHigh(),outvn->getHigh(),false))
+	return;
+    }
+  }
 
-  PcodeOp *newop;
+  // Snip the INDIRECT itself
-
+  PcodeOp *newop = allocateCopyTrim(invn0, indop->getAddr(), indop);
-  newop = allocateCopyTrim(invn0, indop->getAddr(), indop);
   SymbolEntry *entry = outvn->getSymbolEntry();
   if (entry != (SymbolEntry *)0 && entry->getSymbol()->getType()->needsResolution()) {
     data.inheritResolution(entry->getSymbol()->getType(), newop, -1, indop, -1);

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

@@ -95,12 +95,10 @@ class Merge {
   static bool compareCopyByInVarnode(PcodeOp *op1,PcodeOp *op2);
   static bool shadowedVarnode(const Varnode *vn);
   static void findAllIntoCopies(HighVariable *high,vector<PcodeOp *> &copyIns,bool filterTemps);
-  void collectCovering(vector<Varnode *> &vlist,HighVariable *high,PcodeOp *op);
+  void collectInputs(HighVariable *high,vector<PcodeOpNode> &oplist,PcodeOp *op);
-  bool collectCorrectable(const vector<Varnode *> &vlist,list<PcodeOp *> &oplist,vector<int4> &slotlist,
-			   PcodeOp *op);
   PcodeOp *allocateCopyTrim(Varnode *inVn,const Address &addr,PcodeOp *trimOp);
   void snipReads(Varnode *vn,list<PcodeOp *> &markedop);
-  void snipIndirect(PcodeOp *indop);
+  bool snipOutputInterference(PcodeOp *indop);
   void eliminateIntersect(Varnode *vn,const vector<BlockVarnode> &blocksort);
   void unifyAddress(VarnodeLocSet::const_iterator startiter,VarnodeLocSet::const_iterator enditer);
   void trimOpOutput(PcodeOp *op);

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

@@ -258,6 +258,7 @@ struct PcodeOpNode {
   PcodeOpNode(void) { op = (PcodeOp *)0; slot = 0; }	///< Unused constructor
   PcodeOpNode(PcodeOp *o,int4 s) { op = o; slot = s; }	///< Constructor
   bool operator<(const PcodeOpNode &op2) const;		///< Simple comparator for putting edges in a sorted container
+  static bool compareByHigh(const PcodeOpNode &a,const PcodeOpNode &b);	///< Compare Varnodes by their HighVariable
 };
 
 /// \brief A node in a tree structure of CPUI_PIECE operations
@@ -376,5 +377,15 @@ inline bool PcodeOpNode::operator<(const PcodeOpNode &op2) const
   return false;
 }
 
+/// Allow a sorting that groups together input Varnodes with the same HighVariable
+/// \param a is the first Varnode to compare
+/// \param b is the second Varnode to compare
+/// \return true is \b a should come before \b b
+inline bool PcodeOpNode::compareByHigh(const PcodeOpNode &a, const PcodeOpNode &b)
+
+{
+  return a.op->getIn(a.slot)->getHigh() < b.op->getIn(b.slot)->getHigh();
+}
+
 } // End namespace ghidra
 #endif