From ae5662a50fb9cf59e192eaefa4a717a27b3de1c3 Mon Sep 17 00:00:00 2001
From: ghidravore <ghidravore@users.noreply.github.com>
Date: Mon, 12 Aug 2019 12:44:59 -0400
Subject: [PATCH] GT-3079 Added checking to binary loader to check for memory
 conflicts before loading in the "add to program" case.

---
 .../ghidra/app/util/MemoryBlockUtils.java     | 23 ++++++++--
 .../opinion/AbstractLibrarySupportLoader.java |  4 +-
 .../util/opinion/AbstractProgramLoader.java   |  2 +-
 .../ghidra/app/util/opinion/BinaryLoader.java | 42 ++++++++++---------
 .../ghidra/app/util/opinion/CoffLoader.java   |  4 +-
 .../ghidra/app/util/opinion/ElfLoader.java    |  4 +-
 .../ghidra/app/util/opinion/GdtLoader.java    |  2 +-
 .../ghidra/app/util/opinion/GzfLoader.java    |  2 +-
 .../app/util/opinion/IntelHexLoader.java      |  2 +-
 .../app/util/opinion/IntelHexMemImage.java    | 21 ++--------
 .../java/ghidra/app/util/opinion/Loader.java  |  5 ++-
 .../app/util/opinion/MotorolaHexLoader.java   | 25 +++--------
 .../ghidra/app/util/opinion/PeLoader.java     |  4 +-
 .../ghidra/app/util/opinion/XmlLoader.java    |  2 +-
 .../plugin/importer/AddToProgramDialog.java   | 24 ++++++++---
 .../plugin/importer/ImporterDialog.java       | 13 +++---
 .../ghidra/app/util/MemoryBlockUtilTest.java  | 38 ++++++++++++++---
 .../generic/test/AbstractGenericTest.java     | 10 ++---
 .../main/java/skeleton/SkeletonLoader.java    |  4 +-
 19 files changed, 132 insertions(+), 99 deletions(-)

diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/MemoryBlockUtils.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/MemoryBlockUtils.java
index 54115d3abf..de68519eb7 100644
--- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/MemoryBlockUtils.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/MemoryBlockUtils.java
@@ -225,14 +225,13 @@ public class MemoryBlockUtils {
 				block = program.getMemory().createInitializedBlock(name, start, fileBytes, offset,
 					length, isOverlay);
 			}
-			catch (MemoryConflictException e) {
-				block = program.getMemory().createInitializedBlock(name, start, fileBytes, offset,
-					length, true);
+			catch (MemoryConflictException | DuplicateNameException e) {
+				block = createBlockNoDuplicateName(program, name, start, fileBytes, offset, length);
 				log.appendMsg("Conflict attempting to create memory block: " + name +
 					" at address " + start.toString() + " Created block in new overlay instead");
 			}
 		}
