Multi-merge functionality

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

@@ -899,7 +899,16 @@ bool Funcdata::syncVarnodesWithSymbol(VarnodeLocSet::const_iterator &iter,uint4
     vn = *iter++;
     if (vn->isFree()) continue;
     vnflags = vn->getFlags();
-    if ((vnflags & mask) != flags) { // We have a change
+    if (vn->mapentry != (SymbolEntry *)0) {		// If there is already an attached SymbolEntry (dynamic)
+      uint4 localMask = mask & ~Varnode::mapped;	// Make sure 'mapped' bit is unchanged
+      uint4 localFlags = flags & localMask;
+      if ((vnflags & localMask) != localFlags) {
+	updateoccurred = true;
+	vn->setFlags(localFlags);
+	vn->clearFlags((~localFlags)&localMask);
+      }
+    }
+    else if ((vnflags & mask) != flags) { // We have a change
       updateoccurred = true;
       vn->setFlags(flags);
       vn->clearFlags((~flags)&mask);

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

@@ -2214,7 +2214,8 @@ bool PrintC::emitScopeVarDecls(const Scope *scope,int4 cat)
   list<SymbolEntry>::const_iterator iter_d = scope->beginDynamic();
   list<SymbolEntry>::const_iterator enditer_d = scope->endDynamic();
   for(;iter_d!=enditer_d;++iter_d) {
-    if ((*iter_d).isPiece()) continue; // Don't do a partial entry
+    const SymbolEntry *entry = &(*iter_d);
+    if (entry->isPiece()) continue; // Don't do a partial entry
     Symbol *sym = (*iter_d).getSymbol();
     if (sym->getCategory() != cat) continue;
     if (sym->getName().size() == 0) continue;
@@ -2222,6 +2223,10 @@ bool PrintC::emitScopeVarDecls(const Scope *scope,int4 cat)
       continue;
     if (dynamic_cast<LabSymbol *>(sym) != (LabSymbol *)0)
       continue;
+    if (sym->isMultiEntry()) {
+      if (sym->getFirstWholeMap() != entry)
+	continue;
+    }
     notempty = true;
     emitVarDeclStatement(sym);
   }

Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/IsolateVariableAction.java

@@ -80,7 +80,7 @@ public class IsolateVariableAction extends AbstractDecompilerAction {
 			new IsolateVariableTask(context.getTool(), context.getProgram(),
 				context.getDecompilerPanel(), tokenAtCursor, highSymbol, SourceType.USER_DEFINED);
 
-		newVariableTask.runTask();
+		newVariableTask.runTask(false);
 	}
 
 }

Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/RenameFieldAction.java

@@ -74,6 +74,6 @@ public class RenameFieldAction extends AbstractDecompilerAction {
 		RenameStructureFieldTask nameTask =
 			new RenameStructureFieldTask(tool, context.getProgram(), context.getDecompilerPanel(),
 				tokenAtCursor, dt, offset);
-		nameTask.runTask();
+		nameTask.runTask(true);
 	}
 }

Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/RenameGlobalAction.java

@@ -81,6 +81,6 @@ public class RenameGlobalAction extends AbstractDecompilerAction {
 		RenameGlobalVariableTask nameTask =
 			new RenameGlobalVariableTask(tool, context.getProgram(), context.getDecompilerPanel(),
 				tokenAtCursor, addr);
-		nameTask.runTask();
+		nameTask.runTask(true);
 	}
 }

Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/RenameLocalAction.java

@@ -82,7 +82,7 @@ public class RenameLocalAction extends AbstractDecompilerAction {
 			context.getDecompilerPanel(),
 			tokenAtCursor, highSymbol, SourceType.USER_DEFINED);
 
-		nameTask.runTask();
+		nameTask.runTask(true);
 	}
 
 }

Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/RenameTask.java

