GP-268 - Listing - fixed slow painting on functions with an outrageous

number of variables

Closes #2351

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/references/ReferenceDBManager.java

@@ -18,6 +18,9 @@ package ghidra.program.database.references;
 import java.io.IOException;
 import java.util.*;
 
+import org.apache.commons.collections4.map.LazyMap;
+import org.apache.commons.collections4.map.LazySortedMap;
+
 import db.*;
 import db.util.ErrorHandler;
 import ghidra.program.database.*;
@@ -63,6 +66,7 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 	 * @param openMode one of ProgramDB.CREATE, UPDATE, UPGRADE, or READ_ONLY
 	 * @param lock the program synchronization lock
 	 * @param monitor Task monitor for upgrading
+	 * @throws CancelledException if the user cancels the loading of this db
 	 * @throws IOException if a database io error occurs.
 	 * @throws VersionException if the database version is different from the expected version
 	 */
@@ -70,8 +74,8 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 			TaskMonitor monitor) throws CancelledException, IOException, VersionException {
 		this.addrMap = addrMap;
 		this.lock = lock;
-		fromCache = new DBObjectCache<RefList>(100);
+		fromCache = new DBObjectCache<>(100);
-		toCache = new DBObjectCache<RefList>(100);
+		toCache = new DBObjectCache<>(100);
 
 		VersionException versionExc = null;
 		try {
@@ -193,19 +197,13 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 				}
 			}
 
-			int refCnt = 0;
 			if (newAddr == null) {
 				// This is an unexpected situation
-				refCnt = removeAllTo(oldAddr);
+				removeAllTo(oldAddr);
 			}
 			else {
-				refCnt = moveReferencesTo(oldAddr, newAddr, monitor);
+				moveReferencesTo(oldAddr, newAddr, monitor);
 			}
-
-//			if (oldAddr.isVariableAddress()) {
-//				varStoreMgr.oldVariableReferencesRemoved(oldAddr, refCnt);
-//			}
-
 		}
 
 	}
@@ -224,17 +222,16 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 		}
 		int cnt = toRefs.getNumRefs();
 		Reference[] refs = toRefs.getAllRefs();
-		for (int i = 0; i < refs.length; i++) {
+		for (Reference ref : refs) {
-			RefList fromRefs = getFromRefs(refs[i].getFromAddress());
+			RefList fromRefs = getFromRefs(ref.getFromAddress());
-			fromRefs.removeRef(toAddr, refs[i].getOperandIndex());
+			fromRefs.removeRef(toAddr, ref.getOperandIndex());
 			if (fromRefs.isEmpty()) {
 				fromCache.delete(fromRefs.getKey());
 			}
-			referenceRemoved(refs[i]);
+			referenceRemoved(ref);
 		}
 		toRefs.removeAll();
 		toCache.delete(toRefs.getKey());
-		lastRefRemovedTo(toAddr);
 		return cnt;
 	}
 
@@ -278,8 +275,8 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 	 * @param ref existing reference
 	 * @param isOffset true if new reference is an offset reference
 	 * @param isShifted true if new reference is a shifted reference
