diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/DebuggerResources.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/DebuggerResources.java index 509ec8d339..3db1d4c8ed 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/DebuggerResources.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/DebuggerResources.java @@ -61,7 +61,8 @@ import resources.ResourceManager; import resources.icons.RotateIcon; public interface DebuggerResources { - String OPTIONS_CATEGORY_WORKFLOW = "Debugger.Workflow"; + String OPTIONS_CATEGORY_DEBUGGER = "Debugger"; + String OPTIONS_CATEGORY_WORKFLOW = "Workflow"; ImageIcon ICON_DEBUGGER = ResourceManager.loadImage("images/debugger.png"); diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/workflow/DebuggerWorkflowServicePlugin.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/workflow/DebuggerWorkflowServicePlugin.java index a662a7d37d..8eeda6d604 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/workflow/DebuggerWorkflowServicePlugin.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/workflow/DebuggerWorkflowServicePlugin.java @@ -128,7 +128,7 @@ public class DebuggerWorkflowServicePlugin extends Plugin /* testing */ final List allBots = new ArrayList<>(); // Cannot auto-wire, since they're dynamically populated - private final ToolOptions options; + private final Options options; @SuppressWarnings("hiding") // I'm FrontEndOnly protected final FrontEndTool tool; @@ -140,8 +140,9 @@ public class DebuggerWorkflowServicePlugin extends Plugin this.tool = (FrontEndTool) tool; // I'm FrontEndOnly this.autoServiceWiring = AutoService.wireServicesProvidedAndConsumed(this); - this.options = tool.getOptions(DebuggerResources.OPTIONS_CATEGORY_WORKFLOW); - this.options.addOptionsChangeListener(this); + ToolOptions rootOptions = tool.getOptions(DebuggerResources.OPTIONS_CATEGORY_DEBUGGER); + rootOptions.addOptionsChangeListener(this); + this.options = rootOptions.getOptions(DebuggerResources.OPTIONS_CATEGORY_WORKFLOW); } diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/block.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/block.cc index 5584dbf4d5..6110093336 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/block.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/block.cc @@ -3425,6 +3425,11 @@ FlowBlock *BlockSwitch::nextFlowAfter(const FlowBlock *bl) const { if (getBlock(0) == bl) return (FlowBlock *)0; // Don't know what will execute + + // Can only evaluate this if bl is a case block that falls through to another case block. + // Otherwise there is a break statement in the flow + if (bl->getType() != t_goto) // Fallthru must be a goto block + return (FlowBlock *)0; int4 i; // Look for block to find flow after for(i=0;isize) size = where+(*iter).second.size; - if ((*iter).second.pass < pass) + if ((*iter).second.pass < pass) { intersect = 1; + pass = (*iter).second.pass; + } themap.erase(iter++); } iter = themap.insert(pair( addr, SizePass() )).first; @@ -200,6 +205,18 @@ void Heritage::clearInfoList(void) void Heritage::removeRevisitedMarkers(const vector &remove,const Address &addr,int4 size) { + HeritageInfo *info = getInfo(addr.getSpace()); + if (info->deadremoved > 0) { + bumpDeadcodeDelay(addr.getSpace()); + if (!info->warningissued) { + info->warningissued = true; + ostringstream errmsg; + errmsg << "Heritage AFTER dead removal. Revisit: "; + addr.printRaw(errmsg); + fd->warningHeader(errmsg.str()); + } + } + vector newInputs; list::iterator pos; for(int4 i=0;i &freeStores) /// of LOAD/STORE/CALL ops. /// \param addr is the starting address of the given range /// \param size is the number of bytes in the given range +/// \param guardPerformed is true if a guard has been previously performed on the range /// \param read is the set of Varnode values reading from the range /// \param write is the set of written Varnodes in the range /// \param inputvars is the set of Varnodes in the range already marked as input -void Heritage::guard(const Address &addr,int4 size,vector &read,vector &write, - vector &inputvars) +void Heritage::guard(const Address &addr,int4 size,bool guardPerformed, + vector &read,vector &write,vector &inputvars) { uint4 fl; Varnode *vn; vector::iterator iter; - bool guardneeded = true; for(iter=read.begin();iter!=read.end();++iter) { vn = *iter; @@ -1113,28 +1130,13 @@ void Heritage::guard(const Address &addr,int4 size,vector &read,vecto if (vn->getSize() < size) *iter = vn = normalizeWriteSize(vn,addr,size); vn->setActiveHeritage(); - if (vn->isAddrForce()) - guardneeded = false; - else { - if (vn->isWritten()) { - if (vn->getDef()->code() == CPUI_INDIRECT) // Evidence of a previous guard - guardneeded = false; - } - } } - if (read.empty() && write.empty() && inputvars.empty()) return; - - // This may need to be adjusted in the future - // Basically we need to take into account the possibility - // that the full syntax tree may form over several stages - // so there is the possibility that we will see a new - // free for an address that has already been guarded before - // Because INDIRECTs for a single call or store really - // issue simultaneously, having multiple INDIRECT guards - // for the same address confuses the renaming algorithm - // SO we don't guard if we think we've guarded before - if (guardneeded) { + // The full syntax tree may form over several stages, so we see a new + // free for an address that has already been guarded before. + // Because INDIRECTs for a single CALL or STORE really issue simultaneously, having multiple INDIRECT guards + // for the same address confuses the renaming algorithm, so we don't add guards if we've added them before. + if (!guardPerformed) { fl = 0; // Query for generic properties of address (use empty usepoint) fd->getScopeLocal()->queryProperties(addr,size,Address(),fl); @@ -2344,17 +2346,16 @@ void Heritage::renameRecurse(BlockBasic *bl,VariableStack &varstack) } } -/// \brief Increase the heritage delay for the given Varnode and request a restart +/// \brief Increase the heritage delay for the given AddrSpace and request a restart /// -/// If applicable, look up the heritage stats for the address space for the given -/// Varnode and increment the delay. The address space must allow an additional +/// If applicable, look up the heritage stats for the address space +/// and increment the delay. The address space must allow an additional /// delay and can only be incremented once. If the increment succeeds, the /// function is marked as having a \e restart pending. -/// \param vn is the given Varnode -void Heritage::bumpDeadcodeDelay(Varnode *vn) +/// \param spc is the given AddrSpace +void Heritage::bumpDeadcodeDelay(AddrSpace *spc) { - AddrSpace *spc = vn->getSpace(); if ((spc->getType() != IPTR_PROCESSOR)&&(spc->getType() != IPTR_SPACEBASE)) return; // Not the right kind of space if (spc->getDelay() != spc->getDeadcodeDelay()) @@ -2388,19 +2389,16 @@ void Heritage::placeMultiequals(void) vector writevars; vector inputvars; vector removevars; - PcodeOp *multiop; - Varnode *vnin; - BlockBasic *bl; - int4 max; for(iter=disjoint.begin();iter!=disjoint.end();++iter) { Address addr = (*iter).first; int4 size = (*iter).second.size; + bool guardPerformed = (*iter).second.pass < pass; readvars.clear(); writevars.clear(); inputvars.clear(); removevars.clear(); - max = collect(addr,size,readvars,writevars,inputvars,removevars); // Collect reads/writes + int4 max = collect(addr,size,readvars,writevars,inputvars,removevars); // Collect reads/writes if ((size > 4)&&(max < size)) { if (refinement(addr,size,readvars,writevars,inputvars)) { iter = disjoint.find(addr); @@ -2412,22 +2410,26 @@ void Heritage::placeMultiequals(void) collect(addr,size,readvars,writevars,inputvars,removevars); } } - if (readvars.empty() && (addr.getSpace()->getType() == IPTR_INTERNAL)) - continue; + if (readvars.empty()) { + if (writevars.empty() && inputvars.empty()) { + continue; + } + if (addr.getSpace()->getType() == IPTR_INTERNAL || guardPerformed) + continue; + } if (!removevars.empty()) removeRevisitedMarkers(removevars, addr, size); guardInput(addr,size,inputvars); - guard(addr,size,readvars,writevars,inputvars); - if (readvars.empty()&&writevars.empty()) continue; + guard(addr,size,guardPerformed,readvars,writevars,inputvars); calcMultiequals(writevars); // Calculate where MULTIEQUALs go for(int4 i=0;inewOp(bl->sizeIn(),bl->getStart()); + BlockBasic *bl = (BlockBasic *) merge[i]; + PcodeOp *multiop = fd->newOp(bl->sizeIn(),bl->getStart()); Varnode *vnout = fd->newVarnodeOut(size,addr,multiop); vnout->setActiveHeritage(); fd->opSetOpcode(multiop,CPUI_MULTIEQUAL); // Create each MULTIEQUAL for(int4 j=0;jsizeIn();++j) { - vnin = fd->newVarnode(size,addr); + Varnode *vnin = fd->newVarnode(size,addr); fd->opSetInput(multiop,vnin,j); } fd->opInsertBegin(multiop,bl); // Insert at beginning of block @@ -2505,19 +2507,19 @@ void Heritage::heritage(void) if (vn->hasNoDescend()) continue; if ((!needwarning)&&(info->deadremoved>0)&&!fd->isJumptableRecoveryOn()) { needwarning = true; - bumpDeadcodeDelay(vn); + bumpDeadcodeDelay(vn->getSpace()); warnvn = vn; } - disjoint.add((*liter).first,(*liter).second.size,pass,prev); + disjoint.add((*liter).first,(*liter).second.size,(*liter).second.pass,prev); } else { // Partially contained in old range, but may contain new stuff - disjoint.add((*liter).first,(*liter).second.size,pass,prev); + disjoint.add((*liter).first,(*liter).second.size,(*liter).second.pass,prev); if ((!needwarning)&&(info->deadremoved>0)&&!fd->isJumptableRecoveryOn()) { // TODO: We should check if this varnode is tiled by previously heritaged ranges if (vn->isHeritageKnown()) continue; // Assume that it is tiled and produced by merging // In most cases, a truly new overlapping read will hit the bumpDeadcodeDelay either here or in prev==2 needwarning = true; - bumpDeadcodeDelay(vn); + bumpDeadcodeDelay(vn->getSpace()); warnvn = vn; } } diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/heritage.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/heritage.hh index e3609d42f9..76f731a586 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/heritage.hh +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/heritage.hh @@ -246,7 +246,8 @@ class Heritage { bool protectFreeStores(AddrSpace *spc,vector &freeStores); bool discoverIndexedStackPointers(AddrSpace *spc,vector &freeStores,bool checkFreeStores); void reprocessFreeStores(AddrSpace *spc,vector &freeStores); - void guard(const Address &addr,int4 size,vector &read,vector &write,vector &inputvars); + void guard(const Address &addr,int4 size,bool guardPerformed, + vector &read,vector &write,vector &inputvars); void guardInput(const Address &addr,int4 size,vector &input); void guardCallOverlappingInput(FuncCallSpecs *fc,const Address &addr,const Address &transAddr,int4 size); bool guardCallOverlappingOutput(FuncCallSpecs *fc,const Address &addr,int4 size,vector &write); @@ -265,7 +266,7 @@ class Heritage { void visitIncr(FlowBlock *qnode,FlowBlock *vnode); void calcMultiequals(const vector &write); void renameRecurse(BlockBasic *bl,VariableStack &varstack); - void bumpDeadcodeDelay(Varnode *vn); + void bumpDeadcodeDelay(AddrSpace *spc); void placeMultiequals(void); void rename(void); public: diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.cc index 9701f642bf..973a7a31d9 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.cc @@ -2858,10 +2858,27 @@ int4 RuleIndirectCollapse::applyOp(PcodeOp *op,Funcdata &data) Varnode *vn2 = op->getOut(); int4 res = vn1->characterizeOverlap(*vn2); if (res > 0) { // Copy has an effect of some sort - if (res == 2) { // vn1 and vn2 are the same storage -> do complete replace + if (res == 2) { // vn1 and vn2 are the same storage + // Convert INDIRECT to COPY + data.opUninsert(op); data.opSetInput(op,vn1,0); data.opRemoveInput(op,1); data.opSetOpcode(op,CPUI_COPY); + data.opInsertAfter(op, indop); + return 1; + } + if (vn1->contains(*vn2) == 0) { // INDIRECT output is properly contained in COPY output + // Convert INDIRECT to a SUBPIECE + uintb trunc; + if (vn1->getSpace()->isBigEndian()) + trunc = vn1->getOffset() + vn1->getSize() - (vn2->getOffset() + vn2->getSize()); + else + trunc = vn2->getOffset() - vn1->getOffset(); + data.opUninsert(op); + data.opSetInput(op,vn1,0); + data.opSetInput(op,data.newConstant(4,trunc),1); + data.opSetOpcode(op, CPUI_SUBPIECE); + data.opInsertAfter(op, indop); return 1; } data.warning("Ignoring partial resolution of indirect",indop->getAddr()); diff --git a/Ghidra/Framework/Project/src/main/java/ghidra/framework/plugintool/mgr/OptionsManager.java b/Ghidra/Framework/Project/src/main/java/ghidra/framework/plugintool/mgr/OptionsManager.java index 41cf86845c..fe83f61979 100644 --- a/Ghidra/Framework/Project/src/main/java/ghidra/framework/plugintool/mgr/OptionsManager.java +++ b/Ghidra/Framework/Project/src/main/java/ghidra/framework/plugintool/mgr/OptionsManager.java @@ -33,6 +33,7 @@ import ghidra.framework.plugintool.dialog.KeyBindingsPanel; import ghidra.framework.plugintool.util.OptionsService; import ghidra.util.HelpLocation; import ghidra.util.Msg; +import ghidra.util.exception.AssertException; /** * Created by PluginTool to manage the set of Options for each category. @@ -61,6 +62,12 @@ public class OptionsManager implements OptionsService, OptionsChangeListener { @Override public ToolOptions getOptions(String category) { + if (category.contains(Options.DELIMITER_STRING)) { + throw new AssertException( + "Options category cannot contain the options path delimiter '" + Options.DELIMITER + + "'"); + } + ToolOptions opt = optionsMap.get(category); if (opt == null) { opt = new ToolOptions(category); @@ -245,8 +252,7 @@ public class OptionsManager implements OptionsService, OptionsChangeListener { } private void removeUnusedOptions(List deleteList) { - for (int i = 0; i < deleteList.size(); i++) { - String name = deleteList.get(i); + for (String name : deleteList) { ToolOptions options = optionsMap.remove(name); options.removeOptionsChangeListener(this); }