Merge remote-tracking branch 'origin/GP-6788_CopyTrim' (Closes #8970)

Ghidra/Features/Decompiler/certification.manifest

@@ -24,6 +24,7 @@ src/decompile/datatests/condconstsub.xml||GHIDRA||||END|
 src/decompile/datatests/condexesub.xml||GHIDRA||||END|
 src/decompile/datatests/condmulti.xml||GHIDRA||||END|
 src/decompile/datatests/convert.xml||GHIDRA||||END|
+src/decompile/datatests/copytrim.xml||GHIDRA||||END|
 src/decompile/datatests/deadvolatile.xml||GHIDRA||||END|
 src/decompile/datatests/deindirect.xml||GHIDRA||||END|
 src/decompile/datatests/deindirect2.xml||GHIDRA||||END|

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

@@ -1042,7 +1042,7 @@ void BlockGraph::findSpanningTree(vector<FlowBlock *> &preorder,vector<FlowBlock
     bool extraroots = false;
     int4 rpostcount = list.size();
     int4 rootindex = 0;
-    clearEdgeFlags(~((uint4)0));	// Clear all edge flags
+    clearEdgeFlags(f_irreducible|f_tree_edge|f_forward_edge|f_cross_edge|f_back_edge|f_loop_edge|f_loop_exit_edge);	// Clear spanning tree
     while(preorder.size() < list.size()) {
       FlowBlock *startbl = (FlowBlock *)0;
       while(rootindex<rootlist.size()) { // Go thru blocks with no in edges
@@ -1244,6 +1244,7 @@ void BlockGraph::clear(void)
   for(iter=list.begin();iter!=list.end();++iter)
     delete *iter;
   list.clear();
+  clearAllFlags();
 }
 
 void BlockGraph::markUnstructured(void)
@@ -1352,13 +1353,16 @@ FlowBlock *BlockGraph::nextFlowAfter(const FlowBlock *bl) const
   return nextbl;
 }
 
-void BlockGraph::finalTransform(Funcdata &data)
+void BlockGraph::finalTransform(Funcdata &data,bool allowOpMoves)
 
 {
+  if (hasFinalTransform())
+    return;			// Already performed
   // Recurse into all the substructures
   vector<FlowBlock *>::const_iterator iter;
   for(iter=list.begin();iter!=list.end();++iter)
-    (*iter)->finalTransform(data);
+    (*iter)->finalTransform(data,allowOpMoves);
+  setFlag(f_final_transform);	// Mark that transform has been performed
 }
 
 void BlockGraph::finalizePrinting(Funcdata &data) const
@@ -2365,14 +2369,23 @@ FlowBlock *BlockBasic::getSplitPoint(void)
   return this;
 }
 
-int4 BlockBasic::flipInPlaceTest(vector<PcodeOp *> &fliplist) const
+int4 BlockBasic::flipInPlaceTest(vector<PcodeOp *> &fliplist,bool allowOpRemoval) const
 
 {
   if (op.empty()) return 2;
   PcodeOp *lastop = op.back();
   if (lastop->code() != CPUI_CBRANCH)
     return 2;
-  return Funcdata::opFlipInPlaceTest(lastop,fliplist);
+  int4 res;
+  if (lastop->isBooleanFlip()) {
+    // If the flip bit is set, don't change any ops to accomplish flip
+    vector<PcodeOp *> unusedOps;
+    res = Funcdata::opFlipInPlaceTest(lastop,unusedOps,allowOpRemoval);
+  }
+  else {
+    res = Funcdata::opFlipInPlaceTest(lastop,fliplist,allowOpRemoval);
+  }
+  return res;
 }
 
 void BlockBasic::flipInPlaceExecute(void)
@@ -2381,7 +2394,9 @@ void BlockBasic::flipInPlaceExecute(void)
   PcodeOp *lastop = op.back();
   // This is similar to negateCondition but we don't need to set the boolean_flip flag on lastop
   // because it is getting explicitly changed
-  lastop->flipFlag(PcodeOp::fallthru_true); // Flip whether the fallthru block is true/false
+  lastop->flipFlag(PcodeOp::fallthru_true);	// Flip whether the fallthru block is true/false
+  if (lastop->isBooleanFlip())			// If the flip flag is set
+    lastop->flipFlag(PcodeOp::boolean_flip);	// unflip it (instead of flipping ops)
   FlowBlock::negateCondition(true); // Flip the order of outof this
 }
 
@@ -2443,6 +2458,18 @@ bool BlockBasic::isComplex(void) const
   return false;
 }
 
+void BlockBasic::finalTransform(Funcdata &data,bool allowOpMoves)
+
+{
+  if (sizeOut() != 2) return;
+  PcodeOp *cbranch = lastOp();
+  if (cbranch == (PcodeOp *)0 || cbranch->code() != CPUI_CBRANCH)
+    return;
+  if (!cbranch->isBooleanFlip())
+    return;
+  data.opNormalizeFlip(cbranch);
+}
+
 /// \param encoder is the stream encoder
 void FlowBlock::encodeHeader(Encoder &encoder) const
 
@@ -2552,13 +2579,14 @@ bool BlockBasic::unblockedMulti(int4 outslot) const
 				// outlists
   }
   if (redundlist.empty()) return true;