-	 * @param offsetOrShift
+	 * @param offsetOrShift the offset or shift amount 
-	 * @return
+	 * @return true if incompatible
 	 */
 	private boolean isIncompatible(Reference ref, boolean isOffset, boolean isShifted,
 			long offsetOrShift) {
@@ -300,8 +297,8 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 	/**
 	 * When adding a reference on top of an existing reference, attempt to combine
 	 * the reference types giving preference to the most specific type.
-	 * @param newType
+	 * @param newType the new type
-	 * @param oldType
+	 * @param oldType the old type
 	 * @return combined reference type, or the newType if unable to combine
 	 */
 	private RefType combineReferenceType(RefType newType, RefType oldType) {
@@ -338,9 +335,6 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 		return newType;
 	}
 
-	/**
-	 * Add the given memory reference.
-	 */
 	private ReferenceDB addRef(Address fromAddr, Address toAddr, RefType type,
 			SourceType sourceType, int opIndex, boolean isOffset, boolean isShifted,
 			long offsetOrShift) throws IOException {
@@ -401,7 +395,8 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 			}
 
 			ReferenceDB r = toRefs == null || fromRefs.getNumRefs() < toRefs.getNumRefs()
-					? fromRefs.getRef(toAddr, opIndex) : toRefs.getRef(fromAddr, opIndex);
+					? fromRefs.getRef(toAddr, opIndex)
+					: toRefs.getRef(fromAddr, opIndex);
 
 			referenceAdded(r);
 			return r;
@@ -623,7 +618,8 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 	@Override
 	public Variable getReferencedVariable(Reference reference) {
 		RefType refType = reference.getReferenceType();
-		return program.getFunctionManager().getReferencedVariable(reference.getFromAddress(),
+		return program.getFunctionManager()
+				.getReferencedVariable(reference.getFromAddress(),
 					reference.getToAddress(), 0,
 					!refType.isWrite() && (refType.isRead() || refType.isIndirect()));
 	}
@@ -651,47 +647,16 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 				return NO_REFS;
 			}
 
-			int firstUseOffset = var.getFirstUseOffset();
+			functionCacher.setFunction(function);
-			int outOfScopeOffset = Integer.MAX_VALUE;
+
 			VariableStorage storage = var.getVariableStorage();
-
+			Scope scope = findVariableScope(function, varSymbol, var);
-			Address variableAddr = null;
+			List<Reference> matchingReferences =
-			try {
+				getScopedVariableReferences(storage, function, scope);
-				variableAddr = (varSymbol != null) ? varSymbol.getAddress()
+			if (matchingReferences.isEmpty()) {
-						: symbolMgr.findVariableStorageAddress(storage);
-			}
-			catch (IOException e) {
-				dbError(e);
-			}
-
-			if (variableAddr != null) {
-				if (firstUseOffset < 0) {
-					firstUseOffset = Integer.MAX_VALUE - firstUseOffset;
-				}
-
-				// There could be more than one variable with the same address
-				// Determine scope of variable within function
-				for (Symbol sym : symbolMgr.getSymbols(function.getID())) {
-					if (!sym.getAddress().equals(variableAddr)) {
-						continue;
-					}
-					Variable v = (Variable) sym.getObject();
-					int nextVarOffset = v.getFirstUseOffset();
-					if (nextVarOffset < 0) {
-						nextVarOffset = Integer.MAX_VALUE - nextVarOffset;
-					}
-					if (nextVarOffset < outOfScopeOffset && nextVarOffset > firstUseOffset) {
-						outOfScopeOffset = nextVarOffset;
-					}
-				}
-			}
-
-			ArrayList<Reference> matchingReferences =
-				getScopedVariableReferences(storage, function, firstUseOffset, outOfScopeOffset);
-
-			if (matchingReferences.size() == 0) {
 				return NO_REFS;
 			}
+
 			Reference[] refs = new Reference[matchingReferences.size()];
 			matchingReferences.toArray(refs);
 			return refs;
@@ -701,27 +666,61 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 		}
 	}
 
