diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.cc index 840c5b7b23..ada5a2762a 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.cc @@ -4931,6 +4931,7 @@ void universal_action(Architecture *conf) act->addAction( new ActionMergeRequired("merge") ); act->addAction( new ActionMarkExplicit("merge") ); act->addAction( new ActionMarkImplied("merge") ); // This must come BEFORE general merging + act->addAction( new ActionMergeMultiEntry("merge") ); act->addAction( new ActionMergeCopy("merge") ); act->addAction( new ActionDominantCopy("merge") ); act->addAction( new ActionMarkIndirectOnly("merge") ); // Must come after required merges but before speculative diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.hh index 817dd6c258..1e97cd7fbc 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.hh +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.hh @@ -380,6 +380,17 @@ public: virtual int4 apply(Funcdata &data) { data.getMerge().mergeOpcode(CPUI_COPY); return 0; } }; +/// \brief Try to merge Varnodes specified by Symbols with multiple SymbolEntrys +class ActionMergeMultiEntry : public Action { +public: + ActionMergeMultiEntry(const string &g) : Action(rule_onceperfunc,"mergemultientry",g) {} ///< Constructor + virtual Action *clone(const ActionGroupList &grouplist) const { + if (!grouplist.contains(getGroup())) return (Action *)0; + return new ActionMergeMultiEntry(getGroup()); + } + virtual int4 apply(Funcdata &data) { data.getMerge().mergeMultiEntry(); return 0; } +}; + /// \brief Try to merge Varnodes of the same type (if they don't hold different values at the same time) class ActionMergeType : public Action { public: diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/database.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/database.cc index 1fc34f407b..ecc0e89680 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/database.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/database.cc @@ -1656,6 +1656,8 @@ SymbolEntry *ScopeInternal::addMapInternal(Symbol *sym,uint4 exfl,const Address list::iterator iter = rangemap->insert(initdata,addr.getOffset(),lastaddress.getOffset()); // Store reference to map in symbol sym->mapentry.push_back(iter); + if (sym->mapentry.size() == 2) + multiEntrySet.insert(sym); return &(*iter); } @@ -1666,6 +1668,8 @@ SymbolEntry *ScopeInternal::addDynamicMapInternal(Symbol *sym,uint4 exfl,uint8 h list::iterator iter = dynamicentry.end(); --iter; sym->mapentry.push_back(iter); // Store reference to map entry in symbol + if (sym->mapentry.size() == 2) + multiEntrySet.insert(sym); return &dynamicentry.back(); } @@ -1890,6 +1894,8 @@ void ScopeInternal::removeSymbol(Symbol *symbol) list.pop_back(); } + if (symbol->mapentry.size() > 1) + multiEntrySet.erase(symbol); // Remove each mapping of the symbol for(iter=symbol->mapentry.begin();iter!=symbol->mapentry.end();++iter) { AddrSpace *spc = (*(*iter)).getAddr().getSpace(); @@ -1907,10 +1913,14 @@ void ScopeInternal::removeSymbol(Symbol *symbol) void ScopeInternal::renameSymbol(Symbol *sym,const string &newname) { - nametree.erase(sym); // Erase under old name + nametree.erase(sym); // Erase under old name + if (sym->mapentry.size() > 1) + multiEntrySet.erase(sym); // The multi-entry set is sorted by name, remove string oldname = sym->name; sym->name = newname; insertNameTree(sym); + if (sym->mapentry.size() > 1) + multiEntrySet.insert(sym); // Reenter into the multi-entry set now that name is changed } void ScopeInternal::retypeSymbol(Symbol *sym,Datatype *ct) diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/database.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/database.hh index 9686a36013..80f1428c3e 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/database.hh +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/database.hh @@ -179,7 +179,8 @@ public: force_oct = 3, ///< Force octal printing of constant symbol force_bin = 4, ///< Force binary printing of constant symbol force_char = 5, ///< Force integer to be printed as a character constant - size_typelock = 8 ///< Only the size of the symbol is typelocked + size_typelock = 8, ///< Only the size of the symbol is typelocked + isolate = 16 ///< Symbol should not speculatively merge automatically }; /// \brief Construct given a name and data-type Symbol(Scope *sc,const string &nm,Datatype *ct) @@ -204,6 +205,8 @@ public: Scope *getScope(void) const { return scope; } ///< Get the scope owning \b this Symbol SymbolEntry *getFirstWholeMap(void) const; ///< Get the first entire mapping of the symbol SymbolEntry *getMapEntry(const Address &addr) const; ///< Get first mapping of the symbol that contains the given Address + int4 numEntries(void) const { return mapentry.size(); } ///< Return the number of SymbolEntrys + SymbolEntry *getMapEntry(int4 i) const { return &(*mapentry[i]); } ///< Return the i-th SymbolEntry for \b this Symbol int4 getResolutionDepth(const Scope *useScope) const; ///< Get the number of scope names to print to resolve symbol in given context void saveXmlHeader(ostream &s) const; ///< Save basic Symbol properties as XML attributes void restoreXmlHeader(const Element *el); ///< Restore basic Symbol properties from XML @@ -718,6 +721,7 @@ protected: vector maptable; ///< Rangemaps of SymbolEntry, one map for each address space vector > category; ///< References to Symbol objects organized by category list dynamicentry; ///< Dynamic symbol entries + SymbolNameTree multiEntrySet; ///< Set of symbols with multiple entries uint8 nextUniqueId; ///< Next available symbol id public: ScopeInternal(const string &nm,Architecture *g); ///< Construct the Scope @@ -765,6 +769,8 @@ public: virtual int4 getCategorySize(int4 cat) const; virtual Symbol *getCategorySymbol(int4 cat,int4 ind) const; virtual void setCategory(Symbol *sym,int4 cat,int4 ind); + set::const_iterator beginMultiEntry(void) const { return multiEntrySet.begin(); } ///< Start of symbols with more than one entry + set::const_iterator endMultiEntry(void) const { return multiEntrySet.end(); } ///< End of symbols with more than one entry static void savePathXml(ostream &s,const vector &vec); ///< Save a path with \ tags static void restorePathXml(vector &vec,const Element *el); ///< Restore path from \ tags }; diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata.hh index 8500775b58..2b4c90fe08 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata.hh +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata.hh @@ -384,6 +384,7 @@ public: void clearDeadOps(void) { obank.destroyDead(); } ///< Delete any dead PcodeOps Symbol *linkSymbol(Varnode *vn); ///< Find or create Symbol associated with given Varnode Symbol *linkSymbolReference(Varnode *vn); ///< Discover and attach Symbol to a constant reference + void findLinkedVarnodes(SymbolEntry *entry,vector res) const; ///< Find Varnodes that map to the given SymbolEntry void buildDynamicSymbol(Varnode *vn); ///< Build a \e dynamic Symbol associated with the given Varnode bool attemptDynamicMapping(SymbolEntry *entry,DynamicHash &dhash); Merge &getMerge(void) { return covermerge; } ///< Get the Merge object for \b this function diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata_varnode.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata_varnode.cc index 788540fbe7..716be14239 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata_varnode.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/funcdata_varnode.cc @@ -1010,6 +1010,32 @@ Symbol *Funcdata::linkSymbolReference(Varnode *vn) return entry->getSymbol(); } +/// Look for Varnodes that are (should be) mapped to the given SymbolEntry and +/// add them to the end of the result list. +/// \param entry is the given SymbolEntry to match +/// \param res is the container holding the result list of matching Varnodes +void Funcdata::findLinkedVarnodes(SymbolEntry *entry,vector res) const + +{ + if (entry->isDynamic()) { + DynamicHash dhash; + Varnode *vn = dhash.findVarnode(this,entry->getFirstUseAddress(),entry->getHash()); + if (vn != (Varnode *)0) + res.push_back(vn); + } + else { + VarnodeLocSet::const_iterator iter = beginLoc(entry->getSize(),entry->getAddr()); + VarnodeLocSet::const_iterator enditer = endLoc(entry->getSize(),entry->getAddr()); + for(;iter!=enditer;++iter) { + Varnode *vn = *iter; + Address addr = vn->getUsePoint(*this); + if (entry->inUse(addr)) { + res.push_back(vn); + } + } + } +} + /// If a Symbol is already attached, no change is made. Otherwise a special \e dynamic Symbol is /// created that is associated with the Varnode via a hash of its local data-flow (rather /// than its storage address). diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/merge.cc b/Ghidra/Features/Decompiler/src/decompile/cpp/merge.cc index 351717ac85..005b6ab983 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/merge.cc +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/merge.cc @@ -102,6 +102,15 @@ bool Merge::mergeTestRequired(HighVariable *high_out,HighVariable *high_in) } else if (high_out->isExtraOut()) return false; + if (high_in->isMapped() && high_out->isMapped()) { + Symbol *symbolIn = high_in->getSymbol(); + Symbol *symbolOut = high_out->getSymbol(); + if (symbolIn != (Symbol *)0 && symbolOut != (Symbol *)0) { + if (symbolIn != symbolOut) return false; // Map to different symbols + if (high_in->getSymbolOffset() != high_out->getSymbolOffset()) + return false; // Map to different parts of same symbol + } + } return true; } @@ -151,9 +160,6 @@ bool Merge::mergeTestSpeculative(HighVariable *high_out,HighVariable *high_in) { if (!mergeTestAdjacent(high_out,high_in)) return false; - // Don't merge a mapped variable speculatively - if (high_out->isMapped()) return false; - if (high_in->isMapped()) return false; // Don't merge anything with a global speculatively if (high_out->isPersist()) return false; if (high_in->isPersist()) return false; @@ -268,7 +274,7 @@ void Merge::mergeOpcode(OpCode opc) vn2 = op->getIn(j); if (!mergeTestBasic(vn2)) continue; if (mergeTestRequired(vn1->getHigh(),vn2->getHigh())) - merge(vn1->getHigh(),vn2->getHigh(),false); + merge(vn1->getHigh(),vn2->getHigh(),true); // Treat as speculative } } } @@ -798,6 +804,61 @@ void Merge::mergeMarker(void) } } +/// \brief Merge together Varnodes mapped to SymbolEntrys from the same Symbol +/// +/// Symbols that have more than one SymbolEntry may attach to more than one Varnode. +/// These Varnodes need to be merged to properly represent a single variable. +void Merge::mergeMultiEntry(void) + +{ + SymbolNameTree::const_iterator iter = data.getScopeLocal()->beginMultiEntry(); + SymbolNameTree::const_iterator enditer = data.getScopeLocal()->endMultiEntry(); + for(;iter!=enditer;++iter) { + vector mergeList; + Symbol *symbol = *iter; + int4 numEntries = symbol->numEntries(); + int4 mergeCount = 0; + int4 skipCount = 0; + int4 conflictCount = 0; + for(int4 i=0;igetMapEntry(i), mergeList); + if (mergeList.size() == prevSize) + skipCount += 1; // Did not discover any Varnodes corresponding to a particular SymbolEntry + } + if (mergeList.empty()) continue; + HighVariable *high = mergeList[0]->getHigh(); + Datatype *ct = high->getType(); + updateHigh(high); + for(int4 i=0;igetHigh(); + if (newHigh == high) continue; // Varnodes already merged + updateHigh(newHigh); + if (!mergeTestRequired(high, newHigh)) { + conflictCount += 1; + continue; + } + if (!merge(high,newHigh,false)) { // Attempt the merge + conflictCount += 1; + continue; + } + mergeCount += 1; + } + if (skipCount != 0 || conflictCount !=0) { + ostringstream s; + s << "Unable to"; + if (mergeCount != 0) + s << " fully"; + s << " merge symbol: " << symbol->getName(); + if (skipCount > 0) + s << " -- Some instance varnodes not found."; + if (conflictCount > 0) + s << " -- Some merges are forbidden"; + data.warningHeader(s.str()); + } + } +} + /// \brief Speculatively merge Varnodes that are input/output to the same p-code op /// /// If a single p-code op has an input and output HighVariable that share the same data-type, diff --git a/Ghidra/Features/Decompiler/src/decompile/cpp/merge.hh b/Ghidra/Features/Decompiler/src/decompile/cpp/merge.hh index eede994e1a..7e6b8fc1cc 100644 --- a/Ghidra/Features/Decompiler/src/decompile/cpp/merge.hh +++ b/Ghidra/Features/Decompiler/src/decompile/cpp/merge.hh @@ -125,6 +125,7 @@ public: void mergeAddrTied(void); void mergeMarker(void); void mergeAdjacent(void); + void mergeMultiEntry(void); bool hideShadows(HighVariable *high); void processCopyTrims(void); void markInternalCopies(void);