GP-5927 Add support for SUBPIECE removal in ConditionalExecution

Ghidra/Features/Decompiler/certification.manifest

@@ -17,6 +17,7 @@ src/decompile/datatests/ccmp.xml||GHIDRA||||END|
 src/decompile/datatests/concat.xml||GHIDRA||||END|
 src/decompile/datatests/concatsplit.xml||GHIDRA||||END|
 src/decompile/datatests/condconst.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/deadvolatile.xml||GHIDRA||||END|

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

@@ -106,14 +106,6 @@ bool ConditionalExecution::verifySameCondition(void)
 
   if (tester.getFlip())
     init2a_true = !init2a_true;
-  int4 multislot = tester.getMultiSlot();
-  if (multislot != -1) {
-    // This is a direct split
-    directsplit = true;
-    posta_outslot = (multislot == prea_inslot) ? 0 : 1;
-    if (init2a_true)
-      posta_outslot = 1 - posta_outslot;
-  }
   return true;
 }
 
@@ -126,15 +118,12 @@ bool ConditionalExecution::testMultiRead(Varnode *vn,PcodeOp *op)
 
 {
   if (op->getParent() == iblock) {
-    if (!directsplit) {
+    if (op->code() == CPUI_COPY || op->code() == CPUI_SUBPIECE) // The copy-like tested separately
-      if (op->code() == CPUI_COPY) // The COPY is tested separately
       return true;		// If the COPY's output reads can be altered, then -vn- can be altered
     return false;
   }
-  }
   if (op->code() == CPUI_RETURN) {
     if ((op->numInput() < 2)||(op->getIn(1) != vn)) return false; // Only test for flow thru to return value
-    returnop.push_back(op);	// mark that CPUI_RETURN needs special handling
   }
   return true;
 }