-		catch (LockException | DuplicateNameException | MemoryConflictException e) {
+		catch (LockException | MemoryConflictException e) {
 			throw new RuntimeException(e);
 		}
 
@@ -241,6 +240,22 @@ public class MemoryBlockUtils {
 		return block;
 	}
 
+	private static MemoryBlock createBlockNoDuplicateName(Program program, String blockName,
+			Address start, FileBytes fileBytes, long offset, long length)
+			throws LockException, MemoryConflictException, AddressOverflowException {
+		int count = 1;
+		String name = blockName;
+		while (true) {
+			try {
+				return program.getMemory().createInitializedBlock(name, start, fileBytes, offset,
+					length, true);
+			}
+			catch (DuplicateNameException e) {
+				name = blockName + "_" + count++;
+			}
+		}
+	}
+
 	/**
 	 * Creates a new initialized block in memory using the bytes from the given input stream.
 	 * If there is a conflict when creating this block (some other block occupies at least some
diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/AbstractLibrarySupportLoader.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/AbstractLibrarySupportLoader.java
index 69cc915f9a..4dbf12095e 100644
--- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/AbstractLibrarySupportLoader.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/AbstractLibrarySupportLoader.java
@@ -139,7 +139,7 @@ public abstract class AbstractLibrarySupportLoader extends AbstractProgramLoader
 	}
 
 	@Override
-	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options) {
+	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options, Program program) {
 
 		if (options != null) {
 			for (Option option : options) {
@@ -151,7 +151,7 @@ public abstract class AbstractLibrarySupportLoader extends AbstractProgramLoader
 				}
 			}
 		}
-		return super.validateOptions(provider, loadSpec, options);
+		return super.validateOptions(provider, loadSpec, options, program);
 	}
 
 	@Override
diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/AbstractProgramLoader.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/AbstractProgramLoader.java
index ebac11493e..7d22e19791 100644
--- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/AbstractProgramLoader.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/AbstractProgramLoader.java
@@ -188,7 +188,7 @@ public abstract class AbstractProgramLoader implements Loader {
 	}
 
 	@Override
-	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options) {
+	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options, Program program) {
 		if (options != null) {
 			for (Option option : options) {
 				String name = option.getName();
diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/BinaryLoader.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/BinaryLoader.java
index 289cb21234..e749a3b43f 100644
--- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/BinaryLoader.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/BinaryLoader.java
@@ -23,16 +23,14 @@ import ghidra.app.util.bin.ByteProvider;
 import ghidra.app.util.importer.MessageLog;
 import ghidra.framework.model.DomainFolder;
 import ghidra.framework.model.DomainObject;
-import ghidra.framework.store.LockException;
 import ghidra.program.database.mem.FileBytes;
 import ghidra.program.model.address.*;
 import ghidra.program.model.lang.*;
 import ghidra.program.model.listing.Program;
-import ghidra.program.model.mem.*;
+import ghidra.program.model.mem.Memory;
 import ghidra.util.Msg;
 import ghidra.util.NumericUtilities;
 import ghidra.util.exception.CancelledException;
-import ghidra.util.exception.DuplicateNameException;
 import ghidra.util.task.TaskMonitor;
 
 public class BinaryLoader extends AbstractProgramLoader {
@@ -91,7 +89,8 @@ public class BinaryLoader extends AbstractProgramLoader {
 	}
 
 	@Override
-	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options) {
+	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options,
+			Program program) {
 		Address baseAddr = null;
 		long length = 0;
 		long fileOffset = 0;
@@ -194,7 +193,12 @@ public class BinaryLoader extends AbstractProgramLoader {
 		if (length == -1) {
 			return "Invalid length specified";
 		}
-		return super.validateOptions(provider, loadSpec, options);
+		if (program != null) {
+			if (program.getMemory().intersects(baseAddr, baseAddr.add(length - 1))) {
+				return "Memory Conflict: Use <Options...> to change the base address!";
+			}
+		}
+		return super.validateOptions(provider, loadSpec, options, program);
 	}
 
 	private Address getBaseAddr(List<Option> options) {
@@ -322,27 +326,27 @@ public class BinaryLoader extends AbstractProgramLoader {
 			if (blockName == null || blockName.length() == 0) {
 				blockName = generateBlockName(prog, isOverlay, baseAddr.getAddressSpace());
 			}
-			try {
-				MemoryBlock block = prog.getMemory().createInitializedBlock(blockName, baseAddr,
-					fileBytes, 0, length, isOverlay);
-				block.setRead(true);
-				block.setWrite(isOverlay ? false : true);
-				block.setExecute(isOverlay ? false : true);
-				block.setSourceName("Binary Loader");
-				MemoryBlockUtils.adjustFragment(prog, block.getStart(), blockName);
-			}
-			catch (LockException | MemoryConflictException e) {
-				Msg.error(this, "Unexpected exception creating memory block", e);
-			}
+			createBlock(prog, isOverlay, blockName, baseAddr, fileBytes, length, log);
+
 			return true;
 		}
 		catch (AddressOverflowException e) {
 			throw new IllegalArgumentException("Invalid address range specified: start:" +
 				baseAddr + ", length:" + length + " - end address exceeds address space boundary!");
 		}
-		catch (DuplicateNameException e) {
-			throw new IllegalArgumentException("Duplicate block name specified: " + blockName);
+	}
+
+	private void createBlock(Program prog, boolean isOverlay, String blockName, Address baseAddr,
+			FileBytes fileBytes, long length, MessageLog log)
+			throws AddressOverflowException, IOException {
+
+		if (prog.getMemory().intersects(baseAddr, baseAddr.add(length - 1))) {
+			throw new IOException("Can't load " + length + " bytes at address " + baseAddr +
+				" since it conflicts with existing memory blocks!");
 		}
+		MemoryBlockUtils.createInitializedBlock(prog, isOverlay, blockName, baseAddr, fileBytes, 0,
+			length, null, "Binary Loader", true, !isOverlay, !isOverlay, log);
+
 	}
 
 	private long clipToMemorySpace(long length, MessageLog log, Program program) {
diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/CoffLoader.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/CoffLoader.java
index c5e6a37f18..d2d84777a1 100644
--- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/CoffLoader.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/CoffLoader.java
@@ -153,7 +153,7 @@ public class CoffLoader extends AbstractLibrarySupportLoader {
 	}
 
 	@Override
-	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options) {
+	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options, Program program) {
 		if (options != null) {
 			for (Option option : options) {
 				String name = option.getName();
@@ -164,7 +164,7 @@ public class CoffLoader extends AbstractLibrarySupportLoader {
 				}
 			}
 		}
-		return super.validateOptions(provider, loadSpec, options);
+		return super.validateOptions(provider, loadSpec, options, program);
 	}
 
 	private boolean performFakeLinking(List<Option> options) {
diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/ElfLoader.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/ElfLoader.java
index 7c185d80bb..65bc926587 100644
--- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/ElfLoader.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/ElfLoader.java
@@ -90,14 +90,14 @@ public class ElfLoader extends AbstractLibrarySupportLoader {
 	}
 
 	@Override
-	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options) {
+	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options, Program program) {
 		if (options != null) {
 			String validationErrorStr = ElfLoaderOptionsFactory.validateOptions(loadSpec, options);
 			if (validationErrorStr != null) {
 				return validationErrorStr;
 			}
 		}
-		return super.validateOptions(provider, loadSpec, options);
+		return super.validateOptions(provider, loadSpec, options, program);
 	}
 
 	@Override
diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/GdtLoader.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/GdtLoader.java
index 021d77a1c4..6233f9f99a 100644
--- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/GdtLoader.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/GdtLoader.java
@@ -89,7 +89,7 @@ public class GdtLoader implements Loader {
 	}
 
 	@Override
-	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options) {
+	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options, Program program) {
 		if (options != null && options.size() > 0) {
 			return "GDTLoader takes no options";
 		}
diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/GzfLoader.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/GzfLoader.java
index 544ce7049d..519d022977 100644
--- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/GzfLoader.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/GzfLoader.java
@@ -54,7 +54,7 @@ public class GzfLoader implements Loader {
 	}
 
 	@Override
-	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options) {
+	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options, Program program) {
 		if (options != null && options.size() > 0) {
 			return "GzfLoader takes no options";
 		}
diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/IntelHexLoader.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/IntelHexLoader.java
index 8698c8a315..7d67f4833d 100644
--- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/IntelHexLoader.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/IntelHexLoader.java
@@ -75,7 +75,7 @@ public class IntelHexLoader extends AbstractProgramLoader {
 	}
 
 	@Override
-	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options) {
+	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options, Program program) {
 		Address baseAddr = null;
 
 		for (Option option : options) {
diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/IntelHexMemImage.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/IntelHexMemImage.java
index 743cdb8225..36459b6b8b 100644
--- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/IntelHexMemImage.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/IntelHexMemImage.java
@@ -23,7 +23,6 @@ import ghidra.app.util.importer.MessageLog;
 import ghidra.program.model.address.*;
 import ghidra.program.model.listing.Program;
 import ghidra.util.Msg;
-import ghidra.util.exception.DuplicateNameException;
 import ghidra.util.task.TaskMonitor;
 
 class IntelHexMemImage {
@@ -212,23 +211,9 @@ class IntelHexMemImage {
 			}
 
 			String name = blockName == null ? base.getAddressSpace().getName() : blockName;
-			while (true) {
-				try {
-					MemoryBlockUtils.createInitializedBlock(program, isOverlay, name,
-						blockRange.getMinAddress(), new ByteArrayInputStream(data), data.length,
-						"Generated by " + creator, progFile, true, !isOverlay, !isOverlay, log,
-						monitor);
-					break;
-				}
-				catch (RuntimeException e) {
-					Throwable cause = e.getCause();
-					if (!(cause instanceof DuplicateNameException)) {
-						throw e;
-					}
-					++count;
-					name = blockName + "_" + count;
-				}
-			}
+			MemoryBlockUtils.createInitializedBlock(program, isOverlay, name,
+				blockRange.getMinAddress(), new ByteArrayInputStream(data), data.length,
+				"Generated by " + creator, progFile, true, !isOverlay, !isOverlay, log, monitor);
 		}
 		return log.toString();
 	}
diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/Loader.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/Loader.java
index b4581f8007..adf23d7161 100644
--- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/Loader.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/Loader.java
@@ -124,10 +124,13 @@ public interface Loader extends ExtensionPoint, Comparable<Loader> {
 	 * @param provider The bytes of the thing being loaded.
 	 * @param loadSpec The proposed {@link LoadSpec}.
 	 * @param options The list of {@link Option}s to validate.
+	 * @param program existing program if the loader is adding to an existing program. If it is
+	 * a fresh import, then this will be null. 
 	 * @return null if all {@link Option}s are valid; otherwise, an error message describing the 
 	 *   problem is returned.
 	 */
-	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options);
+	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options,
+			Program program);
 
 	/**
 	 * Gets the {@link Loader}'s name, which is used both for display purposes, and to identify the 
diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/MotorolaHexLoader.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/MotorolaHexLoader.java
index 274efa6fd4..5cf55e1cad 100644
--- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/MotorolaHexLoader.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/MotorolaHexLoader.java
@@ -92,7 +92,8 @@ public class MotorolaHexLoader extends AbstractProgramLoader {
 	}
 
 	@Override
-	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options) {
+	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options,
+			Program program) {
 		Address baseAddr = null;
 
 		for (Option option : options) {
@@ -335,30 +336,16 @@ public class MotorolaHexLoader extends AbstractProgramLoader {
 
 						String name =
 							blockName == null ? baseAddr.getAddressSpace().getName() : blockName;
-						int count = 0;
-						while (true) {
-							try {
-								MemoryBlockUtils.createInitializedBlock(program, isOverlay, name,
-									start, new ByteArrayInputStream(data), data.length, "",
-									provider.getName(), true, isOverlay, isOverlay, log, monitor);
-								break;
-							}
-							catch (RuntimeException e) {
-								Throwable cause = e.getCause();
-								if (!(cause instanceof DuplicateNameException)) {
-									throw e;
-								}
-								++count;
-								name = blockName + "_" + count;
-							}
-						}
+						MemoryBlockUtils.createInitializedBlock(program, isOverlay, name, start,
+							new ByteArrayInputStream(data), data.length, "", provider.getName(),
+							true, isOverlay, isOverlay, log, monitor);
 					}
 					offset = 0;
 					// set up new start address of new block we are starting
 					startAddress = addr;
 					endAddress = addr;
 				}
-				// update endaddress to start of next contiguous line address
+				// update end address to start of next contiguous line address
 				endAddress += numBytes - addrLen - 1;
 
 				// read in the data bytes on the line
diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/PeLoader.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/PeLoader.java
index 75ab90d429..231a08593c 100644
--- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/PeLoader.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/PeLoader.java
@@ -177,7 +177,7 @@ public class PeLoader extends AbstractPeDebugLoader {
 	}
 
 	@Override
-	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options) {
+	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options, Program program) {
 		if (options != null) {
 			for (Option option : options) {
 				String name = option.getName();
@@ -188,7 +188,7 @@ public class PeLoader extends AbstractPeDebugLoader {
 				}
 			}
 		}
-		return super.validateOptions(provider, loadSpec, options);
+		return super.validateOptions(provider, loadSpec, options, program);
 	}
 
 	@Override
diff --git a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/XmlLoader.java b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/XmlLoader.java
index c49ceb0a18..0adb052671 100644
--- a/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/XmlLoader.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/app/util/opinion/XmlLoader.java
@@ -335,7 +335,7 @@ public class XmlLoader extends AbstractProgramLoader {
 	}
 
 	@Override
-	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options) {
+	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options, Program program) {
 		// XXX will this work? is there other state that xmlOptions needs to
 		// know?
 		try {
diff --git a/Ghidra/Features/Base/src/main/java/ghidra/plugin/importer/AddToProgramDialog.java b/Ghidra/Features/Base/src/main/java/ghidra/plugin/importer/AddToProgramDialog.java
index e2b3190b77..8dfb90681f 100644
--- a/Ghidra/Features/Base/src/main/java/ghidra/plugin/importer/AddToProgramDialog.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/plugin/importer/AddToProgramDialog.java
@@ -60,10 +60,12 @@ public class AddToProgramDialog extends ImporterDialog {
 		folderButton.setEnabled(false);
 		languageButton.setEnabled(false);
 		filenameTextField.setEnabled(false);
+		validateFormInput();
 	}
 
 	@Override
 	protected boolean validateFormInput() {
+
 		setOkEnabled(false);
 		optionsButton.setEnabled(false);
 		Loader loader = getSelectedLoader();
@@ -75,10 +77,20 @@ public class AddToProgramDialog extends ImporterDialog {
 			setStatusText(loader.getName() + " does not support add to program.");
 			return false;
 		}
+		optionsButton.setEnabled(true);
+
+		LoadSpec loadSpec = getSelectedLoadSpec(loader);
+
+		String result =
+			loader.validateOptions(byteProvider, loadSpec, getOptions(loadSpec), addToProgram);
+
+		if (result != null) {
+			setStatusText(result);
+			return false;
+		}
 
 		setStatusText("");
 		setOkEnabled(true);
-		optionsButton.setEnabled(true);
 		return true;
 	}
 
@@ -102,18 +114,18 @@ public class AddToProgramDialog extends ImporterDialog {
 			options = selectedLoader.getDefaultOptions(byteProvider, selectedLoadSpec, null, true);
 		}
 		TaskLauncher.launchNonModal("Import File", monitor -> {
-			ImporterUtilities.addContentToProgram(tool, addToProgram, fsrl, selectedLoadSpec, options,
-				monitor);
+			ImporterUtilities.addContentToProgram(tool, addToProgram, fsrl, selectedLoadSpec,
+				options, monitor);
 		});
 		close();
 	}
 
 	@Override
 	protected List<Option> getOptions(LoadSpec loadSpec) {
-		if (options == null) {
-			options = loadSpec.getLoader().getDefaultOptions(byteProvider, loadSpec, null, true);
+		if (options != null) {
+			return options;
 		}
-		return options;
+		return loadSpec.getLoader().getDefaultOptions(byteProvider, loadSpec, addToProgram, true);
 	}
 
 	/**
diff --git a/Ghidra/Features/Base/src/main/java/ghidra/plugin/importer/ImporterDialog.java b/Ghidra/Features/Base/src/main/java/ghidra/plugin/importer/ImporterDialog.java
index 352427e098..f446c65f85 100644
--- a/Ghidra/Features/Base/src/main/java/ghidra/plugin/importer/ImporterDialog.java
+++ b/Ghidra/Features/Base/src/main/java/ghidra/plugin/importer/ImporterDialog.java
@@ -346,10 +346,9 @@ public class ImporterDialog extends DialogComponentProvider {
 			String programPath = removeTrailingSlashes(getName());
 			DomainFolder importFolder = getOrCreateImportFolder(destinationFolder, programPath);
 			String programName = FilenameUtils.getName(programPath);
-			List<Option> localOptions = getOptions(loadSpec);
 			TaskLauncher.launchNonModal("Import File", monitor -> {
 				ImporterUtilities.importSingleFile(tool, programManager, fsrl, importFolder,
-					loadSpec, programName, localOptions, monitor);
+					loadSpec, programName, getOptions(loadSpec), monitor);
 			});
 			close();
 		}
@@ -404,24 +403,24 @@ public class ImporterDialog extends DialogComponentProvider {
 			AddressFactory addressFactory = selectedLanguage.getLanguage().getAddressFactory();
 			LoadSpec loadSpec = getSelectedLoadSpec(loader);
 			OptionValidator validator =
-				optionList -> loader.validateOptions(byteProvider, loadSpec, optionList);
+				optionList -> loader.validateOptions(byteProvider, loadSpec, optionList, null);
 
 			AddressFactoryService service = () -> addressFactory;
 
-			List<Option> defaultOptions = getOptions(loadSpec);
-			if (defaultOptions.isEmpty()) {
+			List<Option> currentOptions = getOptions(loadSpec);
+			if (currentOptions.isEmpty()) {
 				Msg.showInfo(this, null, "Options", "There are no options for this importer!");
 				return;
 			}
 
-			OptionsDialog optionsDialog = new OptionsDialog(defaultOptions, validator, service);
+			OptionsDialog optionsDialog = new OptionsDialog(currentOptions, validator, service);
 			optionsDialog.setHelpLocation(
 				new HelpLocation("ImporterPlugin", getAnchorForSelectedLoader(loader)));
 			tool.showDialog(optionsDialog);
 			if (!optionsDialog.wasCancelled()) {
 				options = optionsDialog.getOptions();
 			}
-
+			validateFormInput();
 		}
 		catch (LanguageNotFoundException e) {
 			Msg.showError(this, null, "Language Error",
diff --git a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/util/MemoryBlockUtilTest.java b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/util/MemoryBlockUtilTest.java
index 4b60b5a28d..0e62de805e 100644
--- a/Ghidra/Features/Base/src/test.slow/java/ghidra/app/util/MemoryBlockUtilTest.java
+++ b/Ghidra/Features/Base/src/test.slow/java/ghidra/app/util/MemoryBlockUtilTest.java
@@ -23,8 +23,12 @@ import java.util.Arrays;
 
 import org.junit.*;
 
+import ghidra.app.util.bin.ByteArrayProvider;
+import ghidra.app.util.bin.ByteProvider;
 import ghidra.app.util.importer.MessageLog;
 import ghidra.program.database.ProgramBuilder;
+import ghidra.program.database.mem.FileBytes;
+import ghidra.program.model.address.Address;
 import ghidra.program.model.address.AddressSpace;
 import ghidra.program.model.listing.Program;
 import ghidra.program.model.mem.MemoryAccessException;
@@ -72,8 +76,7 @@ public class MemoryBlockUtilTest extends AbstractGhidraHeadedIntegrationTest {
 		Arrays.fill(data, (byte) 0xb);
 		is = new ByteArrayInputStream(data);
 		MemoryBlockUtils.createInitializedBlock(prog, false, "b", space.getAddress(3500), is, 1000,
-			"bbbb", "b b b", false,
-			false, false, log, TaskMonitor.DUMMY);
+			"bbbb", "b b b", false, false, false, log, TaskMonitor.DUMMY);
 
 		MemoryBlock[] blocks = prog.getMemory().getBlocks();
 		assertEquals(2, blocks.length);
@@ -111,8 +114,8 @@ public class MemoryBlockUtilTest extends AbstractGhidraHeadedIntegrationTest {
 		byte[] cdata = new byte[1000];
 		Arrays.fill(cdata, (byte) 0xc);
 		MemoryBlockUtils.createInitializedBlock(prog, false, "c", space.getAddress(4000),
-			new ByteArrayInputStream(cdata),
-			1000, "Ccomment", "Csource", false, false, false, log, TaskMonitor.DUMMY);
+			new ByteArrayInputStream(cdata), 1000, "Ccomment", "Csource", false, false, false, log,
+			TaskMonitor.DUMMY);
 
 		MemoryBlock[] blocks = prog.getMemory().getBlocks();
 		assertEquals(2, blocks.length);
@@ -127,5 +130,30 @@ public class MemoryBlockUtilTest extends AbstractGhidraHeadedIntegrationTest {
 		assertTrue(blocks[1].isInitialized());
 	}
 
-	//TODO test bit blocks and code unit clearing...
+	private Address addr(long offset) {
+		return space.getAddress(offset);
+	}
+
+	@Test
+	public void testDuplicateExceptionHandling() throws Exception {
+		ByteProvider byteProvider = new ByteArrayProvider(new byte[1000]);
+		FileBytes fileBytes =
+			MemoryBlockUtils.createFileBytes(prog, byteProvider, TaskMonitor.DUMMY);
+
+		MemoryBlockUtils.createInitializedBlock(prog, true, "test", addr(0), fileBytes, 0, 10, "",
+			"", true, true, true, new MessageLog());
+		MemoryBlockUtils.createInitializedBlock(prog, true, "test", addr(0), fileBytes, 0, 10, "",
+			"", true, true, true, new MessageLog());
+		MemoryBlockUtils.createInitializedBlock(prog, true, "test", addr(0), fileBytes, 0, 10, "",
+			"", true, true, true, new MessageLog());
+
+		MemoryBlock[] blocks = prog.getMemory().getBlocks();
+		assertEquals(3, blocks.length);
+
+		assertEquals("test", blocks[0].getName());
+		assertEquals("test_1", blocks[1].getName());
+		assertEquals("test_2", blocks[2].getName());
+
+	}
+
 }
diff --git a/Ghidra/Framework/Generic/src/main/java/generic/test/AbstractGenericTest.java b/Ghidra/Framework/Generic/src/main/java/generic/test/AbstractGenericTest.java
index b209b92556..7e823b2ae6 100644
--- a/Ghidra/Framework/Generic/src/main/java/generic/test/AbstractGenericTest.java
+++ b/Ghidra/Framework/Generic/src/main/java/generic/test/AbstractGenericTest.java
@@ -15,7 +15,7 @@
  */
 package generic.test;
 
