diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/block.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/block.cc index 3ed18a11a5..42afbbfa24 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/block.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/block.cc @@ -354,6 +354,21 @@ int4 FlowBlock::calcDepth(const FlowBlock *leaf) const return depth; } +/// Return \b true if \b this block \e dominates the given block (or is equal to it). +/// This assumes that block indices have been set with a reverse post order so that having a +/// smaller index is a necessary condition for dominance. +/// \param subBlock is the given block to test against \b this for dominance +/// \return \b true if \b this dominates +bool FlowBlock::dominates(const FlowBlock *subBlock) const + +{ + while(subBlock != (const FlowBlock *)0 && index <= subBlock->index) { + if (subBlock == this) return true; + subBlock = subBlock->getImmedDom(); + } + return false; +} + /// \return \b true if \b this is the top of a loop bool FlowBlock::hasLoopIn(void) const diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/block.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/block.hh index 01fa04b8d6..6c56eedee6 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/block.hh +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/block.hh @@ -214,6 +214,7 @@ public: const FlowBlock *getFrontLeaf(void) const; ///< Get the first leaf FlowBlock FlowBlock *getFrontLeaf(void); ///< Get the first leaf FlowBlock int4 calcDepth(const FlowBlock *leaf) const; ///< Get the depth of the given component FlowBlock + bool dominates(const FlowBlock *subBlock) const; ///< Does \b this block dominate the given block int4 sizeOut(void) const { return outofthis.size(); } ///< Get the number of out edges int4 sizeIn(void) const { return intothis.size(); } ///< Get the number of in edges bool hasLoopIn(void) const; ///< Is there a looping edge coming into \b this block diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.cc index 14b4c81e88..aab1dec579 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.cc @@ -3589,14 +3589,24 @@ int4 ActionCopyMarker::apply(Funcdata &data) op = *iter; switch(op->code()) { case CPUI_COPY: - if (op->getOut()->getHigh() == op->getIn(0)->getHigh()) { - data.opSetFlag(op,PcodeOp::nonprinting); + v1 = op->getOut(); + h1 = v1->getHigh(); + if (h1 == op->getIn(0)->getHigh()) { + data.opSetFlag(op, PcodeOp::nonprinting); count += 1; } - else if (op->getOut()->hasNoDescend()) { // Don't print shadow assignments - if (shadowedVarnode(op->getOut())) { - data.opSetFlag(op,PcodeOp::nonprinting); - count += 1; + else { // COPY between different HighVariables + if (h1->hasCopyIn()) { // If we've seen other COPYs into this high + if (!h1->isCopyProcessed()) // and we haven't searched before, + data.getMerge().markRedundantCopies(h1); // search for redundant COPYs + } + else + h1->setCopyIn(); + if (v1->hasNoDescend()) { // Don't print shadow assignments + if (shadowedVarnode(v1)) { + data.opSetFlag(op, PcodeOp::nonprinting); + count += 1; + } } } break; diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/merge.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/merge.cc index 5165170e93..2507e77eea 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/merge.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/merge.cc @@ -857,6 +857,27 @@ void Merge::findSingleCopy(HighVariable *high,vector &singlelist) } } +/// \brief Compare COPY ops first by Varnode input, then by block containing the op +/// +/// A sort designed to group COPY ops from the same Varnode together. Then within a group, +/// COPYs are sorted by their containing basic block (so that dominating ops come first). +/// \param op1 is the first PcodeOp being compared +/// \param op2 is the second PcodeOp being compared +/// \return \b true if the first PcodeOp should be ordered before the second +bool Merge::compareCopyByInVarnode(PcodeOp *op1,PcodeOp *op2) + +{ + Varnode *inVn1 = op1->getIn(0); + Varnode *inVn2 = op2->getIn(0); + if (inVn1 != inVn2) // First compare by Varnode inputs + return (inVn1->getCreateIndex() < inVn2->getCreateIndex()); + int4 index1 = op1->getParent()->getIndex(); + int4 index2 = op2->getParent()->getIndex(); + if (index1 != index2) + return (index1 < index2); + return (op1->getSeqNum().getOrder() < op2->getSeqNum().getOrder()); +} + /// \brief Hide \e shadow Varnodes related to the given HighVariable by consolidating COPY chains /// /// If two Varnodes are copied from the same common ancestor then they will always contain the @@ -900,6 +921,108 @@ bool Merge::hideShadows(HighVariable *high) return res; } +/// \brief Check if the given PcodeOp COPYs are redundant +/// +/// Both the given COPYs assign to the same HighVariable. One is redundant if there is no other +/// assignment to the HighVariable between the first COPY and the second COPY. +/// The first COPY must come from a block with a smaller or equal index to the second COPY. +/// If the indices are equal, the first COPY must come before the second within the block. +/// \param high is the HighVariable being assigned to +/// \param domOp is the first COPY +/// \param subOp is the second COPY +/// \return \b true if the second COPY is redundant +bool Merge::checkCopyPair(HighVariable *high,PcodeOp *domOp,PcodeOp *subOp) + +{ + FlowBlock *domBlock = domOp->getParent(); + FlowBlock *subBlock = subOp->getParent(); + if (!domBlock->dominates(subBlock)) + return false; + int4 domId = domBlock->getIndex(); + int4 subId = subBlock->getIndex(); + for(int4 i=0;inumInstances();++i) { + Varnode *vn = high->getInstance(i); + if (!vn->isWritten()) continue; + PcodeOp *op = vn->getDef(); + if (op == domOp) continue; + if (op == subOp) continue; + int4 index = op->getParent()->getIndex(); + if (index == domId) { // Assignment same block as domOp + if (op->getSeqNum().getOrder() > domOp->getSeqNum().getOrder()) + return false; + } + if (index == subId) { // Assignment same block as subOp + if (op->getSeqNum().getOrder() < subOp->getSeqNum().getOrder()) + return false; + } + } + // All cover blocks in between domOp and subOp must be empty + if (subBlock != domBlock) { + high->updateCover(); + subBlock = subBlock->getImmedDom(); // Don't need to check first block + while(subBlock != domBlock) { // Don't need to check last block + if (!high->wholecover.getCoverBlock(subBlock->getIndex()).empty()) + return false; + subBlock = subBlock->getImmedDom(); + } + } + return true; +} + +/// \brief Search for and mark redundant COPY ops into the given high as \e non-printing +/// +/// Trimming during the merge process can insert multiple COPYs from the same source. In some +/// cases, one or more COPYs may be redundant and shouldn't be printed. This method searches +/// for redundancy among COPY ops that assign to the given HighVariable. +/// \param high is the given HighVariable +/// \return \b true if any redundant COPYs were discovered +bool Merge::markRedundantCopies(HighVariable *high) + +{ + bool hasRedundant = false; + vector copyIns; + + high->setCopyProcessed(); + // Find all the COPY ops into this HighVariable from a different HighVariable + for(int4 i=0;inumInstances();++i) { + Varnode *vn = high->getInstance(i); + if (!vn->isWritten()) continue; + PcodeOp *op = vn->getDef(); + if (op->code() != CPUI_COPY) continue; + if (op->getIn(0)->getHigh() != high) + copyIns.push_back(op); + } + + // Group COPYs based on the incoming Varnode + sort(copyIns.begin(),copyIns.end(),compareCopyByInVarnode); + int4 pos = 0; + while(pos < copyIns.size()) { + // Find a group of COPYs coming from the same Varnode + Varnode *inVn = copyIns[pos]->getIn(0); + int4 sz = 1; + while(pos + sz < copyIns.size()) { + Varnode *nextVn = copyIns[pos+sz]->getIn(0); + if (nextVn != inVn) break; + sz += 1; + } + if (sz > 1) { // If there is more than one COPY in a group + for(int4 i=sz-1;i>0;--i) { + PcodeOp *subOp = copyIns[pos+i]; + for(int4 j=i-1;j>=0;--j) { + // Make sure earlier index provides dominant op + if (checkCopyPair(high,copyIns[pos+j],subOp)) { + data.opSetFlag(subOp, PcodeOp::nonprinting); + hasRedundant = true; + break; + } + } + } + } + pos += sz; + } + return hasRedundant; +} + /// \brief Perform low-level details of merging two HighVariables if possible /// /// This routine only fails (returning \b false) if there is a Cover intersection between diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/merge.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/merge.hh index f19444534e..f3f5eb3e36 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/merge.hh +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/merge.hh @@ -89,6 +89,7 @@ class Merge { static bool mergeTestBasic(Varnode *vn); static void findSingleCopy(HighVariable *high,vector &singlelist); static bool compareHighByBlock(const HighVariable *a,const HighVariable *b); + static bool compareCopyByInVarnode(PcodeOp *op1,PcodeOp *op2); void collectCovering(vector &vlist,HighVariable *high,PcodeOp *op); bool collectCorrectable(const vector &vlist,list &oplist,vector &slotlist, PcodeOp *op); @@ -103,6 +104,7 @@ class Merge { void mergeIndirect(PcodeOp *indop); void mergeLinear(vector &highvec); bool merge(HighVariable *high1,HighVariable *high2,bool isspeculative); + bool checkCopyPair(HighVariable *high,PcodeOp *domOp,PcodeOp *subOp); public: Merge(Funcdata &fd) : data(fd) {} ///< Construct given a specific function bool intersection(HighVariable *a,HighVariable *b); @@ -116,6 +118,7 @@ public: void mergeMarker(void); void mergeAdjacent(void); bool hideShadows(HighVariable *high); + bool markRedundantCopies(HighVariable *high); }; /// \brief Compare HighVariables by the blocks they cover diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/variable.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/variable.hh index 0c5503674d..40bb6ed8f3 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/variable.hh +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/variable.hh @@ -45,7 +45,9 @@ public: enum { flagsdirty = 1, ///< Boolean properties for the HighVariable are dirty typedirty = 2, ///< The data-type for the HighVariable is dirty - coverdirty = 4 ///< The cover for the HighVariable is dirty + coverdirty = 4, ///< The cover for the HighVariable is dirty + copy_in = 8, ///< There exist COPY ops into \b this HighVariable from other HighVariables + copy_processed = 16 ///< COPY ops into \b this HighVariable have been analyzed }; private: friend class Merge; @@ -61,6 +63,7 @@ private: void updateFlags(void) const; ///< (Re)derive boolean properties of \b this from the member Varnodes void updateCover(void) const; ///< (Re)derive the cover of \b this from the member Varnodes void updateType(void) const; ///< (Re)derive the data-type for \b this from the member Varnodes + void setCopyProcessed(void) const { highflags |= copy_processed; } ///< Mark that \b this has had its COPY ins processed public: HighVariable(Varnode *vn); ///< Construct a HighVariable with a single member Varnode Datatype *getType(void) const { updateType(); return type; } ///< Get the data-type @@ -108,6 +111,9 @@ public: void setMark(void) const { flags |= Varnode::mark; } ///< Set the mark on this variable void clearMark(void) const { flags &= ~Varnode::mark; } ///< Clear the mark on this variable bool isMark(void) const { return ((flags&Varnode::mark)!=0); } ///< Return \b true if \b this is marked + void setCopyIn(void) const { highflags |= copy_in; } ///< Mark the existence of COPY ops into \b this + bool hasCopyIn(void) const { return ((highflags©_in)!=0); } ///< Are there COPY ops into \b this + bool isCopyProcessed(void) const { return ((highflags©_processed)!=0); } ///< Have COPY ops into \b this been processed /// \brief Determine if \b this HighVariable has an associated cover. ///