GP-4477 Speed improvements for binaries with large address tables

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/codebrowser/MarkAndSelectionAction.java

@@ -103,7 +103,7 @@ public class MarkAndSelectionAction extends ToggleDockingAction {
 		if (markedLocation != null) {
 			Address address = markedLocation.getByteAddress();
 			menuName = "Create Selection from " + address;
-			description = "Create seletion from marked location: " + address;
+			description = "Create selection from marked location: " + address;
 			icon = armedIcon;
 		}
 

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/disassembler/AddressTable.java

@@ -17,6 +17,8 @@ package ghidra.app.plugin.core.disassembler;
 
 import java.util.*;
 
+import org.apache.commons.lang3.ArrayUtils;
+
 import ghidra.app.cmd.disassemble.DisassembleCommand;
 import ghidra.app.cmd.function.CreateFunctionCmd;
 import ghidra.app.cmd.label.AddLabelCmd;
@@ -95,6 +97,29 @@ public class AddressTable {
 		this.skipAmount = skipAmount;
 		this.shiftedAddr = shiftedAddr;
 	}
+	
+
+	/**
+	 * Create a new address table from any remaining table entries starting at startPos
+	 * 
+	 * @param startPos new start position in list of existing table entries
+	 * @return new address table if any elements left, null otherwise
+	 */
+	public AddressTable newRemainingAddressTable(int startPos) {
+		if (topIndexAddress != null) {
+			return null;
+		}
+		if (startPos <= 0 || startPos >= tableElements.length) {
+			return null;
+		}
+		int byteLength = getByteLength(0, startPos - 1, false);
+		Address newTop = topAddress.add(byteLength);
+
+		Address newElementArray[] =
+			ArrayUtils.subarray(tableElements, startPos, tableElements.length);
+
+		return new AddressTable(newTop, newElementArray, addrSize, skipAmount, shiftedAddr);
+	}
 
 	/**
 	 * @return the first address of the address table
@@ -1098,15 +1123,6 @@ public class AddressTable {
 
 				// See if the tested address is contained in memory
 				if (!memory.contains(testAddr)) {
-//					if (addrSize == 8) {  // don't try to look up in database, it polutes the key lookup in the DB
-//						break;
-//					}
-//
-//					// TODO: what is this doing?  doesn't seem like anything, always breaks!
-//					Symbol syms[] = program.getSymbolTable().getSymbols(testAddr);
-//					if (syms == null || syms.length == 0 || syms[0].getSource() == SourceType.DEFAULT) {
-//						break;
-//					}
 					break;
 				}
 
@@ -1207,7 +1223,7 @@ public class AddressTable {
 				Data data = definedData.next();
 				// no data found or past end of table
 				Address dataAddr = data.getMinAddress();
-				if (data == null || dataAddr.compareTo(endAddr) > 0) {
+				if (dataAddr.compareTo(endAddr) > 0) {
 					break;
 				}
 				// data found at start of pointer
@@ -1217,6 +1233,12 @@ public class AddressTable {
 						continue;
 					}
 				}
+				
+				// undefined data is OK, could be a pointer
+				if (data.getDataType() instanceof Undefined) {
+					continue;
+				}
+				
 				// data intersects, calculate valid entries and stop looking
 				if (pointerSet.intersects(dataAddr, data.getMaxAddress())) {
 					count = (int) (dataAddr.subtract(topAddr) / (addrSize + skipAmount));

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/disassembler/AddressTableAnalyzer.java

@@ -23,8 +23,10 @@ import ghidra.app.util.PseudoDisassembler;
 import ghidra.app.util.importer.MessageLog;
 import ghidra.framework.options.Options;
 import ghidra.program.model.address.*;
+import ghidra.program.model.data.*;
 import ghidra.program.model.listing.*;
 import ghidra.program.model.mem.*;
+import ghidra.util.exception.CancelledException;
 import ghidra.util.task.TaskMonitor;
 
 /**
@@ -84,12 +86,15 @@ public class AddressTableAnalyzer extends AbstractAnalyzer {
 	private boolean relocationGuideEnabled = OPTION_DEFAULT_RELOCATION_GUIDE_ENABLED;
 
 	private boolean allowOffcutReferences = OPTION_DEFAULT_ALLOW_OFFCUT_REFERENCES;
-
-	private boolean ignoreBookmarks = false;
+	
+	private static final String ADDRESS_TABLE_BOOKMARK_TYPENAME = "Address Table";
 
 	// true if the processor uses Address low bit to refer to code
 	private boolean processorHasLowBitCode = false;
 
+	private long lastID;
+
+
 	public AddressTableAnalyzer() {
 		super("Create Address Tables", DESCRIPTION, AnalyzerType.BYTE_ANALYZER);
 		setPriority(AnalysisPriority.DATA_TYPE_PROPOGATION.before());
@@ -116,13 +121,21 @@ public class AddressTableAnalyzer extends AbstractAnalyzer {
 
 	@Override
 	public boolean added(Program program, AddressSetView addrSet, TaskMonitor monitor,
-			MessageLog log) {
+			MessageLog log) throws CancelledException {
 		AutoAnalysisManager mgr = AutoAnalysisManager.getAnalysisManager(program);
 
+		// remove memory blocks that should not be searched
 		addrSet = removeNonSearchableMemory(program, addrSet);
 
+		// remove defined locations that can't be part of an address table
+		// only do this the first time entering
+		long id = mgr.getProgram().getCurrentTransactionInfo().getID();
+		if (id != lastID) {
+			lastID = id;
+			addrSet = removeDefined(program, addrSet);
+		}
+		
 		if (addrSet.isEmpty()) {
-			ignoreBookmarks = false;
 			return true;
 		}
 
@@ -142,7 +155,10 @@ public class AddressTableAnalyzer extends AbstractAnalyzer {
 
 		AddressIterator addrIter = addrSet.getAddresses(true);
 
-		while (addrIter.hasNext() && !monitor.isCancelled()) {
+		boolean didTable = false;
+		while (!didTable && addrIter.hasNext()) {
+			monitor.checkCancelled();
+			
 			addrCount++;
 			monitor.setProgress(addrCount);
 			Address start = addrIter.next();
@@ -155,90 +171,28 @@ public class AddressTableAnalyzer extends AbstractAnalyzer {
 			if ((addrCount % 2048) == 1) {
 				monitor.setMessage("Analyze Tables " + start);
 			}
-			AddressTable tableEntry =
+
+			AddressTable addressTable =
 				AddressTable.getEntry(program, start, monitor, true, minimumTableSize, ptrAlignment,
 					0, AddressTable.MINIMUM_SAFE_ADDRESS, relocationGuideEnabled);
-			if (tableEntry != null) {
-				int tableLen = checkTable(tableEntry, program);
-				if (tableLen < minimumTableSize) {
-					continue;
-				}
-
-				Bookmark bookmark = program.getBookmarkManager()
-						.getBookmark(tableEntry.getTopAddress(), BookmarkType.ANALYSIS,
-							"Address Table");
-
-				// nothing to see here, already done.
-				if (!ignoreBookmarks && bookmark != null) {
-					// skip over this table, assumes the table is good as found...
-					//   This is likely a good assumption, since the table analysis already did this area.
-					Address nextAddr = start.add(tableEntry.getByteLength());
-					addrIter = addrSet.getAddresses(nextAddr, true);
-					maxAddr = nextAddr;
-					continue;
-				}
-
-				// make the table
-				tableEntry.makeTable(program, 0, tableLen - 1, autoLabelTable, false);
-
-				Address startTable = tableEntry.getTopAddress();
-				Address endTable = start.add(tableEntry.getByteLength());
-				mgr.codeDefined(new AddressSet(startTable, endTable));
-
-				// put info bookmark in
-				if (createBookmarksEnabled) {
-					program.getBookmarkManager()
-							.setBookmark(tableEntry.getTopAddress(), BookmarkType.ANALYSIS,
-								"Address Table", "Address table[" +
-									tableEntry.getNumberAddressEntries() + "] created");
-				}
-
-				// if all are valid code, disassemble
-				List<Address> validCodeList = tableEntry.getFunctionEntries(program, 0);
-				if (validCodeList != null &&
-					validCodeList.size() >= tableEntry.getNumberAddressEntries()) {
-					AddressSet validCodeSet = new AddressSet();
-					AddressSet validFuncSet = new AddressSet();
-					for (Address addr : validCodeList) {
-						// set target context correctly. Target address will get
-						// aligned in DisassembleCmd
-						PseudoDisassembler.setTargetContextForDisassembly(program, addr);
-
-						// even though they are valid code, don't do them if
-						// there is already code there.
-						if (program.getListing().getCodeUnitContaining(addr) == null) {
-							validCodeSet.addRange(addr, addr);
-						}
-						// For Now, Never make functions from address tables,
-						// Could later be an option
-						// if (pdis.isValidSubroutine(addr, true)) {
-						// validFuncSet.addRange(addr, addr);
-						// }
-					}
-					// disassemble valid code
-					if (!validCodeSet.isEmpty()) {
-						mgr.disassemble(validCodeSet,
-							AnalysisPriority.DATA_TYPE_PROPOGATION.before());
-					}
-					// if valid functions, schedule much later, so switch table analysis can pick it up.
-					if (!validFuncSet.isEmpty()) {
-						//  For Now, Never make functions from address tables, Could later be an option
-						//		mgr.createFunction(validFuncSet, true,
-						//		AnalysisPriority.DATA_TYPE_PROPOGATION.getNext());
-					}
-				}
-
-				// jump the address iterator by the size of the table entry
-				int tableByteLen = tableEntry.getByteLength(0, tableLen - 1, false);
-				addrCount += tableByteLen;
-				addrIter = skipBytes(addrIter, addrSet, start, tableByteLen);
-				try {
-					maxAddr = maxAddr.addNoWrap(tableByteLen - 1);
-				}
-				catch (AddressOverflowException e) {
-					// pass
-				}
-				break;
+			
+			if (addressTable == null) {
+				continue;
+			}
+			
+			didTable = processAddressTable(addressTable, program, mgr, monitor);
+			
+			// Assumes that the entire table found was processed
+			long tableByteLen = addressTable.getByteLength();
+			addrCount += tableByteLen;
+			
+			// update maximum addresses consumed by table
+			try {
+				maxAddr = start.addNoWrap(tableByteLen-1);
+				addrIter = addrSet.getAddresses(maxAddr.addNoWrap(1), true);
+			}
+			catch (AddressOverflowException e) {
+				// ignore, just keep using current iterator
 			}
 		} // end of while (addrIt.hasNext())
 
@@ -250,16 +204,148 @@ public class AddressTableAnalyzer extends AbstractAnalyzer {
 		if (!set.isEmpty()) {
 			mgr.scheduleOneTimeAnalysis(this, set);
 		}
-		else {
-			// don't ignore address table bookmarks anymore
-			ignoreBookmarks = false;
-		}
 		return true;
 	}
 
+	/**
+	 * Process the table of addresses.  This could create multiple address tables if the
+	 * table has an entry that breaks the table.
+	 * 
+	 * Note: current algorithm processes the entire table, if this changes, code that
+	 *       assumes this must be re-factored to know how many bytes were consumed
+	 * 
+	 * @param addressTable address table
+	 * @param program program
+	 * @param mgr autoAnalysis manager
+	 * @param monitor monitor for checking if table creation canceled
+	 * @return true if a table was actually made, false otherwise
+	 * @throws CancelledException if use cancels
+	 */
+	private boolean processAddressTable(AddressTable addressTable, Program program, AutoAnalysisManager mgr,
+			TaskMonitor monitor) throws CancelledException {
+		boolean didTable = false;
+		
+		while (addressTable != null) {
+			Address startTableAddr = addressTable.getTopAddress();
+
+			int tableLen = checkTable(addressTable, program, monitor);
+			
+			// check if there is already an address table bookmark here
+			Bookmark bookmark = program.getBookmarkManager()
+					.getBookmark(addressTable.getTopAddress(), BookmarkType.ANALYSIS,
+						ADDRESS_TABLE_BOOKMARK_TYPENAME);
+
+			// if table too small, or bookmark already here
+			// - skip the bad tableLen entries
+			// - continue with any additional entries in the table
+			if (bookmark != null || tableLen < minimumTableSize) {
+				// There might be more to the table. Get a new smaller table.
+				// This will skip one element in the table, that is assumed to have
+				// broken the table up, thus (tableLen+1)
+				//
+				// table entry will be null if no more table entries beyond tableLen+1
+				addressTable = addressTable.newRemainingAddressTable(tableLen+1);
+				continue;
+			}
+
+			// make the table
+			addressTable.makeTable(program, 0, tableLen - 1, autoLabelTable, false);
+
+			int tableByteLen = addressTable.getByteLength(0, tableLen - 1, false);
+			Address endTableAddr = startTableAddr.add(tableByteLen-1);
+			mgr.codeDefined(new AddressSet(startTableAddr, endTableAddr));
+
+			// put info bookmark in
+			if (createBookmarksEnabled) {
+				program.getBookmarkManager()
+						.setBookmark(addressTable.getTopAddress(), BookmarkType.ANALYSIS,
+							ADDRESS_TABLE_BOOKMARK_TYPENAME, "Address table[" + tableLen + "] created");
+			}
+
+			// if all are valid code, disassemble
+			List<Address> validCodeList = addressTable.getFunctionEntries(program, 0);
+			if (validCodeList != null &&
+				validCodeList.size() >= addressTable.getNumberAddressEntries()) {
+				AddressSet validCodeSet = new AddressSet();
+				AddressSet validFuncSet = new AddressSet();
+				for (Address addr : validCodeList) {
+					// set target context correctly. Target address will get
+					// aligned in DisassembleCmd
+					PseudoDisassembler.setTargetContextForDisassembly(program, addr);
+
+					// even though they are valid code, don't do them if
+					// there is already code there.
+					if (program.getListing().getCodeUnitContaining(addr) == null) {
+						validCodeSet.addRange(addr, addr);
+					}
+					// For Now, Never make functions from address tables,
+					// Could later be an option
+					// if (pdis.isValidSubroutine(addr, true)) {
+					// validFuncSet.addRange(addr, addr);
+					// }
+				}
+				// disassemble valid code
+				if (!validCodeSet.isEmpty()) {
+					mgr.disassemble(validCodeSet,
+						AnalysisPriority.DATA_TYPE_PROPOGATION.before());
+				}
+				// if valid functions, schedule much later, so switch table analysis can pick it up.
+				if (!validFuncSet.isEmpty()) {
+					//  For Now, Never make functions from address tables, Could later be an option
+					//		mgr.createFunction(validFuncSet, true,
+					//		AnalysisPriority.DATA_TYPE_PROPOGATION.getNext());
+				}
+			}
+			
+			// There might be more to the table. Get a new smaller table.
+			// This will skip one element in the table, that is assumed to have
+			// broken the table up, thus (tableLen+1)
+			//
+			// table entry will be null if no more table entries beyond tableLen+1
+			addressTable = addressTable.newRemainingAddressTable(tableLen+1);
+			didTable = true;
+		}
+		
+		return didTable;
+	}
+
+	/**
+	 * Remove any addresses from set where instructions or defined data exists that would
+	 * get filtered during address table creation.
+	 */
+	private AddressSetView removeDefined(Program program, AddressSetView addrSet) {
+		AddressSet subSet = new AddressSet();
+
+		// find defined data that should be removed
+		DataIterator definedData = program.getListing().getDefinedData(addrSet, true);
+		for (Data data : definedData) {
+			DataType dataType = data.getDataType();			
+			if (dataType instanceof Undefined) {
+				continue;
+			}
+			
+			if (data.isPointer()) {
+				continue;
+			}
+
+			// anything else, add to subset to be removed
+			subSet.add(data.getMinAddress(), data.getMaxAddress());
+		}
+		addrSet = addrSet.subtract(subSet);
+		
+		// Find defined instructions that should be removed
+		subSet = new AddressSet();
+		InstructionIterator instructions = program.getListing().getInstructions(addrSet, true);
+		for (Instruction instruction : instructions) {
+			subSet.add(instruction.getMinAddress(), instruction.getMaxAddress());
+		}
+		addrSet = addrSet.subtract(subSet);
+		
+		return addrSet;
+	}
+
 	private AddressSetView removeNonSearchableMemory(Program program, AddressSetView addrSet) {
 		// get rid of any non-initialized blocks
-		ignoreBookmarks = ignoreBookmarks | addrSet.hasSameAddresses(program.getMemory());
 
 		addrSet = addrSet.intersect(program.getMemory().getLoadedAndInitializedAddressSet());
 
@@ -283,21 +369,32 @@ public class AddressTableAnalyzer extends AbstractAnalyzer {
 	}
 
 	/**
-	 * @param tableEntry
-	 * @param program
-	 * @return number of entries in table before hitting entry that overlaps a string
+	 * Check the table for consistency and return the number of good entries before a bad
+	 * entry is found.
+	 * <ul>
+	 * <li> check possible strings</li>
+	 * <li> pointers that jump all over memory</li>
+	 * <li> pointers to offcut instructions</li>
+	 * <li> allow offcut strings</li>
+	 * </ul>
+	 * 
+	 * @param tableEntry address table to check
+	 * @param program program
+	 * @return number of entries in table before hitting an inconsistent entry
+	 * @throws CancelledException if user cancels
 	 */
-	private int checkTable(AddressTable tableEntry, Program program) {
+	private int checkTable(AddressTable tableEntry, Program program, TaskMonitor monitor) throws CancelledException {
 		// search for unicode strings first.
 		//   don't create an address table that overlaps a unicode string.
 		AddressSetView addrSet = tableEntry.getTableBody();
-		AddressSet possibleStrings = findPossibleStrings(program, addrSet);
+		AddressSet possibleStrings = findPossibleStrings(program, addrSet, monitor);
 
 		// trim from the first offcut entry on
 		Address start = tableEntry.getTopAddress();
 		int tableLen = tableEntry.getNumberAddressEntries();
 		Address addrs[] = tableEntry.getTableElements();
 		for (int i = 0; i < tableLen; i++) {
+			monitor.checkCancelled();
 			Address tableEntryAddr = start.add(i * 4);
 			Address targetAddr = addrs[i];
 			if (possibleStrings.contains(tableEntryAddr)) {
@@ -323,6 +420,10 @@ public class AddressTableAnalyzer extends AbstractAnalyzer {
 				continue;
 			}
 			boolean atStartOfCU = cu.getMinAddress().equals(targetAddr);
+			// always allow offcuts to strings
+			if (cu instanceof Data data && data.getDataType() instanceof AbstractStringDataType) {
+				continue;
+			}
 			if (!allowOffcutReferences && !atStartOfCU) {
 				// if the processor uses low bit to reference instructions
 				//  allow offcut to an instruction by 1
@@ -334,7 +435,7 @@ public class AddressTableAnalyzer extends AbstractAnalyzer {
 		return tableLen;
 	}
 
-	private AddressSet findPossibleStrings(Program program, AddressSetView addrSet) {
+	private AddressSet findPossibleStrings(Program program, AddressSetView addrSet, TaskMonitor monitor) throws CancelledException {
 		AddressSet possibleStrSet = new AddressSet();
 		Memory memory = program.getMemory();
 
@@ -346,6 +447,8 @@ public class AddressTableAnalyzer extends AbstractAnalyzer {
 
 		while (addrIter.hasNext()) {
 			Address start = addrIter.next();
+			
+			monitor.checkCancelled();
 
 			// skip over anything that smells like a unicode string
 			//

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/exporter/ExporterDialog.java

@@ -557,7 +557,7 @@ public class ExporterDialog extends DialogComponentProvider implements AddressFa
 			}
 
 			// Program selection only relavent if isFrontEndPlugin() is false
-			ProgramSelection selection = getApplicableProgramSeletion();
+			ProgramSelection selection = getApplicableProgramSelection();
 			File outputFile = getSelectedOutputFile();
 
 			try {
@@ -597,7 +597,7 @@ public class ExporterDialog extends DialogComponentProvider implements AddressFa
 		}
 	}
 
-	private ProgramSelection getApplicableProgramSeletion() {
+	private ProgramSelection getApplicableProgramSelection() {
 		if (selectionCheckBox != null && selectionCheckBox.isSelected()) {
 			return currentSelection;
 		}