+  int4 inIndexToThis = blout->getInIndex(this);
   for(iter=blout->op.begin();iter!=blout->op.end();++iter) {
     multiop = *iter;
     if (multiop->code() != CPUI_MULTIEQUAL) continue;
     for(vector<const FlowBlock *>::iterator biter=redundlist.begin();biter!=redundlist.end();++biter) {
       bl = *biter;
       vnredund = multiop->getIn(blout->getInIndex(bl)); // One of the redundant varnodes
-      vnremove = multiop->getIn(blout->getInIndex(this));
+      vnremove = multiop->getIn(inIndexToThis);
       if (vnremove->isWritten()) {
 	othermulti = vnremove->getDef();
 	if ((othermulti->code()==CPUI_MULTIEQUAL)&&(othermulti->getParent()==this))
@@ -2570,6 +2598,26 @@ bool BlockBasic::unblockedMulti(int4 outslot) const
   return true;
 }
 
+/// Was there copy propagation directly out of \b this block into a MULTIEQUAL in the immediate \e out block?
+/// If so, \b this block shouldn't be removed as the COPY may need to be put back during merge.
+/// \param outslot is the output edge to search along
+/// \return \b true if there was no immediate copy propagation, \b false otherwise
+bool BlockBasic::hasNoImmediateCopy(int4 outslot) const
+
+{
+  if (!hasImmedCopyEdge(outslot)) return true;
+  const BlockBasic *blout = (const BlockBasic *)getOut(outslot);
+  int4 inIndexToThis = blout->getInIndex(this);
+  list<PcodeOp *>::const_iterator iter;
+  for(iter=blout->op.begin();iter!=blout->op.end();++iter) {
+    PcodeOp *multiop = *iter;
+    if (multiop->code() != CPUI_MULTIEQUAL) continue;
+    if (multiop->hasCopyImmed(inIndexToThis))
+      return false;
+  }
+  return true;
+}
+
 /// This is a crucial test for whether \b this block is doing anything substantial
 /// or is a candidate for removal.  Even blocks that "do nothing" have some kind of branch
 /// and placeholder operations (MULTIEQUAL and INDIRECT) for data flowing through the block.
@@ -2987,7 +3035,7 @@ void BlockList::printHeader(ostream &s) const
   FlowBlock::printHeader(s);
 }
 
-int4 BlockCondition::flipInPlaceTest(vector<PcodeOp *> &fliplist) const
+int4 BlockCondition::flipInPlaceTest(vector<PcodeOp *> &fliplist,bool allowOpRemoval) const
 
 {
   FlowBlock *split1 = getBlock(0)->getSplitPoint();
@@ -2996,10 +3044,10 @@ int4 BlockCondition::flipInPlaceTest(vector<PcodeOp *> &fliplist) const
   FlowBlock *split2 = getBlock(1)->getSplitPoint();
   if (split2 == (FlowBlock *)0)
     return 2;
-  int4 subtest1 = split1->flipInPlaceTest(fliplist);
+  int4 subtest1 = split1->flipInPlaceTest(fliplist,allowOpRemoval);
   if (subtest1 == 2)
     return 2;
-  int4 subtest2 = split2->flipInPlaceTest(fliplist);
+  int4 subtest2 = split2->flipInPlaceTest(fliplist,allowOpRemoval);
   if (subtest2 == 2)
     return 2;
   return subtest1;
@@ -3090,7 +3138,7 @@ void BlockIf::printHeader(ostream &s) const
   FlowBlock::printHeader(s);
 }
 
-bool BlockIf::preferComplement(Funcdata &data)
+bool BlockIf::preferComplement(Funcdata &data,bool allowOpRemoval)
 
 {
   if (getSize()!=3)		// If we are an if/else
@@ -3100,7 +3148,7 @@ bool BlockIf::preferComplement(Funcdata &data)
   if (split == (FlowBlock *)0)
     return false;
   vector<PcodeOp *> fliplist;
-  if (0 != split->flipInPlaceTest(fliplist))
+  if (0 != split->flipInPlaceTest(fliplist,allowOpRemoval))
     return false;
   split->flipInPlaceExecute();
   data.opFlipInPlaceExecute(fliplist);
@@ -3353,10 +3401,11 @@ FlowBlock *BlockWhileDo::nextFlowAfter(const FlowBlock *bl) const
 /// Determine if \b this block can be printed as a \e for loop, with an \e initializer statement
 /// extracted from the previous block, and an \e iterator statement extracted from the body.
 /// \param data is the function containing \b this loop
-void BlockWhileDo::finalTransform(Funcdata &data)
+/// \param allowOpMoves is \b true if iterator and initializer ops can be moved
+void BlockWhileDo::finalTransform(Funcdata &data,bool allowOpMoves)
 
 {
-  BlockGraph::finalTransform(data);
+  BlockGraph::finalTransform(data,allowOpMoves);
   if (!data.getArch()->analyze_for_loops) return;
   if (hasOverflowSyntax()) return;
   FlowBlock *copyBl = getFrontLeaf();
@@ -3379,6 +3428,7 @@ void BlockWhileDo::finalTransform(Funcdata &data)
   if (iterateOp == (PcodeOp *)0) return;
 
   if (iterateOp != lastOp) {
+    if (!allowOpMoves) return;
     data.opUninsert(iterateOp);
     data.opInsertAfter(iterateOp, lastOp);
   }
@@ -3391,6 +3441,7 @@ void BlockWhileDo::finalTransform(Funcdata &data)
     return;
   }
   if (initializeOp != lastOp) {
+    if (!allowOpMoves) return;
     data.opUninsert(initializeOp);
     data.opInsertAfter(initializeOp, lastOp);
   }

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

@@ -102,7 +102,9 @@ public:
     f_whiledo_overflow = 0x8000,///< Set if the conditional block of a whiledo is too big to print as while(cond) { ...
     f_flip_path = 0x10000,      ///< If true, out edges have been flipped since last time path was traced
     f_joined_block = 0x20000,	///< Block is a merged form of original basic blocks
-    f_duplicate_block = 0x40000	///< Block is a duplicated version of an original basic block
+    f_duplicate_block = 0x40000,	///< Block is a duplicated version of an original basic block
+    f_delayed_donothing = 0x80000,	///< Potential \e do \e nothing block whose removal has been delayed
+    f_final_transform = 0x100000	///< Has the final transform been run
   };
   /// \brief Boolean properties on edges
   enum edge_flags {
@@ -114,7 +116,8 @@ public:
     f_forward_edge = 0x20,	///< An edge that jumps forward in the spanning tree
     f_cross_edge = 0x40,	///< An edge that crosses subtrees in the spanning tree
     f_back_edge = 0x80,		///< Within (reducible) graph, a back edge defining a loop
-    f_loop_exit_edge = 0x100	///< Edge exits the body of a loop
+    f_loop_exit_edge = 0x100,	///< Edge exits the body of a loop
+    f_immed_copy = 0x200	///< Copy propagation has happened across the edge
   };
 private:
   uint4 flags;			///< Collection of block_flags
@@ -154,6 +157,7 @@ private:
 protected:
   void setFlag(uint4 fl) { flags |= fl; }	///< Set a boolean property
   void clearFlag(uint4 fl) { flags &= ~fl; }	///< Clear a boolean property
+  void clearAllFlags(void) { flags = 0; }	///< Clear all properties of \b this block
 public:
   FlowBlock(void);				///< Construct a block with no edges
   virtual ~FlowBlock(void) {}			///< Destructor
@@ -239,9 +243,9 @@ public:
   virtual PcodeOp *lastOp(void) const { return (PcodeOp *)0; }
 
   virtual bool negateCondition(bool toporbottom);	///< Flip the condition computed by \b this
-  virtual bool preferComplement(Funcdata &data);	///< Rearrange \b this hierarchy to simplify boolean expressions
+  virtual bool preferComplement(Funcdata &data,bool allowOpRemoval);	///< Rearrange \b this hierarchy to simplify boolean expressions
   virtual FlowBlock *getSplitPoint(void);		///< Get the leaf splitting block
-  virtual int4 flipInPlaceTest(vector<PcodeOp *> &fliplist) const;
+  virtual int4 flipInPlaceTest(vector<PcodeOp *> &fliplist,bool allowOpRemoval) const;
   virtual void flipInPlaceExecute(void);
 
   /// \brief Is \b this too complex to be a condition (BlockCondition)
@@ -254,7 +258,8 @@ public:
   /// \brief Do any structure driven final transforms
   ///
   /// \param data is the function to transform
-  virtual void finalTransform(Funcdata &data) {}
+  /// \param allowOpMoves is \b true if ops may be moved within their basic block
+  virtual void finalTransform(Funcdata &data,bool allowOpMoves) {}
 
   /// \brief Make any final configurations necessary to emit the block
   ///
@@ -294,6 +299,8 @@ public:
   void setLoopExit(int4 i) { setOutEdgeFlag(i,f_loop_exit_edge); }	///< Label the edge exiting \b this as a loop
   void clearLoopExit(int4 i) { clearOutEdgeFlag(i,f_loop_exit_edge); }	///< Clear the loop exit edge
   void setBackEdge(int4 i) { setOutEdgeFlag(i,f_back_edge); }		///< Label the \e back edge of a loop
+  void setImmedCopyEdge(int4 i) { setOutEdgeFlag(i,f_immed_copy); }	///< Mark that an immediate COPY has propagated across the edge
+  bool hasImmedCopyEdge(int4 i) const { return ((outofthis[i].label & f_immed_copy)!=0); }	///< Has an immediate COPY propagated across the edge
   bool getFlipPath(void) const { return ((flags & f_flip_path)!=0); }	///< Have out edges been flipped
   bool isJumpTarget(void) const;		///< Return \b true if non-fallthru jump flows into \b this
   FlowBlock *getFalseOut(void) const { return outofthis[0].point; }	///< Get the \b false output FlowBlock
@@ -393,7 +400,7 @@ public:
   virtual void emit(PrintLanguage *lng) const { lng->emitBlockGraph(this); }
   virtual PcodeOp *firstOp(void) const;
   virtual FlowBlock *nextFlowAfter(const FlowBlock *bl) const;
-  virtual void finalTransform(Funcdata &data);
+  virtual void finalTransform(Funcdata &data,bool allowOpMoves);
   virtual void finalizePrinting(Funcdata &data) const;
   virtual void encodeBody(Encoder &encoder) const;
   virtual void decodeBody(Decoder &decoder);
@@ -438,6 +445,7 @@ public:
   void calcLoop(void);								///< Calculate loop edges
   void collectReachable(vector<FlowBlock *> &res,FlowBlock *bl,bool un) const;	///< Collect reachable/unreachable FlowBlocks from a given start FlowBlock
   void structureLoops(vector<FlowBlock *> &rootlist);				///< Label loop edges
+  bool hasFinalTransform(void) const { return ((getFlags() & f_final_transform) != 0); }	///< Return \b true if finalTransform() has been run on \b this
 #ifdef BLOCKCONSISTENT_DEBUG
   bool isConsistent(void) const;						///< Check consistency of \b this BlockGraph
 #endif
@@ -490,10 +498,12 @@ public:
   virtual PcodeOp *lastOp(void) const;
   virtual bool negateCondition(bool toporbottom);
   virtual FlowBlock *getSplitPoint(void);
-  virtual int4 flipInPlaceTest(vector<PcodeOp *> &fliplist) const;
+  virtual int4 flipInPlaceTest(vector<PcodeOp *> &fliplist,bool allowOpRemoval) const;
   virtual void flipInPlaceExecute(void);
   virtual bool isComplex(void) const;
-  bool unblockedMulti(int4 outslot) const;		///< Check if \b this block can be removed without introducing inconsistencies
+  virtual void finalTransform(Funcdata &data,bool allowOpMoves);
+  bool unblockedMulti(int4 outslot) const;	///< Check if \b this block can be removed without introducing inconsistencies
+  bool hasNoImmediateCopy(int4 outslot) const;	///< Check if there have been immediate COPYs out of \b this block
   bool hasOnlyMarkers(void) const;		///< Does \b this block contain only MULTIEQUAL and INDIRECT ops
   bool isDoNothing(void) const;			///< Should \b this block should be removed
   list<PcodeOp *>::iterator beginOp(void) { return op.begin(); }	///< Return an iterator to the beginning of the PcodeOps
@@ -504,6 +514,9 @@ public:
   bool noInterveningStatement(void) const;
   PcodeOp *findMultiequal(const vector<Varnode *> &varArray);		///< Find MULTIEQUAL with given inputs
   PcodeOp *earliestUse(Varnode *vn);
+  void setDelayedDonothing(void) { setFlag(f_delayed_donothing); }	///< Mark as \e do \e nothing block with delayed removal
+  void clearDelayedDonothing(void) { clearFlag(f_delayed_donothing); }	///< Clear mark for delayed removal
+  bool isDelayedDonothing(void) const { return ((getFlags() & f_delayed_donothing) != 0); }	///< Is block marked for delayed removal
   static bool liftVerifyUnroll(vector<Varnode *> &varArray,int4 slot);	///< Verify given Varnodes are defined with same PcodeOp
 };
 