-	private ArrayList<Reference> getScopedVariableReferences(VariableStorage storage,
+	private Scope findVariableScope(Function function, Symbol varSymbol, Variable var) {
-			Function function, int firstUseOffset, int outOfScopeOffset) {
 
-		SortedMap<Address, List<Reference>> dataReferences =
+		VariableStorage storage = var.getVariableStorage();
-			functionCacher.getFunctionDataReferences(function);
+		Address variableAddr = null;
-
+		try {
-		ArrayList<Reference> matchingReferences = new ArrayList<Reference>();
+			variableAddr = (varSymbol != null) ? varSymbol.getAddress()
-
+					: symbolMgr.findVariableStorageAddress(storage);
-		Address entry = function.getEntryPoint();
+		}
-
+		catch (IOException e) {
-		for (Varnode varnode : storage.getVarnodes()) {
+			dbError(e);
-			getScopedVarnodeReferences(matchingReferences, varnode, dataReferences, firstUseOffset,
-				outOfScopeOffset, entry);
 		}
 
-		return matchingReferences;
+		int firstUseOffset = var.getFirstUseOffset();
+		int outOfScopeOffset = Integer.MAX_VALUE;
+		if (firstUseOffset < 0) {
+			firstUseOffset = Integer.MAX_VALUE - firstUseOffset;
+		}
+
+		if (variableAddr == null) {
+			return new Scope(firstUseOffset, outOfScopeOffset);
+		}
+
+		// There could be more than one variable with the same address
+		// Determine scope of variable within function
+		for (Variable v : functionCacher.getVariables(variableAddr)) {
+			int nextVarOffset = v.getFirstUseOffset();
+			if (nextVarOffset < 0) {
+				nextVarOffset = Integer.MAX_VALUE - nextVarOffset;
+			}
+			if (nextVarOffset < outOfScopeOffset && nextVarOffset > firstUseOffset) {
+				outOfScopeOffset = nextVarOffset;
+			}
+		}
+
+		return new Scope(var.getFirstUseOffset(), outOfScopeOffset);
+	}
+
+	private List<Reference> getScopedVariableReferences(VariableStorage storage,
+			Function function, Scope scope) {
+
+		SortedMap<Address, List<Reference>> dataReferences =
+			functionCacher.getFunctionDataReferences();
+
+		Address entry = function.getEntryPoint();
+		List<Reference> references = new ArrayList<>();
+		for (Varnode varnode : storage.getVarnodes()) {
+			getScopedVarnodeReferences(references, varnode, dataReferences, scope, entry);
+		}
+
+		return references;
 	}
 
 	private void getScopedVarnodeReferences(List<Reference> matchingReferences, Varnode varnode,
-			SortedMap<Address, List<Reference>> dataReferences, int firstUseOffset,
+			SortedMap<Address, List<Reference>> dataReferences, Scope scope, Address entry) {
-			int outOfScopeOffset, Address entry) {
+
 		Address minStorageAddr = varnode.getAddress();
 		Address maxStorageAddr;
 		try {
@@ -732,13 +731,16 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 			maxStorageAddr = minStorageAddr.getAddressSpace().getMaxAddress();
 		}
 
+		int firstUseOffset = scope.getFirstUseOffset();
+		int outOfScopeOffset = scope.getOutOfScopeOffset();
+
 		SortedMap<Address, List<Reference>> subMap = dataReferences.tailMap(minStorageAddr);
 		Iterator<List<Reference>> refListIter = subMap.values().iterator();
 		while (refListIter.hasNext()) {
+
 			List<Reference> refList = refListIter.next();
-			Iterator<Reference> refIterator = refList.iterator();
+			for (Reference ref : refList) {
-			while (refIterator.hasNext()) {
+
-				Reference ref = refIterator.next();
 				if (ref.getToAddress().compareTo(maxStorageAddr) > 0) {
 					return;
 				}
@@ -752,7 +754,6 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 				}
 			}
 		}
-		return;
 	}
 
 	@Override
@@ -813,10 +814,10 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 	@Override
 	public Reference[] getFlowReferencesFrom(Address addr) {
 		Reference[] refs = getReferencesFrom(addr);
-		ArrayList<Reference> list = new ArrayList<Reference>(refs.length);
+		ArrayList<Reference> list = new ArrayList<>(refs.length);
-		for (int i = 0; i < refs.length; i++) {
+		for (Reference ref : refs) {
-			if (refs[i].getReferenceType().isFlow()) {
+			if (ref.getReferenceType().isFlow()) {
-				list.add(refs[i]);
+				list.add(ref);
 			}
 		}
 		refs = new Reference[list.size()];
@@ -883,15 +884,18 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 
 	/**
 	 * Get all memory references with the given from address at opIndex.
+	 * @param fromAddr the from address
+	 * @param opIndex the operand index
+	 * @return the references
 	 */
-	public Reference[] getReferences(Address fromAddr, int opIndex) {
+	Reference[] getReferences(Address fromAddr, int opIndex) {
 		lock.acquire();
 		try {
 			RefList fromRefs = getFromRefs(fromAddr);
 			if (fromRefs == null) {
 				return NO_REFS;
 			}
-			ArrayList<Reference> list = new ArrayList<Reference>(10);
+			ArrayList<Reference> list = new ArrayList<>(10);
 			ReferenceIterator it = fromRefs.getRefs();
 			while (it.hasNext()) {
 				Reference ref = it.next();
@@ -1106,12 +1110,6 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 
 	}
 
-	/**
-	 * Remove reference
-	 * @param fromAddr
-	 * @param toAddr
-	 * @param opIndex
-	 */
 	void removeReference(Address fromAddr, Address toAddr, int opIndex) {
 		lock.acquire();
 		try {
@@ -1134,7 +1132,6 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 				toRefs.removeRef(fromAddr, opIndex);
 				if (toRefs.isEmpty()) {
 					toCache.delete(toRefs.getKey());
-					lastRefRemovedTo(toAddr);
 				}
 			}
 			if (ref != null) {
@@ -1150,9 +1147,8 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 	}
 
 	/**
-	 * Symbol is about to be removed.
+	 * Symbol is about to be removed
-	 * symbol becomes unusable.
+	 * @param symbol the symbol that will be removed
-	 * @param symbol
 	 */
 	public void symbolRemoved(Symbol symbol) {
 		if (symbol.isDynamic()) {
@@ -1166,7 +1162,7 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 		Address refAddr = symbol.getAddress();
 
 		ReferenceIterator iter = getReferencesTo(refAddr);
-		ArrayList<Reference> list = new ArrayList<Reference>();
+		ArrayList<Reference> list = new ArrayList<>();
 		while (iter.hasNext()) {
 			Reference ref = iter.next();
 			if (symID == ref.getSymbolID()) {
@@ -1201,9 +1197,6 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 		}
 		lock.acquire();
 		try {
-//			if (s.getSymbolType() != SymbolType.CODE) {
-//				throw new IllegalArgumentException("Only code label symbols may be associated with a reference");
-//			}
 			if (s instanceof VariableSymbolDB) {
 				VariableStorage storage = ((VariableSymbolDB) s).getVariableStorage();
 				if (!storage.contains(ref.getToAddress())) {
@@ -1327,7 +1320,10 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 	 * within the ReferenceTo-list.
 	 * @param oldToAddr old reference to address
 	 * @param newToAddr new reference to address
+	 * @param monitor the monitor
 	 * @return number of references updated
+	 * @throws CancelledException if the task is cancelled 
+	 * @throws IOException if a database exception occurs 
 	 */
 	public int moveReferencesTo(Address oldToAddr, Address newToAddr, TaskMonitor monitor)
 			throws CancelledException, IOException {
@@ -1484,9 +1480,8 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 		return SymbolUtilities.UNK_LEVEL;
 	}
 
-	/**
+	/*
-	 * Get address iterator over references that are external entry
+	 * Get address iterator over references that are external entry memory references
-	 * mem references.
 	 */
 	public AddressIterator getExternalEntryIterator() {
 		lock.acquire();
@@ -1506,8 +1501,9 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 	}
 
 	/**
-	 * Return whether the address is an external entry point.
+	 * Return whether the address is an external entry point
 	 * @param toAddr the address to test for external entry point
+	 * @return true if the address is an external entry point
 	 */
 	public boolean isExternalEntryPoint(Address toAddr) {
 		lock.acquire();
@@ -1596,7 +1592,7 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 					refList = toAdapter.getRefList(program, toCache, to, toAddr);
 				}
 				catch (ClosedException e) {
-
+					// TODO this seems wrong here; no other method handles closed exceptions
 				}
 				catch (IOException e) {
 					dbError(e);
@@ -1609,9 +1605,8 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 		}
 	}
 
-	/**
+	/*
-	 * Remove all references that have the "From" address as
+	 * Remove all references that have the "From" address as the given address.
-	 * the given address.
 	 */
 	void removeAllFrom(Address fromAddr) throws IOException {
 		lock.acquire();
@@ -1621,16 +1616,15 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 				return;
 			}
 			Reference[] refs = fromRefs.getAllRefs();
-			for (int i = 0; i < refs.length; i++) {
+			for (Reference ref : refs) {
-				RefList toRefs = getToRefs(refs[i].getToAddress());
+				RefList toRefs = getToRefs(ref.getToAddress());
 				if (toRefs != null) { // cope with buggy situation
-					toRefs.removeRef(fromAddr, refs[i].getOperandIndex());
+					toRefs.removeRef(fromAddr, ref.getOperandIndex());
 					if (toRefs.isEmpty()) {
 						toCache.delete(toRefs.getKey());
-						lastRefRemovedTo(refs[i].getToAddress());
 					}
 				}
-				referenceRemoved(refs[i]);
+				referenceRemoved(ref);
 			}
 			fromRefs.removeAll();
 			fromCache.delete(fromRefs.getKey());
@@ -1642,15 +1636,11 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 
 	private void removeAllFrom(Address fromAddr, int opIndex) {
 		Reference[] refs = getReferences(fromAddr, opIndex);
-		for (int i = 0; i < refs.length; i++) {
+		for (Reference ref : refs) {
-			delete(refs[i]);
+			delete(ref);
 		}
 	}
 
-	/**
-	 * @param ref
-	 * @param symbolID
-	 */
 	void setSymbolID(Reference ref, long symbolID) throws IOException {
 		lock.acquire();
 		try {
@@ -1841,16 +1831,6 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 		removeReference(ref.getFromAddress(), ref.getToAddress(), ref.getOperandIndex());
 	}
 
-	/**
-	 * @param toAddress
-	 */
-	private void lastRefRemovedTo(Address toAddress) {
-//		if (toAddress.isExternalAddress()) {
-//			ExternalManagerDB extMgr = (ExternalManagerDB)program.getExternalManager();
-//			extMgr.removeExternalLocation(toAddress);
-//		}
-	}
-
 	@Override
 	public ReferenceIterator getExternalReferences() {
 		AddressSet set = new AddressSet(AddressSpace.EXTERNAL_SPACE.getMinAddress(),
@@ -1915,8 +1895,8 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 			}
 			Reference[] refs = fromRefs.getAllRefs();
 			int cnt = 0;
-			for (int i = 0; i < refs.length; i++) {
+			for (Reference ref : refs) {
-				if (refs[i].getOperandIndex() == opIndex) {
+				if (ref.getOperandIndex() == opIndex) {
 					cnt++;
 				}
 			}
@@ -1925,9 +1905,9 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 			}
 			retRefs = new Reference[cnt];
 			cnt = 0;
-			for (int i = 0; i < refs.length; i++) {
+			for (Reference ref : refs) {
-				if (refs[i].getOperandIndex() == opIndex) {
+				if (ref.getOperandIndex() == opIndex) {
-					retRefs[cnt++] = refs[i];
+					retRefs[cnt++] = ref;
 				}
 			}
 		}
@@ -1976,9 +1956,6 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 		}
 	}
 
-	/**
-	 * Returns associated program
-	 */
 	ProgramDB getProgram() {
 		return program;
 	}
@@ -1987,17 +1964,15 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 
 		private Function cachedFunction;
 		private SortedMap<Address, List<Reference>> references;
+		private Map<Address, List<Variable>> variablesByAddress;
 
-		synchronized SortedMap<Address, List<Reference>> getFunctionDataReferences(
+		synchronized void setFunction(Function function) {
-				Function theFunction) {
+			if (cachedFunction == function) {
-			if (cachedFunction == theFunction) {
+				return;
-				return references;
 			}
 
-			references = getSortedVariableReferences(theFunction);
+			clearCache();
-			cachedFunction = theFunction;
+			cachedFunction = function;
-
-			return references;
 		}
 
 		synchronized void clearCache() {
@@ -2005,9 +1980,40 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 			references = null;
 		}
 
+		synchronized SortedMap<Address, List<Reference>> getFunctionDataReferences() {
+
+			if (references != null) {
+				return references;
+			}
+
+			references = getSortedVariableReferences(cachedFunction);
+			return references;
+		}
+
+		synchronized List<Variable> getVariables(Address address) {
+
+			if (variablesByAddress != null) {
+				return variablesByAddress.get(address);
+			}
+
+			Map<Address, List<Variable>> map =
+				LazyMap.lazyMap(new HashMap<>(), () -> new ArrayList<>());
+
+			for (Symbol s : symbolMgr.getSymbols(cachedFunction.getID())) {
+				if (!s.getAddress().equals(address)) {
+					continue;
+				}
+				Variable v = (Variable) s.getObject();
+				map.get(address).add(v);
+			}
+
+			variablesByAddress = map;
+			return variablesByAddress.get(address);
+		}
+
 		private SortedMap<Address, List<Reference>> getSortedVariableReferences(Function function) {
 			SortedMap<Address, List<Reference>> newReferencesList =
-				new TreeMap<Address, List<Reference>>();
+				LazySortedMap.lazySortedMap(new TreeMap<>(), () -> new ArrayList<>());
 
 			ReferenceIterator refIter = new FromRefIterator(function.getBody());
 			while (refIter.hasNext()) {
@@ -2016,15 +2022,30 @@ public class ReferenceDBManager implements ReferenceManager, ManagerDB, ErrorHan
 				if (referenceType.isFlow() && !referenceType.isIndirect()) {
 					continue;
 				}
+
 				Address toAddr = ref.getToAddress();
-				List<Reference> refList = newReferencesList.get(toAddr);
+				newReferencesList.get(toAddr).add(ref);
-				if (refList == null) {
-					refList = new ArrayList<Reference>();
-					newReferencesList.put(toAddr, refList);
-				}
-				refList.add(ref);
 			}
 			return newReferencesList;
 		}
 	}
+
+	private class Scope {
+
+		int outOfScopeOffset;
+		int firstUseOffset;
+
+		Scope(int firstUseOffset, int outOfScopeOffset) {
+			this.firstUseOffset = firstUseOffset;
+			this.outOfScopeOffset = outOfScopeOffset;
+		}
+
+		int getFirstUseOffset() {
+			return firstUseOffset;
+		}
+
+		int getOutOfScopeOffset() {
+			return outOfScopeOffset;
+		}
+	}
 }