@@ -148,78 +137,65 @@ bool ConditionalExecution::testOpRead(Varnode *vn,PcodeOp *op)
 
 {
   if (op->getParent() == iblock) return true;
-  if ((op->code() == CPUI_RETURN)&&(!directsplit)) {
+  PcodeOp *writeOp = vn->getDef();
-    if ((op->numInput() < 2)||(op->getIn(1) != vn)) return false; // Only test for flow thru to return value
+  if (writeOp->code() == CPUI_COPY || writeOp->code() == CPUI_SUBPIECE) {
-    PcodeOp *copyop = vn->getDef();
+    Varnode *invn = writeOp->getIn(0);
-    if (copyop->code() == CPUI_COPY) {
+    if (invn->isWritten()) {
-      // Ordinarily, if -vn- is produced by a COPY we want to return false here because the propagation
-      // hasn't had time to happen here.  But if the flow is into a RETURN this can't propagate, so
-      // we allow this as a read that can be altered.  (We have to move the COPY)
-      Varnode *invn = copyop->getIn(0);
-      if (!invn->isWritten()) return false;
       PcodeOp *upop = invn->getDef();
       if ((upop->getParent() == iblock)&&(upop->code() != CPUI_MULTIEQUAL))
 	return false;
-      returnop.push_back(op);
-      return true;
     }
+    else if (invn->isFree())
+      return false;
+    return true;
   }
   return false;
 }
 
-/// \brief Prebuild a replacement MULTIEQUAL for output Varnode of the given PcodeOp in \b posta_block
+/// \param inbranch is the iblock incoming branch to pullback through
-///
+/// \return the output of the previous pullback op, or null
-/// The new op will hold the same data-flow as the original Varnode once a new
+Varnode *ConditionalExecution::findPullback(int4 inbranch)
-/// edge into \b posta_block is created.
-/// \param op is the given PcodeOp
-void ConditionalExecution::predefineDirectMulti(PcodeOp *op)
 
 {
-  PcodeOp *newop = fd->newOp(posta_block->sizeIn()+1,posta_block->getStart());
+  while(pullback.size() <= inbranch)
-  Varnode *outvn = op->getOut();
+    pullback.push_back((Varnode *)0);
-  Varnode *newoutvn;
+  return pullback[inbranch];
-  newoutvn = fd->newVarnodeOut(outvn->getSize(),outvn->getAddr(),newop);
+}
-  fd->opSetOpcode(newop,CPUI_MULTIEQUAL);
+
-  Varnode *vn;
+/// Create a duplicate SUBPIECE outside the iblock.  If the SUBPIECE input is defined by a MULTIEQUAL in the iblock,
-  int4 inslot = iblock->getOutRevIndex(posta_outslot);
+/// the duplicate's input will be selected from the MULTIEQUAL input.
-  for(int4 i=0;i<posta_block->sizeIn();++i) {
+/// \param subOp is the SUBPIECE in the iblock being replaced
-    if (i==inslot)
+/// \param inbranch is the direction to pullback from
-      vn = op->getIn(1-camethruposta_slot);
+/// \return the output Varnode of the new SUBPIECE
+Varnode *ConditionalExecution::pullbackSubpiece(PcodeOp *subOp,int4 inbranch)
+
+{
+  Varnode *invn = findPullback(inbranch);	// Look for pullback constructed for a previous read
+  if (invn != (Varnode *)0)
+    return invn;
+  invn = subOp->getIn(0);
+  BlockBasic *bl;
+  if (invn->isWritten()) {
+    PcodeOp *defOp = invn->getDef();
+    if (defOp->getParent() == iblock) {
+      bl = (BlockBasic *)iblock->getIn(inbranch);
+      invn = defOp->getIn(inbranch);		// defOp must by MULTIEQUAL
+    }
     else
-      vn = newoutvn;
+      bl = defOp->getParent();
-    fd->opSetInput(newop,vn,i);
   }
-  fd->opSetInput(newop,op->getIn(camethruposta_slot),posta_block->sizeIn());
+  else {
-  fd->opInsertBegin(newop,posta_block);
+    bl = (BlockBasic *)iblock->getImmedDom();
-
-  // Cache this new data flow holder
-  replacement[posta_block->getIndex()] = newoutvn;
-}
-
-/// In the \e direct \e split case, MULTIEQUALs in the body block (\b posta_block)
-/// must update their flow to account for \b iblock being removed and a new
-/// block flowing into the body block.
-void ConditionalExecution::adjustDirectMulti(void)
-
-{
-  list<PcodeOp *>::const_iterator iter;
-  PcodeOp *op;
-  iter = posta_block->beginOp();
-  int4 inslot = iblock->getOutRevIndex(posta_outslot);
-  while(iter != posta_block->endOp()) {
-    op = *iter++;
-    if (op->code() != CPUI_MULTIEQUAL) continue;
-    Varnode *vn = op->getIn(inslot);
-    if (vn->isWritten()&&(vn->getDef()->getParent() == iblock)) {
-      if (vn->getDef()->code() != CPUI_MULTIEQUAL)
-	throw LowlevelError("Cannot push non-trivial operation");
-      // Flow that stays in iblock, comes from modified side
-      fd->opSetInput(op,vn->getDef()->getIn(1-camethruposta_slot),inslot);
-      // Flow from unmodified side, forms new branch
-      vn = vn->getDef()->getIn(camethruposta_slot);
-    }
-    fd->opInsertInput(op,vn,op->numInput());
   }
+  PcodeOp *newOp = fd->newOp(2,subOp->getAddr());
+  Varnode *origOutVn = subOp->getOut();
+  Varnode *outVn = fd->newVarnodeOut(origOutVn->getSize(),origOutVn->getAddr(),newOp);
+  fd->opSetOpcode(newOp,CPUI_SUBPIECE);
+  fd->opSetInput(newOp,invn,0);
+  fd->opSetInput(newOp,subOp->getIn(1),1);
+  fd->opInsertEnd(newOp, bl);
+  pullback[inbranch] = outVn;		// Cache pullback in case there are other reads
+  return outVn;
 }
 
 /// \brief Create a MULTIEQUAL in the given block that will hold data-flow from the given PcodeOp
@@ -249,6 +225,67 @@ Varnode *ConditionalExecution::getNewMulti(PcodeOp *op,BlockBasic *bl)
   return newoutvn;
 }
 
+/// Given an op in the \b iblock and the basic block of another op that reads the output Varnode,
+/// calculate the replacement Varnode for the read.
+/// \param op is the given op in the \b iblock
+/// \param bl is the basic block of the read
+/// \return the replacement Varnode
+Varnode *ConditionalExecution::resolveRead(PcodeOp *op,BlockBasic *bl)
+
+{
+  Varnode *res;
+  if (bl->sizeIn()==1) {
+    // Since dominator is iblock, In(0) must be iblock
+    // Figure what side of -iblock- we came through
+    int4 slot = (bl->getInRevIndex(0) == posta_outslot) ? camethruposta_slot : 1-camethruposta_slot;
+    res = resolveIblockRead(op,slot);
+  }
+  else
+    res = getNewMulti(op,bl);
+  return res;
+}
+
+/// \param op is the \b iblock op whose output is being read
+/// \param inbranch is the known direction of the reading op
+/// \return the replacement Varnode to use for the read
+Varnode *ConditionalExecution::resolveIblockRead(PcodeOp *op,int4 inbranch)
+
+{
+  if (op->code() == CPUI_COPY) {
+    Varnode *vn = op->getIn(0);
+    if (vn->isWritten()) {
+      PcodeOp *defOp = vn->getDef();
+      if (defOp->code() == CPUI_MULTIEQUAL && defOp->getParent() == iblock)
+	op = defOp;
+    }
+    else
+      return vn;
+  }
+  if (op->code() == CPUI_MULTIEQUAL)
+   return op->getIn(inbranch);
+  else if (op->code() == CPUI_SUBPIECE) {
+    return pullbackSubpiece(op, inbranch);
+  }
+  throw LowlevelError("Conditional execution: Illegal op in iblock");
+}
+
+/// \brief Get the replacement Varnode for the output of a MULTIEQUAL in the \b iblock, given the op reading it
+///
+/// \param op is the MULTIEQUAL from \b iblock
+/// \param readop is the PcodeOp reading the output Varnode
+/// \param slot is the input slot being read
+/// \return the Varnode to use as a replacement
+Varnode *ConditionalExecution::getMultiequalRead(PcodeOp *op,PcodeOp *readop,int4 slot)
+
+{
+  BlockBasic *bl = readop->getParent();
+  BlockBasic *inbl = (BlockBasic *)bl->getIn(slot);
+  if (inbl != iblock)
+    return getReplacementRead(op, inbl);
+  int4 s = (bl->getInRevIndex(slot) == posta_outslot) ? camethruposta_slot : 1-camethruposta_slot;
+  return resolveIblockRead(op,s);
+}
+
 /// \brief Find a replacement Varnode for the output of the given PcodeOp that is read in the given block
 ///
 /// The replacement Varnode must be valid for everything below (dominated) by the block.
@@ -278,15 +315,7 @@ Varnode *ConditionalExecution::getReplacementRead(PcodeOp *op,BlockBasic *bl)
     replacement[bl->getIndex()] = (*iter).second;
     return (*iter).second;
   }
-  Varnode *res;
+  Varnode *res = resolveRead(op,curbl);
-  if (curbl->sizeIn()==1) {
-    // Since dominator is iblock, In(0) must be iblock
-    // Figure what side of -iblock- we came through
-    int4 slot = (curbl->getInRevIndex(0) == posta_outslot) ? camethruposta_slot : 1-camethruposta_slot;
-    res = op->getIn(slot);
-  }
-  else
-    res = getNewMulti(op,curbl);
   replacement[curbl->getIndex()] = res;
   if (curbl != bl)
     replacement[bl->getIndex()] = res;
@@ -299,14 +328,8 @@ Varnode *ConditionalExecution::getReplacementRead(PcodeOp *op,BlockBasic *bl)
 void ConditionalExecution::doReplacement(PcodeOp *op)
 
 {
-  if (op->code() == CPUI_COPY) {
-    if (op->getOut()->hasNoDescend()) // Verify that this has been dealt with by fixReturnOp
-      return;
-    // It could be a COPY internal to iblock, we need to remove it like any other op
-  }
   replacement.clear();
-  if (directsplit)
+  pullback.clear();
-    predefineDirectMulti(op);
   Varnode *vn = op->getOut();
   list<PcodeOp *>::const_iterator iter = vn->beginDescend();
   while(iter != vn->endDescend()) {
@@ -315,20 +338,22 @@ void ConditionalExecution::doReplacement(PcodeOp *op)
     BlockBasic *bl = readop->getParent();
     Varnode *rvn;
     if (bl == iblock) {
-      if (directsplit)
-	fd->opSetInput(readop,op->getIn(1-camethruposta_slot),slot);	// We know op is MULTIEQUAL
-      else
       fd->opUnsetInput(readop,slot);
     }
     else {
       if (readop->code() == CPUI_MULTIEQUAL) {
-	BlockBasic *inbl = (BlockBasic *)bl->getIn(slot);
+	rvn = getMultiequalRead(op, readop, slot);
-	if (inbl == iblock) {
-	  int4 s = (bl->getInRevIndex(slot) == posta_outslot) ? camethruposta_slot : 1-camethruposta_slot;
-	  rvn = op->getIn(s);
       }
-	else
+      else if (readop->code() == CPUI_RETURN) {		// Cannot replace input of RETURN directly, create COPY to hold input
-	  rvn = getReplacementRead(op,inbl);
+	Varnode *retvn = readop->getIn(1);
+	PcodeOp *newcopyop = fd->newOp(1,readop->getAddr());
+	fd->opSetOpcode(newcopyop,CPUI_COPY);
+	Varnode *outvn = fd->newVarnodeOut(retvn->getSize(),retvn->getAddr(),newcopyop); // Preserve the CPUI_RETURN storage address
+	fd->opSetInput(readop,outvn,1);
+	fd->opInsertBefore(newcopyop,readop);
+	readop = newcopyop;
+	slot = 0;
+	rvn = getReplacementRead(op,bl);
       }
       else
 	rvn = getReplacementRead(op,bl);
@@ -339,28 +364,6 @@ void ConditionalExecution::doReplacement(PcodeOp *op)
   }
 }
 
-/// \brief Reproduce COPY data-flow into RETURN ops affected by the removal of \b iblock
-void ConditionalExecution::fixReturnOp(void)
-
-{
-  for(int4 i=0;i<returnop.size();++i) {
-    PcodeOp *retop = returnop[i];
-    Varnode *retvn = retop->getIn(1);
-    PcodeOp *iblockop = retvn->getDef();
-    Varnode *invn;
-    if (iblockop->code() == CPUI_COPY)
-      invn = iblockop->getIn(0); // This must either be from MULTIEQUAL or something written outside of iblock
-    else
-      invn = retvn;
-    PcodeOp *newcopyop = fd->newOp(1,retop->getAddr());
-    fd->opSetOpcode(newcopyop,CPUI_COPY);
-    Varnode *outvn = fd->newVarnodeOut(retvn->getSize(),retvn->getAddr(),newcopyop); // Preserve the CPUI_RETURN storage address
-    fd->opSetInput(newcopyop,invn,0);
-    fd->opSetInput(retop,outvn,1);
-    fd->opInsertBefore(newcopyop,retop);
-  }
-}
-
 /// \param op is the PcodeOp within \b iblock to test
 /// \return \b true if it is removable
 bool ConditionalExecution::testRemovability(PcodeOp *op)
@@ -385,6 +388,7 @@ bool ConditionalExecution::testRemovability(PcodeOp *op)
     if (op->code()==CPUI_INDIRECT) return false;
 
     vn = op->getOut();
+    if (vn->isAddrTied()) return false;
     if (vn != (Varnode *)0) {
       bool hasnodescend = true;
       for(iter=vn->beginDescend();iter!=vn->endDescend();++iter) {
@@ -408,7 +412,6 @@ bool ConditionalExecution::verify(void)
 {
   prea_inslot = 0;
   posta_outslot = 0;
-  directsplit = false;
 
   if (!testIBlock()) return false;
   if (!findInitPre()) return false;
@@ -420,7 +423,6 @@ bool ConditionalExecution::verify(void)
   posta_block = (BlockBasic *)iblock->getOut(posta_outslot);
   postb_block = (BlockBasic *)iblock->getOut(1-posta_outslot);
 
-  returnop.clear();
   list<PcodeOp *>::const_iterator iter;
   iter = iblock->endOp();
   if (iter != iblock->beginOp())
@@ -456,51 +458,8 @@ bool ConditionalExecution::trial(BlockBasic *ib)
 {
   iblock = ib;
   if (!verify()) return false;
-
-  PcodeOp *cbranch_copy;
-  BlockBasic *initblock_copy;
-  BlockBasic *iblock_copy;
-  int4 prea_inslot_copy;
-  bool init2a_true_copy;
-  bool iblock2posta_true_copy;
-  int4 camethruposta_slot_copy;
-  int4 posta_outslot_copy;
-  BlockBasic *posta_block_copy;
-  BlockBasic *postb_block_copy;
-  bool directsplit_copy;
-
-  for(;;) {
-    if (!directsplit) return true;
-    // Save off the data for current iblock
-    cbranch_copy = cbranch;
-    initblock_copy = initblock;
-    iblock_copy = iblock;
-    prea_inslot_copy = prea_inslot;
-    init2a_true_copy = init2a_true;
-    iblock2posta_true_copy = iblock2posta_true;
-    camethruposta_slot_copy = camethruposta_slot;
-    posta_outslot_copy = posta_outslot;
-    posta_block_copy = posta_block;
-    postb_block_copy = postb_block;
-    directsplit_copy = directsplit;
-
-    iblock = posta_block;
-    if (!verify()) {
-      cbranch = cbranch_copy;
-      initblock = initblock_copy;
-      iblock = iblock_copy;
-      prea_inslot = prea_inslot_copy;
-      init2a_true = init2a_true_copy;
-      iblock2posta_true = iblock2posta_true_copy;
-      camethruposta_slot = camethruposta_slot_copy;
-      posta_outslot = posta_outslot_copy;
-      posta_block = posta_block_copy;
-      postb_block = postb_block_copy;
-      directsplit = directsplit_copy;
   return true;
 }
-  }
-}
 
 /// We assume the last call to verify() returned \b true
 void ConditionalExecution::execute(void)
@@ -508,32 +467,21 @@ void ConditionalExecution::execute(void)
 {
   list<PcodeOp *>::iterator iter;
   PcodeOp *op;
+  bool notdone;
 
-  fixReturnOp();		// Patch any data-flow thru to CPUI_RETURN
+  iter = iblock->endOp();		// Remove ops in reverse order
-  if (!directsplit) {
+  --iter;
-    iter = iblock->beginOp();
+  do {
-    while(iter != iblock->endOp()) {
+    op = *iter;
-      op = *iter++;
+    notdone = iter != iblock->beginOp();
+    if (notdone)
+      --iter;
     if (!op->isBranch())
       doReplacement(op);	// Remove all read refs of op
     fd->opDestroy(op);	// Then destroy op
-    }
+  } while(notdone);
   fd->removeFromFlowSplit(iblock,(posta_outslot != camethruposta_slot));
 }
-  else {
-    adjustDirectMulti();
-    iter = iblock->beginOp();
-    while(iter != iblock->endOp()) {
-      op = *iter++;
-      if (op->code() == CPUI_MULTIEQUAL) { // Only adjust MULTIEQUALs
-	doReplacement(op);
-	fd->opDestroy(op);
-      }
-      // Branch stays, other operations stay
-    }
-    fd->switchEdge(iblock->getIn(camethruposta_slot),iblock,posta_block);
-  }
-}
 
 int4 ActionConditionalExe::apply(Funcdata &data)
 

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

@@ -113,9 +113,8 @@ class ConditionalExecution {
   int4 posta_outslot;		///< The \b out edge from iblock to posta
   BlockBasic *posta_block;	///< First block in posta path
   BlockBasic *postb_block;	///< First block in postb path
-  bool directsplit;		///< True if this the \e direct \e split variation
   map<int4,Varnode *> replacement;	///< Map from block to replacement Varnode for (current) Varnode
-  vector<PcodeOp *> returnop;	///< RETURN ops that have flow coming out of the iblock
+  vector<Varnode *> pullback;	///< Outputs of ops that have been pulled back from \b iblock for (current) Varnode
   vector<bool> heritageyes;	///< Boolean array indexed by address space indicating whether the space is heritaged
 
   void buildHeritageArray(void);
@@ -123,14 +122,16 @@ class ConditionalExecution {
   bool findInitPre(void);			///< Find \b initblock, based on \b iblock
   bool verifySameCondition(void);		///< Verify that \b initblock and \b iblock branch on the same condition
   bool testOpRead(Varnode *vn,PcodeOp *op);	///< Can we move the (non MULTIEQUAL) defining p-code of the given Varnode
-  bool testMultiRead(Varnode *vn,PcodeOp *op);	///< Can we mave the MULTIEQUAL defining p-code of the given Varnode
+  bool testMultiRead(Varnode *vn,PcodeOp *op);	///< Can we move the MULTIEQUAL defining p-code of the given Varnode
   bool testRemovability(PcodeOp *op);		///< Test if the given PcodeOp can be removed from \b iblock
-  void predefineDirectMulti(PcodeOp *op);
+  Varnode *findPullback(int4 inbranch);		///< Find previously constructed pull-back op
-  void adjustDirectMulti(void);			///< Update inputs to any MULTIEQUAL in the direct block
+  Varnode *pullbackSubpiece(PcodeOp *subOp,int4 inbranch);	///< Pull-back SUBPIECE out of the iblock
   Varnode *getNewMulti(PcodeOp *op,BlockBasic *bl);
+  Varnode *resolveRead(PcodeOp *op,BlockBasic *bl);	///< Resolve a read op coming through an arbitrary block
+  Varnode *resolveIblockRead(PcodeOp *op,int4 inbranch);	///< Resolve a read op coming through the \b iblock
+  Varnode *getMultiequalRead(PcodeOp *op,PcodeOp *readop,int4 slot);
   Varnode *getReplacementRead(PcodeOp *op,BlockBasic *bl);
   void doReplacement(PcodeOp *op);		///< Replace the data-flow for the given PcodeOp in \b iblock
-  void fixReturnOp(void);
   bool verify(void);				///< Verify that we have a removable \b iblock
 public:
   ConditionalExecution(Funcdata *f);		///< Constructor

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

@@ -4292,11 +4292,13 @@ void ActionConditionalConst::handlePhiNodes(Varnode *varVn,Varnode *constVn,vect
 /// For each read op, check that is in or dominated by a specific block we known
 /// the Varnode is constant in.
 /// \param varVn is the given Varnode
-/// \param constVn is the constant Varnode to replace with
+/// \param constVn is the constant Varnode to replace with (may be null)
+/// \param constVal is the constant value being propagated
 /// \param constBlock is the block which dominates ops reading the constant value
 /// \param useMultiequal is \b true if conditional constants can be applied to MULTIEQUAL ops
 /// \param data is the function being analyzed
-void ActionConditionalConst::propagateConstant(Varnode *varVn,Varnode *constVn,FlowBlock *constBlock,bool useMultiequal,Funcdata &data)
+void ActionConditionalConst::propagateConstant(Varnode *varVn,Varnode *constVn,uintb constVal,
+					       FlowBlock *constBlock,bool useMultiequal,Funcdata &data)
 
 {
   vector<PcodeOpNode> phiNodeEdges;
@@ -4333,6 +4335,8 @@ void ActionConditionalConst::propagateConstant(Varnode *varVn,Varnode *constVn,F
 						// ...unless COPY is into something more interesting
     }
     if (constBlock->dominates(op->getParent())) {
+      if (constVn == (Varnode *)0)
+	constVn = data.newConstant(varVn->getSize(), constVal);
       if (opc == CPUI_RETURN){
           // CPUI_RETURN ops can't directly take constants
           // as inputs
@@ -4350,9 +4354,12 @@ void ActionConditionalConst::propagateConstant(Varnode *varVn,Varnode *constVn,F
       count += 1;			// We made a change
     }
   }
-  if (!phiNodeEdges.empty())
+  if (!phiNodeEdges.empty()) {
+    if (constVn == (Varnode *)0)
+      constVn = data.newConstant(varVn->getSize(), constVal);
     handlePhiNodes(varVn, constVn, phiNodeEdges, data);
   }
+}
 
 int4 ActionConditionalConst::apply(Funcdata &data)
 
@@ -4367,15 +4374,26 @@ int4 ActionConditionalConst::apply(Funcdata &data)
       useMultiequal = false;	// Don't propagate into MULTIEQUAL
   }
   const BlockGraph &blockGraph(data.getBasicBlocks());
+  bool blockdom[2];
   for(int4 i=0;i<blockGraph.getSize();++i) {
     FlowBlock *bl = blockGraph.getBlock(i);
     PcodeOp *cBranch = bl->lastOp();
     if (cBranch == (PcodeOp *)0 || cBranch->code() != CPUI_CBRANCH) continue;
     Varnode *boolVn = cBranch->getIn(1);
+    blockdom[0] = bl->getOut(0)->restrictedByConditional(bl);	// Make sure boolean constant holds down false branch
+    blockdom[1] = bl->getOut(1)->restrictedByConditional(bl);
+    if (!blockdom[0] && !blockdom[1]) continue;
+    bool flipEdge = cBranch->isBooleanFlip();
+    if (boolVn->loneDescend() == (PcodeOp *)0) {	// If the boolean is read more than once
+      // Search for implied constants, bool=0 down false branch, bool=1 down true branch
+      if (blockdom[0])
+	propagateConstant(boolVn, (Varnode *)0, flipEdge ? 1 : 0, bl->getFalseOut(), useMultiequal, data);
+      if (blockdom[1])
+	propagateConstant(boolVn, (Varnode *)0, flipEdge ? 0 : 1, bl->getTrueOut(), useMultiequal, data);
+    }
     if (!boolVn->isWritten()) continue;
     PcodeOp *compOp = boolVn->getDef();
     OpCode opc = compOp->code();
-    bool flipEdge = cBranch->isBooleanFlip();
     if (opc == CPUI_BOOL_NEGATE) {
       flipEdge = !flipEdge;
       boolVn = compOp->getIn(0);
@@ -4400,11 +4418,11 @@ int4 ActionConditionalConst::apply(Funcdata &data)
       constVn = varVn;
       varVn = tmp;
     }
+    if (varVn->loneDescend() != (PcodeOp *)0) continue;
     if (flipEdge)
       constEdge = 1 - constEdge;
-    FlowBlock *constBlock = bl->getOut(constEdge);
+    if (!blockdom[constEdge]) continue;	// Make sure condition holds
-    if (!constBlock->restrictedByConditional(bl)) continue;	// Make sure condition holds
+    propagateConstant(varVn,constVn,0,bl->getOut(constEdge),useMultiequal,data);
-    propagateConstant(varVn,constVn,constBlock,useMultiequal,data);
   }
   return 0;
 }

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

@@ -574,7 +574,7 @@ class ActionConditionalConst : public Action {
   static Varnode *placeCopy(PcodeOp *op,BlockBasic *bl,Varnode *constVn,Funcdata &data);
   static void placeMultipleConstants(vector<PcodeOpNode> &phiNodeEdges,vector<int4> &marks,Varnode *constVn,Funcdata &data);
   void handlePhiNodes(Varnode *varVn,Varnode *constVn,vector<PcodeOpNode> &phiNodeEdges,Funcdata &data);
-  void propagateConstant(Varnode *varVn,Varnode *constVn,FlowBlock *constBlock,bool useMultiequal,Funcdata &data);
+  void propagateConstant(Varnode *varVn,Varnode *constVn,uintb constVal,FlowBlock *constBlock,bool useMultiequal,Funcdata &data);
 public:
   ActionConditionalConst(const string &g) : Action(0,"condconst",g) {}	///< Constructor
   virtual Action *clone(const ActionGroupList &grouplist) const {

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

@@ -0,0 +1,31 @@
+<decompilertest>
+<binaryimage arch="ARM:LE:32:v8:default">
+<!--
+   Example where a SUBPIECE gets placed in an iblock that should still be removed.
+   The predicates should still all collapse to a single "if" body.
+-->
+<bytechunk space="ram" offset="0x402d29a8" readonly="true">
+                10402de9eaffffeb
+24309fe5003093e50120a0e31420c3e5
+000050e30030a0131730c0151630c015
+1530c0151040bde81eff2fe100003040
+</bytechunk>
+<symbol space="ram" offset="0x402d29a8" name="func"/>
+<symbol space="ram" offset="0x402d295c" name="other"/>
+</binaryimage>
+<script>
+  <com>option readonly on</com>
+  <com>parse line struct foo { char buf[0x14]; char w; char x; char y; char z; };</com>
+  <com>parse line extern foo *other(void);</com>
+  <com>map addr r0x40300000 foo *glob1</com>
+  <com>lo fu func</com>
+  <com>decompile</com>
+  <com>print C</com>
+</script>
+<stringmatch name="Conditional Subpiece #1" min="1" max="1">if</stringmatch>
+<stringmatch name="Conditional Subpiece #2" min="0" max="0">\(char\)</stringmatch>
+<stringmatch name="Conditional Subpiece #3" min="1" max="1">glob1-&gt;w = '\\x01';</stringmatch>
+<stringmatch name="Conditional Subpiece #4" min="1" max="1">pfVar1-&gt;x = '\\0';</stringmatch>
+<stringmatch name="Conditional Subpiece #5" min="1" max="1">pfVar1-&gt;y = '\\0';</stringmatch>
+<stringmatch name="Conditional Subpiece #6" min="1" max="1">pfVar1-&gt;z = '\\0';</stringmatch>
+</decompilertest>