@@ -534,6 +547,7 @@ public:
   virtual bool negateCondition(bool toporbottom) { bool res = copy->negateCondition(true); FlowBlock::negateCondition(toporbottom); return res; }
   virtual FlowBlock *getSplitPoint(void) { return copy->getSplitPoint(); }
   virtual bool isComplex(void) const { return copy->isComplex(); }
+  virtual void finalTransform(Funcdata &data,bool allowOpMoves) { return copy->finalTransform(data,allowOpMoves); }
   virtual void encodeHeader(Encoder &encoder) const;
 };
 
@@ -629,7 +643,7 @@ public:
   virtual void emit(PrintLanguage *lng) const { lng->emitBlockCondition(this); }
   virtual bool negateCondition(bool toporbottom);
   virtual FlowBlock *getSplitPoint(void) { return this; }
-  virtual int4 flipInPlaceTest(vector<PcodeOp *> &fliplist) const;
+  virtual int4 flipInPlaceTest(vector<PcodeOp *> &fliplist,bool allowOpRemoval) const;
   virtual void flipInPlaceExecute(void);
   virtual PcodeOp *lastOp(void) const;
   virtual bool isComplex(void) const { return getBlock(0)->isComplex(); }
@@ -668,7 +682,7 @@ public:
   virtual void scopeBreak(int4 curexit,int4 curloopexit);
   virtual void printHeader(ostream &s) const;
   virtual void emit(PrintLanguage *lng) const { lng->emitBlockIf(this); }
-  virtual bool preferComplement(Funcdata &data);
+  virtual bool preferComplement(Funcdata &data,bool allowOpRemoval);
   virtual const FlowBlock *getExitLeaf(void) const;
   virtual PcodeOp *lastOp(void) const;
   virtual FlowBlock *nextFlowAfter(const FlowBlock *bl) const;
@@ -710,7 +724,7 @@ public:
   virtual void printHeader(ostream &s) const;
   virtual void emit(PrintLanguage *lng) const { lng->emitBlockWhileDo(this); }
   virtual FlowBlock *nextFlowAfter(const FlowBlock *bl) const;
-  virtual void finalTransform(Funcdata &data);
+  virtual void finalTransform(Funcdata &data,bool allowOpMoves);
   virtual void finalizePrinting(Funcdata &data) const;
 };
 
@@ -831,8 +845,9 @@ inline void FlowBlock::emit(PrintLanguage *lng) const
 /// For the instructions in this block, decide if the control-flow structure
 /// can be rearranged so that boolean expressions come out more naturally.
 /// \param data is the function to analyze
+/// \param allowOpRemoval if \b true, changes can include removal of ops
 /// \return \b true if a change was made
