Merge remote-tracking branch 'origin/GT-3597_ghidra1_UpgradeReferences'

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/code/CodeManager.java

@@ -2883,14 +2883,14 @@ public class CodeManager implements ErrorHandler, ManagerDB {
 	}
 
 	/**
-	 * Creates the symbol and adds references for the moved instruction.
+	 * Updates the default references for a new or updated instruction.
 	 */
 	private void addReferencesForInstruction(InstructionDB inst) {
 
 		List<Reference> oldRefList = null;
 		if (redisassemblyMode) {
 			for (Reference ref : refManager.getReferencesFrom(inst.getMinAddress())) {
-				if (ref.getSource() != SourceType.DEFAULT) {
+				if (ref.getSource() != SourceType.DEFAULT || !ref.isMemoryReference()) {
 					continue;
 				}
 				if (oldRefList == null) {
@@ -2912,32 +2912,47 @@ public class CodeManager implements ErrorHandler, ManagerDB {
 			//  not any addresses that are added by the user.
 			int refCnt = 0;
 
+			Reference operandPrimaryRef = null;
+
 			// First look through the pieces of the operand to find the addresses
 			ArrayList<Object> opList = prototype.getOpRepresentationList(opIndex, inst);
 			for (Object obj : opList) {
 				if (obj instanceof Address) {
 					Address refAddr = (Address) obj;
 					++refCnt;
-					removeOldReference(oldRefList, refAddr, opIndex);
-					if (addOperandReference(inst, opIndex, flowAddrs, refAddr)) {
+					RefType refType =
+						getOperandMemoryReferenceType(inst, opIndex, flowAddrs, refAddr);
+					if (refType != null) {
+						operandPrimaryRef = addDefaultMemoryReferenceIfMissing(inst, opIndex,
+							refAddr, refType, oldRefList, operandPrimaryRef);
 						--remainingAddrs;
 					}
 				}
 			}
 			// If there are still more addresses on this operand, see if the whole operand has any
-			if (refCnt == 0 && remainingAddrs != 0) {
+			if (refCnt == 0 && remainingAddrs > 0) {
 				Address refAddr = prototype.getAddress(opIndex, inst);
 				if (refAddr != null) {
-					removeOldReference(oldRefList, refAddr, opIndex);
-					if (addOperandReference(inst, opIndex, flowAddrs, refAddr)) {
+					RefType refType =
+						getOperandMemoryReferenceType(inst, opIndex, flowAddrs, refAddr);
+					if (refType != null) {
+						operandPrimaryRef = addDefaultMemoryReferenceIfMissing(inst, opIndex,
+							refAddr, refType, oldRefList, operandPrimaryRef);
 						--remainingAddrs;
 					}
 				}
 			}
+
+			if (operandPrimaryRef != null && !operandPrimaryRef.isPrimary()) {
+				// ensure that we have a primary ref on the operand if one exists
+				refManager.setPrimary(operandPrimaryRef, true);
+			}
 		}
+
+		Reference mnemonicPrimaryRef = null;
+
 		for (Address flowAddr : flowAddrs) {
 			if (flowAddr != null && flowAddr.isMemoryAddress()) {
-				removeOldReference(oldRefList, flowAddr, Reference.MNEMONIC);
 				FlowType flowType = RefTypeFactory.getDefaultFlowType(inst, flowAddr, false);
 				if (flowType == null) {
 					flowType = RefType.INVALID;
@@ -2945,73 +2960,108 @@ public class CodeManager implements ErrorHandler, ManagerDB {
 				boolean isFallthrough =
 					(flowType.isJump() && flowAddr.equals(inst.getMaxAddress().next()));
 				if (!isFallthrough) {
-					refManager.addMemoryReference(inst.getMinAddress(), flowAddr, flowType,
-						SourceType.DEFAULT, Reference.MNEMONIC);
+					mnemonicPrimaryRef = addDefaultMemoryReferenceIfMissing(inst, Reference.MNEMONIC,
+						flowAddr, flowType, oldRefList, mnemonicPrimaryRef);
 				}
 			}
 		}
+
+		if (mnemonicPrimaryRef != null && !mnemonicPrimaryRef.isPrimary()) {
+			// ensure that we have a primary ref on the mnemonic if one exists
+			refManager.setPrimary(mnemonicPrimaryRef, true);
+		}
+
 		if (oldRefList != null && !oldRefList.isEmpty()) {
 			for (Reference ref : oldRefList) {
-				SourceType source = ref.getSource();
-				if (!ref.isMemoryReference() || source == SourceType.IMPORTED ||
-					source == SourceType.USER_DEFINED) {
-					continue;
-				}
-				if (source == SourceType.ANALYSIS) {
-					boolean hasReferencesFrom =
-						refManager.hasReferencesFrom(ref.getFromAddress(), ref.getOperandIndex());
-					if (!hasReferencesFrom) {
-						continue;
-					}
-					// TODO: what other kinds of references should we preserve?
-				}
 				refManager.delete(ref);
 			}
 		}
 	}
 
-	private void removeOldReference(List<Reference> oldRefList, Address toAddr, int opIndex) {
+	/**
+	 * Remove the specified reference is from oldRefList if present, otherwise add to instruction as a DEFAULT.
+	 * Return as preferred primary reference if it previously existed as a primary reference in oldRefList or
+	 * the specified operandPrimaryRef was null.
+	 * @param inst instruction to which references apply
+	 * @param opIndex operand to which reference applies
+	 * @param refAddr default reference to-address
+	 * @param refType default reference type
+	 * @param oldRefList list of old references which exist on instruction which have 
+	 * yet to be accounted for (may be null).
+	 * @param operandPrimaryRef current preferred primary reference for operand
+	 * @return updated preferred primary address for operand (i.e., operandPrimaryRef)
+	 */
+	private Reference addDefaultMemoryReferenceIfMissing(Instruction inst,
+			int opIndex, Address refAddr, RefType refType, List<Reference> oldRefList,
+			Reference operandPrimaryRef) {
+
+		Reference ref = removeOldReference(oldRefList, refAddr, opIndex, refType);
+		if (ref == null) {
+			ref = refManager.addMemoryReference(inst.getMinAddress(), refAddr, refType,
+				SourceType.DEFAULT, opIndex);
+			if (operandPrimaryRef == null) {
+				operandPrimaryRef = ref;
+			}
+		}
+		else if (ref.isPrimary()) {
+			operandPrimaryRef = ref;
+		}
+		return operandPrimaryRef;
+	}
+
+	/**
+	 * Remove matching DEFAULT memory reference from oldRefList
+	 * @param oldRefList list of existing DEFAULT memory references (may be null)
+	 * @param toAddr new reference desination address
+	 * @param opIndex new reference operand
+	 * @param refType new reference type
+	 * @return existing reference if it already exists in oldRefList, else null
+	 */
+	private Reference removeOldReference(List<Reference> oldRefList, Address toAddr, int opIndex,
+			RefType refType) {
 		if (oldRefList == null) {
-			return;
+			return null;
 		}
 		Iterator<Reference> iterator = oldRefList.iterator();
 		while (iterator.hasNext()) {
 			Reference ref = iterator.next();
-			if (toAddr.equals(ref.getToAddress()) && opIndex == ref.getOperandIndex()) {
+			if (opIndex == ref.getOperandIndex() && refType == ref.getReferenceType() &&
+				toAddr.equals(ref.getToAddress())) {
 				iterator.remove();
-				return;
+				return ref;
 			}
 		}
+		return null;
 	}
 
 	/**
-	 * Add operand reference
-	 * @param inst
+	 * Get operand reference type for a new default memory reference
+	 * @param inst instruction
 	 * @param opIndex operand index
-	 * @param flowAddrs known set of flow destination addresses
+	 * @param flowAddrs known set of flow destination addresses.  Any address utilized from this
+	 * list to produce an operand reference will be set to null within this array.
 	 * @param refAddr reference to address
-	 * @return true if reference is a flow and corresponds to one of the flowAddrs, otherwise false
+	 * @return reference type or null if refAddr corresponds to defined register
 	 */
-	private boolean addOperandReference(InstructionDB inst, int opIndex, Address[] flowAddrs,
-			Address refAddr) {
-		boolean isFlow = false;
-		if (program.getRegister(refAddr) == null) {
-			RefType refType = RefTypeFactory.getDefaultMemoryRefType(inst, opIndex, refAddr, true);
-			if (refType.isFlow()) {
-				for (int j = 0; j < flowAddrs.length; j++) {
-					if (refAddr.equals(flowAddrs[j])) {
-						flowAddrs[j] = null;
-						isFlow = true;
-					}
-				}
-				if (refType != RefType.INDIRECTION && !isFlow) {
-					refType = RefType.DATA;
+	private RefType getOperandMemoryReferenceType(InstructionDB inst, int opIndex,
+			Address[] flowAddrs, Address refAddr) {
+		if (program.getRegister(refAddr) != null) {
+			return null;
+		}
+
+		RefType refType = RefTypeFactory.getDefaultMemoryRefType(inst, opIndex, refAddr, true);
+		if (refType.isFlow()) {
+			for (int j = 0; j < flowAddrs.length; j++) {
+				if (refAddr.equals(flowAddrs[j])) {
+					flowAddrs[j] = null;
+					return refType;
 				}
 			}
-			refManager.addMemoryReference(inst.getMinAddress(), refAddr, refType,
-				SourceType.DEFAULT, opIndex);
+			if (refType != RefType.INDIRECTION) {
+				refType = RefType.DATA;
+			}
 		}
-		return isFlow;
+		return refType;
 	}
 
 	/**

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/symbol/ReferenceManager.java

@@ -46,8 +46,8 @@ public interface ReferenceManager {
 	 * @param type reference type - how the location is being referenced.
 	 * @param source the source of this reference
 	 */
-	public Reference addStackReference(Address fromAddr, int opIndex, int stackOffset,
-			RefType type, SourceType source);
+	public Reference addStackReference(Address fromAddr, int opIndex, int stackOffset, RefType type,
+			SourceType source);
 
 	/**
 	 * Add a reference to a register. If a reference already
@@ -63,7 +63,9 @@ public interface ReferenceManager {
 			RefType type, SourceType source);
 
 	/**
-	 * Adds a memory reference.  
+	 * Adds a memory reference.  Only first the first memory reference placed on
+	 * an operand will be made primary by default.  All non-memory references 
+	 * will be removed from the specified operand.
 	 * @param fromAddr address of the codeunit where the reference occurs
 	 * @param toAddr address of the location being referenced.  
 	 * Memory, stack, and register addresses are all permitted.
@@ -76,7 +78,9 @@ public interface ReferenceManager {
 			SourceType source, int opIndex);
 
 	/**
-	 * Add an offset memory reference.
+	 * Add an offset memory reference.  Only first the first memory reference placed on
+	 * an operand will be made primary by default.  All non-memory references 
+	 * will be removed from the specified operand.
 	 * @param fromAddr address for the "from"
 	 * @param toAddr address of the "to" 
 	 * @param offset value added to a base address to get the toAddr
@@ -90,7 +94,9 @@ public interface ReferenceManager {
 	/**
 	 * Add a shifted memory reference; the "to" address is computed as the value
 	 * at the operand at opIndex shifted by some number of bits, specified in the 
-	 * shiftValue parameter.
+	 * shiftValue parameter.  Only first the first memory reference placed on
+	 * an operand will be made primary by default.  All non-memory references 
+	 * will be removed from the specified operand.
 	 * @param fromAddr address for the "from"
 	 * @param toAddr computed as the value of the operand at opIndex shifted
 	 * by the number of bits specified by shiftValue 
@@ -103,7 +109,9 @@ public interface ReferenceManager {
 			RefType type, SourceType source, int opIndex);
 
 	/**
-	 * Adds an external reference.
+	 * Adds an external reference.  If a reference already
+	 * exists for the fromAddr and opIndex, the existing reference is replaced
+	 * with the new reference.
 	 * @param fromAddr from address (source of the reference)
 	 * @param libraryName name of external program
 	 * @param extLabel label within the external program, may be null if extAddr is not null
@@ -119,7 +127,9 @@ public interface ReferenceManager {
 			throws InvalidInputException, DuplicateNameException;
 
 	/**
-	 * Adds an external reference.
+	 * Adds an external reference.  If a reference already
+	 * exists for the fromAddr and opIndex, the existing reference is replaced
+	 * with the new reference.
 	 * @param fromAddr from address (source of the reference)
 	 * @param extNamespace external namespace containing the named external label.
 	 * @param extLabel label within the external program, may be null if extAddr is not null
@@ -130,12 +140,14 @@ public interface ReferenceManager {
 	 * @throws InvalidInputException
 	 * @throws DuplicateNameException 
 	 */
-	public Reference addExternalReference(Address fromAddr, Namespace extNamespace,
-			String extLabel, Address extAddr, SourceType source, int opIndex, RefType type)
+	public Reference addExternalReference(Address fromAddr, Namespace extNamespace, String extLabel,
+			Address extAddr, SourceType source, int opIndex, RefType type)
 			throws InvalidInputException, DuplicateNameException;
 
 	/**
-	 * Adds an external reference.
+	 * Adds an external reference.  If a reference already
+	 * exists for the fromAddr and opIndex, the existing reference is replaced
+	 * with the new reference.
 	 * @param fromAddr from address (source of the reference)
 	 * @param opIndex operand index
 	 * @param location external location