Basic multi-entry merge capability

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

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:

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

@@ -1656,6 +1656,8 @@ SymbolEntry *ScopeInternal::addMapInternal(Symbol *sym,uint4 exfl,const Address
   list<SymbolEntry>::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<SymbolEntry>::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();
@@ -1908,9 +1914,13 @@ void ScopeInternal::renameSymbol(Symbol *sym,const string &newname)
 
 {
   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)

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<EntryMap *> maptable;			///< Rangemaps of SymbolEntry, one map for each address space
   vector<vector<Symbol *> > category;		///< References to Symbol objects organized by category
   list<SymbolEntry> 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<Symbol *>::const_iterator beginMultiEntry(void) const { return multiEntrySet.begin(); }	///< Start of symbols with more than one entry
+  set<Symbol *>::const_iterator endMultiEntry(void) const { return multiEntrySet.end(); }	///< End of symbols with more than one entry
   static void savePathXml(ostream &s,const vector<string> &vec);	///< Save a path with \<val> tags
   static void restorePathXml(vector<string> &vec,const Element *el);	///< Restore path from \<val> tags
 };

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<Varnode *> 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

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<Varnode *> 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).

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<Varnode *> mergeList;
+    Symbol *symbol = *iter;
+    int4 numEntries = symbol->numEntries();
+    int4 mergeCount = 0;
+    int4 skipCount = 0;
+    int4 conflictCount = 0;
+    for(int4 i=0;i<numEntries;++i) {
+      int4 prevSize = mergeList.size();
+      data.findLinkedVarnodes(symbol->getMapEntry(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;i<mergeList.size();++i) {
+      HighVariable *newHigh = mergeList[i]->getHigh();
+      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,

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);