-inline bool FlowBlock::preferComplement(Funcdata &data)
+inline bool FlowBlock::preferComplement(Funcdata &data,bool allowOpRemoval)
 
 {
   return false;
@@ -851,15 +866,17 @@ inline FlowBlock *FlowBlock::getSplitPoint(void)
 /// \brief Test normalizing the conditional branch in \b this
 ///
 /// Find the set of PcodeOp objects that need to be adjusted to flip
-/// the condition \b this FlowBlock calculates.
+/// the condition \b this FlowBlock calculates.  If \b allowOpRemoval is set,
+/// the adjustment can include the removal of (BOOL_NEGATE) ops.
 ///
 /// Return:
 ///   - 0 if the flip would normalize the condition
-///   - 1 if the flip doesn't affect normalization of the condition
+///   - 1 if the flip denormalizes (or doesn't affect normalization)
-///   - 2 if the flip produces an unnormalized condition
+///   - 2 if a flip is not possible
 /// \param fliplist will contain the PcodeOps that need to be adjusted
+/// \param allowOpRemoval if \b true adjustments can include removal of ops
 /// \return 0 if the condition will be normalized, 1 or 2 otherwise
-inline int4 FlowBlock::flipInPlaceTest(vector<PcodeOp *> &fliplist) const
+inline int4 FlowBlock::flipInPlaceTest(vector<PcodeOp *> &fliplist,bool allowOpRemoval) const
 
 {
   return 2;	// By default a block will not normalize

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

@@ -2110,7 +2110,7 @@ void ConditionalJoin::clear(void)
 int4 ActionStructureTransform::apply(Funcdata &data)
 
 {
-  data.getStructure().finalTransform(data);
+  data.getStructure().finalTransform(data,allowOpMoves);
   return 0;
 }
 
@@ -2127,7 +2127,7 @@ int4 ActionNormalizeBranches::apply(Funcdata &data)
     if (cbranch == (PcodeOp *)0) continue;
     if (cbranch->code() != CPUI_CBRANCH) continue;
     fliplist.clear();
-    if (Funcdata::opFlipInPlaceTest(cbranch,fliplist) != 0)
+    if (Funcdata::opFlipInPlaceTest(cbranch,fliplist,true) != 0)
       continue;
     data.opFlipInPlaceExecute(fliplist);
     bb->flipInPlaceExecute();
@@ -2143,6 +2143,7 @@ int4 ActionPreferComplement::apply(Funcdata &data)
   BlockGraph &graph(data.getStructure());
   
   if (graph.getSize() == 0) return 0;
+  if (graph.hasFinalTransform()) return 0;
   vector<BlockGraph *> vec;
   vec.push_back(&graph);
   int4 pos = 0;
@@ -2159,7 +2160,7 @@ int4 ActionPreferComplement::apply(Funcdata &data)
 	continue;
       vec.push_back((BlockGraph *)childbl);
     }
-    if (curbl->preferComplement(data))
+    if (curbl->preferComplement(data,allowOpMods))
       count += 1;
   }
   data.clearDeadOps();		// Clear any ops deleted during this action

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

@@ -268,11 +268,12 @@ public:
 ///
 /// This is currently used to set up \e for loops via BlockWhileDo
 class ActionStructureTransform : public Action {
+  bool allowOpMoves;		///< Are p-code ops allowed to be moved by \b this action
 public:
-  ActionStructureTransform(const string &g) : Action(0,"structuretransform",g) {}	///< Constructor
+  ActionStructureTransform(const string &g,bool allowMoves) : Action(0,"structuretransform",g) { allowOpMoves = allowMoves; }	///< Constructor
   virtual Action *clone(const ActionGroupList &grouplist) const {
     if (!grouplist.contains(getGroup())) return (Action *)0;
-    return new ActionStructureTransform(getGroup());
+    return new ActionStructureTransform(getGroup(),allowOpMoves);
   }
   virtual int4 apply(Funcdata &data);
 };
@@ -298,11 +299,12 @@ public:
 /// This uses the preferComplement() method on structured FlowBlocks to choose between symmetric
 /// structurings, such as an if/else where the \b true and \b false blocks can be swapped.
 class ActionPreferComplement : public Action {
+  bool allowOpMods;		///< Are p-code ops allowed to be modified by \b this action
 public:
-  ActionPreferComplement(const string &g) : Action(0,"prefercomplement",g) {}	///< Constructor
+  ActionPreferComplement(const string &g,bool allowMods) : Action(0,"prefercomplement",g) { allowOpMods = allowMods; }	///< Constructor
   virtual Action *clone(const ActionGroupList &grouplist) const {
     if (!grouplist.contains(getGroup())) return (Action *)0;
-    return new ActionPreferComplement(getGroup());
+    return new ActionPreferComplement(getGroup(),allowOpMods);
   }
   virtual int4 apply(Funcdata &data);
 };

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

