Fixing some bugs in the new transform framework

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

@@ -1807,16 +1807,12 @@ bool SubfloatFlow::traceForward(TransformVar *rvn)
     {
       TransformVar *rvn2 = setReplacement(op->getIn(1-slot));
       if (rvn2 == (TransformVar *)0) return false;
-      TransformOp *rop = newPreexistingOp(2, op->code(), op);
+      if (preexistingGuard(slot, rvn2)) {
-      if (slot == 0) {
+	TransformOp *rop = newPreexistingOp(2, op->code(), op);
-	opSetInput(rop,rvn,0);
+	opSetInput(rop, rvn, 0);
-	opSetInput(rop,rvn2,1);
+	opSetInput(rop, rvn2, 1);
+	terminatorCount += 1;
       }
-      else {
-	opSetInput(rop,rvn2,0);
-	opSetInput(rop,rvn,1);
-      }
-      terminatorCount += 1;
       break;
     }
     case CPUI_FLOAT_TRUNC:

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

@@ -226,6 +226,8 @@ void TransformOp::createReplacement(Funcdata *fd)
     fd->opSetOpcode(op, opc);
     while(input.size() < op->numInput())
       fd->opRemoveInput(op, op->numInput()-1);
+    for(int4 i=0;i<op->numInput();++i)
+      fd->opUnsetInput(op,i);			// Clear any remaining inputs
     while(op->numInput() < input.size())
       fd->opInsertInput(op, (Varnode *)0, op->numInput()-1);
   }

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

@@ -182,6 +182,7 @@ public:
   TransformVar *getSplit(Varnode *vn,const LaneDescription &description,int4 numLanes,int4 startLane);
   void opSetInput(TransformOp *rop,TransformVar *rvn,int4 slot);	///< Mark given variable as input to given op
   void opSetOutput(TransformOp *rop,TransformVar *rvn);		///< Mark given variable as output of given op
+  static bool preexistingGuard(int4 slot,TransformVar *rvn);	///< Should newPreexistingOp be called
 
   void apply(void);		///< Apply the full transform to the function
 };
@@ -226,4 +227,23 @@ inline void TransformManager::opSetOutput(TransformOp *rop,TransformVar *rvn)
   rvn->def = rop;
 }
 
+/// Varnode marking prevents duplicate TransformOp (and TransformVar) records from getting
+/// created, except in the case of a preexisting PcodeOp with 2 (or more) non-constant inputs.
+/// Because the op is preexisting the output Varnode doesn't get marked, and the op will
+/// be visited for each input.  This method determines when the TransformOp object should be
+/// created, with the goal of creating it exactly once even though the op is visited more than once.
+/// It currently assumes the PcodeOp is binary, and the slot along which the op is
+/// currently visited is passed in, along with the TransformVar for the \e other input. It returns
+/// \b true if the TransformOp should be created.
+/// \param slot is the incoming slot along which the op is visited
+/// \param rvn is the other input
+inline bool TransformManager::preexistingGuard(int4 slot,TransformVar *rvn)
+
+{
+  if (slot == 0) return true;	// If we came in on the first slot, build the TransformOp
+  if (rvn->type == TransformVar::piece || rvn->type == TransformVar::piece_temp)
+    return false;		// The op was/will be visited on slot 0, don't create TransformOp now
+  return true;			// The op was not (will not be) visited on slot 0, build now
+}
+
 #endif