@@ -59,9 +59,10 @@ public abstract class RenameTask {
 	
 	/**
 	 * Bring up a dialog that is initialized with the old name, and allows the user to select a new name
+	 * @param oldNameIsCancel is true if the user keeping/entering the old name is considered a cancel
 	 * @return true unless the user canceled
 	 */
-	private boolean runDialog() {
+	private boolean runDialog(boolean oldNameIsCancel) {
 		InputDialogListener listener = new InputDialogListener() {
 			@Override
 			public boolean inputIsValid(InputDialog dialog) {
@@ -91,15 +92,19 @@ public abstract class RenameTask {
         if (renameVarDialog.isCanceled()) {
         	return false;
         }
-        if (newName.equals(oldName)) {
+		if (oldNameIsCancel && newName.equals(oldName)) {
 			return false;
 		}
         return true;		
 		
 	}
 
-	public void runTask() {
+	/**
-		boolean dialogres = runDialog();
+	 * Perform the task of selecting a new name and committing it to the database
+	 * @param oldNameIsCancel is true if the user entering/keeping the old name is considered a cancel
+	 */
+	public void runTask(boolean oldNameIsCancel) {
+		boolean dialogres = runDialog(oldNameIsCancel);
 		if (dialogres) {
 			int transaction = program.startTransaction(getTransactionName());
 			boolean commit = false;

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/HighFunctionDBUtil.java

@@ -216,9 +216,11 @@ public class HighFunctionDBUtil {
 			}
 			String name = sym.getName();
 			try {
-				Variable var = clearConflictingLocalVariables(sym);
+				Variable var =
+					clearConflictingLocalVariables(function, sym.getStorage(), sym.getPCAddress());
 				if (var == null) {
-					var = createLocalVariable(sym, null, null, source);
+					var = createLocalVariable(function, sym.getDataType(), sym.getStorage(),
+						sym.getPCAddress(), source);
 					if (name != null) {
 						var.setName(name, source);
 					}
@@ -237,26 +239,24 @@ public class HighFunctionDBUtil {
 
 	/**
 	 * Create a local DB variable with a default name. Storage and data-type for the variable
-	 * can be provided explicitly, or they can be taken from a decompiler symbol.
+	 * are provided explicitly.
-	 * @param symbol is the decompiler symbol
+	 * @param function is the function owning the new variable
-	 * @param dt is the given data-type or null (to use the symbol's data-type)
+	 * @param dt is the given data-type
-	 * @param storage is the given storage or null (to use the symbol's storage)
+	 * @param storage is the given storage
-	 * @param source is the desired SourceType of the new variable
+	 * @param pcAddr is point where the variable is instantiated or null
+	 * @param source is the source type of the new variable
 	 * @return the new local variable
 	 * @throws InvalidInputException is a valid variable can't be created
 	 */
-	private static Variable createLocalVariable(HighSymbol symbol, DataType dt,
+	private static Variable createLocalVariable(Function function, DataType dt,
-			VariableStorage storage, SourceType source) throws InvalidInputException {
+			VariableStorage storage, Address pcAddr, SourceType source)
-		Function function = symbol.getHighFunction().getFunction();
+			throws InvalidInputException {
 		Program program = function.getProgram();
-		if (storage == null || storage.isUniqueStorage()) {
+		int firstUseOffset = 0;
-			storage = symbol.getStorage();
+		if (pcAddr != null) {
+			firstUseOffset = (int) pcAddr.subtract(function.getEntryPoint());
 		}
-		if (dt == null) {
+		Variable var = new LocalVariableImpl(null, firstUseOffset, dt, storage, program);
-			dt = symbol.getDataType();
-		}
-		Variable var =
-			new LocalVariableImpl(null, symbol.getFirstUseOffset(), dt, storage, program);
 		try {
 			var = function.addLocalVariable(var, source);
 		}
@@ -266,8 +266,8 @@ public class HighFunctionDBUtil {
 
 		Register reg = var.getRegister();
 		if (reg != null) {
-			program.getReferenceManager().addRegisterReference(symbol.getPCAddress(), -1, reg,
+			program.getReferenceManager().addRegisterReference(pcAddr, -1, reg, RefType.WRITE,
-				RefType.WRITE, source);
+				source);
 		}
 
 		return var;
@@ -305,33 +305,84 @@ public class HighFunctionDBUtil {
 		return false;
 	}
 
+	/**
+	 * Given a particular seed Variable, find the set of local Variables that are intended to be
+	 * merged containing that seed. The result will be an array with at least the seed variable in it.
+	 * @param function is the function containing the local variables
+	 * @param seed is the seed local variable
+	 * @return an array of all Variables intended to be merged.
+	 */
+	private static Variable[] gatherMergeSet(Function function, Variable seed) {
+		TreeMap<String, Variable> nameMap = new TreeMap<String, Variable>();
+		for (Variable var : function.getAllVariables()) {
+			nameMap.put(var.getName(), var);
+		}
+		String baseName = seed.getName();
+		int pos = baseName.lastIndexOf('$');
+		if (pos >= 0) {
+			baseName = baseName.substring(0, pos);
+		}
+		DataType dataType = seed.getDataType();
+		Variable currentVar = nameMap.get(baseName);
+		int index = 0;
+		boolean sawSeed = false;
+		ArrayList<Variable> mergeArray = new ArrayList<Variable>();
+		for (;;) {
+			if (currentVar == null) {
+				break;
+			}
+			if (!currentVar.getDataType().equals(dataType)) {
+				break;
+			}
+			if (index != 0 && currentVar instanceof Parameter) {
+				break;
+			}
+			if (index != 0 && currentVar.hasStackStorage()) {
+				break;
+			}
+			if (currentVar == seed) {
+				sawSeed = true;
+			}
+			mergeArray.add(currentVar);
+			index += 1;
+			String newName = baseName + '$' + Integer.toString(index);
+			currentVar = nameMap.get(newName);
+		}
+		Variable[] res;
+		if (!sawSeed) {
+			res = new Variable[1];
+			res[0] = seed;
+		}
+		else {
+			res = new Variable[mergeArray.size()];
+			mergeArray.toArray(res);
+		}
+		return res;
+	}
+
 	/**
 	 * Low-level routine for clearing any variables in the
 	 * database which conflict with this variable and return
 	 * one of them for re-use.  The returned variable still
 	 * exists within the function at the same first-use-offset.
+	 * @param function is the function containing the local variables
+	 * @param storage is the storage area to clear
+	 * @param pcAddr is the point of use
 	 * @return existing variable with identical storage and first-use offset or null
 	 */
-	private static Variable clearConflictingLocalVariables(HighSymbol local) {
+	private static Variable clearConflictingLocalVariables(Function function,
+			VariableStorage storage, Address pcAddr) {
 
-		SymbolEntry entry = local.getFirstWholeMap();
+		int firstUseOffset = 0;
-		if (entry instanceof MappedEntry) {
+		if (pcAddr != null) {
-			if (local.isParameter()) {	// Don't clear parameters
+			firstUseOffset = (int) pcAddr.subtract(function.getEntryPoint());
-				throw new IllegalArgumentException();
-			}
 		}
+		if (storage.isHashStorage()) {
 
-		HighFunction highFunction = local.getHighFunction();
+			long hashVal = storage.getFirstVarnode().getOffset();
-		Function func = highFunction.getFunction();
+			for (Variable ul : function.getLocalVariables(VariableFilter.UNIQUE_VARIABLE_FILTER)) {
-
-		VariableStorage storage = local.getStorage();
-		int firstUseOffset = local.getFirstUseOffset();
-		if (entry instanceof DynamicEntry) {
-
-			DynamicEntry dynamicEntry = (DynamicEntry) entry;
-			for (Variable ul : func.getLocalVariables(VariableFilter.UNIQUE_VARIABLE_FILTER)) {
 				// Note: assumes there is only one hash method used for unique locals
-				if (ul.getFirstStorageVarnode().getOffset() == dynamicEntry.getHash()) {
+				if (ul.getFirstStorageVarnode().getOffset() == hashVal) {
 					return ul;
 				}
 			}
@@ -339,7 +390,7 @@ public class HighFunctionDBUtil {
 		}
 
 		Variable matchingVariable = null;
-		for (Variable otherVar : func.getLocalVariables()) {
+		for (Variable otherVar : function.getLocalVariables()) {
 			if (otherVar.getFirstUseOffset() != firstUseOffset) {
 				// other than parameters we will have a hard time identifying
 				// local variable conflicts due to differences in scope (i.e., first-use)
@@ -353,7 +404,7 @@ public class HighFunctionDBUtil {
 					matchingVariable = otherVar;
 					continue;
 				}
-				func.removeVariable(otherVar);
+				function.removeVariable(otherVar);
 			}
 		}
 
@@ -453,22 +504,31 @@ public class HighFunctionDBUtil {
 			}
 		}
 		else if (!highSymbol.isGlobal()) {
-			Variable var;
+			Variable[] varList = null;
 			VariableStorage storage = highSymbol.getStorage();
+			Address pcAddr = highSymbol.getPCAddress();
 			HighVariable tmpHigh = highSymbol.getHighVariable();
 			if (!storage.isHashStorage() && tmpHigh != null &&
 				tmpHigh.requiresDynamicStorage()) {
 				storage =
 					DynamicEntry.buildDynamicStorage(tmpHigh.getRepresentative(), highFunction);
-				var = null;
 			}
 			else {
-				var = clearConflictingLocalVariables(highSymbol);
+				Variable var = clearConflictingLocalVariables(function, storage, pcAddr);
+				if (var != null) {
+					if (!resized) {
+						varList = gatherMergeSet(function, var);	// Cannot resize a whole multi-merge
+					}
+					else {
+						varList = new Variable[1];
+						varList[0] = var;
+					}
+				}
 			}
 			boolean usesHashStorage = storage.isHashStorage();
 			if (dataType == null) {
-				if (var != null) {
+				if (varList != null) {
-					dataType = var.getDataType();	// Use preexisting datatype if it fits in desired storage
+					dataType = varList[0].getDataType();	// Use preexisting datatype if it fits in desired storage
 				}
 				else {
 					dataType = Undefined.getUndefinedDataType(highSymbol.getSize());
@@ -484,19 +544,34 @@ public class HighFunctionDBUtil {
 				storage = VariableUtilities.resizeStorage(storage, dataType, true, function);
 			}
 
-			if (var == null) {
+			if (varList == null) {
-				var = createLocalVariable(highSymbol, dataType, storage, source);
+				Variable var = createLocalVariable(function, dataType, storage, pcAddr, source);
+				varList = new Variable[1];
+				varList[0] = var;
+			}
+			else if (resized) {
+				// Set resized data-type on existing Variable
+				varList[0].setDataType(dataType, storage, true, source);
 			}
 			else {
-				// fixup reused variable
+				// Set data-type on existing merge set
-				var.setDataType(dataType, storage, true, source);
+				for (Variable var : varList) {
+					var.setDataType(dataType, source);
+				}
 			}
 			if (name == null) {
 				name = highSymbol.getName(); // must update name if not specified
 			}
+			Variable renameVar = null;
 			try {
-				// must set/correct name
+				int index = 0;
-				var.setName(name, source);
+				String curName = name;
+				for (Variable var : varList) {
+					renameVar = var;
+					var.setName(curName, source);
+					index += 1;
+					curName = name + '$' + Integer.toString(index);
+				}
 			}
 			catch (DuplicateNameException e) {
 				if (isRename) {
@@ -507,7 +582,7 @@ public class HighFunctionDBUtil {
 					Msg.error(HighFunctionDBUtil.class,
 						"Name conflict while naming local variable: " + function.getName() + ":" +
 							name);
-					var.setName(null, SourceType.DEFAULT);
+					renameVar.setName(null, SourceType.DEFAULT);
 				}
 				catch (DuplicateNameException e1) {
 					throw new AssertException("Unexpected exception with default name", e);

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/LocalSymbolMap.java

@@ -75,11 +75,85 @@ public class LocalSymbolMap {
 		return key;
 	}
 
+	/**
+	 * Construct and return a map from a HighSymbol's name to the HighSymbol object
+	 * @return the new name to symbol map
+	 */
+	public Map<String, HighSymbol> getNameToSymbolMap() {
+		Map<String, HighSymbol> newMap = new TreeMap<String, HighSymbol>();
+		for (HighSymbol highSymbol : symbolMap.values()) {
+			newMap.put(highSymbol.getName(), highSymbol);
+		}
+		return newMap;
+	}
+
+	/**
+	 * Remove the given HighSymbol from this container.
+	 * The key is removed from the main symbolMap.  It is also removed from the MappedEntry map
+	 * and from the list of parameter symbols if applicable.
+	 * @param highSymbol is the given symbol
+	 */
+	private void removeSymbol(HighSymbol highSymbol) {
+		SymbolEntry mapEntry = highSymbol.getFirstWholeMap();
+		if (mapEntry instanceof MappedEntry) {
+			MappedVarKey key = new MappedVarKey(mapEntry.getStorage(), mapEntry.getPCAdress());
+			addrMappedSymbols.remove(key);
+		}
+		symbolMap.remove(highSymbol.getId());
+		if (highSymbol.isParameter()) {
+			int index = highSymbol.getCategoryIndex();
+			HighSymbol[] newArray = new HighSymbol[paramSymbols.length - 1];
+			for (int i = 0; i < index; ++i) {
+				newArray[i] = paramSymbols[i];
+			}
+			for (int i = index + 1; i < paramSymbols.length; ++i) {
+				HighSymbol paramSym = paramSymbols[i];
+				newArray[i - 1] = paramSym;
+				paramSym.categoryIndex -= 1;
+			}
+		}
+	}
+
+	/**
+	 * Given names of the form:  "baseName", "baseName$1", "baseName@2", ...
+	 * find the corresponding HighSymbols in this container and merge them into a single HighSymbol.
+	 * The name passed into this method must be of the form "baseName$1", the base name is extracted from it.
+	 * @param name is a string with the base name concatenated with "$1"
+	 * @param nameMap is a map from all symbols names in this container to the corresponding HighSymbol
+	 */
+	private void mergeNamedSymbols(String name, Map<String, HighSymbol> nameMap) {
+		String baseName = name.substring(0, name.length() - 2);
+		HighSymbol baseSymbol = nameMap.get(baseName);
+		if (baseSymbol == null || !baseSymbol.isTypeLocked() ||
+			(baseSymbol instanceof EquateSymbol)) {
+			return;
+		}
+		DataType baseDataType = baseSymbol.getDataType();
+		for (int index = 1;; ++index) {
+			String nextName = baseName + '$' + Integer.toString(index);
+			HighSymbol nextSymbol = nameMap.get(nextName);
+			if (nextSymbol == null || !nextSymbol.isTypeLocked() || nextSymbol.isParameter() ||
+				(baseSymbol instanceof EquateSymbol)) {
+				break;
+			}
+			if (!nextSymbol.getDataType().equals(baseDataType)) {	// Data-types of symbols being merged must match
+				break;
+			}
+			SymbolEntry mapEntry = nextSymbol.getFirstWholeMap();
+			if (mapEntry.getPCAdress() == null) {					// Don't merge from an address tied symbol
+				break;
+			}
+			baseSymbol.addMapEntry(mapEntry);
+			removeSymbol(nextSymbol);
+		}
+	}
+
 	/**
 	 * Populate the local variable map from information attached to the Program DB's function.
 	 * @param includeDefaultNames is true if default symbol names should be considered locked
 	 */
 	public void grabFromFunction(boolean includeDefaultNames) {
+		ArrayList<String> mergeNames = null;
 		Function dbFunction = func.getFunction();
 		Variable locals[] = dbFunction.getLocalVariables();
 		for (Variable local : locals) {
@@ -94,6 +168,15 @@ public class LocalSymbolMap {
 				istypelock = false;
 			}
 			String name = local.getName();
+			if (name.length() > 2 && name.charAt(name.length() - 2) == '$') {
+				// An indication of names like "name", "name@1", "name@2"
+				if (name.charAt(name.length() - 1) == '1') {
+					if (mergeNames == null) {
+						mergeNames = new ArrayList<String>();
+					}
+					mergeNames.add(name);
+				}
+			}
 
 			VariableStorage storage = local.getVariableStorage();
 			long id = 0;
@@ -132,6 +215,15 @@ public class LocalSymbolMap {
 			}
 			DataType dt = var.getDataType();
 			String name = var.getName();
+			if (name.length() > 2 && name.charAt(name.length() - 2) == '$') {
+				// An indication of names like "name", "name@1", "name@2"
+				if (name.charAt(name.length() - 1) == '1') {
+					if (mergeNames == null) {
+						mergeNames = new ArrayList<String>();
+					}
+					mergeNames.add(name);
+				}
+			}
 			VariableStorage storage = var.getVariableStorage();
 			Address resAddr = storage.isStackStorage() ? null : pcaddr;
 			long id = 0;
@@ -154,6 +246,7 @@ public class LocalSymbolMap {
 		Arrays.sort(paramSymbols, PARAM_SYMBOL_SLOT_COMPARATOR);
 
 		grabEquates(dbFunction);
+		grabMerges(mergeNames);
 	}
 
 	private boolean isUserDefinedName(String name) {
@@ -450,10 +543,18 @@ public class LocalSymbolMap {
 		}
 	}
 
+	private void grabMerges(ArrayList<String> mergeNames) {
+		if (mergeNames == null) {
+			return;
+		}
+		Map<String, HighSymbol> nameToSymbolMap = getNameToSymbolMap();
+		for (String name : mergeNames) {
+			mergeNamedSymbols(name, nameToSymbolMap);
+		}
+	}
+
 	/**
 	 * Hashing keys for Local variables
-	 * 
-	 *
 	 */
 	class MappedVarKey {
 		private Address addr;