@@ -3552,12 +3552,11 @@ int4 ActionUnreachable::apply(Funcdata &data)
 int4 ActionDoNothing::apply(Funcdata &data)
 
 {				// Remove blocks that do nothing
-  int4 i;
   const BlockGraph &graph(data.getBasicBlocks());
-  BlockBasic *bb;
 
-  for(i=0;i<graph.getSize();++i) {
+  for(int4 i=0;i<graph.getSize();++i) {
-    bb = (BlockBasic *) graph.getBlock(i);
+    BlockBasic *bb = (BlockBasic *) graph.getBlock(i);
+    bb->clearDelayedDonothing();
     if (bb->isDoNothing()) {
       if ((bb->sizeOut()==1)&&(bb->getOut(0)==bb)) { // Infinite loop
 	if (!bb->isDonothingLoop()) {
@@ -3566,15 +3565,71 @@ int4 ActionDoNothing::apply(Funcdata &data)
 	}
       }
       else if (bb->unblockedMulti(0)) {
-	data.removeDoNothingBlock(bb);
+	if (data.isNormalizationOn() || bb->hasNoImmediateCopy(0)) {
-	count += 1;
+	  data.removeDoNothingBlock(bb);
-	return 0;
+	  count += 1;
+	  return 0;
+	}
+	else {
+	  // If there were immediate COPYs but the block was otherwise removable,
+	  // mark the block for possible late removal.
+	  bb->setDelayedDonothing();
+	}
       }
     }
   }
   return 0;
 }
 
+/// For each input to \b bl, check if all other out edges go to the same \e out block as \b bl.
+/// \param bl is the block being removed
+/// \return \b true if a redundancy would be created
+bool ActionLateDoNothing::removingCreatesRedundancy(FlowBlock *bl)
+
+{
+  FlowBlock *outbl = bl->getOut(0);
+  for(int4 i=0;i<bl->sizeIn();++i) {
+    FlowBlock *inbl = bl->getIn(i);
+    if (inbl->sizeOut() == 1) continue;
+    int4 count;
+    for(count=0;count<inbl->sizeOut();++count) {
+      FlowBlock *curbl = inbl->getOut(count);
+      if (curbl != bl && curbl != outbl)		// Check if this edge goes to outbl (possibly via bl)
+	break;
+    }
+    if (count == inbl->sizeOut())			// All edges lead to outbl
+      return true;
+  }
+  return false;
+}
+
+int4 ActionLateDoNothing::apply(Funcdata &data)
+
+{
+  const BlockGraph &graph(data.getBasicBlocks());
+  vector<BlockBasic *> removeList;
+
+  for(int4 i=0;i<graph.getSize();++i) {
+    BlockBasic *bb = (BlockBasic *) graph.getBlock(i);
+    if (!bb->isDelayedDonothing()) continue;
+    if (bb->isDoNothing()) {
+      if (removingCreatesRedundancy(bb)) continue;
+      if ((bb->sizeOut() == 1) && (bb->getOut(0) == bb)) { // Infinite loop
+	if (!bb->isDonothingLoop()) {
+	  bb->setDonothingLoop();
+	  data.warning("Do nothing block with infinite loop",bb->getStart());
+	}
+      }
+      else if (bb->unblockedMulti(0)) removeList.push_back(bb);
+    }
+  }
+  for(int4 i=0;i<removeList.size();++i) {
+    data.removeDoNothingBlock(removeList[i]);
+    count += 1;
+  }
+  return 0;
+}
+
 int4 ActionRedundBranch::apply(Funcdata &data)
 
 {
@@ -5594,10 +5649,10 @@ void ActionDatabase::universalAction(Architecture *conf)
       actmainloop->addAction( new ActionRestrictLocal("localrecovery") ); // Do before dead code removed
       actmainloop->addAction( new ActionDeadCode("deadcode") );
       actmainloop->addAction( new ActionDynamicMapping("dynamic") ); // Must come before restructurevarnode and infertypes
-      actmainloop->addAction( new ActionRestructureVarnode("localrecovery") );
       actmainloop->addAction( new ActionSpacebase("base") );	// Must come before infertypes and nonzeromask
       actmainloop->addAction( new ActionNonzeroMask("analysis") );
       actmainloop->addAction( new ActionInferTypes("typerecovery") );
+      actmainloop->addAction( new ActionRestructureVarnode("localrecovery") );
       actstackstall = new ActionGroup(Action::rule_repeatapply,"stackstall");
       {
 	actprop = new ActionPool(Action::rule_repeatapply,"oppool1");
@@ -5809,8 +5864,8 @@ void ActionDatabase::universalAction(Architecture *conf)
   }
   act->addAction( actcleanup );
 
-  act->addAction( new ActionPreferComplement("blockrecovery") );
+  act->addAction( new ActionPreferComplement("blockrecovery", true) );
-  act->addAction( new ActionStructureTransform("blockrecovery") );
+  act->addAction( new ActionStructureTransform("blockrecovery", true) );	// Allow mods
   act->addAction( new ActionNormalizeBranches("normalizebranches") );
   act->addAction( new ActionAssignHigh("merge") );
   act->addAction( new ActionMergeRequired("merge") );
@@ -5825,6 +5880,10 @@ void ActionDatabase::universalAction(Architecture *conf)
   act->addAction( new ActionMergeType("merge") );
   act->addAction( new ActionHideShadow("merge") );
   act->addAction( new ActionCopyMarker("merge") );
+  act->addAction( new ActionLateDoNothing("blockrecovery") );
+  act->addAction( new ActionBlockStructure("blockrecovery") );
+  act->addAction( new ActionPreferComplement("blockrecovery", false) );		// Don't allow mods
+  act->addAction( new ActionStructureTransform("blockrecovery", false) );	// Don't allow mods
   act->addAction( new ActionOutputPrototype("localrecovery") );
   act->addAction( new ActionInputPrototype("fixateproto") );
   act->addAction( new ActionMapGlobals("fixateglobals") );

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

@@ -510,6 +510,18 @@ public:
   virtual int4 apply(Funcdata &data);
 };
 
+/// \brief Remove blocks that do nothing after variable merging has occurred
+class ActionLateDoNothing : public Action {
+  static bool removingCreatesRedundancy(FlowBlock *bl);	///< Does removing the given block create a redundant branch point
+public:
+  ActionLateDoNothing(const string &g) : Action(0,"latedonothing",g) {}	///< Constructor
+  virtual Action *clone(const ActionGroupList &grouplist) const {
+    if (!grouplist.contains(getGroup())) return (Action *)0;
+    return new ActionLateDoNothing(getGroup());
+  }
+  virtual int4 apply(Funcdata &data);
+};
+
 /// \brief Get rid of \b redundant branches: duplicate edges between the same input and output block
 class ActionRedundBranch : public Action {
 public:
@@ -629,6 +641,7 @@ public:
 class ActionNormalizeSetup : public Action {
 public:
   ActionNormalizeSetup(const string &g) : Action(rule_onceperfunc,"normalizesetup",g) {}	///< Constructor
+  virtual void reset(Funcdata &data) { data.setNormalization(true); }
   virtual Action *clone(const ActionGroupList &grouplist) const {
     if (!grouplist.contains(getGroup())) return (Action *)0;
     return new ActionNormalizeSetup(getGroup());

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

@@ -86,7 +86,7 @@ void Funcdata::clear(void)
 {				// Clear everything associated with decompilation (analysis)
 
   flags &= ~(highlevel_on|blocks_generated|processing_started|typerecovery_start|typerecovery_on|
-      double_precis_on|restart_pending);
+      double_precis_on|restart_pending|normalization_on);
   clean_up_index = 0;
   high_level_index = 0;
   cast_phase_index = 0;

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

@@ -69,7 +69,8 @@ class Funcdata {
     unimplemented_present = 0x800,	///< Set if function contains unimplemented instructions
     baddata_present = 0x1000,	///< Set if function flowed into bad data
     double_precis_on = 0x2000,	///< Set if we are performing double precision recovery
-    typerecovery_exceeded= 0x4000	///< Set if data-type propagation passes reached maximum
+    typerecovery_exceeded= 0x4000,	///< Set if data-type propagation passes reached maximum
+    normalization_on = 0x8000	///< Set if normalization will be performed
   };
   uint4 flags;			///< Boolean properties associated with \b this function
   uint4 clean_up_index;		///< Creation index of first Varnode created after start of cleanup
@@ -116,6 +117,7 @@ class Funcdata {
 				// Low level block functions
   void blockRemoveInternal(BlockBasic *bb,bool unreachable);
   void branchRemoveInternal(BlockBasic *bb,int4 num);
+  Varnode *createReplaceVarnode(Varnode *origvn,bool makeUnique);	///< Create a replacement for a Varnode that may be merged
   void pushMultiequals(BlockBasic *bb);		///< Push MULTIEQUAL Varnodes of the given block into the output block
   void clearBlocks(void);			///< Clear all basic blocks
   void structureReset(void);			///< Calculate initial basic block structures (after a control-flow change)
@@ -150,6 +152,7 @@ public:
   bool isTypeRecoveryOn(void) const { return ((flags&typerecovery_on)!=0); }	///< Will data-type analysis be performed
   bool hasTypeRecoveryStarted(void) const { return ((flags&typerecovery_start)!=0); }	///< Has data-type recovery processes started
   bool isTypeRecoveryExceeded(void) const { return ((flags&typerecovery_exceeded)!=0); }	///< Has maximum propagation passes been reached
+  bool isNormalizationOn(void) const { return ((flags&normalization_on)!=0); }	///< Will normalization be performed
   bool hasNoCode(void) const { return ((flags & no_code)!=0); }		///< Return \b true if \b this function has no code body
   void setNoCode(bool val) { if (val) flags |= no_code; else flags &= ~no_code; }	///< Toggle whether \b this has a body
   void setLanedRegGenerated(void) { minLanedSize = 1000000; }	///< Mark that laned registers have been collected
@@ -180,6 +183,11 @@ public:
   /// \param val is \b true if data-type analysis is enabled
   void setTypeRecovery(bool val) { flags = val ? (flags | typerecovery_on) : (flags & ~typerecovery_on); }
   void setTypeRecoveryExceeded(void) { flags |= typerecovery_exceeded; }	///< Mark propagation passes have reached maximum
+
+  /// \brief Toggle whether normalization transforms will be performed on \b this function
+  ///
+  /// \param val is \b true if normalization is enabled
+  void setNormalization(bool val) { flags = val ? (flags | normalization_on) : (flags & ~normalization_on); }
   void startCastPhase(void) { cast_phase_index = vbank.getCreateIndex(); }	///< Start the \b cast insertion phase
   uint4 getCastPhaseIndex(void) const { return cast_phase_index; }	///< Get creation index at the start of \b cast insertion
   uint4 getHighLevelIndex(void) const { return high_level_index; }	///< Get creation index at the start of HighVariable creation
@@ -494,8 +502,9 @@ public:
   PcodeOp *opStackStore(AddrSpace *spc,uintb off,PcodeOp *op,bool insertafter);
   Varnode *opBoolNegate(Varnode *vn,PcodeOp *op,bool insertafter);
   void opUndoPtradd(PcodeOp *op,bool finalize);	///< Convert a CPUI_PTRADD back into a CPUI_INT_ADD
-  static int4 opFlipInPlaceTest(PcodeOp *op,vector<PcodeOp *> &fliplist);
+  static int4 opFlipInPlaceTest(PcodeOp *op,vector<PcodeOp *> &fliplist,bool allowOpRemoval);
   void opFlipInPlaceExecute(vector<PcodeOp *> &fliplist);
+  bool opNormalizeFlip(PcodeOp *cbranch);
 
   /// \brief Start of PcodeOp objects with the given op-code
   list<PcodeOp *>::const_iterator beginOp(OpCode opc) const { return obank.begin(opc); }

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

@@ -77,6 +77,27 @@ void Funcdata::removeJumpTable(JumpTable *jt)
   jumpvec = remain;
 }
 
+/// The replacement has the same address and size as the given Varnode but can optionally turned into a new \e unique.
+/// We assume the given Varnode will be destroyed but that it may already be merged into a HighVariable.
+/// If a HighVariable exists, the given Varnode is removed and the replacement is added.
+/// \param origvn is the given Varnode to be replaced
+/// \param makeUnique is \b true if the replacement should be a new \e unique
+/// \return the new replacement Varnode
+Varnode *Funcdata::createReplaceVarnode(Varnode *origvn,bool makeUnique)
+
+{
+  Varnode *replacevn;
+  if (makeUnique)
+    replacevn = newUnique(origvn->getSize(),origvn->getType());
+  else
+    replacevn = newVarnode(origvn->getSize(),origvn->getAddr(),origvn->getType());
+  if (isHighOn()) {
+    origvn->replaceInHigh(replacevn);
+    replacevn->setExplicit();
+  }
+  return replacevn;
+}
+
 /// Assuming the given basic block is being removed, force any Varnode defined by
 /// a MULTIEQUAL in the block to be defined in the output block instead. This is used
 /// as part of the basic block removal process to patch up data-flow.
@@ -128,11 +149,8 @@ void Funcdata::pushMultiequals(BlockBasic *bb)
     }
     if (!needreplace) continue;
 				// Construct artificial MULTIEQUAL
+    replacevn = createReplaceVarnode(origvn, neednewunique);
     vector<Varnode *> branches;
-    if (neednewunique)
-      replacevn = newUnique(origvn->getSize());
-    else
-      replacevn = newVarnode(origvn->getSize(),origvn->getAddr());
     for(int4 i=0;i<outblock->sizeIn();++i) {
       if (outblock->getIn(i) == bb)
 	branches.push_back(origvn);
@@ -208,7 +226,7 @@ void Funcdata::branchRemoveInternal(BlockBasic *bb,int4 num)
   bblocks.removeEdge(bb,bbout); // Sever (one) connection between bb and bbout
   for(iter=bbout->beginOp();iter!=bbout->endOp();++iter) {
     op = *iter;
-    if (op->code() != CPUI_MULTIEQUAL) continue;
+    if (op->code() != CPUI_MULTIEQUAL) break;
     opRemoveInput(op,blocknum);
     opZeroMulti(op);
   }

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

@@ -1216,11 +1216,12 @@ Varnode *Funcdata::buildCopyTemp(Varnode *vn,PcodeOp *point)
 ///
 /// The boolean Varnode is either the output of the given PcodeOp or the
 /// first input if the PcodeOp is a CBRANCH. The list of ops that need flipping is
-/// returned in an array
+/// returned in an array.
 /// \param op is the given PcodeOp
 /// \param fliplist is the array that will hold the ops to flip
-/// \return 0 if the change normalizes, 1 if the change is ambivalent, 2 if the change does not normalize
+/// \param allowOpRemoval if \b true allow for removal of BOOL_NEGATE ops to achieve flip
-int4 Funcdata::opFlipInPlaceTest(PcodeOp *op,vector<PcodeOp *> &fliplist)
+/// \return 0 if the change normalizes, 1 if the change denormalizes or is ambivalent, 2 if flip in place is not possible
+int4 Funcdata::opFlipInPlaceTest(PcodeOp *op,vector<PcodeOp *> &fliplist,bool allowOpRemoval)
 
 {
   Varnode *vn;
@@ -1230,12 +1231,19 @@ int4 Funcdata::opFlipInPlaceTest(PcodeOp *op,vector<PcodeOp *> &fliplist)
     vn = op->getIn(1);
     if (vn->loneDescend() != op) return 2;
     if (!vn->isWritten()) return 2;
-    return opFlipInPlaceTest(vn->getDef(),fliplist);
+    subtest1 = opFlipInPlaceTest(vn->getDef(),fliplist,allowOpRemoval);
+    if (subtest1 != 2 && op->isBooleanFlip())
+      subtest1 = 1-subtest1;
+    return subtest1;
   case CPUI_INT_EQUAL:
   case CPUI_FLOAT_EQUAL:
     fliplist.push_back(op);
     return 1;
   case CPUI_BOOL_NEGATE:
+    if (!allowOpRemoval)
+      return 2;
+    fliplist.push_back(op);
+    return 0;
   case CPUI_INT_NOTEQUAL:
   case CPUI_FLOAT_NOTEQUAL:
     fliplist.push_back(op);
@@ -1257,13 +1265,13 @@ int4 Funcdata::opFlipInPlaceTest(PcodeOp *op,vector<PcodeOp *> &fliplist)
     vn = op->getIn(0);
     if (vn->loneDescend() != op) return 2;
     if (!vn->isWritten()) return 2;
-    subtest1 = opFlipInPlaceTest(vn->getDef(),fliplist);
+    subtest1 = opFlipInPlaceTest(vn->getDef(),fliplist,allowOpRemoval);
     if (subtest1 == 2)
       return 2;
     vn = op->getIn(1);
     if (vn->loneDescend() != op) return 2;
     if (!vn->isWritten()) return 2;
-    subtest2 = opFlipInPlaceTest(vn->getDef(),fliplist);
+    subtest2 = opFlipInPlaceTest(vn->getDef(),fliplist,allowOpRemoval);
     if (subtest2 == 2)
       return 2;
     fliplist.push_back(op);
@@ -1314,6 +1322,31 @@ void Funcdata::opFlipInPlaceExecute(vector<PcodeOp *> &fliplist)
   }
 }
 
+/// \brief Remove the \b boolean_flip flag on a CBRANCH op, without changing behavior
+///
+/// Try to flip the true/false meaning of the CBRANCH and negate the meaning of the comparison op feeding the CBRANCH.
+/// Only the \b boolean_flip flag and the comparison op's opcode are affected.
+/// \param cbranch is CBRANCH to modify
+/// \return \b true if the ops were successfully modified
+bool Funcdata::opNormalizeFlip(PcodeOp *cbranch)
+
+{
+  Varnode *boolVn = cbranch->getIn(1);
+  if (!boolVn->isWritten()) return false;
+  if (boolVn->loneDescend() != cbranch) return false;
+  PcodeOp *condOp = boolVn->getDef();
+  bool flipyes;
+  OpCode opc = get_booleanflip(condOp->code(), flipyes);
+  if (opc == CPUI_MAX) return false;
+  opSetOpcode(condOp,opc); // Set the negated opcode
+  if (flipyes)			// Do we need to reverse the two operands
+    opSwapInput(condOp,0,1);
+  cbranch->flipFlag(PcodeOp::boolean_flip);
+  if (opc == CPUI_INT_LESSEQUAL || opc == CPUI_INT_SLESSEQUAL)
+    replaceLessequal(condOp);
+  return true;
+}
+
 /// \brief Find a duplicate calculation of a given PcodeOp reading a specific Varnode
 ///
 /// We only match 1 level of calculation.  Additionally the duplicate must occur in the

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

@@ -124,6 +124,29 @@ bool PcodeOp::isCollapsible(void) const
   return true;
 }
 
+/// \param slot is the specific input edge being marked
+void PcodeOp::setCopyImmed(int4 slot)
+
+{
+  FlowBlock *inbl = parent->getIn(slot);
+  int4 outedge = parent->getInRevIndex(slot);
+  inbl->setImmedCopyEdge(outedge);
+  addlflags |= immed_copy;
+}
+
+/// \param slot is the specific input edge to test
+/// \return \b true if an immediate COPY has been propagated along the edge
+bool PcodeOp::hasCopyImmed(int4 slot) const
+
+{
+  if ((addlflags & immed_copy) != 0) {
+    FlowBlock *inbl = parent->getIn(slot);
+    int4 outedge = parent->getInRevIndex(slot);
+    return inbl->hasImmedCopyEdge(outedge);
+  }
+  return false;
+}
+
 /// Produce a hash of the following attributes: output size, the opcode, and the identity
 /// of each input varnode.  This is suitable for determining if two PcodeOps calculate identical values
 /// \return the calculated hash or 0 if the op is not cse hashable

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

@@ -116,7 +116,8 @@ public:
     hold_output = 0x80,		///< Output varnode (of call) should not be removed if it is unread
     concat_root = 0x100,	///< Output of \b this is root of a CONCAT tree
     no_indirect_collapse = 0x200,	///< Do not collapse \b this INDIRECT (via RuleIndirectCollapse)
-    store_unmapped = 0x400	///< If STORE collapses to a stack Varnode, force it to be unmapped
+    store_unmapped = 0x400,	///< If STORE collapses to a stack Varnode, force it to be unmapped
+    immed_copy = 0x800		///< Copy has propagated into input of \b this op
   };
 private:
   TypeOp *opcode;		///< Pointer to class providing behavioral details of the operation
@@ -224,6 +225,8 @@ public:
   void setNoIndirectCollapse(void) { addlflags |= no_indirect_collapse; }	///< Prevent collapse of INDIRECT
   bool isStoreUnmapped(void) const { return ((addlflags & store_unmapped)!=0); }	///< Is STORE location supposed to be unmapped
   void setStoreUnmapped(void) const { addlflags |= store_unmapped; }	///< Mark that STORE location should be unmapped
+  void setCopyImmed(int4 slot);		///< Mark that a COPY propagation from the immediate input block has happened
+  bool hasCopyImmed(int4 slot) const;	///< Return \b true if a COPY propagation from an immediate input block has happened
   /// \brief Return \b true if this LOADs or STOREs from a dynamic \e spacebase pointer
   bool usesSpacebasePtr(void) const { return ((flags&PcodeOp::spacebase_ptr)!=0); }
   uintm getCseHash(void) const;	///< Return hash indicating possibility of common subexpression elimination

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

@@ -3969,6 +3969,9 @@ int4 RulePropagateCopy::applyOp(PcodeOp *op,Funcdata &data)
       if (invn->isAddrTied() && op->getOut()->isAddrTied() && 
 	  (op->getOut()->getAddr() != invn->getAddr()))
 	continue;		// We must not allow merging of different addrtieds
+      if (op->code() == CPUI_MULTIEQUAL && copyop->getParent() == op->getParent()->getIn(i)) {
+	op->setCopyImmed(i);
+      }
     }
     data.opSetInput(op,invn,i); // otherwise propagate just a single copy
     return 1;
