Merge remote-tracking branch 'origin/patch'

Ghidra/Configurations/Public_Release/src/global/docs/ChangeHistory.html

@@ -32,6 +32,7 @@
     <li><I>Debugger</I>. Fixed issue with Debugger ignoring JAVA_HOME when launching child JVM. (GP-1143, Issue #3231)</li>
     <li><I>Debugger</I>. Fixed command-reply matching issue when using GDB via SSH. (GP-1153, Issue #3238)</li>
     <li><I>Debugger:Emulator</I>. Fixed bug in Trace Emulation causing ArrayIndexOutOfBoundsExceptions. (GP-1058)</li>
+    <li><I>Decompiler</I>. Fixed issue causing <code>Offset must be between...</code> AddressOutOfBoundsException, when decompiling real-mode x86 programs. (GP-1163, Issue #239, #2948)</li>
     <li><I>Decompiler</I>. The decompiler now shows results when a HighGlobal has no associated symbol reference in the program. (GP-1184)</li>
     <li><I>DWARF</I>. Changed processing to ignore incomplete DWARF parameter lists in Rust binaries. (GP-1121, Issue #3060)</li>
     <li><I>Exporter</I>. The C/C++ Exporter now emits semicolons after function prototypes when using the <B>Create Header File</B> option. (GP-1145, Issue #1644)</li>
@@ -39,12 +40,13 @@
     <li><I>Graphing</I>. Corrected graph magnification behavior when using a high resolution mouse wheel. (GP-1181, Issue #3281, #3284)</li>
     <li><I>GUI</I>. Fixed NullPointerException when Hovering in Decompiler over a function that is not in memory. (GP-1131)</li>
     <li><I>GUI</I>. Fixed bug in <B>Find References to</B> search results that prevented '<' characters from being rendered. (GP-1137, Issue #3217)</li>
-    <li><I>GUI</I>. Fixed issue where duplicate label names could cause the symbol tree to become unstable, evidenced by broken display and scrolling actions. Also, improved grouping algorithm. (GP-1159, Issue #3264)</li>
+    <li><I>GUI</I>. Fixed issue where duplicate label names could cause the symbol tree to become unstable, evidenced by broken display and scrolling actions. Also, improved grouping algorithm. (GP-1159, Issue #3263)</li>
     <li><I>GUI</I>. Fixed <B>Enter</B> key in Set Equates dialog to choose the selected table row. Updated the Function Signature Editor dialog to allow the <B>Cancel</B> key to close the dialog when the focus is in the top text editor. (GP-1162, Issue #3235)</li>
     <li><I>Headless</I>. Fixed a regression in analyzeHeadless.bat that prevented the headless analyzer from running on Windows in some cases. (GP-1156, Issue #3261)</li>
     <li><I>Importer</I>. The MzLoader now populates the relocation table when relocations are performed. (GP-1160)</li>
     <li><I>Importer:ELF</I>. Corrected dynamic GOT/PLT markup problem for images which do not contain section headers.  In cases where image does not define symbols within the PLT, analysis may be relied upon for its disassembly.  ELF Importer's goal is to migrate symbols which may be defined within the PLT to the External symbol space. (GP-1110, Issue #3198)</li>
     <li><I>Importer:Mach-O</I>. The Mach-O importer now correctly interprets indirect symbols as references to symbols within another .dylib. (GP-1120)</li>
+    <li><I>Importer:PE</I>. Improved ControlFlowGuard markup and creation of functions (GP-1179, Issue #1547, #1565)</li>
     <li><I>Processors</I>. Fixed bug in SuperH4 <code>fmov.s</code> pcode. (GP-1152)</li>
     <li><I>Processors</I>. The ARM instruction semantics for the mulitple-single-element forms of the  <code>vld1</code>/<code>vst1</code> vector instructions have been corrected. (GP-1167)</li>
     <li><I>Sleigh</I>. Fixed a string formatting error in the sleigh compiler. (GP-1124, Issue #3168)</li>

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pe/ControlFlowGuard.java

@@ -15,11 +15,15 @@
  */
 package ghidra.app.util.bin.format.pe;
 
+import java.util.ArrayList;
+import java.util.List;
+
+import ghidra.app.cmd.data.CreateArrayCmd;
 import ghidra.app.util.bin.format.pe.LoadConfigDirectory.GuardFlags;
 import ghidra.app.util.importer.MessageLog;
+import ghidra.app.util.opinion.AbstractProgramLoader;
 import ghidra.program.model.address.*;
-import ghidra.program.model.data.DataType;
+import ghidra.program.model.data.*;
-import ghidra.program.model.data.ImageBaseOffset32DataType;
 import ghidra.program.model.listing.Data;
 import ghidra.program.model.listing.Program;
 import ghidra.program.model.mem.Memory;
@@ -37,6 +41,9 @@ import ghidra.util.exception.InvalidInputException;
  * Creator's update. 
  */
 public class ControlFlowGuard {
+	public static String GuardCFFunctionTableName = "GuardCFFunctionTable";
+	public static String GuardCFAddressTakenIatTableName = "GuardCFAddressTakenIatTable";
+	public static String GuardCfgTableEntryName = "GuardCfgTableEntry";
 
 	/**
 	 * Perform markup on the supported ControlFlowGuard and ReturnFlowGuard functions and 
@@ -56,28 +63,29 @@ public class ControlFlowGuard {
 		SymbolTable symbolTable = program.getSymbolTable();
 
 		// ControlFlowGuard
-		markupCfgCheckFunction(lcd, is64bit, space, mem, symbolTable);
+		markupCfgCheckFunction(lcd, program, is64bit, space, mem, symbolTable);
-		markupCfgDispatchFunction(lcd, is64bit, space, mem, symbolTable);
+		markupCfgDispatchFunction(lcd, program, is64bit, space, mem, symbolTable);
 		markupCfgFunctionTable(lcd, program, log);
+		markupCfgAddressTakenIatEntryTable(lcd, program, log);
 
 		// ReturnFlowGuard
-		markupRfgFailureRoutine(lcd, space, symbolTable);
+		markupRfgFailureRoutine(lcd, program, space, symbolTable);
-		markupRfgDefaultFailureRoutine(lcd, is64bit, space, mem, symbolTable);
+		markupRfgDefaultFailureRoutine(lcd, program, is64bit, space, mem, symbolTable);
-		markupRfgDefaultStackPointerFunction(lcd, is64bit, space, mem, symbolTable);
+		markupRfgDefaultStackPointerFunction(lcd, program, is64bit, space, mem, symbolTable);
 	}
 
 	/**
 	 * Performs markup on the ControlFlowGuard check function, if it exists.
 	 * 
 	 * @param lcd The PE LoadConfigDirectory.
+	 * @param program The program.
 	 * @param is64bit True if the PE is 64-bit; false if it's 32-bit.
 	 * @param space The program's address space.
 	 * @param mem The program's memory.
 	 * @param symbolTable The program's symbol table.
 	 */
-	private static void markupCfgCheckFunction(LoadConfigDirectory lcd, boolean is64bit,
+	private static void markupCfgCheckFunction(LoadConfigDirectory lcd, Program program,
-			AddressSpace space,
+			boolean is64bit, AddressSpace space, Memory mem, SymbolTable symbolTable) {
-			Memory mem, SymbolTable symbolTable) {
 
 		if (lcd.getCfgCheckFunctionPointer() == 0) {
 			return;
@@ -88,6 +96,8 @@ public class ControlFlowGuard {
 			Address functionAddr = space.getAddress(
 				is64bit ? mem.getLong(functionPointerAddr) : mem.getInt(functionPointerAddr));
 			symbolTable.createLabel(functionAddr, "_guard_check_icall", SourceType.IMPORTED);
+
+			AbstractProgramLoader.markAsFunction(program, null, functionAddr);
 		}
 		catch (MemoryAccessException | AddressOutOfBoundsException | InvalidInputException e) {
 			Msg.warn(ControlFlowGuard.class, "Unable to label ControlFlowGuard check function.", e);
@@ -98,13 +108,14 @@ public class ControlFlowGuard {
 	 * Performs markup on the ControlFlowGuard dispatch function, if it exists.
 	 * 
 	 * @param lcd The PE LoadConfigDirectory.
+	 * @param program The program.
 	 * @param is64bit True if the PE is 64-bit; false if it's 32-bit.
 	 * @param space The program's address space.
 	 * @param mem The program's memory.
 	 * @param symbolTable The program's symbol table.
 	 */
-	private static void markupCfgDispatchFunction(LoadConfigDirectory lcd, boolean is64bit,
+	private static void markupCfgDispatchFunction(LoadConfigDirectory lcd, Program program,
-			AddressSpace space, Memory mem, SymbolTable symbolTable) {
+			boolean is64bit, AddressSpace space, Memory mem, SymbolTable symbolTable) {
 
 		if (lcd.getCfgDispatchFunctionPointer() == 0) {
 			return;
@@ -115,6 +126,8 @@ public class ControlFlowGuard {
 			Address functionAddr = space.getAddress(
 				is64bit ? mem.getLong(functionPointerAddr) : mem.getInt(functionPointerAddr));
 			symbolTable.createLabel(functionAddr, "_guard_dispatch_icall", SourceType.IMPORTED);
+
+			AbstractProgramLoader.markAsFunction(program, null, functionAddr);
 		}
 		catch (MemoryAccessException | AddressOutOfBoundsException | InvalidInputException e) {
 			Msg.warn(ControlFlowGuard.class, "Unable to label ControlFlowGuard dispatch function.",
@@ -142,22 +155,110 @@ public class ControlFlowGuard {
 			return;
 		}
 
-		try {
 		Address tableAddr =
 			program.getAddressFactory().getDefaultAddressSpace().getAddress(tablePointer);
 
 		// Label the start of the table
-			program.getSymbolTable().createLabel(tableAddr, "GuardCFFunctionTable",
+		try {
-				SourceType.IMPORTED);
+			program.getSymbolTable()
+					.createLabel(tableAddr, GuardCFFunctionTableName, SourceType.IMPORTED);
+		}
+		catch (InvalidInputException e) {
+			Msg.warn(ControlFlowGuard.class, "Unable to label ControlFlowGuard function table.", e);
+		}
 
 		// Each table entry is an RVA (32-bit image base offset), followed by 'n' extra bytes
 		GuardFlags guardFlags = lcd.getCfgGuardFlags();
 		int n = (guardFlags.getFlags() &
 			IMAGE_GUARD_CF_FUNCTION_TABLE_SIZE_MASK) >> IMAGE_GUARD_CF_FUNCTION_TABLE_SIZE_SHIFT;
+
+		// Pre-define base data types used to define table entry data type
+		DataType ibo32 = new ImageBaseOffset32DataType();
+		DataType byteType = ByteDataType.dataType;
+
+		CategoryPath categoryPath = new CategoryPath(CategoryPath.ROOT, "CFG");
+		StructureDataType GuardCfgTableEntryType = (StructureDataType) program.getDataTypeManager()
+				.getDataType(categoryPath, GuardCfgTableEntryName);
+
+		if (GuardCfgTableEntryType == null) {
+			GuardCfgTableEntryType = new StructureDataType(categoryPath, GuardCfgTableEntryName, 0);
+			GuardCfgTableEntryType.setPackingEnabled(false);
+			GuardCfgTableEntryType.add(ibo32, "Offset", "");
+			if (n > 0) {
+				ArrayDataType padType =
+					new ArrayDataType(byteType, n / byteType.getLength(), byteType.getLength());
+				GuardCfgTableEntryType.add(padType, "Pad", "");
+			}
+		}
+
+		CreateArrayCmd cmd = new CreateArrayCmd(tableAddr, (int) functionCount,
+			GuardCfgTableEntryType, GuardCfgTableEntryType.getLength());
+		cmd.applyTo(program);
+
+		Data tableData = program.getListing().getDataAt(tableAddr);
+		createCfgFunctions(program, tableData, log);
+	}
+
+	private static void createCfgFunctions(Program program, Data tableData, MessageLog log) {
+		if (tableData == null) {
+			Msg.warn(ControlFlowGuard.class, "Couldn't find Control Flow Guard tables.");
+			return;
+		}
+
+		if (!tableData.isArray() || (tableData.getNumComponents() < 1)) {
+			Msg.warn(ControlFlowGuard.class, "Control Flow Guard table seems to be empty.");
+			return;
+		}
+
+		for (Address target : getFunctionAddressesFromTable(program, tableData)) {
+			AbstractProgramLoader.markAsFunction(program, null, target);
+		}
+	}
+
+	private static List<Address> getFunctionAddressesFromTable(Program program, Data table) {
+		List<Address> list = new ArrayList<Address>();
+
+		// use the array and ibo data in structure to get a list of functions
+		for (int i = 0; i < table.getNumComponents(); i++) {
+			Data entry = table.getComponent(i);
+			Data iboData = entry.getComponent(0);
+			Object value = iboData.getValue();
+			if (value instanceof Address) {
+				list.add((Address) value);
+			}
+		}
+		return list;
+	}
+
+	/**
+	 * Performs markup on the ControlFlowGuard address taken IAT table, if it exists.
+	 * 
+	 * @param lcd The PE LoadConfigDirectory.
+	 * @param program The program.
+	 * @param log The log.
+	 */
+	private static void markupCfgAddressTakenIatEntryTable(LoadConfigDirectory lcd, Program program,
+			MessageLog log) {
+
+		long tablePointer = lcd.getGuardAddressIatTableTablePointer();
+		long functionCount = lcd.getGuardAddressIatTableCount();
+
+		if (tablePointer == 0 || functionCount <= 0) {
+			return;
+		}
+
+		try {
+			Address tableAddr =
+				program.getAddressFactory().getDefaultAddressSpace().getAddress(tablePointer);
+
+			// Label the start of the table
+			program.getSymbolTable()
+					.createLabel(tableAddr, GuardCFAddressTakenIatTableName, SourceType.IMPORTED);
+			// Each table entry is an RVA (32-bit image base offset)
 			DataType ibo32 = new ImageBaseOffset32DataType();
 			for (long i = 0; i < functionCount; i++) {
-				Data d = PeUtils.createData(program, tableAddr.add(i * (ibo32.getLength() + n)),
+				Data d =
-					ibo32, log);
+					PeUtils.createData(program, tableAddr.add(i * ibo32.getLength()), ibo32, log);
 				if (d == null) {
 					// If we failed to create data on a table entry, just assume the rest will fail
 					break;
@@ -165,7 +266,7 @@ public class ControlFlowGuard {
 			}
 		}
 		catch (AddressOutOfBoundsException | InvalidInputException e) {
-			Msg.warn(ControlFlowGuard.class, "Unable to label ControlFlowGuard function table.", e);
+			Msg.warn(ControlFlowGuard.class, "Unable to label ControlFlowGuard IAT table.", e);
 		}
 	}
 
@@ -173,11 +274,12 @@ public class ControlFlowGuard {
 	 * Performs markup on the ReturnFlowGuard failure routine, if it exists.
 	 * 
 	 * @param lcd The PE LoadConfigDirectory.
+	 * @param program The program
 	 * @param space The program's address space.
 	 * @param symbolTable The program's symbol table.
 	 */
-	private static void markupRfgFailureRoutine(LoadConfigDirectory lcd, AddressSpace space,
+	private static void markupRfgFailureRoutine(LoadConfigDirectory lcd, Program program,
-			SymbolTable symbolTable) {
+			AddressSpace space, SymbolTable symbolTable) {
 
 		if (lcd.getRfgFailureRoutine() == 0) {
 			return;
@@ -186,6 +288,8 @@ public class ControlFlowGuard {
 		try {
 			Address routineAddr = space.getAddress(lcd.getRfgFailureRoutine());
 			symbolTable.createLabel(routineAddr, "_guard_ss_verify_failure", SourceType.IMPORTED);
+
+			AbstractProgramLoader.markAsFunction(program, null, routineAddr);
 		}
 		catch (AddressOutOfBoundsException | InvalidInputException e) {
 			Msg.warn(ControlFlowGuard.class, "Unable to label ReturnFlowGuard failure routine.", e);
@@ -196,13 +300,14 @@ public class ControlFlowGuard {
 	 * Performs markup on the ReturnFlowGuard "default" failure routine function, if it exists.
 	 * 
 	 * @param lcd The PE LoadConfigDirectory.
+	 * @param program The program
 	 * @param is64bit True if the PE is 64-bit; false if it's 32-bit.
 	 * @param space The program's address space.
 	 * @param mem The program's memory.
 	 * @param symbolTable The program's symbol table.
 	 */
-	private static void markupRfgDefaultFailureRoutine(LoadConfigDirectory lcd, boolean is64bit,
+	private static void markupRfgDefaultFailureRoutine(LoadConfigDirectory lcd, Program program,
-			AddressSpace space, Memory mem, SymbolTable symbolTable) {
+			boolean is64bit, AddressSpace space, Memory mem, SymbolTable symbolTable) {
 
 		if (lcd.getRfgFailureRoutineFunctionPointer() == 0) {
 			return;
@@ -215,6 +320,9 @@ public class ControlFlowGuard {
 				is64bit ? mem.getLong(functionPointerAddr) : mem.getInt(functionPointerAddr));
 			symbolTable.createLabel(functionAddr, "_guard_ss_verify_failure_default",
 				SourceType.IMPORTED);
+
+			AbstractProgramLoader.markAsFunction(program, null, functionAddr);
+
 		}
 		catch (MemoryAccessException | AddressOutOfBoundsException | InvalidInputException e) {
 			Msg.warn(ControlFlowGuard.class,
@@ -226,13 +334,15 @@ public class ControlFlowGuard {
 	 * Performs markup on the ReturnFlowGuard verify stack pointer function, if it exists.
 	 * 
 	 * @param lcd The PE LoadConfigDirectory.
+	 * @param program The program
 	 * @param is64bit True if the PE is 64-bit; false if it's 32-bit.
 	 * @param space The program's address space.
 	 * @param mem The program's memory.
 	 * @param symbolTable The program's symbol table.
 	 */
 	private static void markupRfgDefaultStackPointerFunction(LoadConfigDirectory lcd,
-			boolean is64bit, AddressSpace space, Memory mem, SymbolTable symbolTable) {
+			Program program, boolean is64bit, AddressSpace space, Memory mem,
+			SymbolTable symbolTable) {
 
 		if (lcd.getRfgVerifyStackPointerFunctionPointer() == 0) {
 			return;
@@ -245,6 +355,9 @@ public class ControlFlowGuard {
 				is64bit ? mem.getLong(functionPointerAddr) : mem.getInt(functionPointerAddr));
 			symbolTable.createLabel(functionAddr, "_guard_ss_verify_sp_default",
 				SourceType.IMPORTED);
+
+			AbstractProgramLoader.markAsFunction(program, null, functionAddr);
+
 		}
 		catch (MemoryAccessException | AddressOutOfBoundsException | InvalidInputException e) {
 			Msg.warn(ControlFlowGuard.class,

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/pe/LoadConfigDirectory.java

@@ -254,6 +254,24 @@ public class LoadConfigDirectory implements StructConverter {
 		return guardCfFunctionCount;
 	}
 
+	/**
+	 * Gets the ControlFlowGuard IAT table pointer address.
+	 * 
+	 * @return The ControlFlowGuard IAT table function pointer address. Could be 0 if ControlFlowGuard is not being used
+	 */
+	public long getGuardAddressIatTableTablePointer() {
+		return guardAddressTakenIatEntryTable;
+	}
+
+	/**
+	 * Gets the ControlFlowGuard IAT entries count.
+	 * 
+	 * @return The ControlFlowGuard IAT entries count.  Could be 0 if ControlFlowGuard is not being used
+	 */
+	public long getGuardAddressIatTableCount() {
+		return guardAddressTakenIatEntryCount;
+	}
+
 	/**
 	 * Gets the ReturnFlowGuard failure routine address.
 	 * 

Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/AbstractProgramLoader.java

@@ -29,10 +29,10 @@ import ghidra.framework.model.DomainFolder;
 import ghidra.framework.model.DomainObject;
 import ghidra.framework.store.LockException;
 import ghidra.program.database.ProgramDB;
+import ghidra.program.database.function.OverlappingFunctionException;
 import ghidra.program.model.address.*;
 import ghidra.program.model.lang.*;
-import ghidra.program.model.listing.Function;
+import ghidra.program.model.listing.*;
-import ghidra.program.model.listing.Program;
 import ghidra.program.model.mem.MemoryConflictException;
 import ghidra.program.model.symbol.*;
 import ghidra.program.model.util.AddressLabelInfo;
@@ -347,6 +347,33 @@ public abstract class AbstractProgramLoader implements Loader {
 		}
 	}
 
+	/**
+	 * Mark this address as a function by creating a one byte function.  The single byte body
+	 * function is picked up by the function analyzer, disassembled, and the body fixed.
+	 * Marking the function this way keeps disassembly and follow on analysis out of the loaders.
+	 * 
+	 * @param program the program
+	 * @param name name of function, null if name not known
+	 * @param funcStart starting address of the function
+	 */
+	public static void markAsFunction(Program program, String name, Address funcStart) {
+		FunctionManager functionMgr = program.getFunctionManager();
+
+		if (functionMgr.getFunctionAt(funcStart) != null) {
+			return;
+		}
+		try {
+			functionMgr.createFunction(name, funcStart, new AddressSet(funcStart, funcStart),
+				SourceType.IMPORTED);
+		}
+		catch (InvalidInputException e) {
+			// ignore
+		}
+		catch (OverlappingFunctionException e) {
+			// ignore
+		}
+	}
+
 	/**
 	 * Gets the {@link Loader}'s language service.
 	 * <p>

Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/CoffLoader.java

@@ -28,7 +28,6 @@ import ghidra.app.util.bin.format.coff.relocation.CoffRelocationHandler;
 import ghidra.app.util.bin.format.coff.relocation.CoffRelocationHandlerFactory;
 import ghidra.app.util.importer.MessageLog;
 import ghidra.framework.model.DomainObject;
-import ghidra.program.database.function.OverlappingFunctionException;
 import ghidra.program.database.mem.FileBytes;
 import ghidra.program.model.address.*;
 import ghidra.program.model.data.*;
@@ -153,7 +152,8 @@ public class CoffLoader extends AbstractLibrarySupportLoader {
 	}
 
 	@Override
-	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options, Program program) {
+	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options,
+			Program program) {
 		if (options != null) {
 			for (Option option : options) {
 				String name = option.getName();
@@ -277,8 +277,8 @@ public class CoffLoader extends AbstractLibrarySupportLoader {
 
 					symbolsMap.put(symbol, sym);
 
-					externalAddress = externalAddress.add(
+					externalAddress = externalAddress
-						getPointerSizeAligned(externalAddress.getAddressSpace()));
+							.add(getPointerSizeAligned(externalAddress.getAddressSpace()));
 				}
 				else if (sectionNum <= -2) {
 					log.appendMsg("Strange symbol " + symbol + " : " + symbol.getBasicType() +
@@ -342,7 +342,7 @@ public class CoffLoader extends AbstractLibrarySupportLoader {
 						symbol.getStorageClass() != CoffSymbolStorageClass.C_STAT) {
 						// ONLY DO THIS IF THE SYMBOL IS A FUNCTION!
 						symbolTable.addExternalEntryPoint(address);
-						createOneByteFunction(program, sym.getName(), address);
+						markAsFunction(program, sym.getName(), address);
 					}
 
 					symbolsMap.put(symbol, sym);
@@ -470,8 +470,8 @@ public class CoffLoader extends AbstractLibrarySupportLoader {
 				// don't create a block, but record the section to get at the address!
 				block = program.getMemory().getBlock(sectionAddr);
 				try {
-					program.getSymbolTable().createLabel(sectionAddr, section.getName(),
+					program.getSymbolTable()
-						SourceType.IMPORTED);
+							.createLabel(sectionAddr, section.getName(), SourceType.IMPORTED);
 					// TODO: sectionSize somewhere for case where flags==0 ?
 				}
 				catch (InvalidInputException e) {
@@ -702,7 +702,8 @@ public class CoffLoader extends AbstractLibrarySupportLoader {
 					}
 				}
 
-				program.getRelocationTable().add(address, relocation.getType(),
+				program.getRelocationTable()
+						.add(address, relocation.getType(),
 							new long[] { relocation.getSymbolIndex() }, origBytes,
 							symbol != null ? symbol.getName() : "<null>");
 			}
@@ -711,27 +712,11 @@ public class CoffLoader extends AbstractLibrarySupportLoader {
 
 	private void handleRelocationError(Program program, MessageLog log, Address address,
 			String message) {
-		program.getBookmarkManager().setBookmark(address, BookmarkType.ERROR, "Relocations",
+		program.getBookmarkManager()
-			message);
+				.setBookmark(address, BookmarkType.ERROR, "Relocations", message);
 		log.appendMsg(message);
 	}
 
-	private void createOneByteFunction(Program program, String name, Address address) {
-		FunctionManager functionMgr = program.getFunctionManager();
-		if (functionMgr.getFunctionAt(address) != null) {
-			return;
-		}
-		try {
-			functionMgr.createFunction(name, address, new AddressSet(address), SourceType.IMPORTED);
-		}
-		catch (InvalidInputException e) {
-			// ignore
-		}
-		catch (OverlappingFunctionException e) {
-			// ignore
-		}
-	}
-
 	@Override
 	public String getName() {
 		return COFF_NAME;

Ghidra/Features/Base/src/test.slow/java/ghidra/app/plugin/core/analysis/AnalysisOptionsTest.java

@@ -102,6 +102,7 @@ public class AnalysisOptionsTest extends AbstractGhidraHeadedIntegrationTest {
 		assertTrue(isAnalyzerEnabled("Reference"));
 		assertTrue(isAnalyzerEnabled("ASCII Strings"));
 	}
+
 	@Test
 	public void testDeselectAll() throws Exception {
 
@@ -140,6 +141,7 @@ public class AnalysisOptionsTest extends AbstractGhidraHeadedIntegrationTest {
 		assertTrue(isAnalyzerEnabled("Reference"));
 		assertTrue(isAnalyzerEnabled("ASCII Strings"));
 	}
+
 	@Test
 	public void testSaveConfiguration() {
 		assertComboboxEquals("Current Program Options");
@@ -153,6 +155,7 @@ public class AnalysisOptionsTest extends AbstractGhidraHeadedIntegrationTest {
 		assertComboboxEquals("foo");
 
 	}
+
 	@Test
 	public void testDeleteConfiguration() {
 		assertComboboxEquals("Current Program Options");
@@ -206,7 +209,9 @@ public class AnalysisOptionsTest extends AbstractGhidraHeadedIntegrationTest {
 		assertFalse(isAnalyzerEnabled("Stack"));
 		assertFalse(isAnalyzerEnabled("Reference"));
 
-		pressButtonByText(optionsDialog, "Cancel");
+		pressButtonByText(optionsDialog, "Cancel", false);
+		OptionDialog yesNoDialog = waitForDialogComponent(OptionDialog.class);
+		pressButtonByText(yesNoDialog.getComponent(), "No");
 
 		assertTrue(isAnalyzerEnabledInProgramOptions("Stack"));
 		assertTrue(isAnalyzerEnabledInProgramOptions("Reference"));
@@ -254,7 +259,8 @@ public class AnalysisOptionsTest extends AbstractGhidraHeadedIntegrationTest {
 	private void assertComboboxEquals(String name) {
 		AnalysisPanel panel = (AnalysisPanel) getInstanceField("panel", optionsDialog);
 		@SuppressWarnings("unchecked")
-		GhidraComboBox<Options> combo = (GhidraComboBox<Options>) getInstanceField("defaultOptionsCombo", panel);
+		GhidraComboBox<Options> combo =
+			(GhidraComboBox<Options>) getInstanceField("defaultOptionsCombo", panel);
 		assertEquals(name, ((Options) combo.getSelectedItem()).getName());
 	}
 
@@ -262,7 +268,8 @@ public class AnalysisOptionsTest extends AbstractGhidraHeadedIntegrationTest {
 		runSwing(() -> {
 			AnalysisPanel panel = (AnalysisPanel) getInstanceField("panel", optionsDialog);
 			@SuppressWarnings("unchecked")
-			GhidraComboBox<Options> combo = (GhidraComboBox<Options>) getInstanceField("defaultOptionsCombo", panel);
+			GhidraComboBox<Options> combo =
+				(GhidraComboBox<Options>) getInstanceField("defaultOptionsCombo", panel);
 			ComboBoxModel<Options> model = combo.getModel();
 			for (int i = 0; i < model.getSize(); i++) {
 				Options elementAt = model.getElementAt(i);
@@ -282,6 +289,7 @@ public class AnalysisOptionsTest extends AbstractGhidraHeadedIntegrationTest {
 		waitForSwing();
 
 	}
+
 	private void confirmDelete() {
 		OptionDialog dialog = waitForDialogComponent(OptionDialog.class);
 		pressButtonByText(dialog, "Yes");
@@ -298,7 +306,6 @@ public class AnalysisOptionsTest extends AbstractGhidraHeadedIntegrationTest {
 		pressButtonByText(optionsDialog, "Analyze");
 	}
 
-
 	private boolean isAnalyzerEnabledInProgramOptions(String analyzerName) {
 		Options options = program.getOptions(Program.ANALYSIS_PROPERTIES);
 		return options.getBoolean(analyzerName, false);
@@ -363,4 +370,3 @@ public class AnalysisOptionsTest extends AbstractGhidraHeadedIntegrationTest {
 	}
 
 }
-

Ghidra/Features/Base/src/test.slow/java/ghidra/util/bean/opteditor/OptionsDialogTest.java

@@ -38,6 +38,7 @@ import docking.actions.KeyBindingUtils;
 import docking.options.editor.*;
 import docking.tool.ToolConstants;
 import docking.widgets.MultiLineLabel;
+import docking.widgets.OptionDialog;
 import docking.widgets.filechooser.GhidraFileChooser;
 import docking.widgets.table.RowObjectFilterModel;
 import docking.widgets.tree.GTree;
@@ -554,9 +555,9 @@ public class OptionsDialogTest extends AbstractGhidraHeadedIntegrationTest {
 		// change to "LEFT"
 		runSwing(() -> ps.setSelectedIndex(0));
 
-		final JButton cancelButton = findButtonByText(dialog.getComponent(), "Cancel");
+		pressButtonByText(dialog, "Cancel", false);
-		assertTrue(cancelButton.isEnabled());
+		OptionDialog yesNoDialog = waitForDialogComponent(OptionDialog.class);
-		runSwing(() -> cancelButton.getActionListeners()[0].actionPerformed(null));
+		pressButtonByText(yesNoDialog.getComponent(), "No");
 
 		Options options = tool.getOptions(ToolConstants.TOOL_OPTIONS);
 		GhidraOptions.CURSOR_MOUSE_BUTTON_NAMES mouseButton =
@@ -564,7 +565,7 @@ public class OptionsDialogTest extends AbstractGhidraHeadedIntegrationTest {
 				(GhidraOptions.CURSOR_MOUSE_BUTTON_NAMES) null);
 
 		assertEquals("MIDDLE", mouseButton.toString());
-		assertTrue(!dialog.isShowing());
+		assertFalse(runSwing(() -> dialog.isShowing()));
 	}
 
 	@Test

Ghidra/Features/Decompiler/src/decompile/cpp/architecture.cc

@@ -1363,8 +1363,8 @@ Address SegmentedResolver::resolve(uintb val,int4 sz,const Address &point,uintb
       uintb base = glb->context->getTrackedValue(segop->getResolve(),point);
       fullEncoding = (base << 8 * innersz) + (val & calc_mask(innersz));
       vector<uintb> seginput;
-      seginput.push_back(val);
       seginput.push_back(base);
+      seginput.push_back(val);
       val = segop->execute(seginput);
       return Address(spc,AddrSpace::addressToByte(val,spc->getWordSize()));
     }
@@ -1375,8 +1375,8 @@ Address SegmentedResolver::resolve(uintb val,int4 sz,const Address &point,uintb
     uintb base = (val >> 8*innersz) & calc_mask(outersz);
     val = val & calc_mask(innersz);
     vector<uintb> seginput;
-    seginput.push_back(val);
     seginput.push_back(base);
+    seginput.push_back(val);
     val = segop->execute(seginput);
     return Address(spc,AddrSpace::addressToByte(val,spc->getWordSize()));
   }

Ghidra/Features/Decompiler/src/decompile/cpp/coreaction.cc

@@ -644,12 +644,12 @@ int4 ActionSegmentize::apply(Funcdata &data)
       }
 
       if (segdef->getNumVariableTerms()==1)
-	bindlist[1] = data.newConstant(4,0);
+	bindlist[0] = data.newConstant(4,0);
       // Redefine the op as a segmentop
       data.opSetOpcode(segroot,CPUI_SEGMENTOP);
       data.opSetInput(segroot,data.newVarnodeSpace(spc),0);
-      data.opSetInput(segroot,bindlist[1],1);
+      data.opSetInput(segroot,bindlist[0],1);
-      data.opSetInput(segroot,bindlist[0],2);
+      data.opSetInput(segroot,bindlist[1],2);
       for(int4 j=segroot->numInput()-1;j>2;--j) // Remove anything else
 	data.opRemoveInput(segroot,j);
       count += 1;

Ghidra/Features/Decompiler/src/decompile/cpp/emulateutil.cc

@@ -127,8 +127,8 @@ void EmulatePcodeOp::executeSegmentOp(void)
   uintb in1 = getVarnodeValue(currentOp->getIn(1));
   uintb in2 = getVarnodeValue(currentOp->getIn(2));
   vector<uintb> bindlist;
-  bindlist.push_back(in2);
   bindlist.push_back(in1);
+  bindlist.push_back(in2);
   uintb res = segdef->execute(bindlist);
   setVarnodeValue(currentOp->getOut(), res);
 }

Ghidra/Features/Decompiler/src/decompile/cpp/ghidra_process.cc

@@ -36,7 +36,7 @@ void connect_to_console(Funcdata *fd)
   if (remote == (RemoteSocket *)0) {
     remote = new RemoteSocket();
     if (remote->open("/tmp/ghidrasocket")) {
-      ghidra_dcp = new IfaceStatus("[ghidradbg]> ",*remote->getInputStream(),*remote->getOutputStream());
+      ghidra_dcp = new IfaceTerm("[ghidradbg]> ",*remote->getInputStream(),*remote->getOutputStream());
       IfaceCapability::registerAllCommands(ghidra_dcp);
     }
   }

Ghidra/Features/Decompiler/src/decompile/cpp/ruleaction.cc

@@ -7489,8 +7489,8 @@ int4 RuleSegment::applyOp(PcodeOp *op,Funcdata &data)
 
   if (vn1->isConstant() && vn2->isConstant()) {
     vector<uintb> bindlist;
-    bindlist.push_back(vn2->getOffset());
     bindlist.push_back(vn1->getOffset());
+    bindlist.push_back(vn2->getOffset());
     uintb val = segdef->execute(bindlist);
     data.opRemoveInput(op,2);
     data.opRemoveInput(op,1);

Ghidra/Features/Decompiler/src/decompile/cpp/userop.cc

@@ -143,13 +143,13 @@ bool SegmentOp::unify(Funcdata &data,PcodeOp *op,
     innervn = op->getIn(2);
     if (basevn->isConstant())
       basevn = data.newConstant(baseinsize,basevn->getOffset());
-    bindlist[1] = basevn;
+    bindlist[0] = basevn;
   }
   else
-    bindlist[1] = (Varnode *)0;
+    bindlist[0] = (Varnode *)0;
   if (innervn->isConstant())
     innervn = data.newConstant(innerinsize,innervn->getOffset());
-  bindlist[0] = innervn;
+  bindlist[1] = innervn;
   return true;
 }
 

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/pcode/HighFunctionDBUtil.java

@@ -254,8 +254,8 @@ public class HighFunctionDBUtil {
 
 		Register reg = var.getRegister();
 		if (reg != null) {
-			program.getReferenceManager().addRegisterReference(pcAddr, -1, reg, RefType.WRITE,
+			program.getReferenceManager()
-				source);
+					.addRegisterReference(pcAddr, -1, reg, RefType.WRITE, source);
 		}
 
 		return var;
@@ -496,8 +496,7 @@ public class HighFunctionDBUtil {
 			VariableStorage storage = highSymbol.getStorage();
 			Address pcAddr = highSymbol.getPCAddress();
 			HighVariable tmpHigh = highSymbol.getHighVariable();
-			if (!storage.isHashStorage() && tmpHigh != null &&
+			if (!storage.isHashStorage() && tmpHigh != null && tmpHigh.requiresDynamicStorage()) {
-				tmpHigh.requiresDynamicStorage()) {
 				DynamicEntry entry = DynamicEntry.build(tmpHigh.getRepresentative());
 				storage = entry.getStorage();
 				pcAddr = entry.getPCAdress();	// The address may change from original Varnode
@@ -728,17 +727,25 @@ public class HighFunctionDBUtil {
 		}
 		if (op.getOpcode() == PcodeOp.PTRSUB) {
 			Varnode vnode = op.getInput(0);
+			Varnode cnode = op.getInput(1);
 			if (vnode.isRegister()) {
 				AddressSpace stackspace = program.getAddressFactory().getStackSpace();
 				if (stackspace != null) {
-					Address caddr = op.getInput(1).getAddress();
+					storageAddress = stackspace.getAddress(cnode.getOffset());
-					storageAddress = stackspace.getAddress(caddr.getOffset());
 				}
 			}
 			else {
-				Address caddr = op.getInput(1).getAddress();
+				AddressSpace space = program.getAddressFactory().getDefaultAddressSpace();
-				storageAddress = program.getAddressFactory().getDefaultAddressSpace().getAddress(
+				if (space instanceof SegmentedAddressSpace) {
-					caddr.getOffset());
+					// Assume this is a "full" encoding of the offset
+					int innersize = space.getPointerSize();
+					int base = (int) (cnode.getOffset() >>> 8 * innersize);
+					int off = (int) cnode.getOffset() & ((1 << 8 * innersize) - 1);
+					storageAddress = ((SegmentedAddressSpace) space).getAddress(base, off);
+				}
+				else {
+					storageAddress = space.getAddress(cnode.getOffset());
+				}
 			}
 		}
 		return storageAddress;

Ghidra/Processors/HCS12/data/languages/HCS12.pspec

@@ -8,8 +8,8 @@
   
   <segmentop space="RAM" userop="segment" farpointer="no">
     <pcode>
-      <input name="inner" size="2"/>
       <input name="base" size="3"/>
+      <input name="inner" size="2"/>
       <output name="res" size="3"/>
       <body><![CDATA[
         res = base ^ zext(inner);

Ghidra/Processors/Z80/data/languages/z80.cspec

@@ -11,8 +11,8 @@
   <stackpointer register="SP" space="ram"/>
   <segmentop space="ram" userop="segment" farpointer="yes">
     <pcode>
-      <input name="inner" size="2"/>
       <input name="base" size="2"/>
+      <input name="inner" size="2"/>
       <output name="res" size="2"/>
       <body><![CDATA[
         res = (base << 12) + inner;

Ghidra/Processors/x86/data/languages/x86-16-real.pspec

@@ -9,8 +9,8 @@
   <segmented_address space="ram" type="real" />
   <segmentop space="ram" userop="segment" farpointer="yes">
     <pcode>
-      <input name="inner" size="2"/>
       <input name="base" size="2"/>
+      <input name="inner" size="2"/>
       <output name="res" size="4"/>
       <body><![CDATA[
         res = (zext(base) << 4) + zext(inner);

Ghidra/Processors/x86/data/languages/x86-16.pspec

@@ -10,8 +10,8 @@
   <segmented_address space="ram" type="protected"/>
   <segmentop space="ram" userop="segment" farpointer="yes">
     <pcode>
-      <input name="inner" size="2"/>
       <input name="base" size="2"/>
+      <input name="inner" size="2"/>
       <output name="res" size="4"/>
       <body><![CDATA[
         res = (zext(base) << 16) + zext(inner);