diff --git a/Ghidra/Features/Decompiler/certification.manifest b/Ghidra/Features/Decompiler/certification.manifest index e28f4aa00e..3bba734922 100644 --- a/Ghidra/Features/Decompiler/certification.manifest +++ b/Ghidra/Features/Decompiler/certification.manifest @@ -28,6 +28,7 @@ src/decompile/datatests/forloop_loaditer.xml||GHIDRA||||END| src/decompile/datatests/forloop_thruspecial.xml||GHIDRA||||END| src/decompile/datatests/forloop_varused.xml||GHIDRA||||END| src/decompile/datatests/forloop_withskip.xml||GHIDRA||||END| +src/decompile/datatests/ifswitch.xml||GHIDRA||||END| src/decompile/datatests/impliedfield.xml||GHIDRA||||END| src/decompile/datatests/indproto.xml||GHIDRA||||END| src/decompile/datatests/injectoverride.xml||GHIDRA||||END| diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/block.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/block.cc index 9e5d86e703..2a6114188c 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/block.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/block.cc @@ -2613,30 +2613,47 @@ void BlockBasic::printRaw(ostream &s) const } } -/// \brief Check if there is meaningful activity between two branch instructions +/// \brief Check for values created in \b this block that flow outside the block. /// -/// The first branch is assumed to be a CBRANCH one edge of which flows into -/// the other branch. The flow can be through 1 or 2 blocks. If either block -/// performs an operation other than MULTIEQUAL, INDIRECT (or the branch), then -/// return \b false. -/// \param first is the CBRANCH operation -/// \param path is the index of the edge to follow to the other branch -/// \param last is the other branch operation -/// \return \b true if there is no meaningful activity -bool BlockBasic::noInterveningStatement(PcodeOp *first,int4 path,PcodeOp *last) +/// The block can calculate a value for a BRANCHIND or CBRANCH and can copy values and this method will still +/// return \b true. But calculating any value used outside the block, writing to an addressable location, +/// or performing a CALL or STORE causes the method to return \b false. +/// \return \b true if no value is created that can be used outside of the block +bool BlockBasic::noInterveningStatement(void) const { - BlockBasic *curbl = (BlockBasic *)first->getParent()->getOut(path); - for(int4 i=0;i<2;++i) { - if (!curbl->hasOnlyMarkers()) return false; - if (curbl != last->getParent()) { - if (curbl->sizeOut() != 1) return false; // Intervening conditional branch + list::const_iterator iter; + const PcodeOp *bop; + OpCode opc; + + for(iter=op.begin();iter!=op.end();++iter) { + bop = *iter; + if (bop->isMarker()) continue; + if (bop->isBranch()) continue; + if (bop->getEvalType() == PcodeOp::special) { + if (bop->isCall()) + return false; + opc = bop->code(); + if (opc == CPUI_STORE || opc == CPUI_NEW) + return false; + } + else { + opc = bop->code(); + if (opc == CPUI_COPY || opc == CPUI_SUBPIECE) + continue; + } + const Varnode *outvn = bop->getOut(); + if (outvn->isAddrTied()) + return false; + list::const_iterator iter = outvn->beginDescend(); + while(iter!=outvn->endDescend()) { + PcodeOp *op = *iter; + if (op->getParent() != this) + return false; + ++iter; } - else - return true; - curbl = (BlockBasic *)curbl->getOut(0); } - return false; + return true; } /// If there exists a CPUI_MULTIEQUAL PcodeOp in \b this basic block that takes the given exact list of Varnodes diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/block.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/block.hh index 2020df08f9..aade37d7e6 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/block.hh +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/block.hh @@ -489,7 +489,7 @@ public: list::const_iterator beginOp(void) const { return op.begin(); } ///< Return an iterator to the beginning of the PcodeOps list::const_iterator endOp(void) const { return op.end(); } ///< Return an iterator to the end of the PcodeOps bool emptyOp(void) const { return op.empty(); } ///< Return \b true if \b block contains no operations - static bool noInterveningStatement(PcodeOp *first,int4 path,PcodeOp *last); + bool noInterveningStatement(void) const; PcodeOp *findMultiequal(const vector &varArray); ///< Find MULTIEQUAL with given inputs static bool liftVerifyUnroll(vector &varArray,int4 slot); ///< Verify given Varnodes are defined with same PcodeOp }; diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/jumptable.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/jumptable.cc index 1e175ae436..e7696bf905 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/jumptable.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/jumptable.cc @@ -31,6 +31,8 @@ ElementId ELEM_NORMADDR = ElementId("normaddr",215); ElementId ELEM_NORMHASH = ElementId("normhash",216); ElementId ELEM_STARTVAL = ElementId("startval",217); +const uint8 JumpValues::NO_LABEL = 0xBAD1ABE1BAD1ABE1; + /// \param encoder is the stream encoder void LoadTable::encode(Encoder &encoder) const @@ -1355,42 +1357,38 @@ bool JumpBasic::foldInOneGuard(Funcdata *fd,GuardRecord &guard,JumpTable *jump) { PcodeOp *cbranch = guard.getBranch(); - int4 indpath = guard.getPath(); // Get stored path to indirect block BlockBasic *cbranchblock = cbranch->getParent(); - if (cbranchblock->getFlipPath()) // Based on whether out branches have been flipped - indpath = 1 - indpath; // get actual path to indirect block - BlockBasic *guardtarget = (BlockBasic *)cbranchblock->getOut(1-indpath); - bool change = false; - int4 pos; - // Its possible the guard branch has been converted between the switch recovery and now if (cbranchblock->sizeOut() != 2) return false; // In which case, we can't fold it in + int4 indpath = guard.getPath(); // Get stored path to indirect block + if (cbranchblock->getFlipPath()) // Based on whether out branches have been flipped + indpath = 1 - indpath; // get actual path to indirect block BlockBasic *switchbl = jump->getIndirectOp()->getParent(); + if (cbranchblock->getOut(indpath) != switchbl) // Guard must go directly into switch block + return false; + BlockBasic *guardtarget = (BlockBasic *)cbranchblock->getOut(1-indpath); + int4 pos; + for(pos=0;possizeOut();++pos) if (switchbl->getOut(pos) == guardtarget) break; + if (jump->hasFoldedDefault() && jump->getDefaultBlock() != pos) // There can be only one folded target + return false; + + if (!switchbl->noInterveningStatement()) + return false; if (pos == switchbl->sizeOut()) { - if (BlockBasic::noInterveningStatement(cbranch,indpath,switchbl->lastOp())) { - // Adjust tables and control flow graph - // for new jumptable destination - jump->addBlockToSwitch(guardtarget,0xBAD1ABE1); - jump->setLastAsMostCommon(); - fd->pushBranch(cbranchblock,1-indpath,switchbl); - guard.clear(); - change = true; - } + jump->addBlockToSwitch(guardtarget,JumpValues::NO_LABEL); // Add new destination to table without a label + jump->setLastAsDefault(); // treating it as either the default case or an exit + fd->pushBranch(cbranchblock,1-indpath,switchbl); // Turn branch target into target of the switch instead } else { - // We should probably check that there are no intervening - // statements between the guard and the switch. But the - // fact that the guard target is also a switch target - // is a good indicator that there are none uintb val = ((indpath==0)!=(cbranch->isBooleanFlip())) ? 0 : 1; fd->opSetInput(cbranch,fd->newConstant(cbranch->getIn(0)->getSize(),val),1); jump->setDefaultBlock(pos); // A guard branch generally targets the default case - guard.clear(); - change = true; } - return change; + jump->setFoldedDefault(); // Mark that the default branch has been folded (and cannot take a label) + guard.clear(); + return true; } JumpBasic::~JumpBasic(void) @@ -1504,12 +1502,12 @@ void JumpBasic::buildLabels(Funcdata *fd,vector
&addresstable,vectorwarning("This code block may not be properly labeled as switch case",addresstable[label.size()]); else if (needswarning==2) @@ -1524,7 +1522,7 @@ void JumpBasic::buildLabels(Funcdata *fd,vector
&addresstable,vectorwarning("Bad switch case",addresstable[label.size()]); - label.push_back(0xBAD1ABE1); + label.push_back(JumpValues::NO_LABEL); } } @@ -1628,7 +1626,7 @@ bool JumpBasic2::foldInOneGuard(Funcdata *fd,GuardRecord &guard,JumpTable *jump) // So we don't make any special mods, in case there are extra statements in these blocks // The final block in the table is the single value produced by the model2 guard - jump->setLastAsMostCommon(); // It should be the default block + jump->setLastAsDefault(); // It should be the default block guard.clear(); // Mark that we are folded return true; } @@ -1977,7 +1975,7 @@ void JumpBasicOverride::buildLabels(Funcdata *fd,vector
&addresstable,v try { addr = backup2Switch(fd,values[i],normalvn,switchvn); } catch(EvaluationError &err) { - addr = 0xBAD1ABE1; + addr = JumpValues::NO_LABEL; } label.push_back(addr); if (label.size() >= addresstable.size()) break; // This should never happen @@ -1985,7 +1983,7 @@ void JumpBasicOverride::buildLabels(Funcdata *fd,vector
&addresstable,v while(label.size() < addresstable.size()) { fd->warning("Bad switch case",addresstable[label.size()]); // This should never happen - label.push_back(0xBAD1ABE1); + label.push_back(JumpValues::NO_LABEL); } } @@ -2170,7 +2168,7 @@ void JumpAssisted::buildLabels(Funcdata *fd,vector
&addresstable,vector label.push_back(output); } } - label.push_back(0xBAD1ABE1); // Add fake label to match the defaultAddress + label.push_back(JumpValues::NO_LABEL); // Add fake label to match the defaultAddress } Varnode *JumpAssisted::foldInNormalization(Funcdata *fd,PcodeOp *indop) @@ -2192,7 +2190,7 @@ bool JumpAssisted::foldInGuards(Funcdata *fd,JumpTable *jump) { int4 origVal = jump->getDefaultBlock(); - jump->setLastAsMostCommon(); // Default case is always the last block + jump->setLastAsDefault(); // Default case is always the last block return (origVal != jump->getDefaultBlock()); } @@ -2348,6 +2346,7 @@ JumpTable::JumpTable(Architecture *g,Address ad) maxext = 1; recoverystage = 0; collectloads = false; + defaultIsFolded = false; } /// This is a partial clone of another jump-table. Objects that are specific @@ -2368,6 +2367,7 @@ JumpTable::JumpTable(const JumpTable *op2) maxext = op2->maxext; recoverystage = op2->recoverystage; collectloads = op2->collectloads; + defaultIsFolded = false; // We just clone the addresses themselves addresstable = op2->addresstable; loadpoints = op2->loadpoints; @@ -2453,7 +2453,7 @@ int4 JumpTable::getIndexByBlock(const FlowBlock *bl,int4 i) const throw LowlevelError("Could not get jumptable index for block"); } -void JumpTable::setLastAsMostCommon(void) +void JumpTable::setLastAsDefault(void) { defaultBlock = lastBlock; @@ -2742,7 +2742,7 @@ void JumpTable::encode(Encoder &encoder) const if (spc != (AddrSpace *)0) spc->encodeAttributes(encoder,off); if (i *loadpoints); ///< Perform sanity check on recovered address targets @@ -600,9 +604,11 @@ public: int4 numIndicesByBlock(const FlowBlock *bl) const; int4 getIndexByBlock(const FlowBlock *bl,int4 i) const; Address getAddressByIndex(int4 i) const { return addresstable[i]; } ///< Get the i-th address table entry - void setLastAsMostCommon(void); ///< Set the most common jump-table target to be the last address in the table + void setLastAsDefault(void); ///< Set the \e default jump-table target to be the last address in the table void setDefaultBlock(int4 bl) { defaultBlock = bl; } ///< Set out-edge of the switch destination considered to be \e default void setLoadCollect(bool val) { collectloads = val; } ///< Set whether LOAD records should be collected + void setFoldedDefault(void) { defaultIsFolded = true; } ///< Mark that the \e default block is a folded CBRANCH target + bool hasFoldedDefault(void) const { return defaultIsFolded; } ///< Return \b true if the \e default block is a folded CBRANCH target void addBlockToSwitch(BlockBasic *bl,uintb lab); ///< Force a given basic-block to be a switch destination void switchOver(const FlowInfo &flow); ///< Convert absolute addresses to block indices uintb getLabelByIndex(int4 index) const { return label[index]; } ///< Given a \e case index, get its label diff --git a/Ghidra/Features/Decompiler/src/decompile/datatests/ifswitch.xml b/Ghidra/Features/Decompiler/src/decompile/datatests/ifswitch.xml new file mode 100644 index 0000000000..2e80215fc4 --- /dev/null +++ b/Ghidra/Features/Decompiler/src/decompile/datatests/ifswitch.xml @@ -0,0 +1,44 @@ + + + + +f30f1efa83ff637e0769c7e8030000c3 +83ff147727488d0d3800000089fa4863 +04914801c83effe0b806000000c3b80a +000000c38d04fd00000000c38d47f3c3 +4863c7c1ff1f4869c06766666648c1f8 +2129f8c3daffffffecffffffd4ffffff +e0ffffffe8ffffffd4ffffffe0ffffff +e0ffffffe0ffffffe8ffffffecffffff +e8ffffffe8ffffffe8ffffffe8ffffff +e8ffffffe8ffffffe8ffffffe8ffffff +d4ffffffecffffff04000000 + + + + +default: + return param_1 \+ -0xd; +if \(99 < param_1\) \{ + return param_1 \* 1000; +case 0: + return 10; +case 1: + case 10: + case 0x14: + return param_1 / 5; +case 2: + case 5: + case 0x13: + return 6; +case 3: + case 6: + case 7: + case 8: + return param_1 \* 8; +