@@ -5496,6 +5499,8 @@ int4 RuleCondNegate::applyOp(PcodeOp *op,Funcdata &data)
   Varnode *vn,*outvn;
 
   if (!op->isBooleanFlip()) return 0;
+  if (data.opNormalizeFlip(op))
+    return 1;
 
   vn = op->getIn(1);
   newop = data.newOp(1,op->getAddr());

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

@@ -531,6 +531,19 @@ void HighVariable::remove(Varnode *vn)
   }
 }
 
+/// \b this is assigned directly to the Varnode, losing any reference to a previous HighVariable,
+/// so the caller must take this into account.
+/// \param newvn is the Varnode to add to \b this
+/// \param mergeGroup is the group to associate with this merge
+void HighVariable::insert(Varnode *newvn,int2 mergeGroup)
+
+{
+  vector<Varnode *>::iterator iter;
+  iter = lower_bound(inst.begin(),inst.end(),newvn,compareJustLoc);
+  inst.insert(iter,newvn);
+  newvn->setHigh(this,mergeGroup);
+}
+
 /// Assuming there is a Symbol attached to \b this, run through the Varnode members
 /// until we find one with a SymbolEntry corresponding to the Symbol and return it.
 /// \return the SymbolEntry that mapped the Symbol to \b this or null if no Symbol is attached

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

