GP-2015 Adjustments to DynamicHash for "Force Union Field" action

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

@@ -19,7 +19,7 @@
 
 // Table for how to hash opcodes, lumps certain operators (i.e. ADD SUB PTRADD PTRSUB) into one hash
 // zero indicates the operator should be skipped
-uint4 DynamicHash::transtable[] = {
+const uint4 DynamicHash::transtable[] = {
   0,
   CPUI_COPY,  CPUI_LOAD,  CPUI_STORE,  CPUI_BRANCH,  CPUI_CBRANCH,  CPUI_BRANCHIND,
 
@@ -199,11 +199,30 @@ void DynamicHash::clear(void)
 void DynamicHash::calcHash(const PcodeOp *op,int4 slot,uint4 method)
 
 {
+  const Varnode *root;
+
+  // slot may be from a hash unassociated with op
+  // we need to check that slot indicates a valid Varnode
+  if (slot < 0) {
+    root = op->getOut();
+    if (root == (const Varnode *)0) {
+      hash = 0;
+      addrresult = Address();
+      return;		// slot does not fit op
+    }
+  }
+  else {
+    if (slot >= op->numInput()) {
+      hash = 0;
+      addrresult = Address();
+      return;		// slot does not fit op
+    }
+    root = op->getIn(slot);
+  }
   vnproc = 0;
   opproc = 0;
   opedgeproc = 0;
 
-  const Varnode *root = (slot < 0) ? op->getOut() : op->getIn(slot);
   opedge.push_back(ToOpEdge(op,slot));
   switch(method) {
     case 4:
@@ -354,6 +373,32 @@ void DynamicHash::pieceTogetherHash(const Varnode *root,uint4 method)
   addrresult = op->getSeqNum().getAddr();
 }
 
+/// For a DynamicHash on a PcodeOp, the op must not be a CAST or other skipped opcode.
+/// Test if the given op is a skip op, and if so follow data-flow indicated by the
+/// slot to another PcodeOp until we find one that isn't a skip op. Pass back the new PcodeOp
+/// and slot. Pass back null if the data-flow path ends.
+/// \param op is the given PcodeOp to modify
+/// \param slot is the slot to modify
+void DynamicHash::moveOffSkip(const PcodeOp *&op,int4 &slot)
+
+{
+  while(transtable[op->code()] == 0) {
+    if (slot >= 0) {
+      const Varnode *vn = op->getOut();
+      op = vn->loneDescend();
+      if (op == (PcodeOp*)0) {
+	return;	// Indicate the end of the data-flow path
+      }
+      slot = op->getSlot(vn);
+    }
+    else {
+      const Varnode *vn = op->getIn(0);
+      if (!vn->isWritten()) return;	// Indicate the end of the data-flow path
+      op = vn->getDef();
+    }
+  }
+}
+
 /// Collect the set of Varnodes at the same address as the given Varnode.
 /// Starting with method 0, increment the method and calculate hashes
 /// of the Varnodes until the given Varnode has a unique hash within the set.
@@ -424,6 +469,12 @@ void DynamicHash::uniqueHash(const Varnode *root,Funcdata *fd)
   addrresult = tmpaddr;
 }
 
+/// Different hash methods are cycled through until a hash is found that distinguishes the given
+/// op from other PcodeOps at the same address. The final hash encoding and address of the PcodeOp are
+/// built for retrieval using getHash() and getAddress().
+/// \param op is the given PcodeOp
+/// \param slot is the particular slot to encode in the hash
+/// \param fd is the function containing the given PcodeOp
 void DynamicHash::uniqueHash(const PcodeOp *op,int4 slot,Funcdata *fd)
 
 {
@@ -435,6 +486,12 @@ void DynamicHash::uniqueHash(const PcodeOp *op,int4 slot,Funcdata *fd)
   Address tmpaddr;
   uint4 maxduplicates = 8;
 
+  moveOffSkip(op, slot);
+  if (op == (const PcodeOp *)0) {
+    hash = (uint8)0;
+    addrresult = Address();	// Hash cannot be calculated
+    return;
+  }
   gatherOpsAtAddress(oplist,fd,op->getAddr());
   for(method=4;method<7;++method) {
     clear();

Ghidra/Features/Decompiler/src/decompile/cpp/dynamic.hh

@@ -76,6 +76,7 @@ class DynamicHash {
   void gatherUnmarkedVn(void);		///< Move staged Varnodes into the sub-graph and mark them
   void gatherUnmarkedOp(void);		///< Mark any new PcodeOps in the sub-graph
   void pieceTogetherHash(const Varnode *root,uint4 method);	///< Clean-up and piece together formal hash value
+  static void moveOffSkip(const PcodeOp *&op,int4 &slot);	///< Convert given PcodeOp to a non-skip op by following data-flow
 public:
   void clear(void);			///< Called for each additional hash (after the first)
   void calcHash(const Varnode *root,uint4 method);	///< Calculate the hash for given Varnode and method
@@ -97,7 +98,7 @@ public:
   static bool getIsNotAttached(uint8 h);		///< Retrieve the attachment boolean from a hash
   static void clearTotalPosition(uint8 &h);		///< Clear the collision total and position fields within a hash
   static uint4 getComparable(uint8 h) { return (uint4)h; }	///< Get only the formal hash for comparing
-  static uint4 transtable[];				///< Translation of op-codes to hash values
+  static const uint4 transtable[];				///< Translation of op-codes to hash values
 };
 
 #endif

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

@@ -122,17 +122,21 @@ public class ForceUnionAction extends AbstractDecompilerAction {
 		int opcode = accessOp.getOpcode();
 		if (opcode == PcodeOp.PTRSUB) {
 			parentDt = typeIsUnionRelated(accessOp.getInput(0));
+			accessVn = accessOp.getInput(0);
+			accessSlot = 0;
 			if (accessOp.getInput(1).getOffset() == 0) {	// Artificial op
-				accessVn = accessOp.getOutput();
+				do {
-				accessOp = accessVn.getLoneDescend();
+					Varnode tmpVn = accessOp.getOutput();
-				if (accessOp == null) {
+					PcodeOp tmpOp = tmpVn.getLoneDescend();
-					return;
+					if (tmpOp == null) {
+						break;
+					}
+					accessOp = tmpOp;
+					accessVn = tmpVn;
+					accessSlot = accessOp.getSlot(accessVn);
 				}
-				accessSlot = accessOp.getSlot(accessVn);
+				while (accessOp.getOpcode() == PcodeOp.PTRSUB &&
-			}
+					accessOp.getInput(1).getOffset() == 0);
-			else {
-				accessVn = accessOp.getInput(0);
-				accessSlot = 0;
 			}
 		}
 		else {
@@ -179,6 +183,9 @@ public class ForceUnionAction extends AbstractDecompilerAction {
 		}
 		for (DataTypeComponent component : components) {
 			String nm = component.getFieldName();
+			if (nm == null || nm.length() == 0) {
+				nm = component.getDefaultFieldName();
+			}
 			allFields.add(nm);
 			if (size == 0 || component.getDataType().getLength() == size) {
 				res.add(nm);

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

@@ -84,7 +84,7 @@ public class DynamicHash {
 	 * in the sub-graph.  The edge can either be from an input Varnode to the PcodeOp
 	 * that reads it, or from a PcodeOp to the Varnode it defines.
 	 */
-	private class ToOpEdge implements Comparable<ToOpEdge> {
+	private static class ToOpEdge implements Comparable<ToOpEdge> {
 		private PcodeOp op;
 		private int slot;			// slot containing varnode we are coming from
 
@@ -221,6 +221,41 @@ public class DynamicHash {
 		opedge.clear();
 	}
 
+	/**
+	 * For a DynamicHash on a PcodeOp, the op must not be a CAST or other skipped opcode.
+	 * Test if the given op is a skip op, and if so follow data-flow indicated by the
+	 * slot to another PcodeOp until we find one that isn't a skip op. Return null, if
+	 * the initial op is not skipped, otherwise return a ToOpEdge indicating the
+	 * new (op,slot) pair.
+	 * @param op is the given PcodeOp
+	 * @param slot is the slot
+	 * @return null or a new (op,slot) pair
+	 */
+	private static ToOpEdge moveOffSkip(PcodeOp op, int slot) {
+		if (transtable[op.getOpcode()] != 0) {
+			return null;
+		}
+		do {
+			if (slot >= 0) {
+				Varnode vn = op.getOutput();
+				op = vn.getLoneDescend();
+				if (op == null) {
+					return new ToOpEdge(null, 0);	// Indicate the end of the data-flow path
+				}
+				slot = op.getSlot(vn);
+			}
+			else {
+				Varnode vn = op.getInput(0);
+				op = vn.getDef();
+				if (op == null) {
+					return new ToOpEdge(null, 0);	// Indicate the end of the data-flow path
+				}
+			}
+		}
+		while (transtable[op.getOpcode()] == 0);
+		return new ToOpEdge(op, slot);
+	}
+
 	/**
 	 * Encode a particular PcodeOp and slot
 	 * @param op is the PcodeOp to preserve
@@ -228,11 +263,27 @@ public class DynamicHash {
 	 * @param method is the method to use for encoding (4, 5, or 6)
 	 */
 	private void calcHash(PcodeOp op, int slot, int method) {
+		Varnode root;
+		if (slot < 0) {
+			root = op.getOutput();
+			if (root == null) {
+				hash = 0;
+				addrresult = Address.NO_ADDRESS;
+				return;
+			}
+		}
+		else {
+			if (slot >= op.getNumInputs()) {
+				hash = 0;
+				addrresult = Address.NO_ADDRESS;
+				return;
+			}
+			root = op.getInput(slot);
+		}
 		vnproc = 0;
 		opproc = 0;
 		opedgeproc = 0;
 		markset = new HashSet<>();
-		Varnode root = (slot < 0) ? op.getOutput() : op.getInput(slot);
 		opedge.add(new ToOpEdge(op, slot));
 		switch (method) {
 			case 4:
@@ -382,6 +433,16 @@ public class DynamicHash {
 		Address tmpaddr = null;
 		int maxduplicates = 8;
 
+		ToOpEdge move = moveOffSkip(op, slot);
+		if (move != null) {
+			op = move.getOp();
+			slot = move.getSlot();
+			if (op == null) {
+				hash = 0;
+				addrresult = Address.NO_ADDRESS;
+				return;
+			}
+		}
 		gatherOpsAtAddress(oplist, fd, op.getSeqnum().getTarget());
 		for (method = 4; method < 7; ++method) {
 			clear();

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

@@ -755,6 +755,8 @@ public class HighFunctionDBUtil {
 	public static void writeUnionFacet(Function function, DataType dt, int fieldNum, Address addr,
 			long hash, SourceType source) throws InvalidInputException, DuplicateNameException {
 		int firstUseOffset = (int) addr.subtract(function.getEntryPoint());
+		String symbolName = UnionFacetSymbol.buildSymbolName(fieldNum, addr);
+		boolean nameCollision = false;
 		Variable[] localVariables =
 			function.getLocalVariables(VariableFilter.UNIQUE_VARIABLE_FILTER);
 		Variable preexistingVar = null;
@@ -762,10 +764,14 @@ public class HighFunctionDBUtil {
 			if (var.getFirstUseOffset() == firstUseOffset &&
 				var.getFirstStorageVarnode().getOffset() == hash) {
 				preexistingVar = var;
-				break;
+			}
+			else if (var.getName().startsWith(symbolName)) {
+				nameCollision = true;
 			}
 		}
-		String symbolName = UnionFacetSymbol.buildSymbolName(fieldNum, addr);
+		if (nameCollision) {	// Uniquify the name if necessary
+			symbolName = symbolName + '_' + Integer.toHexString(DynamicHash.getComparable(hash));
+		}
 		if (preexistingVar != null) {
 			if (preexistingVar.getName().equals(symbolName)) {
 				return;			// No change to make