GT-3597 improved default reference update during language upgrade

redisassembly

Ghidra/Features/Base/developer_scripts/FixLangId.java

@@ -104,7 +104,7 @@ public class FixLangId extends GhidraScript {
 		List<LanguageDescription> descriptions =
 			DefaultLanguageService.getLanguageService().getLanguageDescriptions(true);
 		List<String> choices = new ArrayList<>(descriptions.size());
-		for (int i = 0; i < choices.size(); i++) {
+		for (int i = 0; i < descriptions.size(); i++) {
 			choices.add(descriptions.get(i).getLanguageID().getIdAsString());
 		}
 

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,14 +2912,28 @@ 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);
+					RefType refType =
-					if (addOperandReference(inst, opIndex, flowAddrs, refAddr)) {
+						getOperandMemoryReferenceType(inst, opIndex, flowAddrs, refAddr);
+					if (refType != null) {
+						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;
+						}
 						--remainingAddrs;
 					}
 				}
@@ -2928,16 +2942,35 @@ public class CodeManager implements ErrorHandler, ManagerDB {
 			if (refCnt == 0 && remainingAddrs != 0) {
 				Address refAddr = prototype.getAddress(opIndex, inst);
 				if (refAddr != null) {
-					removeOldReference(oldRefList, refAddr, opIndex);
+					RefType refType =
-					if (addOperandReference(inst, opIndex, flowAddrs, refAddr)) {
+						getOperandMemoryReferenceType(inst, opIndex, flowAddrs, refAddr);
+					if (refType != null) {
+						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;
+						}
 						--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 +2978,87 @@ public class CodeManager implements ErrorHandler, ManagerDB {
 				boolean isFallthrough =
 					(flowType.isJump() && flowAddr.equals(inst.getMaxAddress().next()));
 				if (!isFallthrough) {
-					refManager.addMemoryReference(inst.getMinAddress(), flowAddr, flowType,
+					Reference ref =
-						SourceType.DEFAULT, Reference.MNEMONIC);
+						removeOldReference(oldRefList, flowAddr, Reference.MNEMONIC, flowType);
+					if (ref == null) {
+						ref = refManager.addMemoryReference(inst.getMinAddress(), flowAddr,
+							flowType, SourceType.DEFAULT, Reference.MNEMONIC);
+						if (mnemonicPrimaryRef == null) {
+							mnemonicPrimaryRef = ref;
+						}
+					}
+					else if (ref.isPrimary()) {
+						mnemonicPrimaryRef = ref;
+					}
 				}
 			}
 		}
+
+		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 matching DEFAULT memory reference from oldRefList
+	 * @param oldRefList list of existing DEFAULT memory references
+	 * @param toAddr new reference desination address
+	 * @param opIndex new reference operand
+	 * @param refType new reference type
+	 * @return existing reference if it already exists, 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
+	 * Get operand reference type for a new default memory reference
-	 * @param inst
+	 * @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,
+	private RefType getOperandMemoryReferenceType(InstructionDB inst, int opIndex,
-			Address refAddr) {
+			Address[] flowAddrs, Address refAddr) {
-		boolean isFlow = false;
+		if (program.getRegister(refAddr) != null) {
-		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++) {
+		RefType refType = RefTypeFactory.getDefaultMemoryRefType(inst, opIndex, refAddr, true);
-					if (refAddr.equals(flowAddrs[j])) {
+		if (refType.isFlow()) {
-						flowAddrs[j] = null;
+			for (int j = 0; j < flowAddrs.length; j++) {
-						isFlow = true;
+				if (refAddr.equals(flowAddrs[j])) {
-					}
+					flowAddrs[j] = null;
-				}
+					return refType;
-				if (refType != RefType.INDIRECTION && !isFlow) {
-					refType = RefType.DATA;
 				}
 			}
-			refManager.addMemoryReference(inst.getMinAddress(), refAddr, refType,
+			if (refType != RefType.INDIRECTION) {
-				SourceType.DEFAULT, opIndex);
+				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,
+	public Reference addStackReference(Address fromAddr, int opIndex, int stackOffset, RefType type,
-			RefType type, SourceType source);
+			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,
+	public Reference addExternalReference(Address fromAddr, Namespace extNamespace, String extLabel,
-			String extLabel, Address extAddr, SourceType source, int opIndex, RefType type)
+			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