@@ -156,6 +156,7 @@ private:
   bool hasCopyIn1(void) const { return ((highflags&copy_in1)!=0); }	///< Is there at least one COPY into \b this
   bool hasCopyIn2(void) const { return ((highflags&copy_in2)!=0); }	///< Is there at least two COPYs into \b this
   void remove(Varnode *vn);				///< Remove a member Varnode from \b this
+  void insert(Varnode *newvn,int2 mergeGroup);		///< Directly insert a Varnode into \b this
   void mergeInternal(HighVariable *tv2,bool isspeculative);	///< Merge another HighVariable into \b this
   void merge(HighVariable *tv2,HighIntersectTest *testCache,bool isspeculative);	///< Merge with another HighVariable taking into account groups
   void setSymbol(Varnode *vn) const;		///< Update Symbol information for \b this from the given member Varnode

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

@@ -347,6 +347,25 @@ void Varnode::destroyDescend(void)
   descend.clear();
 }
 
+/// We assume the replacement Varnode is initially a singleton in its own HighVariable.
+/// We swap \b this and the replacement between their respective HighVariables.
+/// \param replacevn is the replacement Varnode
+void Varnode::replaceInHigh(Varnode *replacevn)
+
+{
+#ifdef CPUI_DEBUG
+  if (replacevn->high->inst.size() != 1)
+    throw LowlevelError("Attempting to replace with merged Varnode");
+#endif
+  high->remove(this);
+  HighVariable *replaceHigh = replacevn->high;
+  replacevn->high = (HighVariable *)0;
+  replaceHigh->inst[0] = this;
+  high->insert(replacevn,mergegroup);
+  high = replaceHigh;
+  mergegroup = 0;
+}
+
 /// Set desired boolean attributes on this Varnode and then set dirty bits if appropriate
 /// \param fl is the mask containing the list of attributes to set
 void Varnode::setFlags(uint4 fl) const

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