-import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.*;
 
 import java.awt.*;
 import java.awt.event.*;
@@ -661,10 +661,10 @@ public abstract class AbstractGenericTest extends AbstractGTest {
 		if (button == null) {
 			throw new AssertionError("Couldn't find button " + buttonText + ".");
 		}
-		if (!button.isShowing()) {
+		if (!runSwing(() -> button.isShowing())) {
 			throw new AssertionError("Button " + buttonText + " is not showing.");
 		}
-		if (!button.isEnabled()) {
+		if (!runSwing(() -> button.isEnabled())) {
 			throw new AssertionError("Button " + buttonText + " is not enabled.");
 		}
 		pressButton(button, waitForCompletion);
@@ -700,10 +700,10 @@ public abstract class AbstractGenericTest extends AbstractGTest {
 		if (button == null) {
 			throw new AssertionError("Couldn't find button " + buttonName + ".");
 		}
-		if (!button.isVisible()) {
+		if (!runSwing(() -> button.isShowing())) {
 			throw new AssertionError("Button " + buttonName + " is not showing.");
 		}
-		if (!button.isEnabled()) {
+		if (!runSwing(() -> button.isEnabled())) {
 			throw new AssertionError("Button " + buttonName + " is not enabled.");
 		}
 		pressButton(button, waitForCompletion);
diff --git a/GhidraBuild/Skeleton/src/main/java/skeleton/SkeletonLoader.java b/GhidraBuild/Skeleton/src/main/java/skeleton/SkeletonLoader.java
index b1a47437be..5011db17d2 100644
--- a/GhidraBuild/Skeleton/src/main/java/skeleton/SkeletonLoader.java
+++ b/GhidraBuild/Skeleton/src/main/java/skeleton/SkeletonLoader.java
@@ -73,11 +73,11 @@ public class SkeletonLoader extends AbstractLibrarySupportLoader {
 	}
 
 	@Override
-	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options) {
+	public String validateOptions(ByteProvider provider, LoadSpec loadSpec, List<Option> options, Program program) {
 
 		// TODO: If this loader has custom options, validate them here.  Not all options require
 		// validation.
 
-		return super.validateOptions(provider, loadSpec, options);
+		return super.validateOptions(provider, loadSpec, options, program);
 	}
 }