GP-4535 Corrected storage of default context on disassembly errors and switch table targets. Improves disassembly for programs with no stored context as context table will be empty.

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/analysis/ConstantPropagationAnalyzer.java

@@ -66,7 +66,7 @@ public class ConstantPropagationAnalyzer extends AbstractAnalyzer {
 	protected static final String MAX_THREAD_COUNT_OPTION_NAME = "Max Threads";
 	protected static final String MAX_THREAD_COUNT_OPTION_DESCRIPTION =
 		"Maximum threads for constant propagation.  Too many threads causes thrashing in DB.";
-	protected static final int MAX_THREAD_COUNT_OPTION_DEFAULT_VALUE = 2;
+	protected static final int MAX_THREAD_COUNT_OPTION_DEFAULT_VALUE = 1;
 
 	protected static final String MIN_KNOWN_REFADDRESS_OPTION_NAME = "Min absolute reference";
 	protected static final String MIN_KNOWN_REFADDRESS_OPTION_DESCRIPTION =

Ghidra/Features/Decompiler/src/main/java/ghidra/app/cmd/function/DecompilerSwitchAnalysisCmd.java

@@ -184,25 +184,12 @@ public class DecompilerSwitchAnalysisCmd extends BackgroundCommand<Program> {
 				if (disSetList.contains(caseStart)) {
 					continue;
 				}
-				if (switchContext != null) {
-					try {
-						// Combine flowed switch context with context register value at case address
-						RegisterValue curContext =
-							programContext.getRegisterValue(baseContextRegister, caseStart);
-						if (curContext != null) {
-							curContext = curContext.combineValues(switchContext);
-
-							// lay down the new merged context
-							programContext.setRegisterValue(caseStart, caseStart, curContext);
-						}
-						else {
-							programContext.setRegisterValue(caseStart, caseStart, switchContext);
-						}
-					}
-					catch (ContextChangeException e) {
-						// This can occur when two or more threads are working on the same function
-						continue;
-					}
+				try {
+					setSwitchTargetContext(programContext, caseStart, switchContext);
+				}
+				catch (ContextChangeException e) {
+					// This can occur when two or more threads are working on the same function
+					continue;
 				}
 				disSetList.add(caseStart);
 			}
@@ -229,6 +216,26 @@ public class DecompilerSwitchAnalysisCmd extends BackgroundCommand<Program> {
 		}
 	}
 
+	private void setSwitchTargetContext(ProgramContext programContext, Address targetStart, RegisterValue switchContext) throws ContextChangeException {
+		if (switchContext == null) {
+			return;
+		}
+		
+		// Combine flowed switch context with context register value at case address
+		RegisterValue curContext =
+			programContext.getNonDefaultValue(switchContext.getRegister(), targetStart);
+		if (curContext != null) {
+			switchContext = curContext.combineValues(switchContext);
+		}
+		
+		if (switchContext == null || !switchContext.hasAnyValue()) {
+			return;
+		}
+
+		// only store if different than what is already there, which could be a default value
+		program.getProgramContext().setRegisterValue(targetStart, targetStart, switchContext);
+	}
+
 	private void labelSwitch(JumpTable table, TaskMonitor monitor) throws CancelledException {
 		AddLabelCmd tableNameLabel =
 			new AddLabelCmd(table.getSwitchAddress(), "switchD", SourceType.ANALYSIS);

Ghidra/Framework/DB/src/main/java/db/Table.java

@@ -583,7 +583,7 @@ public class Table {
 	 * @return record count
 	 */
 	public int getRecordCount() {
-		return tableRecord.getRecordCount();
+		return (tableRecord == null ? 0 : tableRecord.getRecordCount());
 	}
 
 	/**

Ghidra/Framework/DB/src/main/java/db/TableRecord.java

@@ -250,7 +250,7 @@ class TableRecord implements Comparable<TableRecord> {
 	 * @return table's record count
 	 */
 	int getRecordCount() {
-		return record.getIntValue(RECORD_COUNT_COLUMN);
+		return (record == null ? 0 : record.getIntValue(RECORD_COUNT_COLUMN));
 	}
 
 	/**

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/util/AddressRangeMapDB.java

@@ -21,6 +21,7 @@ import java.util.List;
 
 import db.*;
 import db.util.ErrorHandler;
+import generic.stl.Pair;
 import ghidra.program.database.map.AddressKeyRecordIterator;
 import ghidra.program.database.map.AddressMap;
 import ghidra.program.model.address.*;
@@ -85,9 +86,8 @@ public class AddressRangeMapDB implements DBListener {
 	private Schema rangeMapSchema;
 	private Table rangeMapTable;
 
-	// caching
-	private Field lastValue;
-	private AddressRange lastRange;
+	// caching, single value, so safer to check
+	private Pair<AddressRange, Field> lastValue;
 
 	private int modCount;
 
@@ -136,10 +136,16 @@ public class AddressRangeMapDB implements DBListener {
 	 * @throws DuplicateNameException if there is already range map with that name
 	 */
 	public boolean setName(String newName) throws DuplicateNameException {
-		String newTableName = RANGE_MAP_TABLE_PREFIX + newName;
-		if (rangeMapTable == null || rangeMapTable.setName(newTableName)) {
-			tableName = newTableName;
-			return true;
+		lock.acquire();
+		try {
+			String newTableName = RANGE_MAP_TABLE_PREFIX + newName;
+			if (rangeMapTable == null || rangeMapTable.setName(newTableName)) {
+				tableName = newTableName;
+				return true;
+			}
+		}
+		finally {
+			lock.release();
 		}
 		return false;
 	}
@@ -149,13 +155,8 @@ public class AddressRangeMapDB implements DBListener {
 	 * @return true if this map is empty
 	 */
 	public boolean isEmpty() {
-		lock.acquire();
-		try {
-			return rangeMapTable == null || rangeMapTable.getRecordCount() == 0;
-		}
-		finally {
-			lock.release();
-		}
+		Table localTable = rangeMapTable;
+		return localTable == null || localTable.getRecordCount() == 0;
 	}
 
 	/**
@@ -165,7 +166,8 @@ public class AddressRangeMapDB implements DBListener {
 	 * @return record count
 	 */
 	public int getRecordCount() {
-		return rangeMapTable != null ? rangeMapTable.getRecordCount() : 0;
+		Table localTable = rangeMapTable;
+		return localTable == null ? 0 : localTable.getRecordCount();
 	}
 
 	/**
@@ -174,23 +176,26 @@ public class AddressRangeMapDB implements DBListener {
 	 * @return value or null no value exists
 	 */
 	public Field getValue(Address address) {
+		if (isEmpty()) {
+			return null;
+		}
+		// check last cached range
+		Pair<AddressRange, Field> localValue = lastValue;
+		if (localValue != null && localValue.first.contains(address)) {
+			return localValue.second;
+		}
 		lock.acquire();
 		try {
 			if (rangeMapTable == null) {
 				return null;
 			}
-			// check last cached range
-			if (lastRange != null && lastRange.contains(address)) {
-				return lastValue;
-			}
-
 			DBRecord record = findRecordContaining(address);
 			List<AddressRange> ranges = getRangesForRecord(record);
 			for (AddressRange range : ranges) {
 				if (range.contains(address)) {
-					lastRange = range;
-					lastValue = record.getFieldValue(VALUE_COL);
-					return lastValue;
+					Field fieldValue = record.getFieldValue(VALUE_COL);
+					lastValue = new Pair<AddressRange, Field>(range, fieldValue);
+					return fieldValue;
 				}
 			}
 		}
@@ -214,16 +219,19 @@ public class AddressRangeMapDB implements DBListener {
 	 * @throws IllegalArgumentException if the end address is greater then the start address
 	 */
 	public void paintRange(Address startAddress, Address endAddress, Field value) {
+		if (value == null && isEmpty()) {
+			return;
+		}
 		AddressRange.checkValidRange(startAddress, endAddress);
 		lock.acquire();
 		try {
+			if (value == null && rangeMapTable == null) {
+				return;
+			}
 			clearCache();
 			++modCount;
 
 			if (rangeMapTable == null) {
-				if (value == null) {
-					return;
-				}
 				createTable();
 			}
 			doPaintRange(startAddress, endAddress, value);
@@ -246,7 +254,7 @@ public class AddressRangeMapDB implements DBListener {
 	 */
 	public void moveAddressRange(Address fromAddr, Address toAddr, long length, TaskMonitor monitor)
 			throws CancelledException {
-		if (length <= 0 || rangeMapTable == null) {
+		if (length <= 0 || isEmpty()) {
 			return;
 		}
 
@@ -254,6 +262,9 @@ public class AddressRangeMapDB implements DBListener {
 		AddressRangeMapDB tmpMap = null;
 		lock.acquire();
 		try {
+			if (rangeMapTable == null) {
+				return;
+			}
 			tmpDb = dbHandle.getScratchPad();
 			tmpMap = new AddressRangeMapDB(tmpDb, addressMap, lock, "TEMP", errHandler, valueField,
 				indexed);
@@ -310,9 +321,15 @@ public class AddressRangeMapDB implements DBListener {
 	 * @return set of addresses where a values has been set
 	 */
 	public AddressSet getAddressSet() {
+		AddressSet set = new AddressSet();
+		if (isEmpty()) {
+			return set;
+		}
 		lock.acquire();
 		try {
-			AddressSet set = new AddressSet();
+			if (rangeMapTable == null) {
+				return set;
+			}
 			AddressRangeIterator addressRanges = getAddressRanges();
 			for (AddressRange addressRange : addressRanges) {
 				set.add(addressRange);
@@ -331,11 +348,15 @@ public class AddressRangeMapDB implements DBListener {
 	 */
 	public AddressSet getAddressSet(Field value) {
 		AddressSet set = new AddressSet();
-		if (rangeMapTable == null) {
+		if (isEmpty()) {
 			return set;
 		}
 		lock.acquire();
 		try {
+			if (rangeMapTable == null) {
+				return set;
+			}
+			
 			RecordIterator it = rangeMapTable.indexIterator(VALUE_COL, value, value, true);
 			while (it.hasNext()) {
 				DBRecord record = it.next();
@@ -357,7 +378,7 @@ public class AddressRangeMapDB implements DBListener {
 	 * @return AddressRangeIterator that iterates over all occupied ranges in the map
 	 */
 	public AddressRangeIterator getAddressRanges() {
-		if (rangeMapTable == null) {
+		if (isEmpty()) {
 			return new EmptyAddressRangeIterator();
 		}
 		try {
@@ -377,7 +398,7 @@ public class AddressRangeMapDB implements DBListener {
 	 * given start address
 	 */
 	public AddressRangeIterator getAddressRanges(Address startAddress) {
-		if (rangeMapTable == null) {
+		if (isEmpty()) {
 			return new EmptyAddressRangeIterator();
 		}
 		try {
@@ -398,7 +419,7 @@ public class AddressRangeMapDB implements DBListener {
 	 * given start address
 	 */
 	public AddressRangeIterator getAddressRanges(Address startAddress, Address endAddr) {
-		if (rangeMapTable == null) {
+		if (isEmpty()) {
 			return new EmptyAddressRangeIterator();
 		}
 		try {
@@ -492,14 +513,13 @@ public class AddressRangeMapDB implements DBListener {
 	 * @return an address range that contains the given address and has all the same value
 	 */
 	public AddressRange getAddressRangeContaining(Address address) {
-
+		// check cache
+		Pair<AddressRange, Field> localValue = lastValue;
+		if (localValue != null && localValue.first.contains(address)) {
+			return localValue.first;
+		}
 		lock.acquire();
 		try {
-			// check cache
-			if (lastRange != null && lastRange.contains(address)) {
-				return lastRange;
-			}
-
 			// look for a stored value range that contains that address
 			AddressRange range = findValueRangeContainingAddress(address);
 			if (range == null) {
@@ -522,8 +542,8 @@ public class AddressRangeMapDB implements DBListener {
 		List<AddressRange> rangesForRecord = getRangesForRecord(record);
 		for (AddressRange range : rangesForRecord) {
 			if (range.contains(address)) {
-				lastRange = range;
-				lastValue = record.getFieldValue(VALUE_COL);
+				Field fieldValue = record.getFieldValue(VALUE_COL);
+				lastValue = new Pair<AddressRange, Field>(range, fieldValue);
 				return range;
 			}
 		}
@@ -786,14 +806,7 @@ public class AddressRangeMapDB implements DBListener {
 	 * Clears the "last range" cache
 	 */
 	private void clearCache() {
-		lock.acquire();
-		try {
-			lastRange = null;
-			lastValue = null;
-		}
-		finally {
-			lock.release();
-		}
+		lastValue = null;
 	}
 
 	DBRecord getAddressWrappingRecord() throws IOException {

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/disassemble/Disassembler.java

@@ -1389,7 +1389,12 @@ public class Disassembler implements DisassemblerConflictHandler {
 			RegisterValue contextValue = conflict.getParseContextValue();
 			if (contextValue != null) {
 				try {
-					program.getProgramContext().setRegisterValue(address, address, contextValue);
+					RegisterValue curContextValue = program.getProgramContext().getRegisterValue(contextValue.getRegister(), address);
+
+					// only store if different than what is already there, which could be a default value
+					if (!contextValue.equals(curContextValue)) {
+						program.getProgramContext().setRegisterValue(address, address, contextValue);
+					}
 				}
 				catch (ContextChangeException e) {
 					// ignore - existing instruction likely blocked context modification

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/util/AbstractStoredProgramContext.java

@@ -143,7 +143,7 @@ abstract public class AbstractStoredProgramContext extends AbstractProgramContex
 		}
 
 		RegisterValueStore store = map.get(register.getBaseRegister());
-		if (store == null) {
+		if (store == null || store.isEmpty()) {
 			return null;
 		}
 
@@ -153,8 +153,8 @@ abstract public class AbstractStoredProgramContext extends AbstractProgramContex
 	@Override
 	public AddressRangeIterator getRegisterValueAddressRanges(Register register) {
 		RegisterValueStore store = registerValueMap.get(register.getBaseRegister());
-		if (store == null) {
-			return new AddressSet().getAddressRanges();
+		if (store == null || store.isEmpty()) {
+			return new EmptyAddressRangeIterator();
 		}
 		return new RegisterAddressRangeIterator(register, store.getAddressRangeIterator(),
 			registerValueMap);
@@ -163,7 +163,7 @@ abstract public class AbstractStoredProgramContext extends AbstractProgramContex
 	@Override
 	public AddressRange getRegisterValueRangeContaining(Register register, Address addr) {
 		RegisterValueStore store = registerValueMap.get(register.getBaseRegister());
-		if (store == null) {
+		if (store == null || store.isEmpty()) {
 			return new AddressRangeImpl(addr, addr);
 		}
 		return store.getValueRangeContaining(addr);
@@ -173,8 +173,8 @@ abstract public class AbstractStoredProgramContext extends AbstractProgramContex
 	public AddressRangeIterator getRegisterValueAddressRanges(Register register, Address start,
 			Address end) {
 		RegisterValueStore store = registerValueMap.get(register.getBaseRegister());
-		if (store == null) {
-			return new AddressSet().getAddressRanges();
+		if (store == null || store.isEmpty()) {
+			return new EmptyAddressRangeIterator();
 		}
 		return new RegisterAddressRangeIterator(register, store.getAddressRangeIterator(start, end),
 			registerValueMap);
@@ -183,8 +183,8 @@ abstract public class AbstractStoredProgramContext extends AbstractProgramContex
 	@Override
 	public AddressRangeIterator getDefaultRegisterValueAddressRanges(Register register) {
 		RegisterValueStore store = defaultRegisterValueMap.get(register.getBaseRegister());
-		if (store == null) {
-			return new AddressSet().getAddressRanges();
+		if (store == null || store.isEmpty()) {
+			return new EmptyAddressRangeIterator();
 		}
 		return new RegisterAddressRangeIterator(register, store.getAddressRangeIterator(),
 			defaultRegisterValueMap);
@@ -194,8 +194,8 @@ abstract public class AbstractStoredProgramContext extends AbstractProgramContex
 	public AddressRangeIterator getDefaultRegisterValueAddressRanges(Register register,
 			Address start, Address end) {
 		RegisterValueStore store = defaultRegisterValueMap.get(register.getBaseRegister());
-		if (store == null) {
-			return new AddressSet().getAddressRanges();
+		if (store == null || store.isEmpty()) {
+			return new EmptyAddressRangeIterator();
 		}
 		return new RegisterAddressRangeIterator(register, store.getAddressRangeIterator(start, end),
 			defaultRegisterValueMap);