@@ -174,6 +174,7 @@ private:
   void addDescend(PcodeOp *op);	///< Add a descendant (reading) PcodeOp to this Varnode's list
   void eraseDescend(PcodeOp *op); ///< Erase a descendant (reading) PcodeOp from this Varnode's list
   void destroyDescend(void);	///< Clear all descendant (reading) PcodeOps
+  void replaceInHigh(Varnode *replacevn);	///< Swap the given Varnode into the HighVariable for \b this
 public:
   // only to be used by HighVariable
   void setHigh(HighVariable *tv,int2 mg) { high = tv; mergegroup = mg; } ///< Set the HighVariable owning this Varnode

Ghidra/Features/Decompiler/src/decompile/datatests/copytrim.xml

@@ -0,0 +1,44 @@
+<decompilertest>
+<binaryimage arch="x86:LE:64:default:gcc">
+<bytechunk space="ram" offset="0x100000" readonly="true">
+554889e5897dec8b45ec48988b048500
+1010008945fc837dec007430eb1f8b45
+ec83e80148988b1485001010008b45ec
+489889148500101000836dec01837dec
+0075db8b45fc8905b40f0000905dc355
+4889e5897dfc8975f8837dfc01750b8b
+45f88905a00f0000eb23837dfc02750e
+8b45f883c00789058c0f0000eb0f837d
+fc0475098b45f889057b0f0000905dc3
+</bytechunk>
+<symbol space="ram" offset="0x100000" name="myloop"/>
+<symbol space="ram" offset="0x10004f" name="myglob"/>
+</binaryimage>
+<script>
+  <com>map addr r0x101000 int4 myarr[2]</com>
+  <com>map addr r0x101008 int4 globvar</com>
+  <com>lo fu myloop</com>
+  <com>map addr s0xffffffffffffffe4 int4 i</com>
+  <com>map hash r0x10000c 5fc6319fee int4 size</com>
+  <com>decompile</com>
+  <com>print C</com>
+  <com>lo fu myglob</com>
+  <com>decompile</com>
+  <com>print C</com>
+  <com>quit</com>
+</script>
+<stringmatch name="Copy trim #1" min="1" max="1">size = myarr\[param_1\];</stringmatch>
+<stringmatch name="Copy trim #2" min="0" max="0">for \(.*myarr\[0\] = </stringmatch>
+<stringmatch name="Copy trim #3" min="1" max="1">for \(; i != 0; i = i \+ -1\) \{
+                                                   myarr\[i\] = myarr\[i \+ -1\];
+                                                 \}
+                                                 myarr\[0\] = size;</stringmatch>
+<stringmatch name="Copy trim #4" min="0" max="0">Var1 = param_2;</stringmatch>
+<stringmatch name="Copy trim #5" min="0" max="0">globvar = iVar1;</stringmatch>
+<stringmatch name="Copy trim #6" min="1" max="1">if \(param_1 == 1\) \{
+                                                   globvar = param_2;</stringmatch>
+<stringmatch name="Copy trim #7" min="1" max="1">if \(param_1 == 2\) \{
+                                                   globvar = param_2 \+ 7;</stringmatch>
+<stringmatch name="Copy trim #8" min="1" max="1">if \(param_1 == 4\) \{
+                                                   globvar = param_2;</stringmatch>
+</decompilertest>

Ghidra/Features/Decompiler/src/decompile/datatests/partialunion.xml

@@ -33,15 +33,14 @@
   <com>dec</com>
   <com>print C</com>
   <com>lo fu partial1</com>
-  <com>map unionfacet structunion 1 r0x1006ee 10603f2c20ffa6</com>
+  <com>map unionfacet structunion 1 r0x1006ee 10603fc3e29498</com>
   <com>dec</com>
   <com>print C</com>
   <com>quit</com>
 </script>
 <stringmatch name="Partial union #1" min="1" max="1">globvar\.b\.bval1 = val;</stringmatch>
 <stringmatch name="Partial union #2" min="1" max="1">return globvar\.a\.aval2;</stringmatch>
-<stringmatch name="Partial union #3" min="1" max="1">Var1 = globvar\.a\.aval2;</stringmatch>
+<stringmatch name="Partial union #3" min="2" max="2">globvar\.a\.aval2 = param_2;</stringmatch>
 <stringmatch name="Partial union #4" min="1" max="1">globvar\.a\.aval1 = param_2 \+ 7;</stringmatch>
 <stringmatch name="Partial union #5" min="1" max="1">globvar\.b\.bval1 = 0\.5;</stringmatch>
-<stringmatch name="Partial union #6" min="1" max="1">globvar\.a\.aval2 = .Var1;</stringmatch>
 </decompilertest>