GP-134: Mainline changes cherry-picked from Debugger branch

.gitignore

@@ -53,6 +53,8 @@ Release
 *.aps
 *.vcproj.*
 *.vcxproj.*
+# dump files
+*.mdmp
 .vs/
 
 # Ignore UNIX backup files

Ghidra/Features/Base/certification.manifest

@@ -10,7 +10,7 @@
 ##MODULE IP: Oxygen Icons - LGPL 3.0
 ##MODULE IP: Tango Icons - Public Domain
 .gitignore||GHIDRA||||END|
-.launch/Ghidra.launch||GHIDRA||||END|
+Ghidra.launch||GHIDRA||||END|
 Module.manifest||GHIDRA||||END|
 build.gradle||GHIDRA||||END|
 data/ElfFunctionsThatDoNotReturn||GHIDRA||||END|

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/assembler/AssembleDockingAction.java

@@ -35,7 +35,6 @@ import docking.widgets.fieldpanel.support.FieldLocation;
 import ghidra.app.context.ListingActionContext;
 import ghidra.app.plugin.assembler.Assembler;
 import ghidra.app.plugin.assembler.Assemblers;
-import ghidra.app.plugin.assembler.sleigh.util.GhidraDBTransaction;
 import ghidra.app.plugin.core.assembler.AssemblyDualTextField.*;
 import ghidra.app.plugin.core.codebrowser.CodeViewerProvider;
 import ghidra.app.util.PluginConstants;
@@ -43,6 +42,7 @@ import ghidra.app.util.viewer.field.ListingField;
 import ghidra.app.util.viewer.listingpanel.ListingModelAdapter;
 import ghidra.app.util.viewer.listingpanel.ListingPanel;
 import ghidra.framework.plugintool.PluginTool;
+import ghidra.program.database.util.ProgramTransaction;
 import ghidra.program.model.address.Address;
 import ghidra.program.model.lang.Language;
 import ghidra.program.model.listing.*;
@@ -106,8 +106,8 @@ public class AssembleDockingAction extends DockingAction {
 	/*
 	 * A class for all my callbacks
 	 * 
-	 * For autocompletion, this causes activation of an assembled instruction to actually patch
+	 * For autocompletion, this causes activation of an assembled instruction to actually patch the
-	 * the instruction in.
+	 * instruction in.
 	 * 
 	 * For keyboard, it causes the escape key, if not already consumed by the autocompleter, to
 	 * cancel the assembly action altogether.
@@ -117,8 +117,8 @@ public class AssembleDockingAction extends DockingAction {
 		public void completionActivated(AutocompletionEvent<AssemblyCompletion> ev) {
 			if (ev.getSelection() instanceof AssemblyInstruction) {
 				AssemblyInstruction ins = (AssemblyInstruction) ev.getSelection();
-				try (GhidraDBTransaction trans =
+				try (ProgramTransaction trans =
-					new GhidraDBTransaction(prog, "Assemble @" + addr + ": " + input.getText())) {
+					ProgramTransaction.open(prog, "Assemble @" + addr + ": " + input.getText())) {
 					assembler.patchProgram(ins.getData(), addr);
 					trans.commit();
 					cancel(); // Not really, since I've committed. Just hides the editors.
@@ -219,6 +219,7 @@ public class AssembleDockingAction extends DockingAction {
 	/**
 	 * Retrieve the location in the code viewer's {@link FieldPanel} for the field at the given
 	 * address having the given header text
+	 * 
 	 * @param addr the address
 	 * @param fieldName the name of the field
 	 * @return if found, the {@link FieldLocation}, otherwise {@code null}

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

@@ -109,8 +109,8 @@ public class CodeBrowserPlugin extends Plugin
 	// - Icon -
 	private ImageIcon CURSOR_LOC_ICON =
 		ResourceManager.loadImage("images/cursor_arrow_flipped.gif");
-	private CodeViewerProvider connectedProvider;
+	protected final CodeViewerProvider connectedProvider;
-	private List<CodeViewerProvider> disconnectedProviders = new ArrayList<>();
+	protected List<CodeViewerProvider> disconnectedProviders = new ArrayList<>();
 	private FormatManager formatMgr;
 	private ViewManagerService viewManager;
 	private MarkerService markerService;
@@ -149,7 +149,7 @@ public class CodeBrowserPlugin extends Plugin
 		formatMgr = new FormatManager(displayOptions, fieldOptions);
 		formatMgr.addFormatModelListener(this);
 		formatMgr.setServiceProvider(tool);
-		connectedProvider = new CodeViewerProvider(this, formatMgr, true);
+		connectedProvider = createProvider(formatMgr, true);
 		tool.showComponentProvider(connectedProvider, true);
 		initOptions(fieldOptions);
 		initDisplayOptions(displayOptions);
@@ -164,6 +164,10 @@ public class CodeBrowserPlugin extends Plugin
 		createActions();
 	}
 
+	protected CodeViewerProvider createProvider(FormatManager formatManager, boolean isConnected) {
+		return new CodeViewerProvider(this, formatManager, isConnected);
+	}
+
 	private void createActions() {
 		DockingAction selectAllAction = new SelectAllAction(getName());
 		selectAllAction.getMenuBarData().setMenuSubGroup("a");
@@ -178,7 +182,7 @@ public class CodeBrowserPlugin extends Plugin
 		tool.addAction(selectComplementAction);
 	}
 
-	void viewChanged(AddressSetView addrSet) {
+	protected void viewChanged(AddressSetView addrSet) {
 		ProgramLocation currLoc = getCurrentLocation();
 		currentView = addrSet;
 		if (addrSet != null && !addrSet.isEmpty()) {
@@ -217,7 +221,7 @@ public class CodeBrowserPlugin extends Plugin
 		}
 	}
 
-	private void updateBackgroundColorModel() {
+	protected void updateBackgroundColorModel() {
 		ListingPanel listingPanel = connectedProvider.getListingPanel();
 		if (markerService != null) {
 			AddressIndexMap indexMap = connectedProvider.getListingPanel().getAddressIndexMap();
@@ -234,7 +238,7 @@ public class CodeBrowserPlugin extends Plugin
 	@Override
 	public CodeViewerProvider createNewDisconnectedProvider() {
 		CodeViewerProvider newProvider =
-			new CodeViewerProvider(this, formatMgr.createClone(), false);
+			createProvider(formatMgr.createClone(), false);
 		newProvider.setClipboardService(tool.getService(ClipboardService.class));
 		disconnectedProviders.add(newProvider);
 		if (dndProvider != null) {
@@ -425,7 +429,7 @@ public class CodeBrowserPlugin extends Plugin
 		}
 	}
 
-	private void programClosed(Program closedProgram) {
+	protected void programClosed(Program closedProgram) {
 		Iterator<CodeViewerProvider> iterator = disconnectedProviders.iterator();
 		while (iterator.hasNext()) {
 			CodeViewerProvider provider = iterator.next();
@@ -1008,6 +1012,7 @@ public class CodeBrowserPlugin extends Plugin
 
 	/**
 	 * Positions the cursor to the given location
+	 * 
 	 * @param address the address to goto
 	 * @param fieldName the name of the field to
 	 * @param row the row within the given field
@@ -1020,6 +1025,7 @@ public class CodeBrowserPlugin extends Plugin
 
 	/**
 	 * Positions the cursor to the given location
+	 * 
 	 * @param addr the address to goto
 	 * @param fieldName the name of the field to
 	 * @param occurrence specifies the which occurrence for multiple fields of same type

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

@@ -417,7 +417,7 @@ public class CodeViewerProvider extends NavigatableComponentProviderAdapter
 		contextChanged();
 	}
 
-	void updateTitle() {
+	protected void updateTitle() {
 		String subTitle = program == null ? "" : ' ' + program.getDomainFile().getName();
 		String newTitle = TITLE + subTitle;
 		if (!isConnected()) {
@@ -1004,8 +1004,8 @@ public class CodeViewerProvider extends NavigatableComponentProviderAdapter
 	}
 
 	/**
-	 * A class that allows clients to install transient highlighters while keeping the
+	 * A class that allows clients to install transient highlighters while keeping the middle-mouse
-	 * middle-mouse highlighting on at the same time.
+	 * highlighting on at the same time.
 	 */
 	private class ProgramHighlighterProvider implements HighlightProvider {
 
@@ -1043,6 +1043,7 @@ public class CodeViewerProvider extends NavigatableComponentProviderAdapter
 
 	/**
 	 * Add the ListingDisplayListener to the listing panel
+	 * 
 	 * @param listener the listener to add
 	 */
 	public void addListingDisplayListener(ListingDisplayListener listener) {
@@ -1051,6 +1052,7 @@ public class CodeViewerProvider extends NavigatableComponentProviderAdapter
 
 	/**
 	 * Remove the ListingDisplayListener from the listing panel
+	 * 
 	 * @param listener the listener to remove
 	 */
 	public void removeListingDisplayListener(ListingDisplayListener listener) {

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

@@ -24,27 +24,40 @@ import ghidra.app.util.viewer.listingpanel.ListingBackgroundColorModel;
 import ghidra.app.util.viewer.listingpanel.ListingPanel;
 import ghidra.app.util.viewer.util.AddressIndexMap;
 import ghidra.program.model.address.Address;
+import ghidra.program.model.listing.Program;
 
 /**
  * {@link BackgroundColorModel} for coloring the Listing based on the {@link MarkerService}
  */
 public class MarkerServiceBackgroundColorModel implements ListingBackgroundColorModel {
 	private MarkerService markerService;
+	private Program program;
 	private AddressIndexMap indexMap;
 	private Color defaultBackgroundColor = Color.WHITE;
 
-	public MarkerServiceBackgroundColorModel(MarkerService markerService,
+	public MarkerServiceBackgroundColorModel(MarkerService markerService, Program program,
 			AddressIndexMap indexMap) {
 		this.markerService = markerService;
+		this.program = program;
 		this.indexMap = indexMap;
 	}
 
+	public MarkerServiceBackgroundColorModel(MarkerService markerService,
+			AddressIndexMap indexMap) {
+		this(markerService, null, indexMap);
+	}
+
 	@Override
 	public Color getBackgroundColor(BigInteger index) {
 		Address addr = indexMap.getAddress(index);
 		Color color = null;
 		if (addr != null) {
-			color = markerService.getBackgroundColor(addr);
+			if (program == null) {
+				color = markerService.getBackgroundColor(addr);
+			}
+			else {
+				color = markerService.getBackgroundColor(program, addr);
+			}
 		}
 		if (color == null) {
 			color = defaultBackgroundColor;
@@ -64,7 +77,7 @@ public class MarkerServiceBackgroundColorModel implements ListingBackgroundColor
 
 	@Override
 	public void modelDataChanged(ListingPanel listingPanel) {
+		this.program = listingPanel.getProgram();
 		this.indexMap = listingPanel.getAddressIndexMap();
 	}
-
 }

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/interpreter/InterpreterComponentProvider.java

@@ -158,6 +158,15 @@ public class InterpreterComponentProvider extends ComponentProviderAdapter
 		return panel.getErrWriter();
 	}
 
+	/**
+	 * For testing purposes, but should probably be promoted to InterpreterConsole interface
+	 * 
+	 * @return the prompt;
+	 */
+	public String getPrompt() {
+		return panel.getPrompt();
+	}
+
 	@Override
 	public void setPrompt(String prompt) {
 		panel.setPrompt(prompt);
@@ -186,4 +195,33 @@ public class InterpreterComponentProvider extends ComponentProviderAdapter
 	public void addFirstActivationCallback(Callback activationCallback) {
 		firstActivationCallbacks.add(activationCallback);
 	}
+
+	@Override
+	public boolean isInputPermitted() {
+		return panel.isInputPermitted();
+	}
+
+	@Override
+	public void setInputPermitted(boolean permitted) {
+		panel.setInputPermitted(permitted);
+	}
+
+	@Override
+	public void show() {
+		tool.showComponentProvider(this, true);
+	}
+
+	@Override
+	public void updateTitle() {
+		tool.updateTitle(this);
+	}
+
+	/**
+	 * For testing purposes only
+	 * 
+	 * @return the text in the output buffer
+	 */
+	public String getOutputText() {
+		return panel.getOutputText();
+	}
 }

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/interpreter/InterpreterConsole.java

@@ -31,6 +31,8 @@ public interface InterpreterConsole extends Disposable {
 	 * continuously checks the InterpreterConsole for new input.  It would be much easier
 	 * if the InterpreterConsole could just asynchronously send an update to whoever is using 
 	 * it every time a new thing is entered into it.  
+	 * 
+	 *The same problem applies to debugger interpreters / consoles.
 	 */
 
 	public void clear();
@@ -48,19 +50,54 @@ public interface InterpreterConsole extends Disposable {
 	public void setPrompt(String prompt);
 
 	/**
-	 * Signals that this console is one that the user can remove from the tool as desired.  If
+	 * Signals that this console is one that the user can remove from the tool as desired. If this
-	 * this method is not called, then the user cannot remove the console from the tool, which 
+	 * method is not called, then the user cannot remove the console from the tool, which means that
-	 * means that closing the console only hides it.
+	 * closing the console only hides it.
 	 */
 	public void setTransient();
-	
+
 	public void addAction(DockingAction action);
 
 	/**
-	 * Adds the given callback which will get called the first time the interpreter console
+	 * Adds the given callback which will get called the first time the interpreter console is
-	 * is activated.
+	 * activated.
 	 * 
 	 * @param activationCallback The callback to execute when activation occurs for the first time.
 	 */
 	public void addFirstActivationCallback(Callback activationCallback);
+
+	/**
+	 * Checks whether the user can input commands.
+	 * 
+	 * @return true if permitted, false if prohibited
+	 */
+	public boolean isInputPermitted();
+
+	/**
+	 * Controls whether the user can input commands.
+	 * 
+	 * @param permitted true to permit input, false to prohibit input
+	 */
+	public void setInputPermitted(boolean permitted);
+
+	/**
+	 * Check if the console is visible
+	 * 
+	 * <p>
+	 * Note if the console is on-screen, but occluded by other windows, this still returns
+	 * {@code true}.
+	 * 
+	 * @return true if visible, false if hidden
+	 */
+	public boolean isVisible();
+
+	/**
+	 * Show the console's provider in the tool
+	 */
+	public void show();
+
+	/**
+	 * Notify the tool that this console's title has changed
+	 */
+	public void updateTitle();
 }

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/interpreter/InterpreterPanel.java

@@ -564,6 +564,10 @@ public class InterpreterPanel extends JPanel implements OptionsChangeListener {
 		outputTextPane.setText("");
 	}
 
+	public String getOutputText() {
+		return outputTextPane.getText();
+	}
+
 	public InputStream getStdin() {
 		return stdin;
 	}
@@ -584,6 +588,10 @@ public class InterpreterPanel extends JPanel implements OptionsChangeListener {
 		return errWriter;
 	}
 
+	public String getPrompt() {
+		return promptTextPane.getText();
+	}
+
 	public void setPrompt(String prompt) {
 		try {
 			final Document document = promptTextPane.getDocument();
@@ -654,6 +662,14 @@ public class InterpreterPanel extends JPanel implements OptionsChangeListener {
 		inputAttributes.addAttributes(attributes);
 	}
 
+	public boolean isInputPermitted() {
+		return inputTextPane.isEditable();
+	}
+
+	public void setInputPermitted(boolean permitted) {
+		inputTextPane.setEditable(permitted);
+	}
+
 //==================================================================================================
 // Inner Classes
 //==================================================================================================
@@ -735,6 +751,7 @@ public class InterpreterPanel extends JPanel implements OptionsChangeListener {
 
 		/**
 		 * Overridden to stop this stream from blocking.
+		 * 
 		 * @throws IOException not
 		 */
 		@Override
@@ -760,5 +777,4 @@ public class InterpreterPanel extends JPanel implements OptionsChangeListener {
 			this.notify();
 		}
 	}
-
 }

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/stackeditor/BiDirectionDataType.java

@@ -22,7 +22,7 @@ import ghidra.program.model.data.*;
 import ghidra.util.exception.AssertException;
 
 /**
- * BiDirectionDataType is a special structure data type that allows both positive and negative 
+ * BiDirectionDataType is a special structure data type that allows both positive and negative
  * offset values.
  */
 public abstract class BiDirectionDataType extends StructureDataType
@@ -346,12 +346,12 @@ public abstract class BiDirectionDataType extends StructureDataType
 	}
 
 	/**
-	 * Increases the size of the bidirectional data type
+	 * Increases the size of the bidirectional data type If amount is positive then the positive
-	 * If amount is positive then the positive offset side will grow by the 
+	 * offset side will grow by the indicated amount. If amount is negative, the data type grows on
-	 * indicated amount. If amount is negative, the data type grows on the 
+	 * the negative offsets side.
-	 * negative offsets side.
+	 * 
-	 * @param amount Positive value indicates number of bytes to add to positive side.
+	 * @param amount Positive value indicates number of bytes to add to positive side. Negative
-	 * Negative value indicates number of bytes to add to negative side.
+	 *            value indicates number of bytes to add to negative side.
 	 */
 	@Override
 	public void growStructure(int amount) {
@@ -567,7 +567,7 @@ public abstract class BiDirectionDataType extends StructureDataType
 //	}
 
 	@Override
-	public abstract DataType clone(DataTypeManager dtm);
+	public abstract BiDirectionDataType clone(DataTypeManager dtm);
 
 	@Override
 	public void clearComponent(int index) {
@@ -709,21 +709,20 @@ public abstract class BiDirectionDataType extends StructureDataType
 	}
 
 	/**
-	 * Replace the indicated component with a new component containing the 
+	 * Replace the indicated component with a new component containing the specified data type.
-	 * specified data type.
+	 * 
 	 * @param origDtc the original data type component in this structure.
 	 * @param dataType the data type of the new component
 	 * @param length the length of the new component
 	 * @param newName the field name of the new component
 	 * @param comment the comment for the new component
 	 * @return the new component or null if the new component couldn't fit.
-	 * @throws IllegalArgumentException if the dataType.getLength() is positive 
+	 * @throws IllegalArgumentException if the dataType.getLength() is positive and does not match
-	 * and does not match the given length parameter.
+	 *             the given length parameter.
-	 * @throws IllegalArgumentException if the specified data type is not 
+	 * @throws IllegalArgumentException if the specified data type is not allowed to replace a
-	 * allowed to replace a component in this composite data type.
+	 *             component in this composite data type. For example, suppose dt1 contains dt2.
-	 * For example, suppose dt1 contains dt2. Therefore it is not valid
+	 *             Therefore it is not valid to replace a dt2 component with dt1 since this would
-	 * to replace a dt2 component with dt1 since this would cause a cyclic 
+	 *             cause a cyclic dependency.
-	 * dependency.
 	 */
 	private DataTypeComponent replace(DataTypeComponent origDtc, DataType dataType, int length,
 			String newName, String comment) {

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/stackeditor/StackFrameDataType.java

@@ -15,6 +15,9 @@
  */
 package ghidra.app.plugin.core.stackeditor;
 
+import java.util.Collections;
+import java.util.Iterator;
+
 import ghidra.docking.settings.Settings;
 import ghidra.program.model.data.*;
 import ghidra.program.model.listing.*;
@@ -25,10 +28,7 @@ import ghidra.util.InvalidNameException;
 import ghidra.util.exception.AssertException;
 import ghidra.util.exception.InvalidInputException;
 
-import java.util.Collections;
+/**
-import java.util.Iterator;
-
-/** 
  * StackFrameDataType provides an editable copy of a function stack frame.
  */
 public class StackFrameDataType extends BiDirectionDataType {
@@ -43,17 +43,20 @@ public class StackFrameDataType extends BiDirectionDataType {
 
 	/**
 	 * Constructor for an editable stack frame for use with the editor.
+	 * 
 	 * @param stack the function stack frame to be edited.
 	 */
 	public StackFrameDataType(StackFrame stack, DataTypeManager dtm) {
-		super((stack.getFunction() != null) ? stack.getFunction().getName()
+		super(
-				: "StackWithoutFunction", 0, 0, stack.getParameterOffset(), dtm);
+			(stack.getFunction() != null) ? stack.getFunction().getName() : "StackWithoutFunction",
+			0, 0, stack.getParameterOffset(), dtm);
 		this.stack = stack;
 		initialize();
 	}
 
 	/**
 	 * Constructor for an editable stack frame for use with the editor.
+	 * 
 	 * @param stack the function stack frame to be edited.
 	 */
 	public StackFrameDataType(StackFrameDataType stackDt, DataTypeManager dtm) {
@@ -67,8 +70,8 @@ public class StackFrameDataType extends BiDirectionDataType {
 		this.defaultSettings = stackDt.defaultSettings;
 		for (int i = 0; i < stackDt.components.size(); i++) {
 			DataTypeComponent dtc = stackDt.components.get(i);
-			replaceAtOffset(dtc.getOffset(), dtc.getDataType(), dtc.getLength(),
+			replaceAtOffset(dtc.getOffset(), dtc.getDataType(), dtc.getLength(), dtc.getFieldName(),
-				dtc.getFieldName(), dtc.getComment());
+				dtc.getComment());
 		}
 	}
 
@@ -132,6 +135,7 @@ public class StackFrameDataType extends BiDirectionDataType {
 	/* (non-Javadoc)
 	 * @see ghidra.program.model.data.DataType#getRepresentation(ghidra.program.model.mem.MemBuffer, ghidra.util.settings.Settings, int)
 	 */
+	@Override
 	public String getRepresentation(MemBuffer buf, Settings settings, int length) {
 		return "";
 	}
@@ -145,7 +149,7 @@ public class StackFrameDataType extends BiDirectionDataType {
 	}
 
 	@Override
-	public DataType clone(DataTypeManager dtm) {
+	public StackFrameDataType clone(DataTypeManager dtm) {
 		return new StackFrameDataType(this, dtm);
 	}
 
@@ -164,10 +168,10 @@ public class StackFrameDataType extends BiDirectionDataType {
 	}
 
 	/**
-	 * If a stack variable is defined in the editor at the specified offset,
+	 * If a stack variable is defined in the editor at the specified offset, this retrieves the
-	 * this retrieves the editor element containing that stack variable
+	 * editor element containing that stack variable <BR>
-	 * <BR>Note: if a stack variable isn't defined at the indicated offset
+	 * Note: if a stack variable isn't defined at the indicated offset then null is returned.
-	 * then null is returned.
+	 * 
 	 * @param offset the offset
 	 * @return the stack editor's element at the offset. Otherwise, null.
 	 */
@@ -183,11 +187,11 @@ public class StackFrameDataType extends BiDirectionDataType {
 	}
 
 	/**
-	 * If a stack variable is defined in the editor at the specified ordinal,
+	 * If a stack variable is defined in the editor at the specified ordinal, this retrieves the
-	 * this retrieves the editor element containing that stack variable.
+	 * editor element containing that stack variable. <BR>
-	 * <BR>Note: if a stack variable isn't defined for the indicated ordinal
+	 * Note: if a stack variable isn't defined for the indicated ordinal then null is returned.
-	 * then null is returned.
+	 * 
-	 * @param ordinal the ordinal 
+	 * @param ordinal the ordinal
 	 * @return the stack editor's element at the ordinal. Otherwise, null.
 	 */
 	public DataTypeComponent getDefinedComponentAtOrdinal(int ordinal) {
@@ -315,6 +319,7 @@ public class StackFrameDataType extends BiDirectionDataType {
 
 	/**
 	 * Undefines any defined stack variables in the indicated offset range.
+	 * 
 	 * @param minOffset the range's minimum offset on the stack frame
 	 * @param maxOffset the range's maximum offset on the stack frame
 	 */
@@ -335,6 +340,7 @@ public class StackFrameDataType extends BiDirectionDataType {
 
 	/**
 	 * Deletes the indicated range of bytes from this stack frame data type.
+	 * 
 	 * @param minOffset the range's minimum offset on the stack frame
 	 * @param maxOffset the range's maximum offset on the stack frame
 	 */
@@ -379,9 +385,8 @@ public class StackFrameDataType extends BiDirectionDataType {
 			String fieldName = dtc.getFieldName();
 			int offset = dtc.getOffset();
 			try {
-				vars[i] =
+				vars[i] = new LocalVariableImpl(fieldName, dtc.getDataType(), offset,
-					new LocalVariableImpl(fieldName, dtc.getDataType(), offset,
+					function.getProgram());
-						function.getProgram());
 			}
 			catch (InvalidInputException e) {
 				try {
@@ -402,6 +407,7 @@ public class StackFrameDataType extends BiDirectionDataType {
 
 	/**
 	 * Sets the name of the component at the specified ordinal.
+	 * 
 	 * @param ordinal the ordinal
 	 * @param name the new name. Null indicates the default name.
 	 */
@@ -433,6 +439,7 @@ public class StackFrameDataType extends BiDirectionDataType {
 
 	/**
 	 * Sets the comment at the specified ordinal.
+	 * 
 	 * @param ordinal the ordinal
 	 * @param comment the new comment.
 	 */
@@ -483,6 +490,7 @@ public class StackFrameDataType extends BiDirectionDataType {
 
 	/**
 	 * Currently no validation is done on the name.
+	 * 
 	 * @param ordinal
 	 * @param newName
 	 * @throws InvalidNameException
@@ -509,8 +517,9 @@ public class StackFrameDataType extends BiDirectionDataType {
 	}
 
 	/**
-	 * Effectively moves a component for a defined stack variable if it will fit
+	 * Effectively moves a component for a defined stack variable if it will fit where it is being
-	 * where it is being moved to in the stack frame.
+	 * moved to in the stack frame.
+	 * 
 	 * @param ordinal the ordinal of the component to move by changing its offset.
 	 * @param newOffset the offset to move the variable to.
 	 * @return the component representing the stack variable at the new offset.
@@ -523,8 +532,8 @@ public class StackFrameDataType extends BiDirectionDataType {
 		if (newOffset == oldOffset) {
 			return comp;
 		}
-		if ((oldOffset >= splitOffset) && (newOffset < splitOffset) || (oldOffset < splitOffset) &&
+		if ((oldOffset >= splitOffset) && (newOffset < splitOffset) ||
-			(newOffset >= splitOffset)) {
+			(oldOffset < splitOffset) && (newOffset >= splitOffset)) {
 			throw new InvalidInputException(
 				"Cannot move a stack variable/parameter across the parameter offset.");
 		}
@@ -548,17 +557,17 @@ public class StackFrameDataType extends BiDirectionDataType {
 		String defaultName = getDefaultName(comp);
 		String oldName = comp.getFieldName();
 		boolean isDefault = (oldName == null) || (oldName.equals(defaultName));
-		DataTypeComponent newComp =
+		DataTypeComponent newComp = replaceAtOffset(newOffset, comp.getDataType(), comp.getLength(),
-			replaceAtOffset(newOffset, comp.getDataType(), comp.getLength(), isDefault ? null
+			isDefault ? null : oldName, comp.getComment());
-					: oldName, comp.getComment());
 		return newComp;
 	}
 
 	/**
-	 * Sets a component representing the defined stack variable at the indicated 
+	 * Sets a component representing the defined stack variable at the indicated ordinal to have the
-	 * ordinal to have the specified data type and length.
+	 * specified data type and length.
+	 * 
 	 * @param ordinal the ordinal
-	 * @param type	the data type
+	 * @param type the data type
 	 * @param length the length or size of this variable.
 	 * @return the component representing this stack variable.
 	 */
@@ -569,6 +578,7 @@ public class StackFrameDataType extends BiDirectionDataType {
 
 	/**
 	 * Get the maximum variable size that will fit at the indicated offset if a replace is done.
+	 * 
 	 * @param offset
 	 * @return the maximum size
 	 */
@@ -604,6 +614,7 @@ public class StackFrameDataType extends BiDirectionDataType {
 
 	/**
 	 * Returns the default name for the indicated stack offset.
+	 * 
 	 * @param offset
 	 * @return the default stack variable name.
 	 */
@@ -623,9 +634,8 @@ public class StackFrameDataType extends BiDirectionDataType {
 
 	/**
 	 * @param element
-	 * @return the index number for this parameter
+	 * @return the index number for this parameter (starting at 1 for the first parameter.) 0 if the
-	 * (starting at 1 for the first parameter.)
+	 *         element is not a parameter.
-	 * 0 if the element is not a parameter.
 	 */
 	private int getParameterIndex(DataTypeComponent element) {
 		int numComps = components.size();
@@ -668,6 +678,7 @@ public class StackFrameDataType extends BiDirectionDataType {
 
 	/**
 	 * Returns true if a stack variable is defined at the specified ordinal.
+	 * 
 	 * @param ordinal
 	 * @return true if variable is defined at ordinal or false if undefined.
 	 */

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

@@ -20,13 +20,13 @@ import ghidra.program.model.data.*;
 import ghidra.program.model.mem.MemBuffer;
 
 class MSRichProductInfoDataType extends StructureDataType {
-	
+
 	private final CompId compid;
-	
+
 	public MSRichProductInfoDataType(CompId compid) {
 		this(compid, null);
 	}
-	
+
 	public MSRichProductInfoDataType(CompId compid, DataTypeManager dtm) {
 		super(new CategoryPath("/PE"), "ProductInfo", 0, dtm);
 		this.compid = compid;
@@ -39,7 +39,7 @@ class MSRichProductInfoDataType extends StructureDataType {
 	}
 
 	@Override
-	public DataType clone(DataTypeManager dtm) {
+	public MSRichProductInfoDataType clone(DataTypeManager dtm) {
 		if (dtm == getDataTypeManager()) {
 			return this;
 		}

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

@@ -22,7 +22,7 @@ import ghidra.program.model.mem.MemBuffer;
 class RichTableRecordDataType extends StructureDataType {
 
 	private final RichHeaderRecord record;
-			
+
 	public RichTableRecordDataType(RichHeaderRecord record) {
 		this(null, record);
 	}
@@ -40,7 +40,7 @@ class RichTableRecordDataType extends StructureDataType {
 	}
 
 	@Override
-	public DataType clone(DataTypeManager dtm) {
+	public RichTableRecordDataType clone(DataTypeManager dtm) {
 		if (dtm == getDataTypeManager()) {
 			return this;
 		}
@@ -64,7 +64,7 @@ class RichTableRecordDataType extends StructureDataType {
 
 	@Override
 	public Object getValue(MemBuffer buf, Settings settings, int length) {
-		return record;				
+		return record;
 	}
 
 	@Override
@@ -81,5 +81,5 @@ class RichTableRecordDataType extends StructureDataType {
 		add(new MSRichProductInfoDataType(record.getCompId()), 4, "productInfo", null);
 		add(new RichObjectCountDataType(record.getObjectCount()), 4, "objectCount", null);
 	}
-	
+
 }

Ghidra/Features/Base/src/main/java/ghidra/app/util/viewer/listingpanel/ListingPanel.java

@@ -93,6 +93,7 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 
 	/**
 	 * Constructs a new ListingPanel using the given FormatManager and ServiceProvider.
+	 * 
 	 * @param manager the FormatManager to use.
 	 */
 	public ListingPanel(FormatManager manager) {
@@ -122,6 +123,7 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 
 	/**
 	 * Constructs a new ListingPanel for the given program.
+	 * 
 	 * @param mgr the FormatManager to use.
 	 * @param program the program for which to create a new ListingPanel
 	 */
@@ -132,6 +134,7 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 
 	/**
 	 * Constructs a new ListingPanel with the given FormatManager and ListingLayoutModel
+	 * 
 	 * @param mgr the FormatManager to use
 	 * @param model the ListingLayoutModel to use.
 	 */
@@ -177,6 +180,7 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 
 	/**
 	 * Sets the ProgramLocationListener. Only one listener is supported
+	 * 
 	 * @param listener the ProgramLocationListener to use.
 	 */
 	public void setProgramLocationListener(ProgramLocationListener listener) {
@@ -185,6 +189,7 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 
 	/**
 	 * Sets the ProgramSelectionListener. Only one listener is supported
+	 * 
 	 * @param listener the ProgramSelectionListener to use.
 	 */
 	public void setProgramSelectionListener(ProgramSelectionListener listener) {
@@ -197,6 +202,7 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 
 	/**
 	 * Sets the ListingLayoutModel to use.
+	 * 
 	 * @param newModel the model to use.
 	 */
 	public void setListingModel(ListingModel newModel) {
@@ -215,8 +221,8 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 	}
 
 	/**
-	 * Sets whether or not the field header component is visible at the top of the
+	 * Sets whether or not the field header component is visible at the top of the listing panel
-	 * listing panel
+	 * 
 	 * @param show if true, the header component will be show, otherwise it will be hidden.
 	 */
 	public void showHeader(boolean show) {
@@ -275,8 +281,9 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 
 	/**
 	 * Adds the MarginProvider to this panel
-	 * @param provider the MarginProvider that will provide components to display in this
+	 * 
-	 * panel's left margin area.
+	 * @param provider the MarginProvider that will provide components to display in this panel's
+	 *            left margin area.
 	 */
 	public void addMarginProvider(MarginProvider provider) {
 		if (provider.isResizeable()) {
@@ -366,6 +373,7 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 
 	/**
 	 * Add a change listener to be notified whenever the indexMap changes.
+	 * 
 	 * @param listener the listener to be added.
 	 */
 	public void addIndexMapChangeListener(ChangeListener listener) {
@@ -374,6 +382,7 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 
 	/**
 	 * Removes the change listener to be notified when the indexMap changes.
+	 * 
 	 * @param listener the listener to be removed.
 	 */
 	public void removeIndexMapChangeListener(ChangeListener listener) {
@@ -382,6 +391,7 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 
 	/**
 	 * Removes the given margin provider from this panel
+	 * 
 	 * @param provider the MarginProvider to remove.
 	 */
 	public void removeMarginProvider(MarginProvider provider) {
@@ -390,8 +400,8 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 	}
 
 	/**
-	 * Adds the given OverviewProvider with will be displayed in this panels right margin
+	 * Adds the given OverviewProvider with will be displayed in this panels right margin area.
-	 * area.
+	 * 
 	 * @param provider the OverviewProvider to display.
 	 */
 	public void addOverviewProvider(OverviewProvider provider) {
@@ -402,6 +412,7 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 
 	/**
 	 * Removes the given OverviewProvider from this panel
+	 * 
 	 * @param provider the OverviewProvider to remove.
 	 */
 	public void removeOverviewProvider(OverviewProvider provider) {
@@ -410,8 +421,9 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 	}
 
 	/**
-	 * Adds a ButtonPressedListener to be notified when the user presses the mouse button while
+	 * Adds a ButtonPressedListener to be notified when the user presses the mouse button while over
-	 * over this panel
+	 * this panel
+	 * 
 	 * @param listener the ButtonPressedListener to add.
 	 */
 	public void addButtonPressedListener(ButtonPressedListener listener) {
@@ -422,6 +434,7 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 
 	/**
 	 * Removes the given ButtonPressedListener.
+	 * 
 	 * @param listener the ButtonPressedListener to remove.
 	 */
 	public void removeButtonPressedListener(ButtonPressedListener listener) {
@@ -441,8 +454,8 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 	}
 
 	/**
-	 * Adds a {@link HighlightProvider} to this listing.   This highlight provider will be used
+	 * Adds a {@link HighlightProvider} to this listing. This highlight provider will be used with
-	 * with any other registered providers to paint all the highlights for this listing.
+	 * any other registered providers to paint all the highlights for this listing.
 	 *
 	 * @param highlightProvider The provider to add
 	 */
@@ -492,6 +505,7 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 
 	/**
 	 * Sets the divider location between the left margin areas and the main display.
+	 * 
 	 * @param dividerLocation the location to set on the divider.
 	 */
 	public void setDividerLocation(int dividerLocation) {
@@ -537,8 +551,9 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 	}
 
 	/**
-	 * Moves the cursor to the given program location and repositions the scrollbar to
+	 * Moves the cursor to the given program location and repositions the scrollbar to show that
-	 * show that location in the screen.
+	 * location in the screen.
+	 * 
 	 * @param loc the location to move to.
 	 */
 	public boolean goTo(ProgramLocation loc) {
@@ -546,15 +561,14 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 	}
 
 	/**
-	 * Moves the cursor to the given program location.  Also, repositions the scrollbar to
+	 * Moves the cursor to the given program location. Also, repositions the scrollbar to show that
-	 * show that location, if the location is not on the screen.
+	 * location, if the location is not on the screen.
 	 *
 	 * @param loc the location to move to.
-	 * @param centerWhenNotVisible this variable only has an effect if the given location is not
+	 * @param centerWhenNotVisible this variable only has an effect if the given location is not on
-	 *                             on the screen.  In that case, when this parameter is true, then
+	 *            the screen. In that case, when this parameter is true, then the given location
-	 *                             the given location will be placed in the center of the screen;
+	 *            will be placed in the center of the screen; when the parameter is false, then the
-	 *                             when the parameter is false, then the screen will be scrolled
+	 *            screen will be scrolled only enough to show the cursor.
-	 *                             only enough to show the cursor.
 	 */
 	public boolean goTo(ProgramLocation loc, boolean centerWhenNotVisible) {
 		final FieldLocation floc = getFieldLocation(loc);
@@ -695,6 +709,7 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 
 	/**
 	 * Positions the ListingPanel to the given address.
+	 * 
 	 * @param addr the address at which to position the listing.
 	 */
 	public boolean goTo(Address addr) {
@@ -707,8 +722,9 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 
 	/**
 	 * Positions the ListingPanel to the given address.
+	 * 
 	 * @param currentAddress used to determine which symbol to goto if the goto address has more
-	 * than one
+	 *            than one
 	 * @param gotoAddress the address at which to position to listing.
 	 * @return true if the address exists
 	 */
@@ -749,6 +765,7 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 
 	/**
 	 * Sets the program to be displayed by this listing panel
+	 * 
 	 * @param program the program to display.
 	 */
 	public void setProgram(Program program) {
@@ -782,6 +799,7 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 
 	/**
 	 * Restricts the program's view to the given address set
+	 * 
 	 * @param view the set of address to include in the view.
 	 */
 	public void setView(AddressSetView view) {
@@ -799,11 +817,11 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 	}
 
 	/**
-	 * Sets the externally supplied {@link ListingBackgroundColorModel} to be blended with
+	 * Sets the externally supplied {@link ListingBackgroundColorModel} to be blended with its own
-	 * its own {@link PropertyBasedBackgroundColorModel}.
+	 * {@link PropertyBasedBackgroundColorModel}.
 	 *
-	 * @param colorModel the {@link ListingBackgroundColorModel} to use in conjunction with
+	 * @param colorModel the {@link ListingBackgroundColorModel} to use in conjunction with the
-	 * the built-in {@link PropertyBasedBackgroundColorModel}
+	 *            built-in {@link PropertyBasedBackgroundColorModel}
 	 */
 	public void setBackgroundColorModel(ListingBackgroundColorModel colorModel) {
 		if (colorModel == null) {
@@ -817,8 +835,8 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 	}
 
 	/**
-	 * Sets the background color for the listing panel.  This will set the background for the
+	 * Sets the background color for the listing panel. This will set the background for the main
-	 * main listing display.
+	 * listing display.
 	 */
 	public void setTextBackgroundColor(Color c) {
 		if (fieldPanel != null) {
@@ -854,8 +872,8 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 
 	/**
 	 * Get a program location for the given point.
-	 * @return program location, or null if point does not correspond
+	 * 
-	 * to a program location
+	 * @return program location, or null if point does not correspond to a program location
 	 */
 	public ProgramLocation getProgramLocation(Point point) {
 		FieldLocation dropLoc = new FieldLocation();
@@ -890,13 +908,27 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 
 	/**
 	 * Sets the cursor to the given program location.
+	 * 
 	 * @param loc the location at which to move the cursor.
 	 */
 	public void setCursorPosition(ProgramLocation loc) {
+		setCursorPosition(loc, EventTrigger.API_CALL);
+	}
+
+	/**
+	 * Sets the cursor to the given program location with a given trigger
+	 * 
+	 * This method should only be used in automated testing to programmatically simulate a user
+	 * navigating within the listing panel.
+	 * 
+	 * @param loc the location at which to move the cursor.
+	 * @param trigger the event trigger
+	 */
+	public void setCursorPosition(ProgramLocation loc, EventTrigger trigger) {
 		FieldLocation floc = getFieldLocation(loc);
 		if (floc != null) {
 			fieldPanel.setCursorPosition(floc.getIndex(), floc.getFieldNum(), floc.getRow(),
-				floc.getCol());
+				floc.getCol(), trigger);
 		}
 	}
 
@@ -991,8 +1023,7 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 	}
 
 	/**
-	 * Sets the selection to the complement of the current selection in the
+	 * Sets the selection to the complement of the current selection in the listing view.
-	 * listing view.
 	 */
 	public AddressSet selectComplement() {
 		fieldPanel.requestFocus();
@@ -1006,6 +1037,7 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 
 	/**
 	 * Sets the selection.
+	 * 
 	 * @param sel the new selection
 	 */
 	public void setSelection(ProgramSelection sel) {
@@ -1043,6 +1075,7 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 
 	/**
 	 * Sets the highlight.
+	 * 
 	 * @param highlight the new highlight.
 	 */
 	public void setHighlight(ProgramSelection highlight) {
@@ -1084,7 +1117,7 @@ public class ListingPanel extends JPanel implements FieldMouseListener, FieldLoc
 	}
 
 	/**
-	 * Sets listing panel to never show scroll bars.  This is useful when you want this listing's
+	 * Sets listing panel to never show scroll bars. This is useful when you want this listing's
 	 * parent to always be as big as this listing.
 	 */
 	public void setNeverSroll() {

Ghidra/Framework/Docking/src/main/java/docking/widgets/fieldpanel/FieldPanel.java

@@ -15,7 +15,7 @@
  */
 package docking.widgets.fieldpanel;
 
-import static docking.widgets.EventTrigger.*;
+import static docking.widgets.EventTrigger.INTERNAL_ONLY;
 
 import java.awt.*;
 import java.awt.event.*;
@@ -129,14 +129,12 @@ public class FieldPanel extends JPanel
 	}
 
 	/**
-	 * Makes sure the location is completely visible on the screen. If it
+	 * Makes sure the location is completely visible on the screen. If it already is visible, this
-	 * already is visible, this routine will do nothing. If the location is
+	 * routine will do nothing. If the location is above the screen (at an index less than the first
-	 * above the screen (at an index less than the first line on the
+	 * line on the screen), the view will be scrolled such that the location will appear at the top
-	 * screen), the view will be scrolled such that the location will appear
+	 * of the screen. If the location is below the screen, the view will be scrolled such that the
-	 * at the top of the screen. If the location is below the screen, the view
+	 * location will appear at the bottom the screen. The layouts[] array will be updated to reflect
-	 * will be scrolled such that the location will appear at the bottom the
+	 * the current view.
-	 * screen. The layouts[] array will be updated to reflect the current
-	 * view.
 	 */
 	private void doScrollTo(FieldLocation location) {
 		if (layouts.isEmpty()) {
@@ -299,7 +297,7 @@ public class FieldPanel extends JPanel
 	}
 
 	/**
-	 * Returns true if the given field location is rendered on the screen; false if scrolled 
+	 * Returns true if the given field location is rendered on the screen; false if scrolled
 	 * offscreen
 	 * 
 	 * @param location the location to check
@@ -409,8 +407,7 @@ public class FieldPanel extends JPanel
 	/**
 	 * Sets the default background color
 	 *
-	 * @param c
+	 * @param c the color to use for the background.
-	 *            the color to use for the background.
 	 */
 	public void setBackgroundColor(Color c) {
 		backgroundColorModel.setDefaultBackgroundColor(c);
@@ -582,8 +579,7 @@ public class FieldPanel extends JPanel
 	/**
 	 * Add a new hover provider to be managed.
 	 *
-	 * @param hoverProvider
+	 * @param hoverProvider the new hover provider to be managed.
-	 *            the new hover provider to be managed.
 	 */
 	public void setHoverProvider(HoverProvider hoverProvider) {
 		hoverHandler.setHoverProvider(hoverProvider);
@@ -591,6 +587,7 @@ public class FieldPanel extends JPanel
 
 	/**
 	 * Returns the class responsible for triggering popups for this field panel.
+	 * 
 	 * @return the hover handler.
 	 */
 	public HoverHandler getHoverHandler() {
@@ -598,16 +595,13 @@ public class FieldPanel extends JPanel
 	}
 
 	/**
-	 * Returns the Field at the given x,y coordinates. Note the x,y must
+	 * Returns the Field at the given x,y coordinates. Note the x,y must currently be visible on the
-	 * currently be visible on the screen or else this method will return null.
+	 * screen or else this method will return null.
 	 *
-	 * @param x
+	 * @param x the x mouse coordinate in the component.
-	 *            the x mouse coordinate in the component.
+	 * @param y the y mouse coordinate in the component.
-	 * @param y
+	 * @param loc will be filled in with the FieldLocation for the given point. Values will be
-	 *            the y mouse coordinate in the component.
+	 *            undefined if the Field return value is null.
-	 * @param loc
-	 *            will be filled in with the FieldLocation for the given point.
-	 *            Values will be undefined if the Field return value is null.
 	 * @return Field the Field object the point is over.
 	 */
 	public Field getFieldAt(int x, int y, FieldLocation loc) {
@@ -644,9 +638,7 @@ public class FieldPanel extends JPanel
 	/**
 	 * Sets the cursor color for when this component has focus.
 	 *
-	 * @param color
+	 * @param color Color to use for the cursor when this component has keyboard focus.
-	 *            Color to use for the cursor when this component has keyboard
-	 *            focus.
 	 */
 	public void setFocusedCursorColor(Color color) {
 		paintContext.setFocusedCursorColor(color);
@@ -659,9 +651,7 @@ public class FieldPanel extends JPanel
 	/**
 	 * Sets the cursor color for when this component does not have focus.
 	 *
-	 * @param color
+	 * @param color Color to use for the cursor when this component does not have keyboard focus.
-	 *            Color to use for the cursor when this component does not have
-	 *            keyboard focus.
 	 */
 	public void setNonFocusCursorColor(Color color) {
 		paintContext.setNotFocusedCursorColor(color);
@@ -688,8 +678,7 @@ public class FieldPanel extends JPanel
 	/**
 	 * Sets the current selection.
 	 *
-	 * @param sel
+	 * @param sel the selection to set.
-	 *            the selection to set.
 	 */
 	public void setSelection(FieldSelection sel) {
 		if (!selectionHandler.isSelectionOn()) {
@@ -703,8 +692,7 @@ public class FieldPanel extends JPanel
 	/**
 	 * Sets the current highlight to the specified field selection.
 	 *
-	 * @param sel
+	 * @param sel the selection to set as the highlight.
-	 *            the selection to set as the highlight.
 	 */
 	public void setHighlight(FieldSelection sel) {
 		highlight = new FieldSelection(sel);
@@ -716,8 +704,7 @@ public class FieldPanel extends JPanel
 	 * Sets the cursorPosition to the given location.
 	 *
 	 * @param index the index of the Layout on which to place the cursor.
-	 * @param fieldNum the index of the field within its layout on which to place the
+	 * @param fieldNum the index of the field within its layout on which to place the cursor.
-	 *        cursor.
 	 * @param row the row within the field to place the cursor.
 	 * @param col the col within the row to place the cursor.
 	 * @return true if the cursor changed
@@ -727,7 +714,17 @@ public class FieldPanel extends JPanel
 	}
 
 	// for subclasses to control the event trigger
-	protected boolean setCursorPosition(BigInteger index, int fieldNum, int row, int col,
+	/**
+	 * Sets the cursorPosition to the given location with the given trigger.
+	 * 
+	 * @param index the index of the Layout on which to place the cursor.
+	 * @param fieldNum the index of the field within its layout on which to place the cursor.
+	 * @param row the row within the field to place the cursor.
+	 * @param col the col within the row to place the cursor.
+	 * @param trigger a caller-specified event trigger.
+	 * @return true if the cursor changed
+	 */
+	public boolean setCursorPosition(BigInteger index, int fieldNum, int row, int col,
 			EventTrigger trigger) {
 		if (cursorHandler.doSetCursorPosition(index, fieldNum, row, col, trigger)) {
 			repaint();
@@ -737,11 +734,10 @@ public class FieldPanel extends JPanel
 	}
 
 	/**
-	 * Sets the cursor on or off. When the cursor is turned off, there is no
+	 * Sets the cursor on or off. When the cursor is turned off, there is no visible cursor
-	 * visible cursor displayed on the screen.
+	 * displayed on the screen.
 	 *
-	 * @param cursorOn
+	 * @param cursorOn true turns the cursor on, false turns it off.
-	 *            true turns the cursor on, false turns it off.
 	 */
 	public void setCursorOn(boolean cursorOn) {
 		cursorHandler.setCursorOn(cursorOn);
@@ -759,20 +755,15 @@ public class FieldPanel extends JPanel
 	}
 
 	/**
-	 * Sets the cursor to the given Field location and attempts to show that
+	 * Sets the cursor to the given Field location and attempts to show that location in the center
-	 * location in the center of the screen.
+	 * of the screen.
 	 *
-	 * @param index
+	 * @param index the index of the line to go to.
-	 *            the index of the line to go to.
+	 * @param fieldNum the field on the line to go to.
-	 * @param fieldNum
+	 * @param row the row in the field to go to.
-	 *            the field on the line to go to.
+	 * @param col the column in the field to go to.
-	 * @param row
+	 * @param alwaysCenterCursor if true, centers cursor on screen. Otherwise, only centers cursor
-	 *            the row in the field to go to.
+	 *            if cursor is offscreen.
-	 * @param col
-	 *            the column in the field to go to.
-	 * @param alwaysCenterCursor
-	 *            if true, centers cursor on screen. Otherwise, only centers
-	 *            cursor if cursor is offscreen.
 	 */
 	public void goTo(BigInteger index, int fieldNum, int row, int col, boolean alwaysCenterCursor) {
 		goTo(index, fieldNum, row, col, alwaysCenterCursor, EventTrigger.API_CALL);
@@ -804,12 +795,10 @@ public class FieldPanel extends JPanel
 	}
 
 	/**
-	 * Scrolls the view so that the cursor is at the given offset from the top
+	 * Scrolls the view so that the cursor is at the given offset from the top of the screen
-	 * of the screen
 	 *
-	 * @param offset
+	 * @param offset the pixel distance from the top of the screen at which to scroll the display
-	 *            the pixel distance from the top of the screen at which to
+	 *            such that the cursor is at that offset.
-	 *            scroll the display such that the cursor is at that offset.
 	 */
 	public void positionCursor(int offset) {
 		if (offset < 0) {
@@ -831,8 +820,7 @@ public class FieldPanel extends JPanel
 	/**
 	 * Sets the selection color
 	 *
-	 * @param color
+	 * @param color the color to use for selection.
-	 *            the color to use for selection.
 	 */
 	public void setSelectionColor(Color color) {
 		paintContext.setSelectionColor(color);
@@ -841,20 +829,18 @@ public class FieldPanel extends JPanel
 	/**
 	 * Sets the highlight color
 	 *
-	 * @param color
+	 * @param color the color to use for highlights.
-	 *            the color to use for highlights.
 	 */
 	public void setHighlightColor(Color color) {
 		paintContext.setHighlightColor(color);
 	}
 
 	/**
-	 * Returns a ViewerPosition object which contains the top of screen
+	 * Returns a ViewerPosition object which contains the top of screen information. The
-	 * information. The ViewerPosition will have the index of the layout at the
+	 * ViewerPosition will have the index of the layout at the top of the screen and the yPos of
-	 * top of the screen and the yPos of that layout. For example, if the layout
+	 * that layout. For example, if the layout is completely displayed, yPos will be 0. If part of
-	 * is completely displayed, yPos will be 0. If part of the layout is off the
+	 * the layout is off the top off the screen, then yPos will have a negative value (indicating
-	 * top off the screen, then yPos will have a negative value (indicating that
+	 * that it begins above the displayable part of the screen.
-	 * it begins above the displayable part of the screen.
 	 */
 	public ViewerPosition getViewerPosition() {
 		if (layouts.size() > 0) {
@@ -864,15 +850,12 @@ public class FieldPanel extends JPanel
 	}
 
 	/**
-	 * Scrolls the display to show the layout specified by index at the vertical
+	 * Scrolls the display to show the layout specified by index at the vertical position specified
-	 * position specified by yPos. Generally, the index will be layout at the
+	 * by yPos. Generally, the index will be layout at the top of the screen and the yPos will be
-	 * top of the screen and the yPos will be &lt;= 0, meaning the layout may be
+	 * &lt;= 0, meaning the layout may be partially off the top of the screen.
-	 * partially off the top of the screen.
 	 *
-	 * @param index
+	 * @param index the index of the layout to show at the top of the screen.
-	 *            the index of the layout to show at the top of the screen.
+	 * @param yPos the position to show the layout.
-	 * @param yPos
-	 *            the position to show the layout.
 	 */
 	public void setViewerPosition(BigInteger index, int xPos, int yPos) {
 		if (index.compareTo(BigInteger.ZERO) >= 0 && index.compareTo(model.getNumIndexes()) < 0) {
@@ -894,8 +877,7 @@ public class FieldPanel extends JPanel
 	/**
 	 * Sets the layout model for this field panel
 	 *
-	 * @param model
+	 * @param model the layout model to use.
-	 *            the layout model to use.
 	 */
 	public void setLayoutModel(LayoutModel model) {
 		invalidate();
@@ -1663,18 +1645,18 @@ public class FieldPanel extends JPanel
 		}
 
 		/**
-		 * Basically checks if the the "shift" modifier is on and the "control"
+		 * Basically checks if the the "shift" modifier is on and the "control" modifier is not.
-		 * modifier is not. Note that "control" is operating system dependent.
+		 * Note that "control" is operating system dependent. It is <control> on windows, and
-		 * It is <control> on windows, and <command> on mac.
+		 * <command> on mac.
 		 */
 		private boolean isAddToContiguousSelectionActivator(MouseEvent e) {
 			return (e.isShiftDown() && !DockingUtils.isControlModifier(e));
 		}
 
 		/**
-		 * Basically checks if the the "control" modifier is on and the shift
+		 * Basically checks if the the "control" modifier is on and the shift modifier is not. Note
-		 * modifier is not. Note that "control" is operating system dependent.
+		 * that "control" is operating system dependent. It is <control> on windows, and <command>
-		 * It is <control> on windows, and <command> on mac.
+		 * on mac.
 		 */
 		private boolean isAddRemoveDisjointSelectionActivator(MouseEvent e) {
 			return DockingUtils.isControlModifier(e) && !e.isShiftDown();
@@ -1896,10 +1878,8 @@ public class FieldPanel extends JPanel
 		/**
 		 * Sets the cursor as close to the given point as possible.
 		 *
-		 * @param x
+		 * @param x the horizontal coordinate.
-		 *            the horizontal coordinate.
+		 * @param y the vertical coordinate.
-		 * @param y
-		 *            the vertical coordinate.
 		 */
 		private void setCursorPos(int x, int y, EventTrigger trigger) {
 			currentField = null;

Ghidra/Framework/Generic/src/main/java/ghidra/generic/util/datastruct/SortedList.java

@@ -0,0 +1,72 @@
+/* ###
+ * IP: GHIDRA
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package ghidra.generic.util.datastruct;
+
+import java.util.List;
+
+/**
+ * An interface for sorted lists
+ *
+ * <p>
+ * This might be better described as a NavigableMultiset; however, I wish for the elements to be
+ * retrievable by index, though insertion and mutation is not permitted by index. This implies that
+ * though unordered, the underlying implementation has sorted the elements in some way and wishes to
+ * expose that ordering to its clients.
+ *
+ * @param <E> the type of elements in this list
+ */
+public interface SortedList<E> extends List<E> {
+	/**
+	 * Returns the greatest index in this list whose element is strictly less than the specified
+	 * element
+	 * 
+	 * @param element the element to search for
+	 * @return the index of the found element, or -1
+	 */
+	int lowerIndex(E element);
+
+	/**
+	 * Returns the greatest index in this list whose element is less than or equal to the specified
+	 * element
+	 * 
+	 * <p>
+	 * If multiples of the specified element exist, this returns the least index of that element.
+	 * 
+	 * @param element the element to search for
+	 * @return the index of the found element, or -1
+	 */
+	int floorIndex(E element);
+
+	/**
+	 * Returns the least index in this list whose element is greater than or equal to the specified
+	 * element
+	 * 
+	 * <p>
+	 * If multiples of the specified element exist, this returns the greatest index of that element.
+	 * 
+	 * @param element the element to search for
+	 * @return the index of the found element, or -1
+	 */
+	int ceilingIndex(E element);
+
+	/**
+	 * Returns the least index in this list whose element is strictly greater the specified element
+	 * 
+	 * @param element the element to search for
+	 * @return the index of the found element, or -1
+	 */
+	int higherIndex(E element);
+}

Ghidra/Framework/Generic/src/main/java/ghidra/generic/util/datastruct/TreeSetValuedTreeMap.java

@@ -15,14 +15,13 @@
  */
 package ghidra.generic.util.datastruct;
 
-import java.util.Set;
+import java.util.*;
-import java.util.TreeMap;
-import java.util.TreeSet;
 
 import org.apache.commons.collections4.multimap.AbstractSetValuedMap;
 
 /**
- * A multi-valued dictionary using a tree map of tree sets
+ * A multi-valued map using a tree map of tree sets
+ * 
  * @param <K> the type of key
  * @param <V> the type of value
  */

Ghidra/Framework/Generic/src/main/java/ghidra/generic/util/datastruct/TreeValueSortedMap.java

@@ -22,36 +22,43 @@ import org.apache.commons.collections4.comparators.ComparableComparator;
 import ghidra.util.ReversedListIterator;
 
 /**
- * A map that is sorted by value.
+ * A tree-based implementation of a value-sorted map
- * 
+ *
- * This is an implementation of {@link Map} where entries are sorted by value, rather than by key.
- * Such a tree may be useful as a priority queue where the cost of an entry may change over time.
- * As such, the collections returned by {@link #entrySet()}, {@link #keySet()}, and
- * {@link #values()} all implement {@link Deque}. The order of the entries will be updated on any
- * call to {@link #put(Object, Object)}, or a call to {@link Collection#add(Object)} on the entry
- * set. Additionally, if the values are mutable objects, whose costs may change, there is an
- * {@link #update(Object)} method, which notifies the map that the given key may need to be
- * repositioned. The associated collections also implement the {@link List} interface, providing
- * fairly efficient implementations of {@link List#get(int)} and {@link List#indexOf(Object)}.
- * Sequential access is best performed via {@link Collection#iterator()}, since this will use a
- * linked list.
- * 
  * The underlying implementation is currently an unbalanced binary tree whose nodes also comprise a
  * doubly-linked list. Currently, it is not thread safe.
- * TODO Consider changing to an AVL tree implementation
+ *
- * TODO Consider implementing the {@link NavigableMap} interface
+ * Note this implementation isn't terribly smart, as it makes no efforts to balance the tree. It is
- * TODO Consider making the implementation thread-safe
+ * also not thread safe.
  * 
  * @param <K> the type of the keys
  * @param <V> the type of the values
  */
-public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
+public class TreeValueSortedMap<K, V> extends AbstractMap<K, V> implements ValueSortedMap<K, V> {
+
+	/**
+	 * Create a tree using the values' natural ordering
+	 */
+	public static <K, V extends Comparable<V>> TreeValueSortedMap<K, V> createWithNaturalOrder() {
+		Comparator<V> natural = Comparator.naturalOrder();
+		return new TreeValueSortedMap<>(natural);
+	}
+
+	/**
+	 * Create a tree using a custom comparator to order the values
+	 * 
+	 * @param comparator the comparator, providing a total ordering of the values
+	 */
+	public static <K, V> TreeValueSortedMap<K, V> createWithComparator(Comparator<V> comparator) {
+		return new TreeValueSortedMap<>(comparator);
+	}
+
 	/**
 	 * An iterator of the entries
 	 */
 	protected class EntryListIterator implements ListIterator<Entry<K, V>> {
 		private boolean atEnd = false;
 		private Node next;
+		private Node cur;
 
 		/**
 		 * Construct a list iterator over the entries
@@ -88,7 +95,7 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 			if (!hasNext()) {
 				throw new NoSuchElementException();
 			}
-			Node cur = next;
+			cur = next;
 			next = next.next;
 			atEnd = next == null;
 			return cur;
@@ -109,9 +116,9 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 				atEnd = tail == null;
 			}
 			else {
-				next = next.prev;
+				cur = next = next.prev;
 			}
-			return next;
+			return cur;
 		}
 
 		@Override
@@ -124,8 +131,12 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 
 		@Override
 		public void remove() {
-			nodeMap.remove(next.key);
+			if (cur == null) {
-			next.remove();
+				throw new IllegalStateException();
+			}
+			nodeMap.remove(cur.key);
+			cur.remove();
+			cur = null;
 		}
 
 		@Override
@@ -204,18 +215,18 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 	 */
 	protected class Node implements Entry<K, V> {
 		// Node key and data 
-		private final K key;
+		protected final K key;
-		private V val;
+		protected V val;
 
 		// Tree-related fields
-		private Node parent;
+		protected Node parent;
-		private Node lChild;
+		protected Node lChild;
-		private Node rChild;
+		protected Node rChild;
-		private int sizeLeft;
+		protected int sizeLeft;
 
 		// Linked list-related fields
-		private Node next;
+		protected Node next;
-		private Node prev;
+		protected Node prev;
 
 		@Override
 		public String toString() {
@@ -224,10 +235,11 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 
 		/**
 		 * Construct a new node
+		 * 
 		 * @param key the key
 		 * @param val the data
 		 */
-		private Node(K key, V val) {
+		protected Node(K key, V val) {
 			this.key = key;
 			this.val = val;
 		}
@@ -237,17 +249,24 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 			try {
 				@SuppressWarnings("unchecked")
 				Entry<K, V> that = (Entry<K, V>) obj;
-				return eq(this.key, that.getKey()) && eq(this.val, that.getValue());
+				return Objects.equals(this.key, that.getKey()) &&
+					Objects.equals(this.val, that.getValue());
 			}
 			catch (ClassCastException e) {
 				return false;
 			}
 		}
 
+		@Override
+		public int hashCode() {
+			return Objects.hash(this.key, this.val);
+		}
+
 		/**
 		 * Compute this node's index.
 		 * 
 		 * This uses the {@link #sizeLeft} field to compute the index in O(log n) on average.
+		 * 
 		 * @return the index
 		 */
 		public int computeIndex() {
@@ -268,7 +287,7 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 		 * 
 		 * This really only makes sense at the root
 		 * 
-		 * @param index the index 
+		 * @param index the index
 		 * @return the node at the given index
 		 */
 		private Node getByIndex(int index) {
@@ -310,6 +329,7 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 
 		/**
 		 * Insert a node into this subtree and the linked list
+		 * 
 		 * @param item the node to insert
 		 */
 		void insert(Node item) {
@@ -340,6 +360,7 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 
 		/**
 		 * Insert a node as a successor to this node in the linked list
+		 * 
 		 * NOTE: Called only after the node is inserted into the tree
 		 */
 		private void insertAfter(Node item) {
@@ -357,6 +378,7 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 
 		/**
 		 * Insert a node as a predecessor to this node in the linked list
+		 * 
 		 * NOTE: Called only after the node is inserted into the tree
 		 */
 		private void insertBefore(Node item) {
@@ -456,34 +478,57 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 		/**
 		 * Find the given value in this subtree
 		 * 
-		 * @param val the value to find
+		 * @param value the value to find
 		 * @param mode when the value occurs multiple times, identifies which instance to find
 		 * @return the node containing the given value, or null if not found
 		 */
-		private Node searchValue(V val, SearchMode mode) {
+		private Node searchValue(V value, SearchMode mode) {
 			Node cur = this;
 			Node eq = null;
+			int c;
 			while (true) {
-				int c = comparator.compare(val, cur.val);
+				c = comparator.compare(value, cur.val);
 				if (c == 0) {
 					eq = cur;
 				}
-				if (c < 0 || (c == 0 && mode == SearchMode.FIRST)) {
+				if (c < 0 || (c == 0 && mode.inEq == Comp.LT)) {
 					if (cur.lChild == null) {
-						return eq;
+						break;
 					}
 					cur = cur.lChild;
 				}
-				else if (c > 0 || (c == 0 && mode == SearchMode.LAST)) {
+				else if (c > 0 || (c == 0 && mode.inEq == Comp.GT)) {
 					if (cur.rChild == null) {
-						return eq;
+						break;
 					}
 					cur = cur.rChild;
 				}
-				else { // c == 0 && mode == SearchMode.ANY
+				else {
-					return eq;
+					break;
 				}
 			}
+			if (eq != null) {
+				if (mode.allowEq == BoundType.CLOSED) {
+					return eq;
+				}
+				if (mode.comp == Comp.LT) {
+					return eq.prev;
+				}
+				return eq.next;
+			}
+			if (mode.comp == Comp.LT) {
+				if (c < 0) {
+					return cur.prev; // May be null;
+				}
+				return cur; // c != 0 here, so c > 0
+			}
+			if (mode.comp == Comp.GT) {
+				if (c < 0) {
+					return cur;
+				}
+				return cur.next;
+			}
+			return null; // Other search modes require exact match
 		}
 
 		@Override
@@ -495,19 +540,42 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 		}
 	}
 
+	private enum BoundType {
+		CLOSED, OPEN
+	}
+
+	private enum Comp {
+		NONE, LT, GT
+	}
+
 	/**
 	 * When searching for values, identifies which instance to find
-	 * 
-	 * TODO When/if implementing {@link NavigableMap}, this seems an appropriate place to put
-	 * FLOOR, CEILING, etc.
 	 */
 	private enum SearchMode {
 		/** Find any occurrence */
-		ANY,
+		ANY(BoundType.CLOSED, Comp.NONE, Comp.NONE),
 		/** Find the first occurrence */
-		FIRST,
+		FIRST(BoundType.CLOSED, Comp.LT, Comp.NONE),
 		/** Find the last occurrence */
-		LAST;
+		LAST(BoundType.CLOSED, Comp.GT, Comp.NONE),
+		/** Find the nearest match less than */
+		LOWER(BoundType.OPEN, Comp.NONE, Comp.LT),
+		/** Find the nearest match less than or equal */
+		FLOOR(BoundType.CLOSED, Comp.LT, Comp.LT),
+		/** Find the nearest match greater than or equal */
+		CEILING(BoundType.CLOSED, Comp.GT, Comp.GT),
+		/** Find the nearest match greater than */
+		HIGHER(BoundType.OPEN, Comp.NONE, Comp.GT);
+
+		final BoundType allowEq;
+		final Comp inEq;
+		final Comp comp;
+
+		SearchMode(BoundType allowEq, Comp inEq, Comp comp) {
+			this.allowEq = allowEq;
+			this.inEq = inEq;
+			this.comp = comp;
+		}
 	}
 
 	/**
@@ -577,13 +645,13 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 	 * A public view of the map as a set of entries
 	 * 
 	 * In addition to {@link Set}, this view implements {@link List} and {@link Deque}, since an
-	 * ordered set ought to behave like a list, and since this implementation is meant to be used
+	 * ordered set ought to behave like a list, and since this implementation is meant to be used as
-	 * as a dynamic-cost priority queue.
+	 * a dynamic-cost priority queue.
 	 * 
 	 * Generally, all of the mutation methods are supported.
 	 */
-	public class ValueSortedTreeMapEntrySet extends AbstractSet<Entry<K, V>>
+	protected class ValueSortedTreeMapEntrySet extends AbstractSet<Entry<K, V>>
-			implements List<Entry<K, V>>, Deque<Entry<K, V>> {
+			implements ValueSortedMapEntryList<K, V> {
 		private ValueSortedTreeMapEntrySet() {
 		}
 
@@ -633,7 +701,7 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 
 		@Override
 		public void clear() {
-			DynamicValueSortedTreeMap.this.clear();
+			TreeValueSortedMap.this.clear();
 		}
 
 		@Override
@@ -645,7 +713,7 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 				@SuppressWarnings("unchecked")
 				Node n = (Node) o;
 				Node m = nodeMap.get(n.key);
-				return eq(n.val, m.val);
+				return Objects.equals(n.val, m.val);
 			}
 			catch (ClassCastException e) {
 				return false;
@@ -658,18 +726,18 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 		}
 
 		@Override
-		public Node element() {
+		public Entry<K, V> element() {
 			return getFirst();
 		}
 
 		@Override
-		public Node get(int index) {
+		public Entry<K, V> get(int index) {
 			return root.getByIndex(index);
 		}
 
 		@Override
-		public Node getFirst() {
+		public Entry<K, V> getFirst() {
-			Node ret = peekFirst();
+			Entry<K, V> ret = peekFirst();
 			if (ret == null) {
 				throw new NoSuchElementException();
 			}
@@ -677,8 +745,8 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 		}
 
 		@Override
-		public Node getLast() {
+		public Entry<K, V> getLast() {
-			Node ret = peekLast();
+			Entry<K, V> ret = peekLast();
 			if (ret == null) {
 				throw new NoSuchElementException();
 			}
@@ -722,6 +790,9 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 
 		@Override
 		public ListIterator<Entry<K, V>> listIterator(int index) {
+			if (root == null) {
+				return Collections.emptyListIterator();
+			}
 			return new EntryListIterator(root.getByIndex(index));
 		}
 
@@ -747,27 +818,27 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 		}
 
 		@Override
-		public Node peek() {
+		public Entry<K, V> peek() {
 			return peekFirst();
 		}
 
 		@Override
-		public Node peekFirst() {
+		public Entry<K, V> peekFirst() {
 			return head;
 		}
 
 		@Override
-		public Node peekLast() {
+		public Entry<K, V> peekLast() {
 			return tail;
 		}
 
 		@Override
-		public Node poll() {
+		public Entry<K, V> poll() {
 			return pollFirst();
 		}
 
 		@Override
-		public Node pollFirst() {
+		public Entry<K, V> pollFirst() {
 			if (head == null) {
 				return null;
 			}
@@ -778,7 +849,7 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 		}
 
 		@Override
-		public Node pollLast() {
+		public Entry<K, V> pollLast() {
 			if (tail == null) {
 				return tail;
 			}
@@ -789,7 +860,7 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 		}
 
 		@Override
-		public Node pop() {
+		public Entry<K, V> pop() {
 			return removeFirst();
 		}
 
@@ -799,12 +870,12 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 		}
 
 		@Override
-		public Node remove() {
+		public Entry<K, V> remove() {
 			return removeFirst();
 		}
 
 		@Override
-		public Node remove(int index) {
+		public Entry<K, V> remove(int index) {
 			Node n = root.getByIndex(index);
 			n.remove();
 			nodeMap.remove(n.key);
@@ -822,7 +893,7 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 					nodeMap.remove(n.key);
 					return true;
 				}
-				if (eq(n.val, rm.val)) {
+				if (Objects.equals(n.val, rm.val)) {
 					nodeMap.remove(rm.key);
 					rm.remove();
 					return true;
@@ -835,8 +906,8 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 		}
 
 		@Override
-		public Node removeFirst() {
+		public Entry<K, V> removeFirst() {
-			Node ret = pollFirst();
+			Entry<K, V> ret = pollFirst();
 			if (ret == null) {
 				throw new NoSuchElementException();
 			}
@@ -849,8 +920,8 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 		}
 
 		@Override
-		public Node removeLast() {
+		public Entry<K, V> removeLast() {
-			Node ret = pollLast();
+			Entry<K, V> ret = pollLast();
 			if (ret == null) {
 				throw new NoSuchElementException();
 			}
@@ -865,13 +936,13 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 		/**
 		 * Modify the entry (key and value) at index
 		 * 
-		 * Because the map is sorted by value, the index of the given entry may not remain the
+		 * Because the map is sorted by value, the index of the given entry may not remain the same
-		 * same after it is modified. In fact, this is equivalent to removing the entry at the
+		 * after it is modified. In fact, this is equivalent to removing the entry at the given
-		 * given index, and then inserting the given entry at its sorted position.
+		 * index, and then inserting the given entry at its sorted position.
 		 */
 		@Override
-		public Node set(int index, Entry<K, V> element) {
+		public Entry<K, V> set(int index, Entry<K, V> element) {
-			Node result = remove(index);
+			Entry<K, V> result = remove(index);
 			add(element);
 			return result;
 		}
@@ -881,11 +952,6 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 			return nodeMap.size();
 		}
 
-		@Override
-		public Spliterator<Entry<K, V>> spliterator() {
-			return Spliterators.spliterator(this, Spliterator.ORDERED | Spliterator.DISTINCT);
-		}
-
 		/**
 		 * This operation is not supported
 		 */
@@ -899,12 +965,13 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 	 * A public view of the map as a set of keys
 	 * 
 	 * In addition to {@link Set}, this view implements {@link List} and {@link Deque}, since an
-	 * ordered set ought to behave like a list, and since this implementation is meant to be used
+	 * ordered set ought to behave like a list, and since this implementation is meant to be used as
-	 * as a dynamic-cost priority queue.
+	 * a dynamic-cost priority queue.
 	 * 
 	 * Generally, only the removal mutation methods are supported, all others are not supported.
 	 */
-	public class ValueSortedTreeMapKeySet extends AbstractSet<K> implements List<K>, Deque<K> {
+	protected class ValueSortedTreeMapKeySet extends AbstractSet<K>
+			implements ValueSortedMapKeyList<K> {
 		private ValueSortedTreeMapKeySet() {
 		}
 
@@ -940,7 +1007,7 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 
 		@Override
 		public void clear() {
-			DynamicValueSortedTreeMap.this.clear();
+			TreeValueSortedMap.this.clear();
 		}
 
 		@Override
@@ -960,17 +1027,17 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 
 		@Override
 		public K get(int index) {
-			return entrySet.get(index).key;
+			return entrySet.get(index).getKey();
 		}
 
 		@Override
 		public K getFirst() {
-			return entrySet.getFirst().key;
+			return entrySet.getFirst().getKey();
 		}
 
 		@Override
 		public K getLast() {
-			return entrySet.getLast().key;
+			return entrySet.getLast().getKey();
 		}
 
 		@Override
@@ -1029,20 +1096,20 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 
 		@Override
 		public K peekFirst() {
-			Node n = entrySet.peekFirst();
+			Entry<K, V> n = entrySet.peekFirst();
 			if (n == null) {
 				return null;
 			}
-			return n.key;
+			return n.getKey();
 		}
 
 		@Override
 		public K peekLast() {
-			Node n = entrySet.peekLast();
+			Entry<K, V> n = entrySet.peekLast();
 			if (n == null) {
 				return null;
 			}
-			return n.key;
+			return n.getKey();
 		}
 
 		@Override
@@ -1052,20 +1119,20 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 
 		@Override
 		public K pollFirst() {
-			Node n = entrySet.pollFirst();
+			Entry<K, V> n = entrySet.pollFirst();
 			if (n == null) {
 				return null;
 			}
-			return n.key;
+			return n.getKey();
 		}
 
 		@Override
 		public K pollLast() {
-			Node n = entrySet.pollLast();
+			Entry<K, V> n = entrySet.pollLast();
 			if (n == null) {
 				return null;
 			}
-			return n.key;
+			return n.getKey();
 		}
 
 		@Override
@@ -1085,32 +1152,32 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 
 		@Override
 		public K remove(int index) {
-			return entrySet.remove(index).key;
+			return entrySet.remove(index).getKey();
 		}
 
 		@Override
 		public boolean remove(Object o) {
-			return DynamicValueSortedTreeMap.this.remove(o) != null;
+			return TreeValueSortedMap.this.remove(o) != null;
 		}
 
 		@Override
 		public K removeFirst() {
-			return entrySet.removeFirst().key;
+			return entrySet.removeFirst().getKey();
 		}
 
 		@Override
 		public boolean removeFirstOccurrence(Object o) {
-			return DynamicValueSortedTreeMap.this.remove(o) != null;
+			return TreeValueSortedMap.this.remove(o) != null;
 		}
 
 		@Override
 		public K removeLast() {
-			return entrySet.removeLast().key;
+			return entrySet.removeLast().getKey();
 		}
 
 		@Override
 		public boolean removeLastOccurrence(Object o) {
-			return DynamicValueSortedTreeMap.this.remove(o) != null;
+			return TreeValueSortedMap.this.remove(o) != null;
 		}
 
 		@Override
@@ -1123,11 +1190,6 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 			return nodeMap.size();
 		}
 
-		@Override
-		public Spliterator<K> spliterator() {
-			return Spliterators.spliterator(this, Spliterator.ORDERED | Spliterator.DISTINCT);
-		}
-
 		/**
 		 * This operation is not supported
 		 */
@@ -1140,14 +1202,14 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 	/**
 	 * A public view of the map as a list of values
 	 * 
-	 * This view implements {@link List} and {@link Deque}, since an ordered collection ought to
+	 * This view implements {@link SortedList} and {@link Deque}, since an ordered collection ought
-	 * behave like a list, and since this implementation is meant to be used as a dynamic-cost
+	 * to behave like a list, and since this implementation is meant to be used as a dynamic-cost
 	 * priority queue.
 	 * 
 	 * Generally, only the removal mutation methods are supported, all others are not supported.
 	 */
-	public class ValueSortedTreeMapValues extends AbstractCollection<V>
+	protected class ValueSortedTreeMapValues extends AbstractCollection<V>
-			implements List<V>, Deque<V> {
+			implements SortedList<V>, Deque<V> {
 		private ValueSortedTreeMapValues() {
 		}
 
@@ -1183,7 +1245,7 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 
 		@Override
 		public void clear() {
-			DynamicValueSortedTreeMap.this.clear();
+			TreeValueSortedMap.this.clear();
 		}
 
 		@Override
@@ -1210,17 +1272,17 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 
 		@Override
 		public V get(int index) {
-			return entrySet.get(index).val;
+			return entrySet.get(index).getValue();
 		}
 
 		@Override
 		public V getFirst() {
-			return entrySet.getFirst().val;
+			return entrySet.getFirst().getValue();
 		}
 
 		@Override
 		public V getLast() {
-			return entrySet.getLast().val;
+			return entrySet.getLast().getValue();
 		}
 
 		@Override
@@ -1228,7 +1290,7 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 			try {
 				@SuppressWarnings("unchecked")
 				V val = (V) o;
-				Node n = root.searchValue(val, SearchMode.FIRST);
+				Node n = searchValue(val, SearchMode.FIRST);
 				if (n == null) {
 					return -1;
 				}
@@ -1239,6 +1301,45 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 			}
 		}
 
+		@Override
+		public int lowerIndex(V element) {
+			if (root == null) {
+				return -1;
+			}
+			Node n = searchValue(element, SearchMode.LOWER);
+			if (n == null) {
+				return -1;
+			}
+			return n.computeIndex();
+		}
+
+		@Override
+		public int floorIndex(V element) {
+			Node n = searchValue(element, SearchMode.FLOOR);
+			if (n == null) {
+				return -1;
+			}
+			return n.computeIndex();
+		}
+
+		@Override
+		public int ceilingIndex(V element) {
+			Node n = searchValue(element, SearchMode.CEILING);
+			if (n == null) {
+				return -1;
+			}
+			return n.computeIndex();
+		}
+
+		@Override
+		public int higherIndex(V element) {
+			Node n = searchValue(element, SearchMode.HIGHER);
+			if (n == null) {
+				return -1;
+			}
+			return n.computeIndex();
+		}
+
 		@Override
 		public boolean isEmpty() {
 			return root == null;
@@ -1297,20 +1398,20 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 
 		@Override
 		public V peekFirst() {
-			Node n = entrySet.peekFirst();
+			Entry<K, V> n = entrySet.peekFirst();
 			if (n == null) {
 				return null;
 			}
-			return n.val;
+			return n.getValue();
 		}
 
 		@Override
 		public V peekLast() {
-			Node n = entrySet.peekLast();
+			Entry<K, V> n = entrySet.peekLast();
 			if (n == null) {
 				return null;
 			}
-			return n.val;
+			return n.getValue();
 		}
 
 		@Override
@@ -1320,20 +1421,20 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 
 		@Override
 		public V pollFirst() {
-			Node n = entrySet.pollFirst();
+			Entry<K, V> n = entrySet.pollFirst();
 			if (n == null) {
 				return null;
 			}
-			return n.val;
+			return n.getValue();
 		}
 
 		@Override
 		public V pollLast() {
-			Node n = entrySet.pollLast();
+			Entry<K, V> n = entrySet.pollLast();
 			if (n == null) {
 				return null;
 			}
-			return n.val;
+			return n.getValue();
 		}
 
 		@Override
@@ -1353,7 +1454,7 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 
 		@Override
 		public V remove(int index) {
-			return entrySet.remove(index).val;
+			return entrySet.remove(index).getValue();
 		}
 
 		@Override
@@ -1363,7 +1464,7 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 
 		@Override
 		public V removeFirst() {
-			return entrySet.removeFirst().val;
+			return entrySet.removeFirst().getValue();
 		}
 
 		@Override
@@ -1386,7 +1487,7 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 
 		@Override
 		public V removeLast() {
-			return entrySet.removeLast().val;
+			return entrySet.removeLast().getValue();
 		}
 
 		@Override
@@ -1426,53 +1527,61 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 		}
 	}
 
-	/**
-	 * A convenience for null-safe comparison
-	 */
-	protected static boolean eq(Object o1, Object o2) {
-		return o1 == null ? o2 == null : o1.equals(o2);
-	}
-
 	// The user-provided comparator
-	private final Comparator<V> comparator;
+	protected final Comparator<V> comparator;
 	// A hash map to locate entries by key
-	private final Map<K, Node> nodeMap = new HashMap<>();
+	protected final Map<K, Node> nodeMap = new HashMap<>();
-	/* Remember, the tree is indexed by *value*, not by key, and more specifically, they are
+	/*
-	 * indexed by the comparator, so an entry's cost may change at any time. Thus, this map
+	 * Remember, the tree is indexed by *value*, not by key, and more specifically, they are indexed
-	 * provides an index by key. This is especially important during an update, since we need to
+	 * by the comparator, so an entry's cost may change at any time. Thus, this map provides an
-	 * locate the affected node, given that it's most likely not in its correct position at the
+	 * index by key. This is especially important during an update, since we need to locate the
-	 * moment. We also use it to ensure each key occurs at most once. */
+	 * affected node, given that it's most likely not in its correct position at the moment. We also
+	 * use it to ensure each key occurs at most once.
+	 */
 
 	// Pre-constructed views. Unlike Java's stock collections, I create these outright
 	// At least one ought to be accessed for this implementation to be useful
-	private transient final ValueSortedTreeMapEntrySet entrySet = new ValueSortedTreeMapEntrySet();
+	private transient final ValueSortedTreeMapEntrySet entrySet = createEntrySet();
-	private transient final ValueSortedTreeMapKeySet keySet = new ValueSortedTreeMapKeySet();
+	private transient final ValueSortedTreeMapKeySet keySet = createKeySet();
-	private transient final ValueSortedTreeMapValues values = new ValueSortedTreeMapValues();
+	private transient final ValueSortedTreeMapValues values = createValues();
 
 	// Pointers into the data structure
-	private Node root; // The root of the binary tree
+	protected Node root; // The root of the binary tree
-	private Node head; // The node with the least value
+	protected Node head; // The node with the least value
-	private Node tail; // The node with the greatest value
+	protected Node tail; // The node with the greatest value
 
-	/**
-	 * Construct a dynamic value-sorted tree map using the values' natural ordering
-	 * 
-	 * If the values do not have a natural ordering, you will eventually encounter a
-	 * {@link ClassCastException}. This condition is not checked at construction.
-	 */
 	@SuppressWarnings({ "rawtypes", "unchecked" })
-	public DynamicValueSortedTreeMap() {
+	protected TreeValueSortedMap() {
 		this(new ComparableComparator());
 	}
 
-	/**
+	protected TreeValueSortedMap(Comparator<V> comparator) {
-	 * Construct a dynamic value-sorted tree map using a custom comparator to order the values
-	 * @param comparator the comparator, providing a total ordering of the values
-	 */
-	public DynamicValueSortedTreeMap(Comparator<V> comparator) {
 		this.comparator = comparator;
 	}
 
+	protected ValueSortedTreeMapEntrySet createEntrySet() {
+		return new ValueSortedTreeMapEntrySet();
+	}
+
+	protected ValueSortedTreeMapKeySet createKeySet() {
+		return new ValueSortedTreeMapKeySet();
+	}
+
+	protected ValueSortedTreeMapValues createValues() {
+		return new ValueSortedTreeMapValues();
+	}
+
+	protected Node createNode(K key, V value) {
+		return new Node(key, value);
+	}
+
+	protected Node searchValue(V value, SearchMode mode) {
+		if (root == null) {
+			return null;
+		}
+		return root.searchValue(value, mode);
+	}
+
 	@Override
 	public void clear() {
 		nodeMap.clear();
@@ -1491,7 +1600,7 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 		try {
 			@SuppressWarnings("unchecked")
 			V val = (V) value;
-			return root.searchValue(val, SearchMode.ANY) != null;
+			return searchValue(val, SearchMode.ANY) != null;
 		}
 		catch (ClassCastException e) {
 			return false;
@@ -1517,6 +1626,26 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 		return n.val;
 	}
 
+	@Override
+	public Entry<K, V> lowerEntryByValue(V value) {
+		return searchValue(value, SearchMode.LOWER);
+	}
+
+	@Override
+	public Entry<K, V> floorEntryByValue(V value) {
+		return searchValue(value, SearchMode.FLOOR);
+	}
+
+	@Override
+	public Entry<K, V> ceilingEntryByValue(V value) {
+		return searchValue(value, SearchMode.CEILING);
+	}
+
+	@Override
+	public Entry<K, V> higherEntryByValue(V value) {
+		return searchValue(value, SearchMode.HIGHER);
+	}
+
 	@Override
 	public boolean isEmpty() {
 		return root == null;
@@ -1524,6 +1653,7 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 
 	/**
 	 * Check if a node is correctly positioned relative to its immediate neighbors
+	 * 
 	 * @param n the node
 	 * @return true if the node need not be moved
 	 */
@@ -1557,7 +1687,7 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 		if (n != null) {
 			return n.setValue(value);
 		}
-		n = new Node(key, value);
+		n = createNode(key, value);
 		nodeMap.put(key, n);
 		if (root == null) {
 			root = n;
@@ -1592,15 +1722,7 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 		return nodeMap.size();
 	}
 
-	/**
+	@Override
-	 * Notify the map of an external change to the cost of a key's associated value
-	 * 
-	 * This is meant to update the entry's position after a change in cost. The position may not
-	 * necessarily change, however, if the cost did not change significantly.
-	 * 
-	 * @param key the key whose associated value has changed in cost
-	 * @return true if the entry's position changed
-	 */
 	public boolean update(K key) {
 		Node n = nodeMap.get(key);
 		if (n == null) {
@@ -1615,6 +1737,7 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 	 * This ought to be called any time the value of a node is modified, whether internall or
 	 * externally. The only way we know of external changes is if the user calls
 	 * {@link #update(Object)}.
+	 * 
 	 * @param n the node whose position to check and update
 	 * @return true if the node's position changed
 	 */
@@ -1636,4 +1759,24 @@ public class DynamicValueSortedTreeMap<K, V> extends AbstractMap<K, V> {
 	public ValueSortedTreeMapValues values() {
 		return values;
 	}
+
+	@Override
+	public ValueSortedMap<K, V> subMapByValue(V fromValue, boolean fromInclusive, V toValue,
+			boolean toInclusive) {
+		return new RestrictedValueSortedMap<>(this, comparator, true, fromValue, fromInclusive,
+			true, toValue, toInclusive);
+	}
+
+	@Override
+	// TODO: Test this implementation and related others
+	public ValueSortedMap<K, V> headMapByValue(V toValue, boolean inclusive) {
+		return new RestrictedValueSortedMap<>(this, comparator, false, null, false, true, toValue,
+			inclusive);
+	}
+
+	@Override
+	public ValueSortedMap<K, V> tailMapByValue(V fromValue, boolean inclusive) {
+		return new RestrictedValueSortedMap<>(this, comparator, true, fromValue, inclusive, false,
+			null, false);
+	}
 }

Ghidra/Framework/Generic/src/main/java/ghidra/generic/util/datastruct/ValueSortedMap.java

@@ -0,0 +1,143 @@
+/* ###
+ * IP: GHIDRA
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package ghidra.generic.util.datastruct;
+
+import java.util.*;
+
+/**
+ * A map that is sorted by value.
+ * 
+ * <p>
+ * This is an extension of {@link Map} where entries are sorted by value, rather than by key. Such a
+ * map may be useful as a priority queue where the cost of an entry may change over time. As such,
+ * the collections returned by {@link #entrySet()}, {@link #keySet()}, and {@link #values()} all
+ * extend {@link Deque}. The order of the entries will be updated on any call to {@link #put(Object,
+ * Object))}, or a call to {@link Collection#add(Object)} on the entry set. Additionally, if the
+ * values are mutable objects, whose order may change, there is an {@link #update(Object)} method,
+ * which notifies the map that the given key may need to be repositioned. The associated collections
+ * also extend the {@link List} interface, providing fairly efficient implementations of
+ * {@link List#get(int)} and {@link List#indexOf(Object)}. Sequential access is best performed via
+ * {@link Collection#iterator()}, since this will use a linked list.
+ * 
+ * @param <K> the type of the keys
+ * @param <V> the type of the values
+ */
+public interface ValueSortedMap<K, V> extends Map<K, V> {
+	public interface ValueSortedMapEntryList<K, V>
+			extends Set<Entry<K, V>>, List<Entry<K, V>>, Deque<Entry<K, V>> {
+		@Override
+		default Spliterator<Entry<K, V>> spliterator() {
+			return Spliterators.spliterator(this, Spliterator.ORDERED | Spliterator.DISTINCT);
+		}
+	}
+
+	public interface ValueSortedMapKeyList<K> extends Set<K>, List<K>, Deque<K> {
+		@Override
+		default Spliterator<K> spliterator() {
+			return Spliterators.spliterator(this, Spliterator.ORDERED | Spliterator.DISTINCT);
+		}
+	}
+
+	@Override
+	ValueSortedMapEntryList<K, V> entrySet();
+
+	/**
+	 * Returns a key-value mapping associated with the greatest value strictly less than the given
+	 * value, or {@code null} if there is no such value.
+	 * 
+	 * @param value the value
+	 * @return the found entry, or {@code null}
+	 */
+	Entry<K, V> lowerEntryByValue(V value);
+
+	/**
+	 * Returns a key-value mapping associated with the greatest value less than or equal to the
+	 * given value, or {@code null} if there is no such value.
+	 * 
+	 * @param value the value
+	 * @return the found entry, or {@code null}
+	 */
+	Entry<K, V> floorEntryByValue(V value);
+
+	/**
+	 * Returns a key-value mapping associated with the least value greater than or equal to the
+	 * given value, or {@code null} if there is no such value.
+	 * 
+	 * @param value the value
+	 * @return the found entry, or {@code null}
+	 */
+	Entry<K, V> ceilingEntryByValue(V value);
+
+	/**
+	 * Returns a key-value mapping associated with the least value strictly greater than the given
+	 * value, or {@code null} if there is no such value.
+	 * 
+	 * @param value the value
+	 * @return the found entry, or {@code null}
+	 */
+	Entry<K, V> higherEntryByValue(V value);
+
+	/**
+	 * Returns a view of the portion of this map whose values range from {@code fromValue} to
+	 * {@code toValue}. The returned map is an unmodifiable view.
+	 * 
+	 * @param fromValue low endpoint of the values in the returned map
+	 * @param fromInclusive {@code true} if the low endpoint is to be included in the returned view
+	 * @param toValue high endpoint of the values in the returned map
+	 * @param toInclusive {@code true} if the high endpoint is to be included in the returned view
+	 * @return the view
+	 */
+	ValueSortedMap<K, V> subMapByValue(V fromValue, boolean fromInclusive, V toValue,
+			boolean toInclusive);
+
+	/**
+	 * Returns a view of the portion of this map whose values are less than (or equal to, if
+	 * {@code inclusive} is true) {@code toValue}. The returned map is an unmodifiable view.
+	 * 
+	 * @param toValue high endpoint of the values in the returned map
+	 * @param inclusive {@code true} if the high endpoint is to be included in the returned view
+	 * @return the view
+	 */
+	ValueSortedMap<K, V> headMapByValue(V toValue, boolean inclusive);
+
+	/**
+	 * Returns a view of the portion of this map whose values are greater than (or equal to, if
+	 * {@code inclusive} is true) {@code toValue}. The returned map is an unmodifiable view.
+	 * 
+	 * @param fromValue low endpoint of the values in the returned map
+	 * @param inclusive {@code true} if the low endpoint is to be included in the returned view
+	 * @return the view
+	 */
+	ValueSortedMap<K, V> tailMapByValue(V fromValue, boolean inclusive);
+
+	@Override
+	ValueSortedMapKeyList<K> keySet();
+
+	/**
+	 * Notify the map of an external change to the cost of a key's associated value
+	 * 
+	 * <p>
+	 * This is meant to update the entry's position after a change in cost. The position may not
+	 * necessarily change, however, if the cost did not change significantly.
+	 * 
+	 * @param key the key whose associated value has changed in cost
+	 * @return true if the entry's position changed
+	 */
+	boolean update(K key);
+
+	@Override
+	SortedList<V> values();
+}

Ghidra/Framework/Generic/src/main/java/ghidra/util/DynamicSortedTreeSet.java

@@ -1,320 +0,0 @@
-/* ###
- * IP: GHIDRA
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- * 
- *      http://www.apache.org/licenses/LICENSE-2.0
- * 
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package ghidra.util;
-
-import java.util.*;
-
-import ghidra.generic.util.datastruct.DynamicValueSortedTreeMap;
-
-/**
- * A set where the ordering of elements may change over time, based on an alternative comparator
- * 
- * This is an implementation of {@link Set} where elements may be sorted by an alternative
- * comparator (usually by "cost"), rather than by the natural ordering. It may seem odd, but the
- * natural ordering is still used to determine the uniqueness of keys. That is, two elements that
- * are unequal -- but are considered equal by the alternative comparator -- may co-exist in the
- * set. (Note: in such cases, the two elements are ordered first-in first-out). Additionally, if
- * the elements are mutable, then their ordering may change over time. This mode of operation is
- * enabled by the {@link #update(Object)} method, which must be called to notify the set of any
- * change to an element that may affect its order. This set also implements the {@link List} and
- * {@link Deque} interfaces. Since the set is ordered, it makes sense to treat it as a list. It
- * provides fairly efficient implementations of {@link #get(int)} and {@link #indexOf(Object)}.
- * Sequential access is best performed via {@link #iterator()}, since this will use a linked list.
- * 
- * The underlying implementation is backed by {@link DynamicValueSortedTreeMap}. Currently, it is
- * not thread safe.
- * 
- * @param <E> the type of the elements
- */
-public class DynamicSortedTreeSet<E> extends AbstractSet<E> implements List<E>, Deque<E> {
-	private final transient DynamicValueSortedTreeMap<E, E>.ValueSortedTreeMapKeySet keys;
-	private final transient DynamicValueSortedTreeMap<E, E> map;
-
-	/**
-	 * Construct a dynamic sorted tree set using the elements' natural ordering
-	 * 
-	 * Other than, perhaps, a more convenient interface, this offers few if any benefits over the
-	 * stock {@link Set}.
-	 */
-	public DynamicSortedTreeSet() {
-		map = new DynamicValueSortedTreeMap<>();
-		keys = map.keySet();
-	}
-
-	/**
-	 * Construct a dynamic sorted tree set using a custom comparator to order the elements
-	 * @param comparator the comparator, providing a total ordering of the values
-	 */
-	public DynamicSortedTreeSet(Comparator<E> comparator) {
-		map = new DynamicValueSortedTreeMap<>(comparator);
-		keys = map.keySet();
-	}
-
-	@Override
-	public boolean add(E e) {
-		return map.put(e, e) == null;
-	}
-
-	/**
-	 * Inserts the element, ignoring index
-	 * 
-	 * @param index ignore since the set is sorted
-	 */
-	@Override
-	public void add(int index, E element) {
-		add(element);
-	}
-
-	/**
-	 * Inserts all elements from the given collection, ignoring index
-	 * 
-	 * @param index ignore since the set is sorted
-	 */
-	@Override
-	public boolean addAll(int index, Collection<? extends E> c) {
-		return addAll(c);
-	}
-
-	/**
-	 * Inserts the element, not necessarily first
-	 */
-	@Override
-	public void addFirst(E e) {
-		add(e);
-	}
-
-	/**
-	 * Inserts the element, not necessarily last
-	 */
-	@Override
-	public void addLast(E e) {
-		add(e);
-	}
-
-	@Override
-	public void clear() {
-		map.clear();
-	}
-
-	@Override
-	public boolean contains(Object o) {
-		return map.containsKey(o);
-	}
-
-	@Override
-	public Iterator<E> descendingIterator() {
-		return keys.descendingIterator();
-	}
-
-	@Override
-	public E element() {
-		return keys.element();
-	}
-
-	@Override
-	public E get(int index) {
-		return keys.get(index);
-	}
-
-	@Override
-	public E getFirst() {
-		return keys.getFirst();
-	}
-
-	@Override
-	public E getLast() {
-		return keys.getLast();
-	}
-
-	@Override
-	public int indexOf(Object o) {
-		return keys.indexOf(o);
-	}
-
-	@Override
-	public boolean isEmpty() {
-		return map.isEmpty();
-	}
-
-	@Override
-	public Iterator<E> iterator() {
-		return keys.iterator();
-	}
-
-	@Override
-	public int lastIndexOf(Object o) {
-		return keys.lastIndexOf(o);
-	}
-
-	@Override
-	public ListIterator<E> listIterator() {
-		return keys.listIterator();
-	}
-
-	@Override
-	public ListIterator<E> listIterator(int index) {
-		return keys.listIterator(index);
-	}
-
-	@Override
-	public boolean offer(E e) {
-		return add(e);
-	}
-
-	/**
-	 * Inserts the element, not necessarily first
-	 */
-	@Override
-	public boolean offerFirst(E e) {
-		return add(e);
-	}
-
-	/**
-	 * Inserts the element, not necessarily last
-	 */
-	@Override
-	public boolean offerLast(E e) {
-		return add(e);
-	}
-
-	@Override
-	public E peek() {
-		return keys.peek();
-	}
-
-	@Override
-	public E peekFirst() {
-		return keys.peekFirst();
-	}
-
-	@Override
-	public E peekLast() {
-		return keys.peekLast();
-	}
-
-	@Override
-	public E poll() {
-		return keys.poll();
-	}
-
-	@Override
-	public E pollFirst() {
-		return keys.pollFirst();
-	}
-
-	@Override
-	public E pollLast() {
-		return keys.pollLast();
-	}
-
-	@Override
-	public E pop() {
-		return keys.pop();
-	}
-
-	@Override
-	public void push(E e) {
-		add(e);
-	}
-
-	@Override
-	public E remove() {
-		return keys.remove();
-	}
-
-	@Override
-	public E remove(int index) {
-		return keys.remove(index);
-	}
-
-	@Override
-	public boolean remove(Object o) {
-		return keys.remove(o);
-	}
-
-	@Override
-	public boolean removeAll(Collection<?> c) {
-		return keys.removeAll(c);
-	}
-
-	@Override
-	public E removeFirst() {
-		return keys.removeFirst();
-	}
-
-	@Override
-	public boolean removeFirstOccurrence(Object o) {
-		return keys.removeFirstOccurrence(o);
-	}
-
-	@Override
-	public E removeLast() {
-		return keys.removeLast();
-	}
-
-	@Override
-	public boolean removeLastOccurrence(Object o) {
-		return keys.removeLastOccurrence(o);
-	}
-
-	@Override
-	public boolean retainAll(Collection<?> c) {
-		return keys.retainAll(c);
-	}
-
-	/**
-	 * Replace the element at the given index with the given element
-	 * 
-	 * Because the set is sorted, the index of the given element may not be the same as
-	 * {@code index}. In fact, this is equivalent to removing the element at the given index, and
-	 * then inserting the given element at its sorted position.
-	 */
-	@Override
-	public E set(int index, E element) {
-		E result = remove(index);
-		add(element);
-		return result;
-	}
-
-	@Override
-	public int size() {
-		return map.size();
-	}
-
-	@Override
-	public Spliterator<E> spliterator() {
-		return Spliterators.spliterator(this, Spliterator.ORDERED | Spliterator.DISTINCT);
-	}
-
-	/**
-	 * This operation is not supported
-	 */
-	@Override
-	public List<E> subList(int fromIndex, int toIndex) {
-		throw new UnsupportedOperationException();
-	}
-
-	/**
-	 * Notify the queue of a change to an elements cost.
-	 * 
-	 * This may cause the element's index to change.
-	 * @param e the element whose cost may have changed
-	 * @return true if the index changed
-	 */
-	public boolean update(E e) {
-		return map.update(e);
-	}
-}

Ghidra/Framework/Generic/src/main/java/ghidra/util/MathUtilities.java

@@ -21,8 +21,8 @@ public class MathUtilities {
 	}
 
 	/**
-	 * Perform unsigned division.  Provides proper handling of all 64-bit unsigned
+	 * Perform unsigned division. Provides proper handling of all 64-bit unsigned values.
-	 * values. 
+	 * 
 	 * @param numerator unsigned numerator
 	 * @param denominator positive divisor
 	 * @return result of unsigned division
@@ -47,8 +47,8 @@ public class MathUtilities {
 	}
 
 	/**
-	 * Perform unsigned modulo.  Provides proper handling of all 64-bit unsigned
+	 * Perform unsigned modulo. Provides proper handling of all 64-bit unsigned values.
-	 * values. 
+	 * 
 	 * @param numerator unsigned numerator
 	 * @param denominator positive divisor
 	 * @return result of unsigned modulo (i.e., remainder)
@@ -97,4 +97,111 @@ public class MathUtilities {
 		}
 	}
 
+	/**
+	 * Compute the minimum, treating the inputs as unsigned
+	 * 
+	 * @param a the first value to consider
+	 * @param b the second value to consider
+	 * @return the minimum
+	 */
+	public static long unsignedMin(long a, long b) {
+		return (Long.compareUnsigned(a, b) < 0) ? a : b;
+	}
+
+	/**
+	 * Compute the minimum, treating the inputs as unsigned
+	 * 
+	 * @param a the first value to consider
+	 * @param b the second value to consider
+	 * @return the minimum
+	 */
+	public static int unsignedMin(int a, int b) {
+		return (Integer.compareUnsigned(a, b) < 0) ? a : b;
+	}
+
+	/**
+	 * Compute the minimum, treating the inputs as unsigned
+	 * 
+	 * <p>
+	 * This method is overloaded to prevent accidental signed-extension on one of the inputs. This
+	 * method will correctly zero-extend the {@code int} parameter before performing any comparison.
+	 * Also note the return type is {@code int}, since b would never be selected if it overflows an
+	 * {@code int}.
+	 * 
+	 * @param a the first value to consider
+	 * @param b the second value to consider
+	 * @return the minimum
+	 */
+	public static int unsignedMin(int a, long b) {
+		return (Long.compareUnsigned(a & 0x0ffffffffL, b) < 0) ? a : (int) b;
+	}
+
+	/**
+	 * Compute the minimum, treating the inputs as unsigned
+	 * 
+	 * <p>
+	 * This method is overloaded to prevent accidental signed-extension on one of the inputs. This
+	 * method will correctly zero-extend the {@code int} parameter before performing any comparison.
+	 * Also note the return type is {@code int}, since b would never be selected if it overflows an
+	 * {@code int}.
+	 * 
+	 * @param a the first value to consider
+	 * @param b the second value to consider
+	 * @return the minimum
+	 */
+	public static int unsignedMin(long a, int b) {
+		return (Long.compareUnsigned(a, b & 0x0ffffffffL) < 0) ? (int) a : b;
+	}
+
+	/**
+	 * Compute the maximum, treating the inputs as unsigned
+	 * 
+	 * @param a the first value to consider
+	 * @param b the second value to consider
+	 * @return the maximum
+	 */
+	public static long unsignedMax(long a, long b) {
+		return (Long.compareUnsigned(a, b) > 0) ? a : b;
+	}
+
+	/**
+	 * Compute the maximum, treating the inputs as unsigned
+	 * 
+	 * @param a the first value to consider
+	 * @param b the second value to consider
+	 * @return the maximum
+	 */
+	public static int unsignedMax(int a, int b) {
+		return (Integer.compareUnsigned(a, b) > 0) ? a : b;
+	}
+
+	/**
+	 * Compute the maximum, treating the inputs as unsigned
+	 * 
+	 * <p>
+	 * This method is overloaded to prevent accidental signed-extension on one of the inputs. This
+	 * method will correctly zero-extend the {@code int} parameter before performing any comparison.
+	 * 
+	 * @param a the first value to consider
+	 * @param b the second value to consider
+	 * @return the maximum
+	 */
+	public static long unsignedMax(int a, long b) {
+		return (Long.compareUnsigned(a & 0x0ffffffffL, b) > 0) ? a : b;
+	}
+
+	/**
+	 * Compute the maximum, treating the inputs as unsigned
+	 * 
+	 * <p>
+	 * This method is overloaded to prevent accidental signed-extension on one of the inputs. This
+	 * method will correctly zero-extend the {@code int} parameter before performing any comparison.
+	 * 
+	 * @param a the first value to consider
+	 * @param b the second value to consider
+	 * @return the maximum
+	 */
+	public static long unsignedMax(long a, int b) {
+		return (Long.compareUnsigned(a, b & 0x0ffffffffL) > 0) ? a : b;
+	}
 }

Ghidra/Framework/Generic/src/main/java/ghidra/util/NumericUtilities.java

@@ -57,9 +57,8 @@ public final class NumericUtilities {
 	}
 
 	/**
-	 * parses the given string as a numeric value, detecting whether
+	 * parses the given string as a numeric value, detecting whether or not it begins with a Hex
-	 * or not it begins with a Hex prefix, and if not, parses as a
+	 * prefix, and if not, parses as a long int value.
-	 * long int value.
 	 */
 	public static long parseNumber(String numStr) {
 		long value = 0;
@@ -85,9 +84,8 @@ public final class NumericUtilities {
 	}
 
 	/**
-	 * parses the given string as a numeric value, detecting whether
+	 * parses the given string as a numeric value, detecting whether or not it begins with a Hex
-	 * or not it begins with a Hex prefix, and if not, parses as a
+	 * prefix, and if not, parses as a long int value.
-	 * long int value.
 	 */
 	public static long parseLong(String numStr) {
 		String origStr = numStr;
@@ -132,9 +130,8 @@ public final class NumericUtilities {
 	}
 
 	/**
-	 * parses the given string as a numeric value, detecting whether
+	 * parses the given string as a numeric value, detecting whether or not it begins with a Hex
-	 * or not it begins with a Hex prefix, and if not, parses as a
+	 * prefix, and if not, parses as a long int value.
-	 * long int value.
 	 */
 	public static long parseOctLong(String numStr) {
 
@@ -193,8 +190,9 @@ public final class NumericUtilities {
 	}
 
 	/**
-	 * returns the value of the specified long as hexadecimal, prefixing
+	 * returns the value of the specified long as hexadecimal, prefixing with the HEX_PREFIX_x
-	 * with the HEX_PREFIX_x string.
+	 * string.
+	 * 
 	 * @param value the long value to convert
 	 */
 	public final static String toHexString(long value) {
@@ -202,8 +200,9 @@ public final class NumericUtilities {
 	}
 
 	/**
-	 * returns the value of the specified long as hexadecimal, prefixing
+	 * returns the value of the specified long as hexadecimal, prefixing with the HEX_PREFIX_x
-	 * with the HEX_PREFIX_x string.
+	 * string.
+	 * 
 	 * @param value the long value to convert
 	 * @param size number of bytes to be represented
 	 */
@@ -215,8 +214,9 @@ public final class NumericUtilities {
 	}
 
 	/**
-	 * returns the value of the specified long as signed hexadecimal, prefixing
+	 * returns the value of the specified long as signed hexadecimal, prefixing with the
-	 * with the HEX_PREFIX_x string.
+	 * HEX_PREFIX_x string.
+	 * 
 	 * @param value the long value to convert
 	 */
 	public final static String toSignedHexString(long value) {
@@ -256,8 +256,9 @@ public final class NumericUtilities {
 	}
 
 	/**
-	 * Get an unsigned aligned value corresponding to the specified unsigned value
+	 * Get an unsigned aligned value corresponding to the specified unsigned value which will be
-	 * which will be greater than or equal the specified value.
+	 * greater than or equal the specified value.
+	 * 
 	 * @param unsignedValue value to be aligned
 	 * @param alignment alignment
 	 * @return aligned value
@@ -280,19 +281,38 @@ public final class NumericUtilities {
 	/**
 	 * Convert a masked value into a hexadecimal-ish string.
 	 *
-	 * Converts the data to hexadecimal, placing an X where a nibble is unknown. Where a nibble
+	 * Converts the data to hexadecimal, placing an X where a nibble is unknown. Where a nibble is
-	 * is partially defined, it is displayed as four bits in brackets []. Bits are displayed
+	 * partially defined, it is displayed as four bits in brackets []. Bits are displayed as x, or
-	 * as x, or the defined value.
+	 * the defined value.
 	 *
-	 * For example, consider the mask 00001111:01011100, and the value 00001001:00011000. This
+	 * For example, consider the mask 00001111:01011100, and the value 00001001:00011000. This will
-	 * will display as {@code X8:[x0x1][10xx]}. To see the correlation, consider the table:
+	 * display as {@code X8:[x0x1][10xx]}. To see the correlation, consider the table:
-	 * <table><caption></caption>
+	 * <table>
-	 *   <tr><th>Display</th><th>{@code X}</th>   <th>{@code 8}</th>   <th>{@code :}</th>
+	 * <caption></caption>
-	 *                           <th>{@code [x0x1]}</th><th>{@code [10xx]}</th></tr>
+	 * <tr>
-	 *   <tr><th>Mask</th>   <td>{@code 0000}</td><td>{@code 1111}</td><td>{@code :}</td>
+	 * <th>Display</th>
-	 *                           <td>{@code 0101}</td>  <td>{@code 1100}</td>  </tr>
+	 * <th>{@code X}</th>
-	 *   <tr><th>Value</th>  <td>{@code 0000}</td><td>{@code 1000}</td><td>{@code :}</td>
+	 * <th>{@code 8}</th>
-	 *                           <td>{@code 0001}</td>  <td>{@code 1000}</td>  </tr>
+	 * <th>{@code :}</th>
+	 * <th>{@code [x0x1]}</th>
+	 * <th>{@code [10xx]}</th>
+	 * </tr>
+	 * <tr>
+	 * <th>Mask</th>
+	 * <td>{@code 0000}</td>
+	 * <td>{@code 1111}</td>
+	 * <td>{@code :}</td>
+	 * <td>{@code 0101}</td>
+	 * <td>{@code 1100}</td>
+	 * </tr>
+	 * <tr>
+	 * <th>Value</th>
+	 * <td>{@code 0000}</td>
+	 * <td>{@code 1000}</td>
+	 * <td>{@code :}</td>
+	 * <td>{@code 0001}</td>
+	 * <td>{@code 1000}</td>
+	 * </tr>
 	 * </table>
 	 *
 	 * @param msk the mask
@@ -364,6 +384,7 @@ public final class NumericUtilities {
 	 * Philosophically, it is hexadecimal, but the only valid digits are 0 and F. Any
 	 * partially-included nibble will be broken down into bracketed bits. Displaying masks in this
 	 * way is convenient when shown proximal to related masked values.
+	 * 
 	 * @param msk the mask
 	 * @param n the number of nibbles, starting at the right
 	 * @param truncate true if leading Xs may be truncated
@@ -420,6 +441,7 @@ public final class NumericUtilities {
 
 	/**
 	 * The reverse of {@link #convertMaskedValueToHexString(long, long, int, boolean, int, String)}
+	 * 
 	 * @param msk an object to receive the resulting mask
 	 * @param val an object to receive the resulting value
 	 * @param hex the input string to parse
@@ -509,7 +531,8 @@ public final class NumericUtilities {
 
 	/**
 	 * Render <code>number</code> in different bases using the default signedness mode.
-	 * <p>This invokes {@linkplain #formatNumber(long, int, SignednessFormatMode)} with a
+	 * <p>
+	 * This invokes {@linkplain #formatNumber(long, int, SignednessFormatMode)} with a
 	 * <code>mode</code> parameter of <code>{@linkplain SignednessFormatMode#DEFAULT}</code>.
 	 *
 	 * @param number The number to represent
@@ -524,30 +547,129 @@ public final class NumericUtilities {
 	/**
 	 * Provide renderings of <code>number</code> in different bases:
 	 * <ul>
-	 * <li> <code>0</code> - renders <code>number</code> as an escaped character sequence</li>
+	 * <li><code>0</code> - renders <code>number</code> as an escaped character sequence</li>
-	 * <li> <code>2</code> - renders <code>number</code> as a <code>base-2</code> integer</li>
+	 * <li><code>2</code> - renders <code>number</code> as a <code>base-2</code> integer</li>
-	 * <li> <code>8</code> - renders <code>number</code> as a <code>base-8</code> integer</li>
+	 * <li><code>8</code> - renders <code>number</code> as a <code>base-8</code> integer</li>
-	 * <li> <code>10</code> - renders <code>number</code> as a <code>base-10</code> integer</li>
+	 * <li><code>10</code> - renders <code>number</code> as a <code>base-10</code> integer</li>
-	 * <li> <code>16</code> (default) - renders <code>number</code> as a <code>base-16</code> integer</li>
+	 * <li><code>16</code> (default) - renders <code>number</code> as a <code>base-16</code>
+	 * integer</li>
 	 * </ul>
-	 * <table><caption></caption>
+	 * <table>
-	 * <tr><th>Number</th><th>Radix</th><th>DEFAULT Mode Alias</th><th style="text-align:center"><i>UNSIGNED</i> Mode Value</th><th><i>SIGNED</i> Mode Value</th></tr>
+	 * <caption></caption>
-	 * <tr><td>&nbsp;</td><td></td><td><i></i></td><td></td><td></td></tr>
+	 * <tr>
-	 * <tr style="text-align:right;font-family: monospace"><td>100</td><td>2</td><td><i>UNSIGNED</i></td><td>1100100b</td><td>1100100b</td></tr>
+	 * <th>Number</th>
-	 * <tr style="text-align:right;font-family: monospace"><td>100</td><td>8</td><td><i>UNSIGNED</i></td><td>144o</td><td>144o</td></tr>
+	 * <th>Radix</th>
-	 * <tr style="text-align:right;font-family: monospace"><td>100</td><td>10</td><td><i>SIGNED</i></td><td>100</td><td>100</td></tr>
+	 * <th>DEFAULT Mode Alias</th>
-	 * <tr style="text-align:right;font-family: monospace"><td>100</td><td>16</td><td><i>UNSIGNED</i></td><td>64h</td><td>64h</td></tr>
+	 * <th style="text-align:center"><i>UNSIGNED</i> Mode Value</th>
-	 * <tr><td>&nbsp;</td><td></td><td><i></i></td><td></td><td></td></tr>
+	 * <th><i>SIGNED</i> Mode Value</th>
-	 * <tr style="text-align:right;font-family: monospace"><td>-1</td><td>2</td><td><i>UNSIGNED</i></td><td>1111111111111111111111111111111111111111111111111111111111111111b</td><td>-1b</td></tr>
+	 * </tr>
-	 * <tr style="text-align:right;font-family: monospace"><td>-1</td><td>8</td><td><i>UNSIGNED</i></td><td>1777777777777777777777o</td><td>-1o</td></tr>
+	 * <tr>
-	 * <tr style="text-align:right;font-family: monospace"><td>-1</td><td>10</td><td><i>SIGNED</i></td><td>18446744073709551615</td><td>-1</td></tr>
+	 * <td>&nbsp;</td>
-	 * <tr style="text-align:right;font-family: monospace"><td>-1</td><td>16</td><td><i>UNSIGNED</i></td><td>ffffffffffffffffh</td><td>-1h</td></tr>
+	 * <td></td>
-	 *<tr><td>&nbsp;</td><td></td><td><i></i></td><td></td><td></td></tr>
+	 * <td><i></i></td>
-	 * <tr style="text-align:right;font-family: monospace"><td>-100</td><td>2</td><td><i>UNSIGNED</i></td><td>1111111111111111111111111111111111111111111111111111111110011100b</td><td>-1100100b</td></tr>
+	 * <td></td>
-	 * <tr style="text-align:right;font-family: monospace"><td>-100</td><td>8</td><td><i>UNSIGNED</i></td><td>1777777777777777777634o</td><td>-144o</td></tr>
+	 * <td></td>
-	 * <tr style="text-align:right;font-family: monospace"><td>-100</td><td>10</td><td><i>SIGNED</i></td><td>18446744073709551516</td><td>-100</td></tr>
+	 * </tr>
-	 * <tr style="text-align:right;font-family: monospace"><td>-100</td><td>16</td><td><i>UNSIGNED</i></td><td>ffffffffffffff9ch</td><td>-64h</td></tr>
+	 * <tr style="text-align:right;font-family: monospace">
+	 * <td>100</td>
+	 * <td>2</td>
+	 * <td><i>UNSIGNED</i></td>
+	 * <td>1100100b</td>
+	 * <td>1100100b</td>
+	 * </tr>
+	 * <tr style="text-align:right;font-family: monospace">
+	 * <td>100</td>
+	 * <td>8</td>
+	 * <td><i>UNSIGNED</i></td>
+	 * <td>144o</td>
+	 * <td>144o</td>
+	 * </tr>
+	 * <tr style="text-align:right;font-family: monospace">
+	 * <td>100</td>
+	 * <td>10</td>
+	 * <td><i>SIGNED</i></td>
+	 * <td>100</td>
+	 * <td>100</td>
+	 * </tr>
+	 * <tr style="text-align:right;font-family: monospace">
+	 * <td>100</td>
+	 * <td>16</td>
+	 * <td><i>UNSIGNED</i></td>
+	 * <td>64h</td>
+	 * <td>64h</td>
+	 * </tr>
+	 * <tr>
+	 * <td>&nbsp;</td>
+	 * <td></td>
+	 * <td><i></i></td>
+	 * <td></td>
+	 * <td></td>
+	 * </tr>
+	 * <tr style="text-align:right;font-family: monospace">
+	 * <td>-1</td>
+	 * <td>2</td>
+	 * <td><i>UNSIGNED</i></td>
+	 * <td>1111111111111111111111111111111111111111111111111111111111111111b</td>
+	 * <td>-1b</td>
+	 * </tr>
+	 * <tr style="text-align:right;font-family: monospace">
+	 * <td>-1</td>
+	 * <td>8</td>
+	 * <td><i>UNSIGNED</i></td>
+	 * <td>1777777777777777777777o</td>
+	 * <td>-1o</td>
+	 * </tr>
+	 * <tr style="text-align:right;font-family: monospace">
+	 * <td>-1</td>
+	 * <td>10</td>
+	 * <td><i>SIGNED</i></td>
+	 * <td>18446744073709551615</td>
+	 * <td>-1</td>
+	 * </tr>
+	 * <tr style="text-align:right;font-family: monospace">
+	 * <td>-1</td>
+	 * <td>16</td>
+	 * <td><i>UNSIGNED</i></td>
+	 * <td>ffffffffffffffffh</td>
+	 * <td>-1h</td>
+	 * </tr>
+	 * <tr>
+	 * <td>&nbsp;</td>
+	 * <td></td>
+	 * <td><i></i></td>
+	 * <td></td>
+	 * <td></td>
+	 * </tr>
+	 * <tr style="text-align:right;font-family: monospace">
+	 * <td>-100</td>
+	 * <td>2</td>
+	 * <td><i>UNSIGNED</i></td>
+	 * <td>1111111111111111111111111111111111111111111111111111111110011100b</td>
+	 * <td>-1100100b</td>
+	 * </tr>
+	 * <tr style="text-align:right;font-family: monospace">
+	 * <td>-100</td>
+	 * <td>8</td>
+	 * <td><i>UNSIGNED</i></td>
+	 * <td>1777777777777777777634o</td>
+	 * <td>-144o</td>
+	 * </tr>
+	 * <tr style="text-align:right;font-family: monospace">
+	 * <td>-100</td>
+	 * <td>10</td>
+	 * <td><i>SIGNED</i></td>
+	 * <td>18446744073709551516</td>
+	 * <td>-100</td>
+	 * </tr>
+	 * <tr style="text-align:right;font-family: monospace">
+	 * <td>-100</td>
+	 * <td>16</td>
+	 * <td><i>UNSIGNED</i></td>
+	 * <td>ffffffffffffff9ch</td>
+	 * <td>-64h</td>
+	 * </tr>
 	 * </table>
+	 * 
 	 * @param number The number to represent
 	 * @param radix The base in which <code>number</code> is represented
 	 * @param mode Specifies how the number is formatted with respect to its signed-ness
@@ -637,8 +759,7 @@ public final class NumericUtilities {
 	}
 
 	/**
-	 * Convert the given byte into a two character String, padding with a leading 0 if
+	 * Convert the given byte into a two character String, padding with a leading 0 if needed.
-	 * needed.
 	 *
 	 * @param b the byte
 	 * @return the byte string
@@ -728,6 +849,7 @@ public final class NumericUtilities {
 
 	/**
 	 * Determine if the provided Number is an integer type -- Byte, Short, Integer, or Long.
+	 * 
 	 * @param number the object to check for for integer-type
 	 * @return true if the provided number is an integer-type, false otherwise
 	 */
@@ -738,6 +860,7 @@ public final class NumericUtilities {
 
 	/**
 	 * Determine if the provided Number class is an integer type.
+	 * 
 	 * @param numClass Class of an object
 	 * @return true if the class parameter is a integer type, false otherwise
 	 */
@@ -747,6 +870,7 @@ public final class NumericUtilities {
 
 	/**
 	 * Determine if the provided Number is a floating-point type -- Float or Double.
+	 * 
 	 * @param number the object to check for for floating-point-type
 	 * @return true if the provided number is a floating-point-type, false otherwise
 	 */
@@ -757,6 +881,7 @@ public final class NumericUtilities {
 
 	/**
 	 * Determine if the provided Number class is a floating-point type.
+	 * 
 	 * @param numClass Class of an object
 	 * @return true if the class parameter is a floating-point type, false otherwise
 	 */
@@ -765,12 +890,12 @@ public final class NumericUtilities {
 	}
 
 	/**
-	 * Provides the protocol for rendering integer-type numbers in different
+	 * Provides the protocol for rendering integer-type numbers in different signed-ness modes.
-	 * signed-ness modes.
 	 */
 	private static interface IntegerRadixRenderer {
 		/**
 		 * Format the given number in the provided radix base.
+		 * 
 		 * @param number the number to render
 		 * @param radix the base in which to render
 		 * @return a string representing the provided number in the given base
@@ -836,8 +961,8 @@ public final class NumericUtilities {
 		/**
 		 * {@inheritDoc}
 		 * <p>
-		 * Values to be rendered in binary, octal, or hexadecimal bases are rendered
+		 * Values to be rendered in binary, octal, or hexadecimal bases are rendered as unsigned,
-		 * as unsigned, numbers rendered in decimal are rendered as signed.
+		 * numbers rendered in decimal are rendered as signed.
 		 */
 		@Override
 		public String toString(long number, int radix) {

Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/AbstractWeakValueMap.java

@@ -0,0 +1,216 @@
+/* ###
+ * IP: GHIDRA
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package ghidra.util.datastruct;
+
+import java.lang.ref.ReferenceQueue;
+import java.lang.ref.WeakReference;
+import java.util.*;
+
+/**
+ * Class to provide a map with weak values, backed by a given map
+ *
+ * @param <K> the type of keys
+ * @param <V> the type of values
+ */
+public abstract class AbstractWeakValueMap<K, V> implements Map<K, V> {
+	protected ReferenceQueue<V> refQueue;
+
+	/**
+	 * Constructs a new weak map
+	 */
+	protected AbstractWeakValueMap() {
+		refQueue = new ReferenceQueue<>();
+	}
+
+	/**
+	 * Returns the backing map
+	 * 
+	 * @return the map
+	 */
+	protected abstract Map<K, WeakValueRef<K, V>> getRefMap();
+
+	@Override
+	public V put(K key, V value) {
+		processQueue();
+		WeakValueRef<K, V> ref = new WeakValueRef<>(key, value, refQueue);
+		WeakValueRef<K, V> oldRef = getRefMap().put(key, ref);
+		if (oldRef != null) {
+			return oldRef.get();
+		}
+		return null;
+	}
+
+	@Override
+	public V get(Object key) {
+		processQueue();
+		WeakValueRef<K, V> ref = getRefMap().get(key);
+		if (ref != null) {
+			return ref.get();
+		}
+		return null;
+	}
+
+	@Override
+	public int size() {
+		processQueue();
+		return getRefMap().size();
+	}
+
+	@Override
+	public void clear() {
+		getRefMap().clear();
+		refQueue = new ReferenceQueue<>();
+	}
+
+	@Override
+	public boolean isEmpty() {
+		processQueue();
+		return getRefMap().isEmpty();
+	}
+
+	@Override
+	public boolean containsKey(Object key) {
+		processQueue();
+		return getRefMap().containsKey(key);
+	}
+
+	@Override
+	public boolean containsValue(Object value) {
+		processQueue();
+		Iterator<WeakValueRef<K, V>> it = getRefMap().values().iterator();
+		while (it.hasNext()) {
+			WeakValueRef<K, V> ref = it.next();
+			if (value.equals(ref.get())) {
+				return true;
+			}
+		}
+		return false;
+	}
+
+	@Override
+	public Collection<V> values() {
+		ArrayList<V> list = new ArrayList<>(getRefMap().size());
+		Iterator<WeakValueRef<K, V>> it = getRefMap().values().iterator();
+		while (it.hasNext()) {
+			WeakValueRef<K, V> ref = it.next();
+			V value = ref.get();
+			if (value != null) {
+				list.add(value);
+			}
+		}
+		return list;
+	}
+
+	@Override
+	public void putAll(Map<? extends K, ? extends V> map) {
+		Iterator<? extends K> it = map.keySet().iterator();
+		while (it.hasNext()) {
+			K key = it.next();
+			V value = map.get(key);
+			if (value != null) {
+				put(key, value);
+			}
+		}
+	}
+
+	@Override
+	public Set<Map.Entry<K, V>> entrySet() {
+		processQueue();
+		Set<Map.Entry<K, V>> list = new HashSet<>();
+		Set<Map.Entry<K, WeakValueRef<K, V>>> entrySet = getRefMap().entrySet();
+		Iterator<Map.Entry<K, WeakValueRef<K, V>>> it = entrySet.iterator();
+		while (it.hasNext()) {
+			Map.Entry<K, WeakValueRef<K, V>> next = it.next();
+			WeakValueRef<K, V> valueRef = next.getValue();
+			V value = valueRef.get();
+			if (value != null) {
+				list.add(new GeneratedEntry(next.getKey(), value));
+			}
+		}
+		return list;
+	}
+
+	@Override
+	public Set<K> keySet() {
+		processQueue();
+		return getRefMap().keySet();
+	}
+
+	@Override
+	public V remove(Object key) {
+		WeakValueRef<K, V> ref = getRefMap().remove(key);
+		if (ref != null) {
+			return ref.get();
+		}
+		return null;
+	}
+
+	@SuppressWarnings("unchecked")
+	protected void processQueue() {
+		WeakValueRef<K, V> ref;
+		while ((ref = (WeakValueRef<K, V>) refQueue.poll()) != null) {
+			getRefMap().remove(ref.key);
+		}
+	}
+
+	/**
+	 * An entry for the "entrySet" method, since internally, entries are of weak-referenced values.
+	 */
+	protected class GeneratedEntry implements Map.Entry<K, V> {
+		K key;
+		V value;
+
+		GeneratedEntry(K key, V value) {
+			this.key = key;
+			this.value = value;
+		}
+
+		@Override
+		public K getKey() {
+			return key;
+		}
+
+		@Override
+		public V getValue() {
+			return value;
+		}
+
+		@Override
+		public V setValue(V value) {
+			this.value = value;
+			return put(key, value);
+		}
+
+	}
+
+	/**
+	 * A weak value ref that also knows its key in the map.
+	 * 
+	 * <p>
+	 * Used for processing the reference queue, so we know which keys to remove.
+	 * 
+	 * @param <K> the type of key
+	 * @param <V> the type of value
+	 */
+	protected static class WeakValueRef<K, V> extends WeakReference<V> {
+		K key;
+
+		WeakValueRef(K key, V value, ReferenceQueue<V> refQueue) {
+			super(value, refQueue);
+			this.key = key;
+		}
+	}
+}

Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/AbstractWeakValueNavigableMap.java

@@ -0,0 +1,216 @@
+/* ###
+ * IP: GHIDRA
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package ghidra.util.datastruct;
+
+import java.util.*;
+
+/**
+ * Class to provide a navigable, e.g., tree-, map with weak values
+ * 
+ * @param <K> the type of keys
+ * @param <V> the type of values
+ */
+public abstract class AbstractWeakValueNavigableMap<K, V> extends AbstractWeakValueMap<K, V>
+		implements NavigableMap<K, V> {
+
+	/**
+	 * A view of this same map that limits or changes the order of the keys
+	 *
+	 * <p>
+	 * TODO: By virtue of extending (indirectly) {@link AbstractWeakValueMap}, this view inherits a
+	 * unique, but totally unused, {@link AbstractWeakValueMap#refQueue}. This is a small and
+	 * harmless, but unnecessary waste.
+	 *
+	 * @param <K> the type of keys
+	 * @param <V> the type of values
+	 */
+	protected static class NavigableView<K, V> extends AbstractWeakValueNavigableMap<K, V> {
+		protected final AbstractWeakValueNavigableMap<K, V> map;
+		protected final NavigableMap<K, WeakValueRef<K, V>> mod;
+
+		public NavigableView(AbstractWeakValueNavigableMap<K, V> map,
+				NavigableMap<K, WeakValueRef<K, V>> sub) {
+			this.map = map;
+			this.mod = Collections.unmodifiableNavigableMap(sub);
+		}
+
+		@Override
+		protected NavigableMap<K, WeakValueRef<K, V>> getRefMap() {
+			map.processQueue();
+			return mod;
+		}
+	}
+
+	@Override
+	protected abstract NavigableMap<K, WeakValueRef<K, V>> getRefMap();
+
+	@Override
+	public Comparator<? super K> comparator() {
+		return getRefMap().comparator();
+	}
+
+	@Override
+	public K firstKey() {
+		processQueue();
+		return getRefMap().firstKey();
+	}
+
+	@Override
+	public K lastKey() {
+		processQueue();
+		return getRefMap().lastKey();
+	}
+
+	/**
+	 * Construct a generated (wrapper) entry, for the entry-retrieval methods.
+	 * 
+	 * <p>
+	 * This handles the null case in one place.
+	 * 
+	 * @param ent the entry to wrap, possibly null
+	 * @return the generated entry, or null
+	 */
+	protected GeneratedEntry generateEntry(Entry<K, WeakValueRef<K, V>> ent) {
+		if (ent == null) {
+			return null;
+		}
+		return new GeneratedEntry(ent.getKey(), ent.getValue().get());
+	}
+
+	@Override
+	public Entry<K, V> lowerEntry(K key) {
+		processQueue();
+		return generateEntry(getRefMap().lowerEntry(key));
+	}
+
+	@Override
+	public K lowerKey(K key) {
+		processQueue();
+		return getRefMap().lowerKey(key);
+	}
+
+	@Override
+	public Entry<K, V> floorEntry(K key) {
+		processQueue();
+		return generateEntry(getRefMap().floorEntry(key));
+	}
+
+	@Override
+	public K floorKey(K key) {
+		processQueue();
+		return getRefMap().floorKey(key);
+	}
+
+	@Override
+	public Entry<K, V> ceilingEntry(K key) {
+		processQueue();
+		return generateEntry(getRefMap().ceilingEntry(key));
+	}
+
+	@Override
+	public K ceilingKey(K key) {
+		processQueue();
+		return getRefMap().ceilingKey(key);
+	}
+
+	@Override
+	public Entry<K, V> higherEntry(K key) {
+		processQueue();
+		return generateEntry(getRefMap().higherEntry(key));
+	}
+
+	@Override
+	public K higherKey(K key) {
+		processQueue();
+		return getRefMap().higherKey(key);
+	}
+
+	@Override
+	public Entry<K, V> firstEntry() {
+		processQueue();
+		return generateEntry(getRefMap().firstEntry());
+	}
+
+	@Override
+	public Entry<K, V> lastEntry() {
+		processQueue();
+		return generateEntry(getRefMap().lastEntry());
+	}
+
+	@Override
+	public Entry<K, V> pollFirstEntry() {
+		processQueue();
+		return generateEntry(getRefMap().pollFirstEntry());
+	}
+
+	@Override
+	public Entry<K, V> pollLastEntry() {
+		processQueue();
+		return generateEntry(getRefMap().pollLastEntry());
+	}
+
+	@Override
+	public NavigableMap<K, V> descendingMap() {
+		processQueue();
+		return new NavigableView<>(this, getRefMap().descendingMap());
+	}
+
+	@Override
+	public NavigableSet<K> navigableKeySet() {
+		return getRefMap().navigableKeySet();
+	}
+
+	@Override
+	public NavigableSet<K> descendingKeySet() {
+		return getRefMap().descendingKeySet();
+	}
+
+	@Override
+	public NavigableMap<K, V> subMap(K fromKey, boolean fromInclusive, K toKey,
+			boolean toInclusive) {
+		processQueue();
+		return new NavigableView<>(this,
+			getRefMap().subMap(fromKey, fromInclusive, toKey, toInclusive));
+	}
+
+	@Override
+	public NavigableMap<K, V> headMap(K toKey, boolean inclusive) {
+		processQueue();
+		return new NavigableView<>(this, getRefMap().headMap(toKey, inclusive));
+	}
+
+	@Override
+	public NavigableMap<K, V> tailMap(K fromKey, boolean inclusive) {
+		processQueue();
+		return new NavigableView<>(this, getRefMap().tailMap(fromKey, inclusive));
+	}
+
+	@Override
+	public SortedMap<K, V> subMap(K fromKey, K toKey) {
+		processQueue();
+		return subMap(fromKey, true, toKey, false);
+	}
+
+	@Override
+	public SortedMap<K, V> headMap(K toKey) {
+		return headMap(toKey, false);
+	}
+
+	@Override
+	public SortedMap<K, V> tailMap(K fromKey) {
+		return tailMap(fromKey, true);
+	}
+}

Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/WeakValueHashMap.java

@@ -15,192 +15,36 @@
  */
 package ghidra.util.datastruct;
 
-import java.lang.ref.ReferenceQueue;
+import java.util.HashMap;
-import java.lang.ref.WeakReference;
+import java.util.Map;
-import java.util.*;
 
 /**
  * Class to provide a hash map with weak values.
  */
 
-public class WeakValueHashMap<K, V> implements Map<K, V> {
+public class WeakValueHashMap<K, V> extends AbstractWeakValueMap<K, V> {
-	private HashMap<K, WeakValueRef<K, V>> hashMap;
+	private Map<K, WeakValueRef<K, V>> refMap;
-	private ReferenceQueue<V> refQueue;
 
 	/**
 	 * Constructs a new weak map
 	 */
 	public WeakValueHashMap() {
-		hashMap = new HashMap<>();
+		super();
-		refQueue = new ReferenceQueue<>();
+		refMap = new HashMap<>();
 	}
 
 	/**
 	 * Constructs a new weak map with the given initial size
+	 * 
 	 * @param initialSize the initial size of the backing map
 	 */
 	public WeakValueHashMap(int initialSize) {
-		hashMap = new HashMap<>(initialSize);
+		super();
-		refQueue = new ReferenceQueue<>();
+		refMap = new HashMap<>(initialSize);
 	}
 
 	@Override
-	public V put(K key, V value) {
+	protected Map<K, WeakValueRef<K, V>> getRefMap() {
-		processQueue();
+		return refMap;
-		WeakValueRef<K, V> ref = new WeakValueRef<>(key, value, refQueue);
-		WeakValueRef<K, V> oldRef = hashMap.put(key, ref);
-		if (oldRef != null) {
-			return oldRef.get();
-		}
-		return null;
 	}
-
-	@Override
-	public V get(Object key) {
-		processQueue();
-		WeakValueRef<K, V> ref = hashMap.get(key);
-		if (ref != null) {
-			return ref.get();
-		}
-		return null;
-	}
-
-	@Override
-	public int size() {
-		processQueue();
-		return hashMap.size();
-	}
-
-	@Override
-	public void clear() {
-		hashMap.clear();
-		refQueue = new ReferenceQueue<>();
-	}
-
-	@Override
-	public boolean isEmpty() {
-		processQueue();
-		return hashMap.isEmpty();
-	}
-
-	@Override
-	public boolean containsKey(Object key) {
-		processQueue();
-		return hashMap.containsKey(key);
-	}
-
-	@Override
-	public boolean containsValue(Object value) {
-		processQueue();
-		Iterator<WeakValueRef<K, V>> it = hashMap.values().iterator();
-		while (it.hasNext()) {
-			WeakValueRef<K, V> ref = it.next();
-			if (value.equals(ref.get())) {
-				return true;
-			}
-		}
-		return false;
-	}
-
-	@Override
-	public Collection<V> values() {
-		ArrayList<V> list = new ArrayList<>(hashMap.size());
-		Iterator<WeakValueRef<K, V>> it = hashMap.values().iterator();
-		while (it.hasNext()) {
-			WeakValueRef<K, V> ref = it.next();
-			V value = ref.get();
-			if (value != null) {
-				list.add(value);
-			}
-		}
-		return list;
-	}
-
-	@Override
-	public void putAll(Map<? extends K, ? extends V> map) {
-		Iterator<? extends K> it = map.keySet().iterator();
-		while (it.hasNext()) {
-			K key = it.next();
-			V value = map.get(key);
-			if (value != null) {
-				put(key, value);
-			}
-		}
-	}
-
-	@Override
-	public Set<Map.Entry<K, V>> entrySet() {
-		processQueue();
-		Set<Map.Entry<K, V>> list = new HashSet<>();
-		Set<Map.Entry<K, WeakValueRef<K, V>>> entrySet = hashMap.entrySet();
-		Iterator<Map.Entry<K, WeakValueRef<K, V>>> it = entrySet.iterator();
-		while (it.hasNext()) {
-			Map.Entry<K, WeakValueRef<K, V>> next = it.next();
-			WeakValueRef<K, V> valueRef = next.getValue();
-			V value = valueRef.get();
-			if (value != null) {
-				list.add(new GeneratedEntry(next.getKey(), value));
-			}
-		}
-		return list;
-	}
-
-	@Override
-	public Set<K> keySet() {
-		processQueue();
-		return hashMap.keySet();
-	}
-
-	@Override
-	public V remove(Object key) {
-		WeakValueRef<K, V> ref = hashMap.remove(key);
-		if (ref != null) {
-			return ref.get();
-		}
-		return null;
-	}
-
-	@SuppressWarnings("unchecked")
-	private void processQueue() {
-		WeakValueRef<K, V> ref;
-		while ((ref = (WeakValueRef<K, V>) refQueue.poll()) != null) {
-			hashMap.remove(ref.key);
-		}
-	}
-
-	class GeneratedEntry implements Map.Entry<K, V> {
-		K key;
-		V value;
-
-		GeneratedEntry(K key, V value) {
-			this.key = key;
-			this.value = value;
-		}
-
-		@Override
-		public K getKey() {
-			return key;
-		}
-
-		@Override
-		public V getValue() {
-			return value;
-		}
-
-		@Override
-		public V setValue(V value) {
-			return put(key, value);
-		}
-
-	}
-
-	static class WeakValueRef<K, V> extends WeakReference<V> {
-		K key;
-
-		WeakValueRef(K key, V value, ReferenceQueue<V> refQueue) {
-			super(value, refQueue);
-			this.key = key;
-		}
-	}
-
 }

Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/WeakValueTreeMap.java

@@ -0,0 +1,48 @@
+/* ###
+ * IP: GHIDRA
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package ghidra.util.datastruct;
+
+import java.util.*;
+
+/**
+ * Class to provide a tree map with weak values.
+ */
+public class WeakValueTreeMap<K, V> extends AbstractWeakValueNavigableMap<K, V> {
+	protected final NavigableMap<K, WeakValueRef<K, V>> refMap;
+
+	/**
+	 * Constructs a new weak map
+	 */
+	public WeakValueTreeMap() {
+		super();
+		refMap = new TreeMap<>();
+	}
+
+	/**
+	 * Constructs a new weak map with keys ordered according to the given comparator
+	 * 
+	 * @param comparator the comparator, or {@code null} for the natural ordering
+	 */
+	public WeakValueTreeMap(Comparator<K> comparator) {
+		super();
+		refMap = new TreeMap<>(comparator);
+	}
+
+	@Override
+	protected NavigableMap<K, WeakValueRef<K, V>> getRefMap() {
+		return refMap;
+	}
+}

Ghidra/Framework/Generic/src/test/java/ghidra/generic/util/datastruct/TreeValueSortedMapTest.java

@@ -13,35 +13,24 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package ghidra.util;
+package ghidra.generic.util.datastruct;
 
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.*;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
 
-import java.util.ArrayList;
+import java.util.*;
-import java.util.Arrays;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.List;
 import java.util.Map.Entry;
-import java.util.Random;
-import java.util.Set;
 
 import org.apache.commons.collections4.comparators.ReverseComparator;
 import org.apache.commons.lang3.ArrayUtils;
 import org.junit.Test;
 
-import ghidra.generic.util.datastruct.DynamicValueSortedTreeMap;
+import ghidra.generic.util.datastruct.TreeValueSortedMap;
-
+import ghidra.generic.util.datastruct.ValueSortedMap;
-public class DynamicValueSortedTreeMapTest {
-	public static class NonComparable {
-	}
 
+public class TreeValueSortedMapTest {
 	@Test
 	public void testNaturalOrder() {
-		DynamicValueSortedTreeMap<String, Integer> queue = new DynamicValueSortedTreeMap<>();
+		ValueSortedMap<String, Integer> queue = TreeValueSortedMap.createWithNaturalOrder();
 		queue.put("2nd", 2);
 		queue.put("1st", 1);
 		queue.put("3rd", 3);
@@ -49,17 +38,10 @@ public class DynamicValueSortedTreeMapTest {
 		assertEquals(Arrays.asList(new String[] { "1st", "2nd", "3rd" }), ordered);
 	}
 
-	@Test(expected = ClassCastException.class)
-	public void testUnorderedError() {
-		DynamicValueSortedTreeMap<String, NonComparable> queue = new DynamicValueSortedTreeMap<>();
-		queue.put("2nd", new NonComparable());
-		queue.put("1st", new NonComparable());
-	}
-
 	@Test
 	public void testExplicitOrdered() {
-		DynamicValueSortedTreeMap<String, Integer> queue =
+		ValueSortedMap<String, Integer> queue =
-			new DynamicValueSortedTreeMap<>(new ReverseComparator<>());
+			TreeValueSortedMap.createWithComparator(new ReverseComparator<>());
 		queue.put("2nd", 2);
 		queue.put("1st", 1);
 		queue.put("3rd", 3);
@@ -67,15 +49,101 @@ public class DynamicValueSortedTreeMapTest {
 		assertEquals(Arrays.asList(new String[] { "3rd", "2nd", "1st" }), ordered);
 	}
 
+	@Test
+	public void testBoundsSearches() {
+		ValueSortedMap<String, Integer> queue = TreeValueSortedMap.createWithNaturalOrder();
+
+		assertNull(queue.lowerEntryByValue(4));
+		assertNull(queue.floorEntryByValue(4));
+		assertNull(queue.ceilingEntryByValue(4));
+		assertNull(queue.higherEntryByValue(4));
+
+		assertEquals(-1, queue.values().lowerIndex(4));
+		assertEquals(-1, queue.values().floorIndex(4));
+		assertEquals(-1, queue.values().ceilingIndex(4));
+		assertEquals(-1, queue.values().higherIndex(4));
+
+		queue.put("4th", 4);
+
+		assertNull(queue.lowerEntryByValue(3));
+		assertEquals(-1, queue.values().lowerIndex(3));
+		assertNull(queue.lowerEntryByValue(4));
+		assertEquals(-1, queue.values().lowerIndex(4));
+		assertEquals("4th", queue.lowerEntryByValue(5).getKey());
+		assertEquals(0, queue.values().lowerIndex(5));
+
+		assertNull(queue.floorEntryByValue(3));
+		assertEquals(-1, queue.values().floorIndex(3));
+		assertEquals("4th", queue.floorEntryByValue(4).getKey());
+		assertEquals(0, queue.values().floorIndex(4));
+		assertEquals("4th", queue.floorEntryByValue(5).getKey());
+		assertEquals(0, queue.values().floorIndex(5));
+
+		assertEquals("4th", queue.ceilingEntryByValue(3).getKey());
+		assertEquals(0, queue.values().ceilingIndex(3));
+		assertEquals("4th", queue.ceilingEntryByValue(4).getKey());
+		assertEquals(0, queue.values().ceilingIndex(4));
+		assertNull(queue.ceilingEntryByValue(5));
+		assertEquals(-1, queue.values().ceilingIndex(5));
+
+		assertEquals("4th", queue.higherEntryByValue(3).getKey());
+		assertEquals(0, queue.values().higherIndex(3));
+		assertNull(queue.higherEntryByValue(4));
+		assertEquals(-1, queue.values().higherIndex(4));
+		assertNull(queue.higherEntryByValue(5));
+		assertEquals(-1, queue.values().higherIndex(5));
+
+		queue.put("2nd", 2);
+		queue.put("6th", 6);
+
+		assertNull(queue.lowerEntryByValue(1));
+		assertNull(queue.lowerEntryByValue(2));
+		assertEquals("2nd", queue.lowerEntryByValue(3).getKey());
+		assertEquals("2nd", queue.lowerEntryByValue(4).getKey());
+		assertEquals("4th", queue.lowerEntryByValue(5).getKey());
+		assertEquals("4th", queue.lowerEntryByValue(6).getKey());
+		assertEquals("6th", queue.lowerEntryByValue(7).getKey());
+		assertEquals(2, queue.values().lowerIndex(7)); // Only this once
+
+		assertNull(queue.floorEntryByValue(1));
+		assertEquals("2nd", queue.floorEntryByValue(2).getKey());
+		assertEquals("2nd", queue.floorEntryByValue(3).getKey());
+		assertEquals("4th", queue.floorEntryByValue(4).getKey());
+		assertEquals("4th", queue.floorEntryByValue(5).getKey());
+		assertEquals("6th", queue.floorEntryByValue(6).getKey());
+		assertEquals("6th", queue.floorEntryByValue(7).getKey());
+
+		assertEquals("2nd", queue.ceilingEntryByValue(1).getKey());
+		assertEquals("2nd", queue.ceilingEntryByValue(2).getKey());
+		assertEquals("4th", queue.ceilingEntryByValue(3).getKey());
+		assertEquals("4th", queue.ceilingEntryByValue(4).getKey());
+		assertEquals("6th", queue.ceilingEntryByValue(5).getKey());
+		assertEquals("6th", queue.ceilingEntryByValue(6).getKey());
+		assertNull(queue.ceilingEntryByValue(7));
+
+		assertEquals("2nd", queue.higherEntryByValue(1).getKey());
+		assertEquals("4th", queue.higherEntryByValue(2).getKey());
+		assertEquals("4th", queue.higherEntryByValue(3).getKey());
+		assertEquals("6th", queue.higherEntryByValue(4).getKey());
+		assertEquals("6th", queue.higherEntryByValue(5).getKey());
+		assertNull(queue.higherEntryByValue(6));
+		assertNull(queue.higherEntryByValue(7));
+	}
+
 	@Test
 	public void testIsEmpty() {
-		DynamicValueSortedTreeMap<String, Integer> queue = new DynamicValueSortedTreeMap<>();
+		ValueSortedMap<String, Integer> queue = TreeValueSortedMap.createWithNaturalOrder();
 		assertTrue(queue.isEmpty());
+
+		assertFalse(queue.containsKey("1st"));
+		assertFalse(queue.containsValue(1));
+		assertEquals(-1, queue.values().indexOf(1));
+
 		queue.put("1st", 1);
 		assertFalse(queue.isEmpty());
 	}
 
-	protected <K, V> void checkConsistent(DynamicValueSortedTreeMap<K, V> queue) {
+	protected <K, V> void checkConsistent(ValueSortedMap<K, V> queue) {
 		Iterator<Entry<K, V>> it = queue.entrySet().iterator();
 		V last = null;
 		Set<K> seen = new HashSet<>();
@@ -119,7 +187,7 @@ public class DynamicValueSortedTreeMapTest {
 		final int COUNT = 1000;
 		final int ROUNDS = 5;
 		Random rand = new Random();
-		DynamicValueSortedTreeMap<String, Integer> queue = new DynamicValueSortedTreeMap<>();
+		ValueSortedMap<String, Integer> queue = TreeValueSortedMap.createWithNaturalOrder();
 		for (int r = 0; r < ROUNDS; r++) {
 			for (int i = 0; i < COUNT; i++) {
 				queue.put("Element" + i, rand.nextInt(50));
@@ -133,7 +201,7 @@ public class DynamicValueSortedTreeMapTest {
 	public void testRemoveRandomly() {
 		final int COUNT = 100;
 		Random rand = new Random();
-		DynamicValueSortedTreeMap<String, Integer> queue = new DynamicValueSortedTreeMap<>();
+		ValueSortedMap<String, Integer> queue = TreeValueSortedMap.createWithNaturalOrder();
 		HashSet<String> all = new HashSet<>();
 		for (int i = 0; i < COUNT; i++) {
 			queue.put("Element" + i, rand.nextInt(50));
@@ -157,7 +225,7 @@ public class DynamicValueSortedTreeMapTest {
 	public void testUpdateRandomly() {
 		final int COUNT = 100;
 		Random rand = new Random();
-		DynamicValueSortedTreeMap<String, Integer> queue = new DynamicValueSortedTreeMap<>();
+		ValueSortedMap<String, Integer> queue = TreeValueSortedMap.createWithNaturalOrder();
 		for (int i = 0; i < COUNT; i++) {
 			queue.put("Element" + i, rand.nextInt(50));
 		}
@@ -176,7 +244,7 @@ public class DynamicValueSortedTreeMapTest {
 	public void testValueIndices() {
 		final int ROUNDS = 1000;
 		Random rand = new Random();
-		DynamicValueSortedTreeMap<String, Integer> queue = new DynamicValueSortedTreeMap<>();
+		ValueSortedMap<String, Integer> queue = TreeValueSortedMap.createWithNaturalOrder();
 		int[] vals = // 0  1  2  3  4  5  6  7  8  9  10 11 12 13 14 15 16 17 18 19 20 21 22
 			new int[] { 0, 0, 1, 1, 1, 2, 3, 4, 4, 5, 5, 6, 6, 6, 6, 6, 6, 6, 6, 6, 8, 8, 10 };
 		for (int r = 0; r < ROUNDS; r++) {
@@ -216,7 +284,7 @@ public class DynamicValueSortedTreeMapTest {
 	public void testAsMonotonicQueue() {
 		final int COUNT = 1000;
 		Random rand = new Random();
-		DynamicValueSortedTreeMap<String, Integer> queue = new DynamicValueSortedTreeMap<>();
+		ValueSortedMap<String, Integer> queue = TreeValueSortedMap.createWithNaturalOrder();
 		for (int i = 0; i < COUNT; i++) {
 			queue.put("ElementA" + i, rand.nextInt(50));
 		}
@@ -238,4 +306,46 @@ public class DynamicValueSortedTreeMapTest {
 		assertEquals(0, queue.size());
 		assertTrue(queue.isEmpty());
 	}
+
+	@Test
+	public void testClearViaKeyIterator() {
+		ValueSortedMap<String, Integer> queue = TreeValueSortedMap.createWithNaturalOrder();
+		for (int i = 0; i < 10; i++) {
+			queue.put("Element" + i, i);
+		}
+		Iterator<String> kit = queue.keySet().iterator();
+		while (kit.hasNext()) {
+			kit.next();
+			kit.remove();
+		}
+
+		assertTrue(queue.isEmpty());
+	}
+
+	@Test
+	public void testRemoveOddsViaValueIterator() {
+		ValueSortedMap<String, Integer> queue = TreeValueSortedMap.createWithNaturalOrder();
+		for (int i = 0; i < 10; i++) {
+			queue.put("Element" + i, i);
+		}
+		Iterator<Integer> vit = queue.values().iterator();
+		while (vit.hasNext()) {
+			int val = vit.next();
+			if (val % 2 == 1) {
+				vit.remove();
+			}
+		}
+
+		for (int val : queue.values()) {
+			assertEquals(0, val % 2);
+		}
+	}
+
+	@Test(expected = IllegalStateException.class)
+	public void testNominalBehaviorIteratorRemoveBeforeNext() {
+		Set<Integer> set = new HashSet<>();
+		set.add(5);
+		Iterator<Integer> it = set.iterator();
+		it.remove();
+	}
 }

Ghidra/Framework/Generic/src/test/java/ghidra/util/DynamicSortedTreeSetTest.java

@@ -1,211 +0,0 @@
-/* ###
- * IP: GHIDRA
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- * 
- *      http://www.apache.org/licenses/LICENSE-2.0
- * 
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package ghidra.util;
-
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Comparator;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Random;
-import java.util.Set;
-
-import org.apache.commons.lang3.ArrayUtils;
-import org.junit.Test;
-
-public class DynamicSortedTreeSetTest {
-	public static class NonComparable {
-		public NonComparable(String key, int cost) {
-			this.key = key;
-			this.cost = cost;
-		}
-
-		@Override
-		public String toString() {
-			return key + "=" + cost;
-		}
-
-		protected String key;
-		protected int cost;
-	}
-
-	public static class TestElem extends NonComparable implements Comparable<TestElem> {
-		public TestElem(String key, int cost) {
-			super(key, cost);
-		}
-
-		@Override
-		public int compareTo(TestElem that) {
-			return key.compareTo(that.key);
-		}
-	}
-
-	public static class CostComparator implements Comparator<TestElem> {
-		@Override
-		public int compare(TestElem a, TestElem b) {
-			return a.cost - b.cost;
-		}
-	}
-
-	@Test
-	public void testNaturalOrder() {
-		DynamicSortedTreeSet<String> queue = new DynamicSortedTreeSet<>();
-		queue.add("2nd");
-		queue.add("1st");
-		queue.add("3rd");
-		List<String> ordered = new ArrayList<>(queue);
-		assertEquals(Arrays.asList(new String[] { "1st", "2nd", "3rd" }), ordered);
-	}
-
-	@Test(expected = ClassCastException.class)
-	public void testUnorderedError() {
-		DynamicSortedTreeSet<NonComparable> queue = new DynamicSortedTreeSet<>();
-		queue.add(new NonComparable("2nd", 2));
-		queue.add(new NonComparable("1st", 1));
-	}
-
-	@Test
-	public void testExplicitOrdered() {
-		DynamicSortedTreeSet<TestElem> queue = new DynamicSortedTreeSet<>(new CostComparator());
-		queue.add(new TestElem("2ndB", 2));
-		queue.add(new TestElem("2ndA", 2));
-		queue.add(new TestElem("1st", 1));
-		queue.add(new TestElem("3rd", 3));
-		List<String> ordered = new ArrayList<>();
-		for (TestElem elem : queue) {
-			ordered.add(elem.key);
-		}
-		assertEquals(Arrays.asList(new String[] { "1st", "2ndB", "2ndA", "3rd" }), ordered);
-	}
-
-	@Test
-	public void testIsEmpty() {
-		DynamicSortedTreeSet<TestElem> queue = new DynamicSortedTreeSet<>(new CostComparator());
-		assertTrue(queue.isEmpty());
-		queue.add(new TestElem("1st", 1));
-		assertFalse(queue.isEmpty());
-	}
-
-	protected <E> void checkConsistent(DynamicSortedTreeSet<E> queue, Comparator<E> comp) {
-		Iterator<E> it = queue.iterator();
-		E last = null;
-		Set<E> seen = new HashSet<>();
-		for (int i = 0; i < queue.size(); i++) {
-			E e = it.next();
-			assertTrue("Indices and iterator did not give same order", queue.get(i) == e);
-			assertEquals("Incorrect computed index", i, queue.indexOf(e));
-			if (!seen.add(e)) {
-				fail("Unique index did not give unique element");
-			}
-			if (last != null && comp.compare(last, e) > 0) {
-				fail("Costs should be monotonic");
-			}
-			last = e;
-		}
-		for (int i = queue.size(); i < queue.size() * 2; i++) {
-			try {
-				queue.get(i);
-				fail();
-			}
-			catch (IndexOutOfBoundsException e) {
-				// pass
-			}
-		}
-		for (int i = -queue.size(); i < 0; i++) {
-			try {
-				queue.get(i);
-				fail();
-			}
-			catch (IndexOutOfBoundsException e) {
-				// pass
-			}
-		}
-	}
-
-	@Test
-	public void testAddRandomly() {
-		final int COUNT = 1000;
-		final int ROUNDS = 10;
-		Random rand = new Random();
-		CostComparator comp = new CostComparator();
-		DynamicSortedTreeSet<TestElem> queue = new DynamicSortedTreeSet<>(comp);
-		for (int r = 0; r < ROUNDS; r++) {
-			for (int i = 0; i < COUNT; i++) {
-				queue.add(new TestElem("Element" + i, rand.nextInt(50)));
-			}
-			checkConsistent(queue, comp);
-			queue.clear();
-		}
-	}
-
-	@Test
-	public void testRemoveRandomly() {
-		final int COUNT = 100;
-		Random rand = new Random();
-		CostComparator comp = new CostComparator();
-		DynamicSortedTreeSet<TestElem> queue = new DynamicSortedTreeSet<>(comp);
-		HashSet<TestElem> all = new HashSet<>();
-		for (int i = 0; i < COUNT; i++) {
-			TestElem e = new TestElem("Element" + i, rand.nextInt(50));
-			queue.add(e);
-			all.add(e);
-		}
-		checkConsistent(queue, comp);
-
-		TestElem[] shuffled = all.toArray(new TestElem[all.size()]);
-		for (int i = 0; i < shuffled.length; i++) {
-			ArrayUtils.swap(shuffled, i, i + rand.nextInt(shuffled.length - i));
-		}
-		for (TestElem e : shuffled) {
-			queue.remove(e);
-			checkConsistent(queue, comp);
-		}
-		assertTrue(queue.isEmpty());
-		assertTrue(queue.size() == 0);
-	}
-
-	@Test
-	public void testUpdateRandomly() {
-		final int COUNT = 100;
-		Random rand = new Random();
-		CostComparator comp = new CostComparator();
-		DynamicSortedTreeSet<TestElem> queue = new DynamicSortedTreeSet<>(comp);
-		for (int i = 0; i < COUNT; i++) {
-			queue.add(new TestElem("Element" + i, rand.nextInt(50)));
-		}
-		checkConsistent(queue, comp);
-
-		for (int i = 0; i < COUNT; i++) {
-			TestElem e = queue.get(rand.nextInt(queue.size()));
-			int oldCost = e.cost;
-			if (rand.nextInt(2) == 0) {
-				e.cost = rand.nextInt(50);
-			}
-			boolean result = queue.update(e);
-			if (oldCost == e.cost) {
-				assertEquals(false, result);
-			}
-			// NOTE: A different cost does not necessarily promote the updated element
-			checkConsistent(queue, comp);
-		}
-	}
-}

Ghidra/Framework/Graph/src/main/java/ghidra/graph/GDirectedGraph.java

@@ -20,17 +20,18 @@ import java.util.*;
 /**
  * A directed graph
  * 
- * Unlike {@link GImplicitDirectedGraph}, this graph is constructed explicitly in memory. Edges and
+ * Unlike {@link GImplicitDirectedGraph}, this graph is constructed explicitly
- * vertices are added and removed like any other collection, and these elements represent the
+ * in memory. Edges and vertices are added and removed like any other
- * entirety of the graph at any given time.
+ * collection, and these elements represent the entirety of the graph at any
+ * given time.
  * 
  * @param <V> the type of vertices
  * @param <E> the type of edges
  */
-public interface GDirectedGraph<V, E extends GEdge<V>> {
+public interface GDirectedGraph<V, E extends GEdge<V>> extends GImplicitDirectedGraph<V, E> {
-
 	/**
 	 * Add a vertex
+	 * 
 	 * @param v the vertex
 	 * @return true if the add was successful, false otherwise
 	 */
@@ -38,6 +39,7 @@ public interface GDirectedGraph<V, E extends GEdge<V>> {
 
 	/**
 	 * Remove a vertex
+	 * 
 	 * @param v the vertex
 	 * @return true
 	 */
@@ -52,12 +54,14 @@ public interface GDirectedGraph<V, E extends GEdge<V>> {
 
 	/**
 	 * Add an edge
+	 * 
 	 * @param e the edge
 	 */
 	public void addEdge(E e);
 
 	/**
 	 * Removes an edge
+	 * 
 	 * @param e the edge
 	 * @return true if the graph contained the given edge
 	 */
@@ -81,18 +85,21 @@ public interface GDirectedGraph<V, E extends GEdge<V>> {
 
 	/**
 	 * Retrieve all the vertices
+	 * 
 	 * @return the vertices
 	 */
 	public Collection<V> getVertices();
 
 	/**
 	 * Retrieve all the edges
+	 * 
 	 * @return the edges
 	 */
 	public Collection<E> getEdges();
 
 	/**
 	 * Test if the graph contains a given vertex
+	 * 
 	 * @param v the vertex
 	 * @return true if the vertex is in the graph, or false
 	 */
@@ -100,6 +107,7 @@ public interface GDirectedGraph<V, E extends GEdge<V>> {
 
 	/**
 	 * Test if the graph contains a given edge
+	 * 
 	 * @param e the ege
 	 * @return true if the edge is in the graph, or false
 	 */
@@ -107,6 +115,7 @@ public interface GDirectedGraph<V, E extends GEdge<V>> {
 
 	/**
 	 * Test if the graph contains an edge from one given vertex to another
+	 * 
 	 * @param from the source vertex
 	 * @param to the destination vertex
 	 * @return true if such an edge exists, or false
@@ -115,18 +124,21 @@ public interface GDirectedGraph<V, E extends GEdge<V>> {
 
 	/**
 	 * Test if the graph is empty, i.e., contains no vertices or edges
+	 * 
 	 * @return true if the graph is empty, or false
 	 */
 	public boolean isEmpty();
 
 	/**
 	 * Count the number of vertices in the graph
+	 * 
 	 * @return the count
 	 */
 	public int getVertexCount();
 
 	/**
 	 * Count the number of edges in the graph
+	 * 
 	 * @return the count
 	 */
 	public int getEdgeCount();
@@ -137,6 +149,7 @@ public interface GDirectedGraph<V, E extends GEdge<V>> {
 	 * @param v the destination vertex
 	 * @return the in-edges to the given vertex
 	 */
+	@Override
 	public Collection<E> getInEdges(V v);
 
 	/**
@@ -145,6 +158,7 @@ public interface GDirectedGraph<V, E extends GEdge<V>> {
 	 * @param v the source vertex
 	 * @return the out-edges from the given vertex
 	 */
+	@Override
 	public Collection<E> getOutEdges(V v);
 
 	/**
@@ -163,11 +177,13 @@ public interface GDirectedGraph<V, E extends GEdge<V>> {
 	/**
 	 * Compute a vertex's predecessors
 	 * 
-	 * <P>The default implementation computes this from the in-edges
+	 * <P>
+	 * The default implementation computes this from the in-edges
 	 * 
 	 * @param v the destination vertex
 	 * @return the predecessors
 	 */
+	@Override
 	public default Collection<V> getPredecessors(V v) {
 		Set<V> result = new LinkedHashSet<>();
 		for (E edge : getInEdges(v)) {
@@ -179,11 +195,13 @@ public interface GDirectedGraph<V, E extends GEdge<V>> {
 	/**
 	 * Compute a vertex's successors
 	 * 
-	 * <P>The default implementation compute this from the out-edges
+	 * <P>
+	 * The default implementation compute this from the out-edges
 	 * 
 	 * @param v the source vertex
 	 * @return the successors
 	 */
+	@Override
 	public default Collection<V> getSuccessors(V v) {
 		Set<V> result = new LinkedHashSet<>();
 		for (E edge : getOutEdges(v)) {
@@ -195,16 +213,19 @@ public interface GDirectedGraph<V, E extends GEdge<V>> {
 	/**
 	 * Copy this graph.
 	 * 
-	 * <P>Note: the vertices and edges in the copy may be the same instances in the new graph
+	 * <P>
-	 * and not themselves copies.
+	 * Note: the vertices and edges in the copy may be the same instances in the
+	 * new graph and not themselves copies.
 	 * 
 	 * @return the new copy
 	 */
+	@Override
 	public GDirectedGraph<V, E> copy();
 
 	/**
-	 * Creates a new instance of this graph with no vertices or edges.  This is useful when 
+	 * Creates a new instance of this graph with no vertices or edges. This is
-	 * you wish to build a new graph using the same type as this graph.
+	 * useful when you wish to build a new graph using the same type as this
+	 * graph.
 	 * 
 	 * @return the new copy
 	 */

Ghidra/Framework/Graph/src/main/java/ghidra/graph/algo/DijkstraShortestPathsAlgorithm.java

@@ -15,33 +15,24 @@
  */
 package ghidra.graph.algo;
 
-import java.util.Collection;
+import java.util.*;
-import java.util.Collections;
-import java.util.Deque;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.LinkedHashMap;
-import java.util.LinkedList;
-import java.util.Map;
 import java.util.Map.Entry;
-import java.util.Set;
 
 import org.apache.commons.collections4.map.LazyMap;
 
 import generic.util.DequePush;
-import ghidra.generic.util.datastruct.DynamicValueSortedTreeMap;
+import ghidra.generic.util.datastruct.TreeValueSortedMap;
-import ghidra.graph.GEdge;
+import ghidra.generic.util.datastruct.ValueSortedMap;
-import ghidra.graph.GEdgeWeightMetric;
+import ghidra.graph.*;
-import ghidra.graph.GImplicitDirectedGraph;
-import ghidra.graph.GWeightedEdge;
 
 /**
  * Dijkstra's shortest-path algorithm
  * 
+ * <p>
  * This implementation computes the shortest paths between two vertices using Dijkstra's
- * single-source shortest path finding algorithm. Any time a new source is given, it explores
+ * single-source shortest path finding algorithm. Any time a new source is given, it explores all
- * all destinations in the graph up to a maximum distance from the source. Thus, this
+ * destinations in the graph up to a maximum distance from the source. Thus, this implementation is
- * implementation is best applied when many queries are anticipated from relatively few sources.
+ * best applied when many queries are anticipated from relatively few sources.
  *
  * @param <V> the type of vertices
  * @param <E> the type of edges
@@ -56,6 +47,7 @@ public class DijkstraShortestPathsAlgorithm<V, E extends GEdge<V>> {
 	/**
 	 * Use Dijkstra's algorithm on the given graph
 	 * 
+	 * <p>
 	 * This constructor assumes the graph's edges are {@link GWeightedEdge}s. If not, you will
 	 * likely encounter a {@link ClassCastException}.
 	 * 
@@ -70,6 +62,7 @@ public class DijkstraShortestPathsAlgorithm<V, E extends GEdge<V>> {
 	/**
 	 * Use Dijkstra's algorithm on the given graph with the given maximum distance
 	 * 
+	 * <p>
 	 * This constructor assumes the graph's edges are {@link GWeightedEdge}s. If not, you will
 	 * likely encounter a {@link ClassCastException}.
 	 * 
@@ -96,8 +89,8 @@ public class DijkstraShortestPathsAlgorithm<V, E extends GEdge<V>> {
 	}
 
 	/**
-	 * Use Dijstra's algorithm on the given graph with the given maximum distance and a custom
+	 * Use Dijstra's algorithm on the given graph with the given maximum distance and a custom edge
-	 * edge weight metric
+	 * weight metric
 	 * 
 	 * @param graph the graph
 	 * @param maxDistance the maximum distance, or null for no maximum
@@ -124,6 +117,7 @@ public class DijkstraShortestPathsAlgorithm<V, E extends GEdge<V>> {
 	/**
 	 * Compute the shortest paths from the given source to the given destination
 	 * 
+	 * <p>
 	 * This implementation differs from typical implementations in that paths tied for the shortest
 	 * distance are all returned. Others tend to choose one arbitrarily.
 	 * 
@@ -139,16 +133,17 @@ public class DijkstraShortestPathsAlgorithm<V, E extends GEdge<V>> {
 	 * A class representing all optimal paths from a given source to every other (reachable) vertex
 	 * in the graph
 	 * 
+	 * <p>
 	 * This is the workhorse of path computation, and implements Dijkstra's Shortest Path algorithm
 	 * from one source to all destinations. We considered using JUNG to store the graph and compute
-	 * the paths, but we could not, because we would like to find all paths having the
+	 * the paths, but we could not, because we would like to find all paths having the optimal
-	 * optimal distance. If there are ties, JUNG's implementation chooses one arbitrarily; we would
+	 * distance. If there are ties, JUNG's implementation chooses one arbitrarily; we would like all
-	 * like all tied paths.
+	 * tied paths.
 	 */
 	protected class OneSourceToAll {
 		// For explored, but unvisited nodes
-		protected final DynamicValueSortedTreeMap<V, Double> queueByDistance =
+		protected final ValueSortedMap<V, Double> queueByDistance =
-			new DynamicValueSortedTreeMap<>();
+			TreeValueSortedMap.createWithNaturalOrder();
 		// For visited nodes, i.e., their optimal distance is known
 		protected final Map<V, Double> visitedDistance = new LinkedHashMap<>();
 		protected final Map<V, Set<E>> bestIns =
@@ -159,6 +154,7 @@ public class DijkstraShortestPathsAlgorithm<V, E extends GEdge<V>> {
 		/**
 		 * Compute the shortest paths from a given vertex to all other reachable vertices in the
 		 * graph
+		 * 
 		 * @param src the source (seed) vertex
 		 */
 		protected OneSourceToAll(V src) {
@@ -169,6 +165,7 @@ public class DijkstraShortestPathsAlgorithm<V, E extends GEdge<V>> {
 
 		/**
 		 * Recover the shortest paths from the source to the given destination, if it is reachable
+		 * 
 		 * @param dst the destination
 		 * @return a collection of the shortest paths from source to destination, or the empty set
 		 */
@@ -179,10 +176,11 @@ public class DijkstraShortestPathsAlgorithm<V, E extends GEdge<V>> {
 		}
 
 		/**
-		 * Add the shortest paths from the source to the given destination into the given
+		 * Add the shortest paths from the source to the given destination into the given collection
-		 * collection
 		 * 
+		 * <p>
 		 * This is used internally to recover the shortest paths
+		 * 
 		 * @param paths a place to store the recovered paths
 		 * @param dst the destination
 		 */
@@ -191,12 +189,14 @@ public class DijkstraShortestPathsAlgorithm<V, E extends GEdge<V>> {
 		}
 
 		/**
-		 * Add the shortest paths from source to a given intermediate, continuing along a given
+		 * Add the shortest paths from source to a given intermediate, continuing along a given path
-		 * path to the final destination, into the given collection
+		 * to the final destination, into the given collection
 		 * 
+		 * <p>
 		 * This is a recursive method for constructing the shortest paths overall. Assuming the
 		 * given path from intermediate to final destination is the shortest, we can show by
 		 * induction, the computed paths from source to destination are the shortest.
+		 * 
 		 * @param paths a place to store the recovered paths
 		 * @param prev the intermediate destination
 		 * @param soFar a (shortest) path from intermediate to final destination
@@ -208,10 +208,11 @@ public class DijkstraShortestPathsAlgorithm<V, E extends GEdge<V>> {
 				paths.add(new LinkedList<>(soFar));
 			}
 			else { // inductive case:
-				/* Dijkstra has computed the best inbound edges. Consider each as a prefix to the
+				/*
-				 * current path from intermediate to final destination. Since we assume that path
+				 * Dijkstra has computed the best inbound edges. Consider each as a prefix to the
-				 * is an optimal path, and we prefix an optimal inbound edge, the prefixed path is
+				 * current path from intermediate to final destination. Since we assume that path is
-				 * an optimal path from a new intermediate source (inbound neighbor) to the final
+				 * an optimal path, and we prefix an optimal inbound edge, the prefixed path is an
+				 * optimal path from a new intermediate source (inbound neighbor) to the final
 				 * destination. So, just recurse, using the new intermediates.
 				 */
 				for (E e : bestIns.get(prev)) {
@@ -226,14 +227,17 @@ public class DijkstraShortestPathsAlgorithm<V, E extends GEdge<V>> {
 		/**
 		 * Update the record for the given destination with a new offer of shortest distance
 		 * 
-		 * If either the record doesn't exist yet, or the new offer beats the current best, then
+		 * <p>
-		 * a new record is created and replaces the current record. If present, the list of best
+		 * If either the record doesn't exist yet, or the new offer beats the current best, then a
+		 * new record is created and replaces the current record. If present, the list of best
 		 * inbound edges is cleared -- because they all correspond to a distance that has just been
 		 * beat. The node is also added and/or moved forward in the queue of unvisited vertices.
 		 * 
-		 * If the record exists, and the new offer ties the current offer, nothing happens, but
+		 * <p>
-		 * the method still returns true, since the corresponding inbound edge could be optimal.
+		 * If the record exists, and the new offer ties the current offer, nothing happens, but the
+		 * method still returns true, since the corresponding inbound edge could be optimal.
 		 * 
+		 * <p>
 		 * If the record's current best beats the offer, nothing happens, and the method returns
 		 * false, indicating the inbound edge is definitely not optimal.
 		 * 
@@ -278,6 +282,7 @@ public class DijkstraShortestPathsAlgorithm<V, E extends GEdge<V>> {
 
 		/**
 		 * Perform one iteration of Dijskstra's path finding algorithm
+		 * 
 		 * @param from the vertex to visit for this iteration
 		 */
 		protected void fillStep(V from, double dist) {

Ghidra/Framework/Graph/src/main/java/ghidra/graph/algo/SorterException.java

@@ -0,0 +1,29 @@
+/* ###
+ * IP: GHIDRA
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package ghidra.graph.algo;
+
+/**
+ * Occurs when a graph cannot be sorted
+ */
+public class SorterException extends Exception {
+	public SorterException(String desc, Object v1, Object v2) {
+		super(desc + ": " + v1 + " ?? " + v2);
+	}
+
+	public SorterException(String desc, Iterable<?> vs) {
+		super(desc + ": " + vs);
+	}
+}

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/DatabaseObject.java

@@ -21,10 +21,10 @@ import db.Record;
 import ghidra.util.Lock;
 
 /**
- * Base class for an cached object in the database. Database objects have keys.  They are marked
+ * Base class for an cached object in the database. Database objects have keys. They are marked as
- * as invalid when a database cache is cleared and can be revived on a refresh as long
+ * invalid when a database cache is cleared and can be revived on a refresh as long as they haven't
- * as they haven't been deleted.  Instantiating an object will cause it to be added
+ * been deleted. Instantiating an object will cause it to be added immediately to the associated
- * immediately to the associated cache.
+ * cache.
  */
 abstract public class DatabaseObject {
 
@@ -36,6 +36,7 @@ abstract public class DatabaseObject {
 
 	/**
 	 * Constructs a new DatabaseObject and adds it to the specified cache.
+	 * 
 	 * @param cache to be used for this object or null if object will not be cached
 	 * @param key database key to uniquely identify this object
 	 */
@@ -64,10 +65,11 @@ abstract public class DatabaseObject {
 	}
 
 	/**
-	 * Returns true if this object has been deleted.  Note: once an object has been deleted,
+	 * Returns true if this object has been deleted. Note: once an object has been deleted, it will
-	 * it will never be "refreshed".  For example, if an object is ever deleted and is
+	 * never be "refreshed". For example, if an object is ever deleted and is resurrected via an
-	 * resurrected via an "undo", you will have get a fresh instance of the object.
+	 * "undo", you will have get a fresh instance of the object.
-	 * @return  true if this object has been deleted.
+	 * 
+	 * @return true if this object has been deleted.
 	 */
 	public boolean isDeleted() {
 		return deleted;
@@ -75,8 +77,8 @@ abstract public class DatabaseObject {
 
 	/**
 	 *
-	 * Invalidate this object. This does not necessarily mean that this object can
+	 * Invalidate this object. This does not necessarily mean that this object can never be used
-	 * never be used again.  If the object can refresh itself, it may still be useable.
+	 * again. If the object can refresh itself, it may still be useable.
 	 */
 	public void setInvalid() {
 		invalidateCount = getCurrentValidationCount() - 1;
@@ -99,8 +101,9 @@ abstract public class DatabaseObject {
 	}
 
 	/**
-	 * Returns true if object is currently invalid.  Calling checkIsValid may
+	 * Returns true if object is currently invalid. Calling checkIsValid may successfully refresh
-	 * successfully refresh object making it valid.
+	 * object making it valid.
+	 * 
 	 * @see #checkIsValid()
 	 */
 	public boolean isInvalid() {
@@ -108,8 +111,8 @@ abstract public class DatabaseObject {
 	}
 
 	/**
-	 * Checks if this object has been deleted, in which case any use of the object is
+	 * Checks if this object has been deleted, in which case any use of the object is not allowed.
-	 * not allowed.
+	 * 
 	 * @throws ConcurrentModificationException if the object has been deleted from the database.
 	 */
 	public void checkDeleted() {
@@ -119,8 +122,9 @@ abstract public class DatabaseObject {
 	}
 
 	/**
-	 * Check whether this object is still valid. If the object is invalid, the object will
+	 * Check whether this object is still valid. If the object is invalid, the object will attempt
-	 * attempt to refresh itself.  If the refresh fails, the object will be marked as deleted.
+	 * to refresh itself. If the refresh fails, the object will be marked as deleted.
+	 * 
 	 * @return true if the object is valid.
 	 */
 	public boolean checkIsValid() {
@@ -128,10 +132,11 @@ abstract public class DatabaseObject {
 	}
 
 	/**
-	 * Check whether this object is still valid. If the object is invalid, the object will
+	 * Check whether this object is still valid. If the object is invalid, the object will attempt
-	 * attempt to refresh itself using the specified record.  If the refresh fails, the
+	 * to refresh itself using the specified record. If the refresh fails, the object will be marked
-	 * object will be marked as deleted and removed from cache.  If this object is already
+	 * as deleted and removed from cache. If this object is already marked as deleted, the record
-	 * marked as deleted, the record can not be used to refresh the object.
+	 * can not be used to refresh the object.
+	 * 
 	 * @param record optional record which may be used to refresh invalid object
 	 * @return true if the object is valid.
 	 */
@@ -153,8 +158,9 @@ abstract public class DatabaseObject {
 	}
 
 	/**
-	 * This method provides a cheap (lock free) way to test if an object is valid.  If
+	 * This method provides a cheap (lock free) way to test if an object is valid. If this object is
-	 * this object is invalid, then the lock will be used to refresh as needed.
+	 * invalid, then the lock will be used to refresh as needed.
+	 * 
 	 * @param lock the lock that will be used if the object needs to be refreshed.
 	 * @return true if object is valid, else false
 	 */
@@ -173,24 +179,25 @@ abstract public class DatabaseObject {
 
 	/**
 	 * Tells the object to refresh its state from the database.
-	 * @return true if the object was able to refresh itself. Return false if the object
+	 * 
-	 * was deleted. Objects that extend this class must implement a refresh method.  If
+	 * @return true if the object was able to refresh itself. Return false if the object was
-	 * an object can never refresh itself, then it should always return false.
+	 *         deleted. Objects that extend this class must implement a refresh method. If an object
+	 *         can never refresh itself, then it should always return false.
 	 */
 	protected abstract boolean refresh();
 
 	/**
-	 * Tells the object to refresh its state from the database using the specified
+	 * Tells the object to refresh its state from the database using the specified record if not
-	 * record if not null.  NOTE: The default implementation ignores the record
+	 * null. NOTE: The default implementation ignores the record and invokes refresh().
-	 * and invokes refresh().  Implementations of this method must take care if
+	 * Implementations of this method must take care if multiple database tables are used since the
-	 * multiple database tables are used since the record supplied could correspond
+	 * record supplied could correspond to another object. In some cases it may be best not to
-	 * to another object.  In some cases it may be best not to override this method
+	 * override this method or ignore the record provided.
-	 * or ignore the record provided.
+	 * 
 	 * @param record valid record associated with object's key (optional, may be null to force
-	 * record lookup or other refresh technique)
+	 *            record lookup or other refresh technique)
-	 * @return true if the object was able to refresh itself. Return false if record is null
+	 * @return true if the object was able to refresh itself. Return false if record is null and
-	 * and object was deleted. Objects that extend this class must implement a refresh method.
+	 *         object was deleted. Objects that extend this class must implement a refresh method.
-	 * If an object can never refresh itself, then it should always return false.
+	 *         If an object can never refresh itself, then it should always return false.
 	 */
 	protected boolean refresh(Record record) {
 		return refresh();

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/DataTypeUtilities.java

@@ -141,11 +141,13 @@ public class DataTypeUtilities {
 
 	/**
 	 * Check to see if the second data type is the same as the first data type or is part of it.
-	 * <br>Note: pointers to the second data type are references and therefore are not considered
+	 * <br>
-	 * to be part of the first and won't cause true to be returned. If you pass a pointer to this
+	 * Note: pointers to the second data type are references and therefore are not considered to be
-	 * method for the first or second parameter, it will return false.
+	 * part of the first and won't cause true to be returned. If you pass a pointer to this method
-	 * @param firstDataType the data type whose components or base type should be checked to see
+	 * for the first or second parameter, it will return false.
-	 * if the second data type is part of it.
+	 * 
+	 * @param firstDataType the data type whose components or base type should be checked to see if
+	 *            the second data type is part of it.
 	 * @param secondDataType the data type to be checked for in the first data type.
 	 * @return true if the second data type is the first data type or is part of it.
 	 */
@@ -184,7 +186,8 @@ public class DataTypeUtilities {
 
 	/**
 	 * Returns true if the two dataTypes have the same sourceArchive and the same UniversalID
-	 * @param dataType1 first data type 
+	 * 
+	 * @param dataType1 first data type
 	 * @param dataType2 second data type
 	 * @return true if types correspond to the same type from a source archive
 	 */
@@ -207,13 +210,14 @@ public class DataTypeUtilities {
 	}
 
 	/**
-	 * Returns true if the two dataTypes have the same sourceArchive and the same UniversalID OR
+	 * Returns true if the two dataTypes have the same sourceArchive and the same UniversalID OR are
-	 * are equivalent
+	 * equivalent
-	 * @param dataType1 first data type (if invoked by DB object or manager, this argument
+	 * 
-	 * must correspond to the DataTypeDB). 
+	 * @param dataType1 first data type (if invoked by DB object or manager, this argument must
+	 *            correspond to the DataTypeDB).
 	 * @param dataType2 second data type
-	 * @return true if types correspond to the same type from a source archive 
+	 * @return true if types correspond to the same type from a source archive or they are
-	 * or they are equivelent, otherwise false
+	 *         equivelent, otherwise false
 	 */
 	public static boolean isSameOrEquivalentDataType(DataType dataType1, DataType dataType2) {
 		// if they contain datatypes that have same ids, then they represent the same dataType
@@ -226,6 +230,7 @@ public class DataTypeUtilities {
 
 	/**
 	 * Get the name of a data type with all conflict naming patterns removed.
+	 * 
 	 * @param dataType data type
 	 * @param includeCategoryPath if true the category path will be included with its
 	 * @return name with without conflict patterns
@@ -238,6 +243,7 @@ public class DataTypeUtilities {
 	/**
 	 * Compares two data type name strings to determine if they are equivalent names, ignoring
 	 * conflict patterns present.
+	 * 
 	 * @param name1 the first name
 	 * @param name2 the second name
 	 * @return true if the names are equivalent when conflict suffixes are ignored.
@@ -249,9 +255,8 @@ public class DataTypeUtilities {
 	}
 
 	/**
-	 * Get the base data type for the specified data type stripping
+	 * Get the base data type for the specified data type stripping away pointers and arrays only. A
-	 * away pointers and arrays only.  A null will be returned for a
+	 * null will be returned for a default pointer.
-	 * default pointer.
 	 *
 	 * @param dt the data type whose base data type is to be determined.
 	 * @return the base data type.
@@ -333,8 +338,9 @@ public class DataTypeUtilities {
 	}
 
 	/**
-	 * Create a data type category path derived from the specified namespace and rooted from
+	 * Create a data type category path derived from the specified namespace and rooted from the
-	 * the specified baseCategory
+	 * specified baseCategory
+	 * 
 	 * @param baseCategory category path from which to root the namespace-base path
 	 * @param namespace the namespace
 	 * @return namespace derived category path
@@ -343,7 +349,7 @@ public class DataTypeUtilities {
 			Namespace namespace) {
 		Namespace ns = namespace;
 		String path = "";
-		while (!(ns instanceof GlobalNamespace) && !(ns instanceof Library)) {
+		while (!ns.isGlobal() && !(ns instanceof Library)) {
 			if (path.length() != 0) {
 				path = "/" + path;
 			}
@@ -361,11 +367,11 @@ public class DataTypeUtilities {
 	}
 
 	/**
-	 * Attempt to find the data type whose dtName and specified namespace match a
+	 * Attempt to find the data type whose dtName and specified namespace match a stored data type
-	 * stored data type within the specified dataTypeManager.  The best match
+	 * within the specified dataTypeManager. The best match will be returned. The namespace will be
-	 * will be returned.  The namespace will be used in checking data type parent categories,
+	 * used in checking data type parent categories, however if no type corresponds to the namespace
-	 * however if no type corresponds to the namespace another type whose name
+	 * another type whose name matches may be returned.
-	 * matches may be returned.
+	 * 
 	 * @param dataTypeManager data type manager
 	 * @param namespace namespace associated with dtName (null indicates no namespace constraint)
 	 * @param dtName name of data type
@@ -379,15 +385,15 @@ public class DataTypeUtilities {
 	}
 
 	/**
-	 * Attempt to find the data type whose dtNameWithNamespace match a
+	 * Attempt to find the data type whose dtNameWithNamespace match a stored data type within the
-	 * stored data type within the specified dataTypeManager.  The best match
+	 * specified dataTypeManager. The best match will be returned. The namespace will be used in
-	 * will be returned.  The namespace will be used in checking data type parent categories,
+	 * checking data type parent categories, however if no type corresponds to the namespace another
-	 * however if no type corresponds to the namespace another type whose name
+	 * type whose name matches may be returned. NOTE: name parsing assumes :: delimiter and can be
-	 * matches may be returned.
+	 * thrown off if name include template information which could contain namespaces.
-	 * NOTE: name parsing assumes :: delimiter and can be thrown off if name include template
+	 * 
-	 * information which could contain namespaces.
 	 * @param dataTypeManager data type manager
-	 * @param dtNameWithNamespace name of data type qualified with namespace (e.g., ns1::ns2::dtname)
+	 * @param dtNameWithNamespace name of data type qualified with namespace (e.g.,
+	 *            ns1::ns2::dtname)
 	 * @param classConstraint optional data type interface constraint (e.g., Structure), or null
 	 * @return best matching data type
 	 */
@@ -403,6 +409,7 @@ public class DataTypeUtilities {
 
 	/**
 	 * Return the appropriate datatype for a given C primitive datatype name.
+	 * 
 	 * @param dataTypeName the datatype name (e.g. "unsigned int", "long long")
 	 * @return the appropriate datatype for a given C primitive datatype name.
 	 */
@@ -452,8 +459,8 @@ public class DataTypeUtilities {
 	}
 
 	/**
-	 * <code>NamespaceMatcher</code> is used to check data type categoryPath
+	 * <code>NamespaceMatcher</code> is used to check data type categoryPath for match against
-	 * for match against preferred namespace.
+	 * preferred namespace.
 	 */
 	private static interface NamespaceMatcher {
 		boolean isNamespaceCategoryMatch(DataType dataType);

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/data/StructureDB.java

@@ -685,10 +685,9 @@ class StructureDB extends CompositeDB implements Structure {
 	}
 
 	/**
-	 * Create copy of structure for target dtm (source archive information is
+	 * Create copy of structure for target dtm (source archive information is discarded). WARNING!
-	 * discarded). WARNING! copying unaligned structures which contain bitfields can
+	 * copying unaligned structures which contain bitfields can produce invalid results when
-	 * produce invalid results when switching endianess due to the differences in
+	 * switching endianess due to the differences in packing order.
-	 * packing order.
 	 * 
 	 * @param dtm target data type manager
 	 * @return cloned structure
@@ -703,16 +702,15 @@ class StructureDB extends CompositeDB implements Structure {
 	}
 
 	/**
-	 * Create cloned structure for target dtm preserving source archive information.
+	 * Create cloned structure for target dtm preserving source archive information. WARNING!
-	 * WARNING! cloning unaligned structures which contain bitfields can produce
+	 * cloning unaligned structures which contain bitfields can produce invalid results when
-	 * invalid results when switching endianess due to the differences in packing
+	 * switching endianess due to the differences in packing order.
-	 * order.
 	 * 
 	 * @param dtm target data type manager
 	 * @return cloned structure
 	 */
 	@Override
-	public DataType clone(DataTypeManager dtm) {
+	public Structure clone(DataTypeManager dtm) {
 		StructureDataType struct =
 			new StructureDataType(getCategoryPath(), getName(), getLength(), getUniversalID(),
 				getSourceArchive(), getLastChangeTime(), getLastChangeTimeInSourceArchive(), dtm);
@@ -793,13 +791,12 @@ class StructureDB extends CompositeDB implements Structure {
 	}
 
 	/**
-	 * Backup from specified ordinal to the first component which contains the
+	 * Backup from specified ordinal to the first component which contains the specified offset. For
-	 * specified offset. For normal components the specified ordinal will be
+	 * normal components the specified ordinal will be returned, however for bit-fields the ordinal
-	 * returned, however for bit-fields the ordinal of the first bit-field
+	 * of the first bit-field containing the specified offset will be returned.
-	 * containing the specified offset will be returned.
 	 * 
 	 * @param ordinal component ordinal
-	 * @param offset  offset within structure
+	 * @param offset offset within structure
 	 * @return index of first defined component containing specific offset.
 	 */
 	private int backupToFirstComponentContainingOffset(int index, int offset) {
@@ -819,13 +816,12 @@ class StructureDB extends CompositeDB implements Structure {
 	}
 
 	/**
-	 * Advance from specified ordinal to the last component which contains the
+	 * Advance from specified ordinal to the last component which contains the specified offset. For
-	 * specified offset. For normal components the specified ordinal will be
+	 * normal components the specified ordinal will be returned, however for bit-fields the ordinal
-	 * returned, however for bit-fields the ordinal of the last bit-field containing
+	 * of the last bit-field containing the specified offset will be returned.
-	 * the specified offset will be returned.
 	 * 
 	 * @param ordinal component ordinal
-	 * @param offset  offset within structure
+	 * @param offset offset within structure
 	 * @return index of last defined component containing specific offset.
 	 */
 	private int advanceToLastComponentContainingOffset(int index, int offset) {
@@ -1137,16 +1133,13 @@ class StructureDB extends CompositeDB implements Structure {
 	}
 
 	/**
-	 * Replaces the internal components of this structure with components of the
+	 * Replaces the internal components of this structure with components of the given structure.
-	 * given structure.
 	 * 
 	 * @param dataType the structure to get the component information from.
-	 * @throws IllegalArgumentException if any of the component data types are not
+	 * @throws IllegalArgumentException if any of the component data types are not allowed to
-	 *                                  allowed to replace a component in this
+	 *             replace a component in this composite data type. For example, suppose dt1
-	 *                                  composite data type. For example, suppose
+	 *             contains dt2. Therefore it is not valid to replace a dt2 component with dt1 since
-	 *                                  dt1 contains dt2. Therefore it is not valid
+	 *             this would cause a cyclic dependency.
-	 *                                  to replace a dt2 component with dt1 since
-	 *                                  this would cause a cyclic dependency.
 	 * @see ghidra.program.database.data.DataTypeDB#replaceWith(ghidra.program.model.data.DataType)
 	 */
 	@Override
@@ -1179,8 +1172,7 @@ class StructureDB extends CompositeDB implements Structure {
 	 * 
 	 * @param struct
 	 * @param notify
-	 * @return true if fully completed else false if pointer component post resolve
+	 * @return true if fully completed else false if pointer component post resolve required
-	 *         required
 	 * @throws DataTypeDependencyException
 	 * @throws IOException
 	 */
@@ -1489,9 +1481,8 @@ class StructureDB extends CompositeDB implements Structure {
 
 	/**
 	 *
-	 * @param definedComponentIndex the index of the defined component that is
+	 * @param definedComponentIndex the index of the defined component that is consuming the bytes.
-	 *                              consuming the bytes.
+	 * @param numBytes the number of undefined bytes to consume
-	 * @param numBytes              the number of undefined bytes to consume
 	 * @return the number of bytes actually consumed
 	 */
 	private int consumeBytesAfter(int definedComponentIndex, int numBytes) {
@@ -1554,14 +1545,14 @@ class StructureDB extends CompositeDB implements Structure {
 	}
 
 	/**
-	 * Replace the indicated component with a new component containing the specified
+	 * Replace the indicated component with a new component containing the specified data type.
-	 * data type. Flex-array component not handled.
+	 * Flex-array component not handled.
 	 * 
-	 * @param origDtc          the original data type component in this structure.
+	 * @param origDtc the original data type component in this structure.
 	 * @param resolvedDataType the data type of the new component
-	 * @param length           the length of the new component
+	 * @param length the length of the new component
-	 * @param name             the field name of the new component
+	 * @param name the field name of the new component
-	 * @param comment          the comment for the new component
+	 * @param comment the comment for the new component
 	 * @return the new component or null if the new component couldn't fit.
 	 */
 	private DataTypeComponent replaceComponent(DataTypeComponent origDtc, DataType resolvedDataType,
@@ -1638,9 +1629,8 @@ class StructureDB extends CompositeDB implements Structure {
 	}
 
 	/**
-	 * Gets the number of Undefined bytes beginning at the indicated component
+	 * Gets the number of Undefined bytes beginning at the indicated component ordinal. Undefined
-	 * ordinal. Undefined bytes that have a field name or comment specified are also
+	 * bytes that have a field name or comment specified are also included.
-	 * included.
 	 * 
 	 * @param ordinal the component ordinal to begin checking at.
 	 * @return the number of contiguous undefined bytes
@@ -1787,7 +1777,7 @@ class StructureDB extends CompositeDB implements Structure {
 					comp.setLength(len, true);
 					shiftOffsets(nextIndex, -bytesNeeded, 0);
 				}
-				else if (comp.getOrdinal() == getLastDefinedComponentIndex()) { 
+				else if (comp.getOrdinal() == getLastDefinedComponentIndex()) {
 					// we are the last defined component, grow structure
 					doGrowStructure(bytesNeeded - bytesAvailable);
 					comp.setLength(len, true);
@@ -1846,9 +1836,8 @@ class StructureDB extends CompositeDB implements Structure {
 	}
 
 	/**
-	 * <code>ComponentComparator</code> provides ability to compare two
+	 * <code>ComponentComparator</code> provides ability to compare two DataTypeComponent objects
-	 * DataTypeComponent objects based upon their ordinal. Intended to be used to
+	 * based upon their ordinal. Intended to be used to sort components based upon ordinal.
-	 * sort components based upon ordinal.
 	 */
 	private static class ComponentComparator implements Comparator<DataTypeComponent> {
 		@Override
@@ -1858,17 +1847,15 @@ class StructureDB extends CompositeDB implements Structure {
 	}
 
 	/**
-	 * Adjust the alignment, packing and padding of components within this structure
+	 * Adjust the alignment, packing and padding of components within this structure based upon the
-	 * based upon the current alignment and packing attributes for this structure.
+	 * current alignment and packing attributes for this structure. This method should be called to
-	 * This method should be called to basically fix up the layout of the internal
+	 * basically fix up the layout of the internal components of the structure after other code has
-	 * components of the structure after other code has changed the attributes of
+	 * changed the attributes of the structure. <BR>
-	 * the structure. <BR>
+	 * When switching between internally aligned and unaligned this method corrects the component
-	 * When switching between internally aligned and unaligned this method corrects
+	 * ordinal numbering also.
-	 * the component ordinal numbering also.
 	 * 
-	 * @param notify if true this method will do data type change notification when
+	 * @param notify if true this method will do data type change notification when it changes the
-	 *               it changes the layout of the components or when it changes the
+	 *            layout of the components or when it changes the overall size of the structure.
-	 *               overall size of the structure.
 	 * @return true if the structure was changed by this method.
 	 */
 	private boolean adjustComponents(boolean notify) {

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

@@ -13,73 +13,77 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package ghidra.app.plugin.assembler.sleigh.util;
+package ghidra.program.database.util;
 
 import ghidra.program.model.listing.Program;
 
 /**
  * A convenience context for transaction IDs on a Ghidra program database
  * 
- * This is meant to be used idiomatically, as in a try-with-resources block:
+ * <p>
+ * This is meant to be used as an idiom in a try-with-resources block:
  * 
  * <pre>
  * {@code
- * try (GhidraDBTransaction t = new GhidraDBTransaction(program, "Demo")) {
+ * try (ProgramTransaction t = ProgramTransaction.open(program, "Demo")) {
  *     program.getMemory().....
  *     t.commit();
  * }
  * }
  * </pre>
  * 
- * This idiom is very useful if there is complex logic in your transaction, it's very easy to
+ * <p>
- * forget to close the transaction, especially if an error occurs, leaving the database in an open
+ * This idiom is very useful if there is complex logic in your transaction, it's very easy to forget
- * transaction indefinitely. Try try-with-resources block will ensure that the transaction is
+ * to close the transaction, especially if an error occurs, leaving the database in an open
- * closed in all circumstances. Note, however, that in order for the transaction to be committed,
+ * transaction indefinitely. The try-with-resources block will ensure that the transaction is closed
- * you must call {@link #commit()}.
+ * in all circumstances. Note, however, that in order for the transaction to be committed, you must
+ * call {@link #commit()}.
  * 
+ * <p>
  * Any exceptions within the block will cause {@code t.commit()} to be skipped, thus aborting the
  * transaction.
  */
-public class GhidraDBTransaction implements AutoCloseable {
+public class ProgramTransaction implements AutoCloseable {
 	protected Program program;
 	protected int tid;
-	protected boolean open;
+	protected boolean commit = false;
 
 	/**
 	 * Start a transaction on the given program with the given description
+	 * 
 	 * @param program the program to modify
 	 * @param description a description of the transaction
 	 */
-	public GhidraDBTransaction(Program program, String description) {
+	public static ProgramTransaction open(Program program, String description) {
+		int tid = program.startTransaction(description);
+		return new ProgramTransaction(program, tid);
+	}
+
+	private ProgramTransaction(Program program, int tid) {
 		this.program = program;
-		this.tid = program.startTransaction(description);
+		this.tid = tid;
-		this.open = true;
 	}
 
 	/**
 	 * Finish the transaction
 	 * 
+	 * <p>
 	 * If this is called before {@link #commit()}, then the transaction is aborted. This is called
 	 * automatically at the close of a try-with-resources block.
 	 */
 	@Override
 	public void close() {
-		if (open) {
+		program.endTransaction(tid, commit);
-			program.endTransaction(tid, false);
-			open = false;
-		}
 	}
 
 	/**
 	 * Finish the transaction, and commit
 	 * 
-	 * This MUST be called in order to commit the transaction. The transaction is immediately
+	 * <p>
-	 * closed, and any further modifications to the database will likely result in an error.
+	 * This MUST be called in order to commit the transaction. The transaction is not committed
+	 * until the close of the try-with-resources block.
 	 */
 	public void commit() {
-		if (open) {
+		commit = true;
-			program.endTransaction(tid, true);
-			open = false;
-		}
 	}
 }

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/AbstractComplexDataType.java

@@ -27,7 +27,48 @@ import ghidra.program.model.mem.WrappedMemBuffer;
  */
 public abstract class AbstractComplexDataType extends BuiltIn {
 
-	private final static long serialVersionUID = 1;
+	protected static AbstractComplexDataType getDefaultComplexDataType(int size) {
+		if (size == 8) {
+			return Complex8DataType.dataType;
+		}
+		if (size == 16) {
+			return Complex16DataType.dataType;
+		}
+		if (size == 32) {
+			return Complex32DataType.dataType;
+		}
+		return null;
+	}
+
+	public static DataType getComplexDataType(int size, DataTypeManager dtm) {
+		if (size < 1) {
+			return DefaultDataType.dataType;
+		}
+		if (size % 2 != 0) {
+			return Undefined.getUndefinedDataType(size);
+		}
+		int floatSize = size / 2;
+		if (dtm != null) {
+			DataOrganization dataOrganization = dtm.getDataOrganization();
+			if (dataOrganization != null) {
+				if (floatSize == dataOrganization.getFloatSize()) {
+					return FloatComplexDataType.dataType.clone(dtm);
+				}
+				if (floatSize == dataOrganization.getDoubleSize()) {
+					return DoubleComplexDataType.dataType.clone(dtm);
+				}
+				if (floatSize == dataOrganization.getLongDoubleSize()) {
+					return LongDoubleComplexDataType.dataType.clone(dtm);
+				}
+			}
+		}
+		DataType dt = getDefaultComplexDataType(size);
+		if (dt == null) {
+			return Undefined.getUndefinedDataType(size);
+		}
+		return dt;
+	}
+
 	private final AbstractFloatDataType floatType;
 
 	public AbstractComplexDataType(String name, AbstractFloatDataType floats, DataTypeManager dtm) {

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/ArchiveType.java

@@ -15,13 +15,12 @@
  */
 package ghidra.program.model.data;
 
-
 public enum ArchiveType {
 	//@formatter:off
-	BUILT_IN, 
+	BUILT_IN,
-	FILE, 
+	FILE,
-	PROJECT, 
+	PROJECT,
-	PROGRAM, 
+	PROGRAM,
 	TEST;
 	//@formatter:on
 

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/Structure.java

@@ -20,22 +20,25 @@ import java.util.Comparator;
 /**
  * The structure interface.
  * <p>
- * NOTE: Structures containing only a flexible array will report a length of 1
+ * NOTE: Structures containing only a flexible array will report a length of 1 which will result in
- * which will result in improper code unit sizing since we are unable to support a 
+ * improper code unit sizing since we are unable to support a defined data of length 0.
- * defined data of length 0.
  * <p>
- * NOTE: The use of zero-length bitfields within unaligned structures is discouraged since
+ * NOTE: The use of zero-length bitfields within unaligned structures is discouraged since they have
- * they have no real affect and are easily misplaced.  Their use should be reserved for 
+ * no real affect and are easily misplaced. Their use should be reserved for aligned/packed
- * aligned/packed structures.
+ * structures.
  */
 public interface Structure extends Composite {
 
+	@Override
+	Structure clone(DataTypeManager dtm);
+
 	/**
-	 * Returns the component of this structure with the indicated ordinal.
+	 * Returns the component of this structure with the indicated ordinal. If the specified ordinal
-	 * If the specified ordinal equals {@link #getNumComponents()} the defined 
+	 * equals {@link #getNumComponents()} the defined flexible array component will be returned,
-	 * flexible array component will be returned, otherwise an out of bounds
+	 * otherwise an out of bounds exception will be thrown. Use of
-	 * exception will be thrown. Use of {@link #getFlexibleArrayComponent()} is preferred 
+	 * {@link #getFlexibleArrayComponent()} is preferred for obtaining this special trailing
-	 * for obtaining this special trailing component.
+	 * component.
+	 * 
 	 * @param ordinal the component's ordinal (zero based).
 	 * @return the data type component.
 	 * @throws ArrayIndexOutOfBoundsException if the ordinal is out of bounds
@@ -44,259 +47,263 @@ public interface Structure extends Composite {
 	public abstract DataTypeComponent getComponent(int ordinal);
 
 	/**
-	 * Gets the immediate child component that contains the byte
+	 * Gets the immediate child component that contains the byte at the given offset. If the
-	 * at the given offset.  If the specified offset corresponds to 
+	 * specified offset corresponds to a bit-field,the first bit-field component containing the
-	 * a bit-field,the first bit-field component containing the offset
+	 * offset will be returned.
-	 * will be returned.
+	 * 
 	 * @param offset the byte offset into this data type
 	 * @return the immediate child component.
 	 */
 	public abstract DataTypeComponent getComponentAt(int offset);
 
 	/**
-	 * Returns the primitive Data Type that is at this offset.  This is useful
+	 * Returns the primitive Data Type that is at this offset. This is useful for prototypes that
-	 * for prototypes that have components that are made up of other components
+	 * have components that are made up of other components If the specified offset corresponds to a
-	 * If the specified offset corresponds to 
+	 * bit-field,the BitFieldDataType of the first bit-field component containing the offset will be
-	 * a bit-field,the BitFieldDataType of the first bit-field component containing 
+	 * returned.
-	 * the offset will be returned.
+	 * 
 	 * @param offset the byte offset into this data type.
 	 * @return the primitive data type at the offset.
 	 */
 	public abstract DataTypeComponent getDataTypeAt(int offset);
 
 	/**
-	 * Inserts a new bitfield at the specified ordinal position in this structure. 
+	 * Inserts a new bitfield at the specified ordinal position in this structure. Within aligned
-	 * Within aligned structures the specified byteWidth and bitOffset will be 
+	 * structures the specified byteWidth and bitOffset will be ignored since packing will occur at
-	 * ignored since packing will occur at the specified ordinal position.
+	 * the specified ordinal position. The resulting component length and bitfield details will
-	 * The resulting component length and bitfield details will reflect the use
+	 * reflect the use of minimal storage sizing.
-	 * of minimal storage sizing.
 	 * <p>
-	 * For unaligned structures, a component shift will only occur if the bitfield placement 
+	 * For unaligned structures, a component shift will only occur if the bitfield placement
-	 * conflicts with another component.  If no conflict occurs, the bitfield will be placed 
+	 * conflicts with another component. If no conflict occurs, the bitfield will be placed at the
-	 * at the specified location consuming any DEFAULT components as needed.  When a conflict 
+	 * specified location consuming any DEFAULT components as needed. When a conflict does occur a
-	 * does occur a shift will be performed at the ordinal position based upon the specified 
+	 * shift will be performed at the ordinal position based upon the specified byteWidth. When
-	 * byteWidth.  When located onto existing bitfields they will be packed together 
+	 * located onto existing bitfields they will be packed together provided they do not conflict,
-	 * provided they do not conflict, otherwise the conflict rule above applies.
+	 * otherwise the conflict rule above applies.
 	 * <p>
-	 * Supported aligned packing starts with bit-0 (lsb) of the first byte for little-endian, and 
+	 * Supported aligned packing starts with bit-0 (lsb) of the first byte for little-endian, and
-	 * with bit-7 (msb) of the first byte for big-endian.  This is the default behavior for most
+	 * with bit-7 (msb) of the first byte for big-endian. This is the default behavior for most
-	 * compilers.  Insertion behavior may not work as expected if packing rules differ from this.
+	 * compilers. Insertion behavior may not work as expected if packing rules differ from this.
+	 * 
 	 * @param ordinal the ordinal where the new datatype is to be inserted.
-	 * @param byteWidth the storage allocation unit width which contains the bitfield.  Must be large
+	 * @param byteWidth the storage allocation unit width which contains the bitfield. Must be large
-	 * enough to contain the "effective bit size" and corresponding bitOffset.  The actual 
+	 *            enough to contain the "effective bit size" and corresponding bitOffset. The actual
-	 * component size used will be recomputed during insertion.
+	 *            component size used will be recomputed during insertion.
-	 * @param bitOffset corresponds to the bitfield left-shift amount with the storage 
+	 * @param bitOffset corresponds to the bitfield left-shift amount with the storage unit when
-	 * unit when viewed as big-endian.  The final offset may be reduced based upon
+	 *            viewed as big-endian. The final offset may be reduced based upon the minimal
-	 * the minimal storage size determined during insertion.  
+	 *            storage size determined during insertion.
 	 * @param baseDataType the bitfield base datatype (certain restrictions apply).
-	 * @param bitSize the declared bitfield size in bits.  The effective bit size may be
+	 * @param bitSize the declared bitfield size in bits. The effective bit size may be adjusted
-	 * adjusted based upon the specified baseDataType.
+	 *            based upon the specified baseDataType.
 	 * @param componentName the field name to associate with this component.
 	 * @param comment the comment to associate with this component.
-	 * @return the bitfield component created whose associated data type will
+	 * @return the bitfield component created whose associated data type will be BitFieldDataType.
-	 * be BitFieldDataType.
+	 * @throws InvalidDataTypeException if the specified baseDataType is not a valid base type for
-	 * @throws InvalidDataTypeException if the specified baseDataType is
+	 *             bitfields.
-	 * not a valid base type for bitfields.
+	 * @throws ArrayIndexOutOfBoundsException if ordinal is less than 0 or greater than the current
-	 * @throws ArrayIndexOutOfBoundsException if ordinal is less than 0 or greater than the 
+	 *             number of components.
-	 * current number of components.
 	 */
 	public DataTypeComponent insertBitField(int ordinal, int byteWidth, int bitOffset,
 			DataType baseDataType, int bitSize, String componentName, String comment)
 			throws InvalidDataTypeException, ArrayIndexOutOfBoundsException;
 
 	/**
-	 * Inserts a new bitfield at the specified location in this composite.  
+	 * Inserts a new bitfield at the specified location in this composite. This method is intended
-	 * This method is intended to be used with unaligned structures where 
+	 * to be used with unaligned structures where the bitfield will be precisely placed. Within an
-	 * the bitfield will be precisely placed.  Within an aligned structure the specified
+	 * aligned structure the specified byteOffset, byteWidth and bitOffset will be used to identify
-	 * byteOffset, byteWidth and bitOffset will be used to identify the appropriate ordinal
+	 * the appropriate ordinal but may not be preserved. The component length will be computed based
-	 * but may not be preserved. The component length will be computed 
+	 * upon the specified parameters and will be reduced from byteWidth to its minimal size for the
-	 * based upon the specified parameters and will be reduced from byteWidth to 
+	 * new component.
-	 * its minimal size for the new component.
 	 * <p>
-	 * For unaligned mode, a component shift will only occur if the bitfield placement 
+	 * For unaligned mode, a component shift will only occur if the bitfield placement conflicts
-	 * conflicts with another component.  If no conflict occurs, the bitfield will be placed 
+	 * with another component. If no conflict occurs, the bitfield will be placed at the specified
-	 * at the specified location consuming any DEFAULT components as needed.  When a conflict 
+	 * location consuming any DEFAULT components as needed. When a conflict does occur a shift will
-	 * does occur a shift will be performed at the point of conflict based upon the specified 
+	 * be performed at the point of conflict based upon the specified byteWidth. When located onto
-	 * byteWidth.  When located onto existing bitfields they will be packed together 
+	 * existing bitfields they will be packed together provided they do not conflict, otherwise the
-	 * provided they do not conflict, otherwise the conflict rule above applies.
+	 * conflict rule above applies.
 	 * <p>
 	 * Supported packing for little-endian fills lsb first, whereas big-endian fills msb first.
 	 * Insertion behavior may not work as expected if packing rules differ from this.
 	 * <p>
-	 * Zero length bitfields may be inserted although they have no real affect for 
+	 * 
-	 * unaligned structures.  Only the resulting byte offset within the structure 
+	 * Zero length bitfields may be inserted although they have no real affect for unaligned
-	 * is of significance in determining its ordinal placement.
+	 * structures. Only the resulting byte offset within the structure is of significance in
-	 * <p> 
+	 * determining its ordinal placement.
-	 * @param byteOffset the first byte offset within this structure which corresponds to the
+	 * <p>
-	 * first byte of the specified storage unit identified by its byteWidth.
+	 * 
-	 * @param byteWidth the storage unit width which contains the bitfield.  Must be large
+	 * @param byteOffset the first byte offset within this structure which corresponds to the first
-	 * enough to contain the specified bitSize and corresponding bitOffset.  The actual 
+	 *            byte of the specified storage unit identified by its byteWidth.
-	 * component size used will be recomputed during insertion.
+	 * @param byteWidth the storage unit width which contains the bitfield. Must be large enough to
-	 * @param bitOffset corresponds to the bitfield left-shift amount with the storage 
+	 *            contain the specified bitSize and corresponding bitOffset. The actual component
-	 * unit when viewed as big-endian.  The final offset may be reduced based upon
+	 *            size used will be recomputed during insertion.
-	 * the minimal storage size determined during insertion. 
+	 * @param bitOffset corresponds to the bitfield left-shift amount with the storage unit when
+	 *            viewed as big-endian. The final offset may be reduced based upon the minimal
+	 *            storage size determined during insertion.
 	 * @param baseDataType the bitfield base datatype (certain restrictions apply).
 	 * @param componentName the field name to associate with this component.
-	 * @param bitSize the bitfield size in bits.  A bitSize of 0 may be specified 
+	 * @param bitSize the bitfield size in bits. A bitSize of 0 may be specified although its name
-	 * although its name will be ignored.
+	 *            will be ignored.
 	 * @param comment the comment to associate with this component.
-	 * @return the componentDataType created whose associated data type will
+	 * @return the componentDataType created whose associated data type will be BitFieldDataType.
-	 * be BitFieldDataType.
+	 * @throws InvalidDataTypeException if the specified data type is not a valid base type for
-	 * @throws InvalidDataTypeException if the specified data type is
+	 *             bitfields.
-	 * not a valid base type for bitfields.
 	 */
 	public DataTypeComponent insertBitFieldAt(int byteOffset, int byteWidth, int bitOffset,
 			DataType baseDataType, int bitSize, String componentName, String comment)
 			throws InvalidDataTypeException;
 
 	/**
-	 * Inserts a new datatype at the specified offset into this structure.  
+	 * Inserts a new datatype at the specified offset into this structure. Inserting a component
-	 * Inserting a component will causing any conflicting component
+	 * will causing any conflicting component to shift down to the extent necessary to avoid a
-	 * to shift down to the extent necessary to avoid a conflict.
+	 * conflict.
-	 * @param offset the byte offset into the structure where the new datatype is to be inserted.	
+	 * 
+	 * @param offset the byte offset into the structure where the new datatype is to be inserted.
 	 * @param dataType the datatype to insert.
-	 * @param length the length to associate with the dataType.
+	 * @param length the length to associate with the dataType. For fixed length types a length
-	 * For fixed length types a length &lt;= 0 will use the length of the resolved dataType.
+	 *            &lt;= 0 will use the length of the resolved dataType.
 	 * @return the componentDataType created.
-	 * @throws IllegalArgumentException if the specified data type is not 
+	 * @throws IllegalArgumentException if the specified data type is not allowed to be inserted
-	 * allowed to be inserted into this composite data type or an invalid length
+	 *             into this composite data type or an invalid length is specified. For example,
-	 * is specified.
+	 *             suppose dt1 contains dt2. Therefore it is not valid to insert dt1 to dt2 since
-	 * For example, suppose dt1 contains dt2. Therefore it is not valid
+	 *             this would cause a cyclic dependency.
-	 * to insert dt1 to dt2 since this would cause a cyclic dependency.
 	 */
 	public DataTypeComponent insertAtOffset(int offset, DataType dataType, int length)
 			throws IllegalArgumentException;
 
 	/**
-	 * Inserts a new datatype at the specified offset into this structure.
+	 * Inserts a new datatype at the specified offset into this structure. Inserting a component
-	 * Inserting a component will causing any conflicting component
+	 * will causing any conflicting component to shift down to the extent necessary to avoid a
-	 * to shift down to the extent necessary to avoid a conflict.
+	 * conflict.
-	 * @param offset the byte offset into the structure where the new datatype is to be inserted.	
+	 * 
+	 * @param offset the byte offset into the structure where the new datatype is to be inserted.
 	 * @param dataType the datatype to insert.
-	 * @param length the length to associate with the dataType.
+	 * @param length the length to associate with the dataType. For fixed length types a length
-	 * For fixed length types a length &lt;= 0 will use the length of the resolved dataType.
+	 *            &lt;= 0 will use the length of the resolved dataType.
 	 * @param name the field name to associate with this component.
 	 * @param comment the comment to associate with this component.
 	 * @return the componentDataType created.
-	 * @throws IllegalArgumentException if the specified data type is not 
+	 * @throws IllegalArgumentException if the specified data type is not allowed to be inserted
-	 * allowed to be inserted into this composite data type or an invalid length is specified.
+	 *             into this composite data type or an invalid length is specified. For example,
-	 * For example, suppose dt1 contains dt2. Therefore it is not valid
+	 *             suppose dt1 contains dt2. Therefore it is not valid to insert dt1 to dt2 since
-	 * to insert dt1 to dt2 since this would cause a cyclic dependency.
+	 *             this would cause a cyclic dependency.
 	 */
 	public DataTypeComponent insertAtOffset(int offset, DataType dataType, int length, String name,
 			String comment) throws IllegalArgumentException;
 
 	/**
-	 * Deletes the component containing the specified offset in this structure.  If the offset
+	 * Deletes the component containing the specified offset in this structure. If the offset
-	 * corresponds to a bit-field, all bit-fields whose base type group contains the offset will 
+	 * corresponds to a bit-field, all bit-fields whose base type group contains the offset will be
-	 * be removed.
+	 * removed.
-	 * @param offset the byte offset into the structure where the datatype is to be deleted.	
+	 * 
+	 * @param offset the byte offset into the structure where the datatype is to be deleted.
 	 */
 	public void deleteAtOffset(int offset);
 
 	/**
-	 * Remove all components from this structure (including flex-array), 
+	 * Remove all components from this structure (including flex-array), effectively setting the
-	 * effectively setting the length to zero.
+	 * length to zero.
 	 */
 	public void deleteAll();
 
 	/**
-	 * Clears the defined component at the given component index.  Clearing a 
+	 * Clears the defined component at the given component index. Clearing a component causes a
-	 * component causes a defined component to be replaced with a number of
+	 * defined component to be replaced with a number of undefined dataTypes to offset the removal
-	 * undefined dataTypes to offset the removal of the defined dataType.
+	 * of the defined dataType.
+	 * 
 	 * @param index the index of the component to clear.
 	 * @throws ArrayIndexOutOfBoundsException if component ordinal is out of bounds
 	 */
 	public void clearComponent(int index) throws ArrayIndexOutOfBoundsException;
 
 	/**
-	 * Replaces the component at the given component index with a new component
+	 * Replaces the component at the given component index with a new component of the indicated
-	 * of the indicated data type.
+	 * data type.
-	 * @param index the index where the datatype is to be replaced.	
+	 * 
+	 * @param index the index where the datatype is to be replaced.
 	 * @param dataType the datatype to insert.
-	 * @param length the length of the dataType to insert.
+	 * @param length the length of the dataType to insert. For fixed length types a length &lt;= 0
-	 * For fixed length types a length &lt;= 0 will use the length of the resolved dataType.
+	 *            will use the length of the resolved dataType.
 	 * @return the new componentDataType at the index.
-	 * @throws IllegalArgumentException if the specified data type is not 
+	 * @throws IllegalArgumentException if the specified data type is not allowed to replace a
-	 * allowed to replace a component in this composite data type or an invalid
+	 *             component in this composite data type or an invalid length is specified. For
-	 * length is specified.
+	 *             example, suppose dt1 contains dt2. Therefore it is not valid to replace a dt2
-	 * For example, suppose dt1 contains dt2. Therefore it is not valid
+	 *             component with dt1 since this would cause a cyclic dependency. In addition, any
-	 * to replace a dt2 component with dt1 since this would cause a cyclic 
+	 *             attempt to replace an existing bit-field component or specify a
-	 * dependency.  In addition, any attempt to replace an existing bit-field
+	 *             {@link BitFieldDataType} will produce this error.
-	 * component or specify a {@link BitFieldDataType} will produce this error.
 	 * @throws ArrayIndexOutOfBoundsException if component index is out of bounds
 	 */
 	public DataTypeComponent replace(int index, DataType dataType, int length)
 			throws ArrayIndexOutOfBoundsException, IllegalArgumentException;
 
 	/**
-	 * Replaces the component at the given component index with a new component
+	 * Replaces the component at the given component index with a new component of the indicated
-	 * of the indicated data type.
+	 * data type.
-	 * @param index the index where the datatype is to be replaced.	
+	 * 
+	 * @param index the index where the datatype is to be replaced.
 	 * @param dataType the datatype to insert.
-	 * @param length the length to associate with the dataType.
+	 * @param length the length to associate with the dataType. For fixed length types a length
-	 * For fixed length types a length &lt;= 0 will use the length of the resolved dataType.
+	 *            &lt;= 0 will use the length of the resolved dataType.
 	 * @param name the field name to associate with this component.
 	 * @param comment the comment to associate with this component.
 	 * @return the new componentDataType at the index.
-	 * @throws IllegalArgumentException if the specified data type is not 
+	 * @throws IllegalArgumentException if the specified data type is not allowed to replace a
-	 * allowed to replace a component in this composite data type or an invalid
+	 *             component in this composite data type or an invalid length is specified. For
-	 * length is specified.
+	 *             example, suppose dt1 contains dt2. Therefore it is not valid to replace a dt2
-	 * For example, suppose dt1 contains dt2. Therefore it is not valid
+	 *             component with dt1 since this would cause a cyclic dependency. In addition, any
-	 * to replace a dt2 component with dt1 since this would cause a cyclic 
+	 *             attempt to replace an existing bit-field component or specify a
-	 * dependency.  In addition, any attempt to replace an existing bit-field
+	 *             {@link BitFieldDataType} will produce this error.
-	 * component or specify a {@link BitFieldDataType} will produce this error.
 	 * @throws ArrayIndexOutOfBoundsException if component index is out of bounds
 	 */
 	public DataTypeComponent replace(int index, DataType dataType, int length, String name,
 			String comment) throws ArrayIndexOutOfBoundsException, IllegalArgumentException;
 
 	/**
-	 * Replaces the component at the specified byte offset with a new component
+	 * Replaces the component at the specified byte offset with a new component of the indicated
-	 * of the indicated data type.   If the offset corresponds to a bit-field, all bit-fields 
+	 * data type. If the offset corresponds to a bit-field, all bit-fields at that offset will be
-	 * at that offset will be removed and replaced by the specified component.  Keep in mind
+	 * removed and replaced by the specified component. Keep in mind bit-field or any component
-	 * bit-field or any component removal must clear sufficient space for an unaligned 
+	 * removal must clear sufficient space for an unaligned structure to complete the replacement.
-	 * structure to complete the replacement.
+	 * 
-	 * @param offset the byte offset into the structure where the datatype is 
+	 * @param offset the byte offset into the structure where the datatype is to be replaced.
-	 * to be replaced.	
 	 * @param dataType the datatype to insert.
-	 * @param length the length to associate with the dataType.
+	 * @param length the length to associate with the dataType. For fixed length types a length
-	 * For fixed length types a length &lt;= 0 will use the length of the resolved dataType.
+	 *            &lt;= 0 will use the length of the resolved dataType.
 	 * @param name the field name to associate with this component.
 	 * @param comment the comment to associate with this component.
 	 * @return the new componentDataType at the index.
-	 * @throws IllegalArgumentException if the specified data type is not 
+	 * @throws IllegalArgumentException if the specified data type is not allowed to replace a
-	 * allowed to replace a component in this composite data type or an invalid 
+	 *             component in this composite data type or an invalid length is specified. For
-	 * length is specified.
+	 *             example, suppose dt1 contains dt2. Therefore it is not valid to replace a dt2
-	 * For example, suppose dt1 contains dt2. Therefore it is not valid
+	 *             component with dt1 since this would cause a cyclic dependency. In addition, any
-	 * to replace a dt2 component with dt1 since this would cause a cyclic 
+	 *             attempt to replace an existing bit-field component or specify a
-	 * dependency.  In addition, any attempt to replace an existing bit-field
+	 *             {@link BitFieldDataType} will produce this error.
-	 * component or specify a {@link BitFieldDataType} will produce this error.
 	 */
 	public DataTypeComponent replaceAtOffset(int offset, DataType dataType, int length, String name,
 			String comment) throws IllegalArgumentException;
 
 	/**
 	 * Determine if a trailing flexible array component has been defined.
+	 * 
 	 * @return true if trailing flexible array component has been defined.
 	 */
 	public boolean hasFlexibleArrayComponent();
 
 	/**
 	 * Get the optional trailing flexible array component associated with this structure.
-	 * @return optional trailing flexible array component associated with this structure or null
+	 * 
-	 * if not present.
+	 * @return optional trailing flexible array component associated with this structure or null if
+	 *         not present.
 	 */
 	public DataTypeComponent getFlexibleArrayComponent();
 
 	/**
 	 * Set the optional trailing flexible array component associated with this structure.
-	 * @param flexType the flexible array dataType (example: for 'char[0]' the type 'char' should be specified)
+	 * 
+	 * @param flexType the flexible array dataType (example: for 'char[0]' the type 'char' should be
+	 *            specified)
 	 * @param name component field name or null for default name
 	 * @param comment component comment
 	 * @return updated flexible array component
-	 * @throws IllegalArgumentException if specified flexType is not permitted (e.g., 
+	 * @throws IllegalArgumentException if specified flexType is not permitted (e.g., self
-	 * self referencing or unsupported type)
+	 *             referencing or unsupported type)
 	 */
 	public DataTypeComponent setFlexibleArrayComponent(DataType flexType, String name,
 			String comment) throws IllegalArgumentException;
@@ -307,38 +314,40 @@ public interface Structure extends Composite {
 	public void clearFlexibleArrayComponent();
 
 	/**
-	 * Increases the size of the structure by the given amount by adding undefined datatypes
+	 * Increases the size of the structure by the given amount by adding undefined datatypes at the
-	 * at the end of the structure.
+	 * end of the structure.
+	 * 
 	 * @param amount the amount by which to grow the structure.
 	 * @throws IllegalArgumentException if amount &lt; 1
 	 */
 	public void growStructure(int amount);
 
 	/**
-	 * Sets the current packing value (usually a power of 2). A value of NOT_PACKING should be passed 
+	 * Sets the current packing value (usually a power of 2). A value of NOT_PACKING should be
-	 * if this isn't a packed data type. Otherwise this value indicates a maximum alignment
+	 * passed if this isn't a packed data type. Otherwise this value indicates a maximum alignment
 	 * for any component within this data type. Calling this method will cause the data type to
-	 * become an internally aligned data type.
+	 * become an internally aligned data type. (Same as {@link Composite#setPackingValue(int)})
-	 * (Same as {@link Composite#setPackingValue(int)})
+	 * 
-	 * @param maxAlignment the new packing value or 0 for NOT_PACKING.
+	 * @param maxAlignment the new packing value or 0 for NOT_PACKING. A negative value will be
-	 * A negative value will be treated the same as 0.
+	 *            treated the same as 0.
 	 */
 	public void pack(int maxAlignment);
 
 	/**
-	 * <code>BitOffsetComparator</code> provides ability to compare an normalized bit offset
+	 * <code>BitOffsetComparator</code> provides ability to compare an normalized bit offset (see
-	 * (see {@link #getNormalizedBitfieldOffset(int, int, int, int, boolean)}) with a
+	 * {@link #getNormalizedBitfieldOffset(int, int, int, int, boolean)}) with a
-	 * {@link DataTypeComponent} object.  The offset will be considered equal (0) if the component 
+	 * {@link DataTypeComponent} object. The offset will be considered equal (0) if the component
-	 * contains the offset.  A normalized component bit numbering is used to establish the footprint
+	 * contains the offset. A normalized component bit numbering is used to establish the footprint
-	 * of each component with an ordinal-based ordering (assumes specific LE/BE allocation rules).  
+	 * of each component with an ordinal-based ordering (assumes specific LE/BE allocation rules).
 	 * Bit offsets for this comparator number the first allocated bit of the structure as 0 and the
-	 * last allocated bit of the structure as (8 * structLength) - 1.  For big-endian bitfields
+	 * last allocated bit of the structure as (8 * structLength) - 1. For big-endian bitfields the
-	 * the msb of the bitfield will be assigned the lower bit-number (assumes msb-allocated-first), 
+	 * msb of the bitfield will be assigned the lower bit-number (assumes msb-allocated-first),
-	 * while little-endian will perform similar numbering assuming byte-swap and bit-reversal of the 
+	 * while little-endian will perform similar numbering assuming byte-swap and bit-reversal of the
-	 * storage unit (assumes lsb-allocated-first).  Both cases result in a normalized view where 
+	 * storage unit (assumes lsb-allocated-first). Both cases result in a normalized view where
 	 * normalized bit-0 is allocated first.
 	 * 
-	 * <pre>{@literal
+	 * <pre>
+	 * {@literal
 	 * Example:
 	 *    
 	 * Big-Endian (normalized view):
@@ -351,7 +360,8 @@ public interface Structure extends Composite {
 	 *    | . . . . . . 6 7 | 8 . . . . . . . |
 	 *    |------------>|                       bit-offset (6, lsb position within storage unit)
 	 *                  |<--->|                 bit-size (3)
-	 * }</pre>
+	 * }
+	 * </pre>
 	 */
 	public static class BitOffsetComparator implements Comparator<Object> {
 
@@ -394,14 +404,14 @@ public interface Structure extends Composite {
 		 * Compute the normalized bit offset of a bitfield relative to the start of a structure.
 		 * 
 		 * NOTE: This implementation currently relies only on endianess to dictate bit allocation
-		 * ordering.  If future support is added for alternate bitfield packing, this implementation will
+		 * ordering. If future support is added for alternate bitfield packing, this implementation
-		 * require modification.
+		 * will require modification.
 		 * 
 		 * @param byteOffset byte offset within structure of storage unit
 		 * @param storageSize storage unit size (i.e., component length)
 		 * @param effectiveBitSize size of bitfield in bits
 		 * @param bitOffset left shift amount for bitfield based upon a big-endian view of the
-		 * storage unit
+		 *            storage unit
 		 * @param bigEndian true if big-endian packing applies
 		 * @return normalized bit-offset
 		 */
@@ -430,9 +440,9 @@ public interface Structure extends Composite {
 	}
 
 	/**
-	 * <code>OffsetComparator</code> provides ability to compare an Integer offset
+	 * <code>OffsetComparator</code> provides ability to compare an Integer offset with a
-	 * with a DataTypeComponent object.  The offset will be consider equal (0) if
+	 * DataTypeComponent object. The offset will be consider equal (0) if the component contains the
-	 * the component contains the offset.
+	 * offset.
 	 */
 	public static class OffsetComparator implements Comparator<Object> {
 
@@ -455,9 +465,9 @@ public interface Structure extends Composite {
 	}
 
 	/**
-	 * <code>OrdinalComparator</code> provides ability to compare an Integer ordinal
+	 * <code>OrdinalComparator</code> provides ability to compare an Integer ordinal with a
-	 * with a DataTypeComponent object.  The offset will be consider equal (0) if
+	 * DataTypeComponent object. The offset will be consider equal (0) if the component corresponds
-	 * the component corresponds to the specified ordinal.
+	 * to the specified ordinal.
 	 */
 	public static class OrdinalComparator implements Comparator<Object> {
 

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/data/StructureDataType.java

@@ -40,54 +40,52 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 	private int alignment = -1;
 
 	/**
-	 * Construct a new structure with the given name and length.
+	 * Construct a new structure with the given name and length. The root category will be used.
-	 * The root category will be used.
+	 * 
 	 * @param name the name of the new structure
-	 * @param length the initial size of the structure in bytes.  If 0 is specified
+	 * @param length the initial size of the structure in bytes. If 0 is specified the structure
-	 * the structure will report its length as 1 and {@link #isNotYetDefined()}
+	 *            will report its length as 1 and {@link #isNotYetDefined()} will return true.
-	 * will return true.
 	 */
 	public StructureDataType(String name, int length) {
 		this(CategoryPath.ROOT, name, length);
 	}
 
 	/**
-	 * Construct a new structure with the given name, length and datatype manager 
+	 * Construct a new structure with the given name, length and datatype manager which conveys data
-	 * which conveys data organization.  The root category will be used.
+	 * organization. The root category will be used.
+	 * 
 	 * @param name the name of the new structure
-	 * @param length the initial size of the structure in bytes.  If 0 is specified
+	 * @param length the initial size of the structure in bytes. If 0 is specified the structure
-	 * the structure will report its length as 1 and {@link #isNotYetDefined()}
+	 *            will report its length as 1 and {@link #isNotYetDefined()} will return true.
-	 * will return true.
+	 * @param dtm the data type manager associated with this data type. This can be null. Also, the
-	 * @param dtm the data type manager associated with this data type. This can be null. 
+	 *            data type manager may not yet contain this actual data type.
-	 * Also, the data type manager may not yet contain this actual data type.
 	 */
 	public StructureDataType(String name, int length, DataTypeManager dtm) {
 		this(CategoryPath.ROOT, name, length, dtm);
 	}
 
 	/**
-	 * Construct a new structure with the given name and length within the
+	 * Construct a new structure with the given name and length within the specified categry path.
-	 * specified categry path.
+	 * 
 	 * @param path the category path indicating where this data type is located.
 	 * @param name the name of the new structure
-	 * @param length the initial size of the structure in bytes.  If 0 is specified
+	 * @param length the initial size of the structure in bytes. If 0 is specified the structure
-	 * the structure will report its length as 1 and {@link #isNotYetDefined()}
+	 *            will report its length as 1 and {@link #isNotYetDefined()} will return true.
-	 * will return true.
 	 */
 	public StructureDataType(CategoryPath path, String name, int length) {
 		this(path, name, length, null);
 	}
 
 	/**
-	 * Construct a new structure with the given name, length and datatype manager
+	 * Construct a new structure with the given name, length and datatype manager within the
-	 * within the specified categry path.
+	 * specified categry path.
+	 * 
 	 * @param path the category path indicating where this data type is located.
 	 * @param name the name of the new structure
-	 * @param length the initial size of the structure in bytes.  If 0 is specified
+	 * @param length the initial size of the structure in bytes. If 0 is specified the structure
-	 * the structure will report its length as 1 and {@link #isNotYetDefined()}
+	 *            will report its length as 1 and {@link #isNotYetDefined()} will return true.
-	 * will return true.
+	 * @param dtm the data type manager associated with this data type. This can be null. Also, the
-	 * @param dtm the data type manager associated with this data type. This can be null. 
+	 *            data type manager may not yet contain this actual data type.
-	 * Also, the data type manager may not yet contain this actual data type.
 	 */
 	public StructureDataType(CategoryPath path, String name, int length, DataTypeManager dtm) {
 		super(path, name, dtm);
@@ -102,18 +100,18 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 
 	/**
 	 * Construct a new structure with the given name and length
+	 * 
 	 * @param path the category path indicating where this data type is located.
 	 * @param name the name of the new structure
-	 * @param length the initial size of the structure in bytes.  If 0 is specified
+	 * @param length the initial size of the structure in bytes. If 0 is specified the structure
-	 * the structure will report its length as 1 and {@link #isNotYetDefined()}
+	 *            will report its length as 1 and {@link #isNotYetDefined()} will return true.
-	 * will return true.
 	 * @param universalID the id for the data type
 	 * @param sourceArchive the source archive for this data type
 	 * @param lastChangeTime the last time this data type was changed
-	 * @param lastChangeTimeInSourceArchive the last time this data type was changed in
+	 * @param lastChangeTimeInSourceArchive the last time this data type was changed in its source
-	 * its source archive.
+	 *            archive.
-	 * @param dtm the data type manager associated with this data type. This can be null. 
+	 * @param dtm the data type manager associated with this data type. This can be null. Also, the
-	 * Also, the data type manager may not yet contain this actual data type.
+	 *            data type manager may not yet contain this actual data type.
 	 */
 	public StructureDataType(CategoryPath path, String name, int length, UniversalID universalID,
 			SourceArchive sourceArchive, long lastChangeTime, long lastChangeTimeInSourceArchive,
@@ -399,21 +397,21 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 	/**
 	 * Add a new component to the end of this structure.
 	 * <p>
-	 * NOTE: This method differs from inserting to the end the structure for the unaligned
+	 * NOTE: This method differs from inserting to the end the structure for the unaligned case in
-	 * case in that this method will always grow the structure by the positive length 
+	 * that this method will always grow the structure by the positive length specified while the
-	 * specified while the insert may limit its growth by the length of a smaller fixed-length
+	 * insert may limit its growth by the length of a smaller fixed-length dataType.
-	 * dataType.
+	 * 
 	 * @param dataType component data type
-	 * @param length maximum component length or -1 to use length of fixed-length dataType 
+	 * @param length maximum component length or -1 to use length of fixed-length dataType after
-	 * after applying structures data organization as determined by data type manager.  
+	 *            applying structures data organization as determined by data type manager. If
-	 * If dataType is Dynamic, a positive length must be specified. 
+	 *            dataType is Dynamic, a positive length must be specified.
-	 * @param isFlexibleArray if true length is ignored and the trailing flexible array will be 
+	 * @param isFlexibleArray if true length is ignored and the trailing flexible array will be set
-	 * set based upon the specified fixed-length dataType;
+	 *            based upon the specified fixed-length dataType;
 	 * @param componentName component name
 	 * @param comment componetn comment
 	 * @return newly added component
-	 * @throws IllegalArgumentException if the specified data type is not 
+	 * @throws IllegalArgumentException if the specified data type is not allowed to be added to
-	 * allowed to be added to this composite data type or an invalid length is specified.
+	 *             this composite data type or an invalid length is specified.
 	 */
 	private DataTypeComponent doAdd(DataType dataType, int length, boolean isFlexibleArray,
 			String componentName, String comment) throws IllegalArgumentException {
@@ -687,10 +685,10 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 	}
 
 	/**
-	 * Backup from specified ordinal to the first component which contains
+	 * Backup from specified ordinal to the first component which contains the specified offset. For
-	 * the specified offset.  For normal components the specified
+	 * normal components the specified ordinal will be returned, however for bit-fields the ordinal
-	 * ordinal will be returned, however for bit-fields the ordinal of the first
+	 * of the first bit-field containing the specified offset will be returned.
-	 * bit-field containing the specified offset will be returned.
+	 * 
 	 * @param ordinal component ordinal
 	 * @param offset offset within structure
 	 * @return index of first defined component containing specific offset.
@@ -712,10 +710,10 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 	}
 
 	/**
-	 * Advance from specified ordinal to the last component which contains
+	 * Advance from specified ordinal to the last component which contains the specified offset. For
-	 * the specified offset.  For normal components the specified
+	 * normal components the specified ordinal will be returned, however for bit-fields the ordinal
-	 * ordinal will be returned, however for bit-fields the ordinal of the last
+	 * of the last bit-field containing the specified offset will be returned.
-	 * bit-field containing the specified offset will be returned.
+	 * 
 	 * @param ordinal component ordinal
 	 * @param offset offset within structure
 	 * @return index of last defined component containing specific offset.
@@ -892,9 +890,10 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 	}
 
 	/**
-	 * Create copy of structure for target dtm (source archive information is discarded).
+	 * Create copy of structure for target dtm (source archive information is discarded). WARNING!
-	 * WARNING! copying unaligned structures which contain bitfields can produce
+	 * copying unaligned structures which contain bitfields can produce invalid results when
-	 * invalid results when switching endianess due to the differences in packing order.
+	 * switching endianess due to the differences in packing order.
+	 * 
 	 * @param dtm target data type manager
 	 * @return cloned structure
 	 */
@@ -907,14 +906,15 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 	}
 
 	/**
-	 * Create cloned structure for target dtm preserving source archive information.
+	 * Create cloned structure for target dtm preserving source archive information. WARNING!
-	 * WARNING! cloning unaligned structures which contain bitfields can produce
+	 * cloning unaligned structures which contain bitfields can produce invalid results when
-	 * invalid results when switching endianess due to the differences in packing order.
+	 * switching endianess due to the differences in packing order.
+	 * 
 	 * @param dtm target data type manager
 	 * @return cloned structure
 	 */
 	@Override
-	public DataType clone(DataTypeManager dtm) {
+	public StructureDataType clone(DataTypeManager dtm) {
 		if (dataMgr == dtm) {
 			return this;
 		}
@@ -945,14 +945,13 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 	}
 
 	/**
-	 * Replaces the internal components of this structure with components of the
+	 * Replaces the internal components of this structure with components of the given structure.
-	 * given structure. 
+	 * 
 	 * @param dataType the structure to get the component information from.
-	 * @throws IllegalArgumentException if any of the component data types 
+	 * @throws IllegalArgumentException if any of the component data types are not allowed to
-	 * are not allowed to replace a component in this composite data type.
+	 *             replace a component in this composite data type. For example, suppose dt1
-	 * For example, suppose dt1 contains dt2. Therefore it is not valid
+	 *             contains dt2. Therefore it is not valid to replace a dt2 component with dt1 since
-	 * to replace a dt2 component with dt1 since this would cause a cyclic 
+	 *             this would cause a cyclic dependency.
-	 * dependency.
 	 */
 	@Override
 	public void replaceWith(DataType dataType) {
@@ -1248,20 +1247,20 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 	}
 
 	/**
-	 * Replace the indicated component with a new component containing the 
+	 * Replace the indicated component with a new component containing the specified data type.
-	 * specified data type.
+	 * 
 	 * @param origDtc the original data type component in this structure.
 	 * @param dataType the data type of the new component
 	 * @param length the length of the new component
 	 * @param componentName the field name of the new component
 	 * @param comment the comment for the new component
 	 * @return the new component or null if the new component couldn't fit.
-	 * @throws IllegalArgumentException if the specified data type is not 
+	 * @throws IllegalArgumentException if the specified data type is not allowed to replace a
-	 * allowed to replace a component in this composite data type.
+	 *             component in this composite data type. For example, suppose dt1 contains dt2.
-	 * For example, suppose dt1 contains dt2. Therefore it is not valid
+	 *             Therefore it is not valid to replace a dt2 component with dt1 since this would
-	 * to replace a dt2 component with dt1 since this would cause a cyclic 
+	 *             cause a cyclic dependency. In addition, any attempt to replace an existing
-	 * dependency.  In addition, any attempt to replace an existing bit-field
+	 *             bit-field component or specify a {@link BitFieldDatatype} will produce this
-	 * component or specify a {@link BitFieldDatatype} will produce this error.
+	 *             error.
 	 */
 	private DataTypeComponent replaceComponent(DataTypeComponentImpl origDtc, DataType dataType,
 			int length, String componentName, String comment) {
@@ -1321,9 +1320,9 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 	}
 
 	/**
-	 * Gets the number of Undefined bytes beginning at the indicated component 
+	 * Gets the number of Undefined bytes beginning at the indicated component index. Undefined
-	 * index. Undefined bytes that have a field name or comment specified are 
+	 * bytes that have a field name or comment specified are also included.
-	 * also included.
+	 * 
 	 * @param index the component index to begin checking at.
 	 * @return the number of contiguous undefined bytes
 	 */
@@ -1384,12 +1383,13 @@ public class StructureDataType extends CompositeDataTypeImpl implements Structur
 	}
 
 	/**
-	 * Adjust the alignment, packing and padding of components within this structure based upon the 
+	 * Adjust the alignment, packing and padding of components within this structure based upon the
-	 * current alignment and packing attributes for this structure. This method should be 
+	 * current alignment and packing attributes for this structure. This method should be called to
-	 * called to fix up the layout of the internal components of the structure 
+	 * fix up the layout of the internal components of the structure after other code has changed
-	 * after other code has changed the attributes of the structure.
+	 * the attributes of the structure. <BR>
-	 * <BR>When switching between internally aligned and unaligned this method corrects the 
+	 * When switching between internally aligned and unaligned this method corrects the component
-	 * component ordinal numbering also.
+	 * ordinal numbering also.
+	 * 
 	 * @return true if the structure was changed by this method.
 	 */
 	protected boolean adjustComponents() {

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/symbol/ReferenceIteratorAdapter.java

@@ -1,6 +1,5 @@
 /* ###
  * IP: GHIDRA
- * REVIEWED: YES
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -20,9 +19,9 @@ import java.util.Iterator;
 
 public class ReferenceIteratorAdapter implements ReferenceIterator {
 
-	private final Iterator<Reference> iterator;
+	private final Iterator<? extends Reference> iterator;
 
-	public ReferenceIteratorAdapter(Iterator<Reference> iterator) {
+	public ReferenceIteratorAdapter(Iterator<? extends Reference> iterator) {
 		this.iterator = iterator;
 	}
 

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/symbol/SymbolIteratorAdapter.java

@@ -1,6 +1,5 @@
 /* ###
  * IP: GHIDRA
- * REVIEWED: YES
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -20,9 +19,9 @@ import java.util.Iterator;
 
 public class SymbolIteratorAdapter implements SymbolIterator {
 
-	private final Iterator<Symbol> iterator;
+	private final Iterator<? extends Symbol> iterator;
 
-	public SymbolIteratorAdapter(Iterator<Symbol> iterator) {
+	public SymbolIteratorAdapter(Iterator<? extends Symbol> iterator) {
 		this.iterator = iterator;
 	}
 

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

@@ -15,24 +15,27 @@
  */
 package ghidra.program.util;
 
+import java.lang.reflect.InvocationTargetException;
+import java.util.Objects;
+
 import ghidra.framework.options.SaveState;
 import ghidra.program.model.address.Address;
 import ghidra.program.model.listing.*;
 import ghidra.util.Msg;
 
 /**
- * <CODE>ProgramLocation</CODE> provides information about a location in a
+ * <CODE>ProgramLocation</CODE> provides information about a location in a program in the most
- * program in the most generic way.
+ * generic way.
  *
- * ProgramLocations refer to a specific location in a program and can be specified down
+ * <p>
- * to an address, a field at that address, and within that field, a row, col, and character
+ * ProgramLocations refer to a specific location in a program and can be specified down to an
- * offset.  The field is not recorded directly, but by the subclass of the ProgramLocation.
+ * address, a field at that address, and within that field, a row, col, and character offset. The
- * The "cursor position" within a field is specified by three variables: row, col, and character
+ * field is not recorded directly, but by the subclass of the ProgramLocation. The "cursor position"
- * offset.  The row is literally the row (line #) the cursor is on  within the field, the
+ * within a field is specified by three variables: row, col, and character offset. The row is
- * column represents the display item on that row (For example, in the bytes field
+ * literally the row (line #) the cursor is on within the field, the column represents the display
- * the column will represent which "byte" the cursor is on.  Most fields only have one
+ * item on that row (For example, in the bytes field the column will represent which "byte" the
- * column item per row.)  And finally, the character offset
+ * cursor is on. Most fields only have one column item per row.) And finally, the character offset
- * is the character position within the display item specified by the row and column.  Simple fields
+ * is the character position within the display item specified by the row and column. Simple fields
  * like the address field and Mnemonic field will always have a row and column of 0.
  */
 public class ProgramLocation implements Comparable<ProgramLocation> {
@@ -48,19 +51,19 @@ public class ProgramLocation implements Comparable<ProgramLocation> {
 
 	/**
 	 * Construct a new ProgramLocation.
-	 * <br>Note: A NullPointerException will be logged if addr is null.
 	 *
 	 * @param program the program of the location
-	 * @param addr address of the location; cannot be null; This could be a
+	 * @param addr address of the location; cannot be null; This could be a code unit minimum
-	 * code unit minimum address where the byteAddr is within the code unit.
+	 *            address where the byteAddr is within the code unit.
 	 * @param byteAddr address of the location; cannot be null
-	 * @param componentPath array of indexes for each nested data component;
+	 * @param componentPath array of indexes for each nested data component; the data index is the
-	 * the data index is the data component's index within its parent; may be null
+	 *            data component's index within its parent; may be null
-	 * @param refAddr the "referred to" address if the location is
+	 * @param refAddr the "referred to" address if the location is over a reference; may be null
-	 * over a reference; may be null
 	 * @param row the row within the field.
-	 * @param col - the display item index on the given row. (Note most fields only have one display item per row)
+	 * @param col the display item index on the given row. (Note most fields only have one display
-	 * @param charOffset - the character offset within the display item.
+	 *            item per row)
+	 * @param charOffset the character offset within the display item.
+	 * @throws NullPointerException if {@code addr} or {@code program} is null
 	 */
 	public ProgramLocation(Program program, Address addr, Address byteAddr, int[] componentPath,
 			Address refAddr, int row, int col, int charOffset) {
@@ -89,20 +92,21 @@ public class ProgramLocation implements Comparable<ProgramLocation> {
 	}
 
 	/**
-	 * Construct a new ProgramLocation for the given address.  The address will be adjusted
+	 * Construct a new ProgramLocation for the given address. The address will be adjusted to the
-	 * to the beginning of the code unit containing that address(if it exists).  The original
+	 * beginning of the {@link CodeUnit code unit} containing that address (if it exists). The
-	 * address can be retrieved using the "getByteAddress()" method.
+	 * original address can be retrieved using the {@link #getByteAddress()}" method.
-	 * <br>Note: A NullPointerException will be logged if addr is null.
+	 * 
-	 * @param program the program associated with this program location (also
+	 * @param program the program associated with this program location (also used to obtain a
-	 * used to obtain a code-unit-aligned address)
+	 *            code-unit-aligned address)
 	 * @param addr address of the location; cannot be null
-	 * @param componentPath array of indexes for each nested data component;
+	 * @param componentPath array of indexes for each nested data component; the index is the data
-	 * the index is the data component's index within its parent; may be null
+	 *            component's index within its parent; may be null
-	 * @param refAddr the "referred to" address if the location is
+	 * @param refAddr the "referred to" address if the location is over a reference; may be null
-	 * over a reference; may be null
 	 * @param row the row within the field.
-	 * @param col - the display item index on the given row. (Note most fields only have one display item per row)
+	 * @param col the display item index on the given row. (Note most fields only have one display
-	 * @param charOffset - the character offset within the display item.
+	 *            item per row)
+	 * @param charOffset the character offset within the display item.
+	 * @throws NullPointerException if {@code addr} or {@code program} is null
 	 */
 	public ProgramLocation(Program program, Address addr, int[] componentPath, Address refAddr,
 			int row, int col, int charOffset) {
@@ -111,41 +115,47 @@ public class ProgramLocation implements Comparable<ProgramLocation> {
 	}
 
 	/**
-	 * Construct a new ProgramLocation for the given address.  The address will be adjusted
+	 * Construct a new ProgramLocation for the given address. The address will be adjusted to the
-	 * to the beginning of the code unit containing that address(if it exists).  The original
+	 * beginning of the {@link CodeUnit code unit} containing that address (if it exists). The
-	 * address can be retrieved using the "getByteAddress()" method.
+	 * original address can be retrieved using the {@link #getByteAddress()} method.
-	 * @param program the program associated with this program location (also
+	 * 
-	 * used to obtain a code-unit-aligned address)
+	 * @param program the program associated with this program location (also used to obtain a
+	 *            code-unit-aligned address)
 	 * @param addr address for the location
+	 * @throws NullPointerException if {@code addr} or {@code program} is null
 	 */
 	public ProgramLocation(Program program, Address addr) {
 		this(program, getCodeUnitAddress(program, addr), addr, null, null, 0, 0, 0);
 	}
 
 	/**
-	 * Construct a new ProgramLocation for the given address.  The address will be adjusted
+	 * Construct a new ProgramLocation for the given address. The address will be adjusted to the
-	 * to the beginning of the code unit containing that address(if it exists).  The original
+	 * beginning of the {@link CodeUnit code unit} containing that address (if it exists). The
-	 * address can be retrieved using the "getByteAddress()" method.
+	 * original address can be retrieved using the {@link #getByteAddress()} method.
-	 * @param program the program associated with this program location (also
+	 * 
-	 * used to obtain a code-unit-aligned address)
+	 * @param program the program associated with this program location (also used to obtain a
+	 *            code-unit-aligned address)
 	 * @param addr address for the location
 	 * @param row the row within the field.
-	 * @param col - the display item index on the given row. (Note most fields only have one display item per row)
+	 * @param col the display item index on the given row. (Note most fields only have one display
-	 * @param charOffset - the character offset within the display item.
+	 *            item per row)
+	 * @param charOffset the character offset within the display item.
+	 * @throws NullPointerException if {@code addr} or {@code program} is null
 	 */
 	public ProgramLocation(Program program, Address addr, int row, int col, int charOffset) {
 		this(program, getCodeUnitAddress(program, addr), addr, null, null, row, col, charOffset);
 	}
 
 	/**
-	 * Construct a new ProgramLocation for the given address.  The address will be adjusted
+	 * Construct a new ProgramLocation for the given address. The address will be adjusted to the
-	 * to the beginning of the code unit containing that address(if it exists).  The original
+	 * beginning of the {@link CodeUnit code unit} containing that address (if it exists). The
-	 * address can be retrieved using the "getByteAddress()" method.
+	 * original address can be retrieved using the {@link #getByteAddress()} method.
-	 * @param program the program associated with this program location (also
+	 * 
-	 * used to obtain a code-unit-aligned address)
+	 * @param program the program associated with this program location (also used to obtain a
+	 *            code-unit-aligned address)
 	 * @param addr address for the location
-	 * @param refAddr the "referred to" address if the location is over a
+	 * @param refAddr the "referred to" address if the location is over a reference
-	 * reference
+	 * @throws NullPointerException if {@code addr} or {@code program} is null
 	 */
 	public ProgramLocation(Program program, Address addr, Address refAddr) {
 		this(program, getCodeUnitAddress(program, addr), addr, null, refAddr, 0, 0, 0);
@@ -158,15 +168,15 @@ public class ProgramLocation implements Comparable<ProgramLocation> {
 	}
 
 	/**
-	 * Returns the componentPath for the codeUnit.  Null will be returned if the
+	 * Returns the componentPath for the {@link CodeUnit code unit}. Null will be returned if the
-	 * object is an Instruction or a top-level Data object.
+	 * object is an {@link Instruction} or a top-level {@link Data} object.
 	 */
 	public int[] getComponentPath() {
 		return componentPath;
 	}
 
 	/**
-	 * Returns program associated with location or null if not specified.
+	 * Returns the program associated with this location.
 	 */
 	public Program getProgram() {
 		return program;
@@ -174,9 +184,11 @@ public class ProgramLocation implements Comparable<ProgramLocation> {
 
 	/**
 	 * Returns the address associated with this location.
-	 * <br>Note: this may not be the same as the byte address. For example, in
+	 * 
-	 * a code unit location this may be the minimum address of the code unit
+	 * <p>
-	 * that contains the byte address.
+	 * Note: this may not be the same as the byte address. For example, in a {@link CodeUnit code
+	 * unit} location this may be the minimum address of the code unit that contains the byte
+	 * address.
 	 */
 	public Address getAddress() {
 		return addr;
@@ -190,8 +202,7 @@ public class ProgramLocation implements Comparable<ProgramLocation> {
 	}
 
 	/**
-	 * Returns the "referred to" address if the location is over an
+	 * Returns the "referred to" address if the location is over an address in some field.
-	 * address in some field.
 	 */
 	public Address getRefAddress() {
 		return refAddr;
@@ -199,6 +210,7 @@ public class ProgramLocation implements Comparable<ProgramLocation> {
 
 	/**
 	 * Save this program location to the given save state object.
+	 * 
 	 * @param obj the save state object for saving the location
 	 */
 	public void saveState(SaveState obj) {
@@ -218,8 +230,8 @@ public class ProgramLocation implements Comparable<ProgramLocation> {
 	}
 
 	/**
-	 * Restore this program location using the given program
+	 * Restore this program location using the given program and save state object.
-	 * and save state object.
+	 * 
 	 * @param program1 program to restore from
 	 * @param obj the save state to restore from
 	 */
@@ -240,6 +252,13 @@ public class ProgramLocation implements Comparable<ProgramLocation> {
 
 	}
 
+	/**
+	 * Get the program location for the given program and save state object.
+	 * 
+	 * @param program the program for the location
+	 * @param saveState the state to restore
+	 * @return the restored program location
+	 */
 	public static ProgramLocation getLocation(Program program, SaveState saveState) {
 		String className = saveState.getString("_CLASSNAME", null);
 		if (className == null) {
@@ -248,7 +267,7 @@ public class ProgramLocation implements Comparable<ProgramLocation> {
 
 		try {
 			Class<?> locClass = Class.forName(className);
-			ProgramLocation loc = (ProgramLocation) locClass.newInstance();
+			ProgramLocation loc = (ProgramLocation) locClass.getConstructor().newInstance();
 			loc.restoreState(program, saveState);
 			if (loc.getAddress() != null) {
 				return loc;
@@ -260,23 +279,20 @@ public class ProgramLocation implements Comparable<ProgramLocation> {
 		catch (ClassNotFoundException e) {
 			// not sure why we are ignoring this--if you know, then please let everyone else know
 		}
-		catch (InstantiationException e) {
+		catch (InstantiationException | IllegalAccessException | NoSuchMethodException e) {
-			Msg.showError(ProgramLocation.class, null, "Programming Error",
-				"Class " + className + " must have default constructor!", e);
-		}
-		catch (IllegalAccessException e) {
 			Msg.showError(ProgramLocation.class, null, "Programming Error",
 				"Class " + className + " must have public default constructor!", e);
 		}
+		catch (InvocationTargetException e) {
+			Msg.showError(ProgramLocation.class, null, "Programming Error",
+				"Class " + className + " default constructor threw an exception!", e);
+		}
 		return null;
 	}
 
 	@Override
 	public int hashCode() {
-		if (addr == null) {
+		return Objects.hash(program, addr);
-			return 0;
-		}
-		return addr.hashCode();
 	}
 
 	@Override
@@ -323,6 +339,9 @@ public class ProgramLocation implements Comparable<ProgramLocation> {
 			return false;
 		}
 		ProgramLocation other = (ProgramLocation) obj;
+		if (program != other.program) {
+			return false;
+		}
 		if (compareAddr(addr, other.addr) != 0) {
 			return false;
 		}
@@ -343,7 +362,7 @@ public class ProgramLocation implements Comparable<ProgramLocation> {
 		if (other == this) {
 			return 0;
 		}
-		int result = ProgramLocationComparator.instance.compare(this, other);
+		int result = ProgramLocationComparator.INSTANCE.compare(this, other);
 		if (result == 0) {
 			result = row - other.row;
 			if (result == 0) {
@@ -419,6 +438,7 @@ public class ProgramLocation implements Comparable<ProgramLocation> {
 
 	/**
 	 * Returns true if this location represents a valid location in the given program
+	 * 
 	 * @param testProgram the program to test if this location is valid.
 	 * @return true if this location represents a valid location in the given program
 	 */
@@ -428,6 +448,7 @@ public class ProgramLocation implements Comparable<ProgramLocation> {
 
 	/**
 	 * Returns the row within the program location.
+	 * 
 	 * @return the row within the program location.
 	 */
 	public int getRow() {
@@ -436,14 +457,15 @@ public class ProgramLocation implements Comparable<ProgramLocation> {
 
 	/**
 	 * Returns the character offset in the display item at the (row,col)
-	 * @return  the character offset in the display item at the (row,col)
+	 * 
+	 * @return the character offset in the display item at the (row,col)
 	 */
 	public int getCharOffset() {
 		return charOffset;
 	}
 
 	/**
-	 * Returns the column index of the display piece represented by this location.  For most
+	 * Returns the column index of the display piece represented by this location. For most
 	 * locations, there is only one display item per row, in which case this value will be 0.
 	 */
 	public int getColumn() {

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

@@ -15,20 +15,33 @@
  */
 package ghidra.program.util;
 
-import java.util.HashMap;
+import java.util.*;
-import java.util.Map;
 
-public class ProgramLocationComparator {
+import ghidra.program.model.listing.Program;
+
+/**
+ * A comparator for the common fields of {@link ProgramLocation}
+ * 
+ * <p>
+ * This comparator only compares the program, address, and class of the program location. To compare
+ * at greater granularity, invoke the {@link ProgramLocation#compareTo(ProgramLocation)} method, or
+ * use the natural ordering. Each particular type of location uses this comparator, and then
+ * compares the more detailed fields, if necessary. If this comparator indicates equality, then the
+ * two locations are definitely of the same class.
+ */
+public class ProgramLocationComparator implements Comparator<ProgramLocation> {
+	/** The singleton instance */
+	public static final ProgramLocationComparator INSTANCE = new ProgramLocationComparator();
 	private static final Class<?>[] PROGRAM_LOCATION_CLASSES = {
 
-	DividerLocation.class, ProgramLocation.class, PlateFieldLocation.class, FunctionLocation.class,
+		DividerLocation.class, ProgramLocation.class, PlateFieldLocation.class,
-		FunctionRepeatableCommentFieldLocation.class, FunctionSignatureFieldLocation.class,
+		FunctionLocation.class, FunctionRepeatableCommentFieldLocation.class,
-		FunctionSignatureSourceFieldLocation.class, FunctionCallFixupFieldLocation.class,
+		FunctionSignatureFieldLocation.class, FunctionSignatureSourceFieldLocation.class,
-		FunctionReturnTypeFieldLocation.class, FunctionCallingConventionFieldLocation.class,
+		FunctionCallFixupFieldLocation.class, FunctionReturnTypeFieldLocation.class,
-		FunctionNameFieldLocation.class, FunctionStartParametersFieldLocation.class,
+		FunctionCallingConventionFieldLocation.class, FunctionNameFieldLocation.class,
-		FunctionParameterFieldLocation.class, FunctionParameterNameFieldLocation.class,
+		FunctionStartParametersFieldLocation.class, FunctionParameterFieldLocation.class,
-		FunctionEndParametersFieldLocation.class, VariableLocation.class,
+		FunctionParameterNameFieldLocation.class, FunctionEndParametersFieldLocation.class,
-		VariableTypeFieldLocation.class, VariableNameFieldLocation.class,
+		VariableLocation.class, VariableTypeFieldLocation.class, VariableNameFieldLocation.class,
 		VariableLocFieldLocation.class, VariableXRefFieldLocation.class,
 		VariableCommentFieldLocation.class,
 
@@ -42,39 +55,58 @@ public class ProgramLocationComparator {
 		PostCommentFieldLocation.class,
 
 		SpaceFieldLocation.class, SpacerFieldLocation.class, SubDataFieldLocation.class,
-		RegisterFieldLocation.class, };
+		RegisterFieldLocation.class,
-	public static final ProgramLocationComparator instance = new ProgramLocationComparator();
+
+	};
+
 	private Map<Class<?>, Integer> priorityMap;
 
 	private ProgramLocationComparator() {
-		priorityMap = new HashMap<Class<?>, Integer>();
+		priorityMap = new HashMap<>();
 		for (int ordinal = 0; ordinal < PROGRAM_LOCATION_CLASSES.length; ordinal++) {
 			priorityMap.put(PROGRAM_LOCATION_CLASSES[ordinal], ordinal);
 		}
 	}
 
+	@Override
 	public int compare(ProgramLocation loc1, ProgramLocation loc2) {
-		int result = loc1.getAddress().compareTo(loc2.getAddress());
+		int result;
-		if (result == 0) {
+		// Try to make a sensible comparison of programs before just using identity hashes
-			Class<?> class1 = loc1.getClass();
+		Program program1 = loc1.getProgram();
-			Class<?> class2 = loc2.getClass();
+		Program program2 = loc2.getProgram();
-			if (class1 == class2) {
+		result = program1.getName().compareTo(program2.getName());
-				return 0;
+		if (result != 0) {
-			}
+			return result;
-			Integer ordinal1 = priorityMap.get(class1);
-			Integer ordinal2 = priorityMap.get(class2);
-			if (ordinal1 == null && ordinal2 == null) {
-				return class1.getName().compareTo(class2.getName());
-			}
-			if (ordinal1 == null) {
-				return 1;
-			}
-			if (ordinal2 == null) {
-				return -1;
-			}
-			result = ordinal1.intValue() - ordinal2.intValue();
 		}
-		return result;
+		result = Integer.compare(program1.hashCode(), program2.hashCode());
+		if (result != 0) {
+			return result;
+		}
+		result = loc1.getAddress().compareTo(loc2.getAddress());
+		if (result != 0) {
+			return result;
+		}
+		Class<?> class1 = loc1.getClass();
+		Class<?> class2 = loc2.getClass();
+		if (class1 == class2) {
+			return 0;
+		}
+		Integer ordinal1 = priorityMap.get(class1);
+		Integer ordinal2 = priorityMap.get(class2);
+		if (ordinal1 == null && ordinal2 == null) {
+			return class1.getName().compareTo(class2.getName());
+		}
+		if (ordinal1 == null) {
+			return 1;
+		}
+		if (ordinal2 == null) {
+			return -1;
+		}
+		result = Integer.compare(ordinal1.intValue(), ordinal2.intValue());
+		if (result != 0) {
+			return result;
+		}
+		return 0;
 	}
 
 }

Ghidra/Processors/Toy/certification.manifest

@@ -16,17 +16,19 @@ data/languages/toy.ldefs||GHIDRA||||END|
 data/languages/toy.pspec||GHIDRA||reviewed||END|
 data/languages/toy.sinc||GHIDRA||||END|
 data/languages/toy64.cspec||GHIDRA||||END|
-data/languages/toy64_be.slaspec||GHIDRA||reviewed||END|
+data/languages/toy64_be.slaspec||GHIDRA||||END|
-data/languages/toy64_le.slaspec||GHIDRA||reviewed||END|
+data/languages/toy64_be_harvard.slaspec||GHIDRA||||END|
+data/languages/toy64_le.slaspec||GHIDRA||||END|
 data/languages/toyInstructions.sinc||GHIDRA||||END|
 data/languages/toyPosStack.cspec||GHIDRA||||END|
 data/languages/toy_be.slaspec||GHIDRA||||END|
 data/languages/toy_be_posStack.slaspec||GHIDRA||||END|
 data/languages/toy_builder.sinc||GHIDRA||||END|
 data/languages/toy_builder_be.slaspec||GHIDRA||||END|
-data/languages/toy_builder_be_align2.slaspec||GHIDRA||reviewed||END|
+data/languages/toy_builder_be_align2.slaspec||GHIDRA||||END|
 data/languages/toy_builder_le.slaspec||GHIDRA||||END|
-data/languages/toy_builder_le_align2.slaspec||GHIDRA||reviewed||END|
+data/languages/toy_builder_le_align2.slaspec||GHIDRA||||END|
+data/languages/toy_harvard.pspec||GHIDRA||||END|
 data/languages/toy_le.slaspec||GHIDRA||||END|
-data/languages/toy_wsz_be.slaspec||GHIDRA||reviewed||END|
+data/languages/toy_wsz_be.slaspec||GHIDRA||||END|
-data/languages/toy_wsz_le.slaspec||GHIDRA||reviewed||END|
+data/languages/toy_wsz_le.slaspec||GHIDRA||||END|

Ghidra/Processors/Toy/data/languages/toy.ldefs

@@ -67,6 +67,17 @@
     <description>Toy (test) processor 64-bit big-endian</description>
     <compiler name="default" spec="toy64.cspec" id="default"/>
   </language>
+  <language processor="Toy"
+            endian="big"
+            size="64"
+            variant="harvard"
+            version="1.0"
+            slafile="toy64_be_harvard.sla"
+            processorspec="toy_harvard.pspec"
+            id="Toy:BE:64:harvard">
+    <description>Toy (test) processor 64-bit big-endian Harvard</description>
+    <compiler name="default" spec="toy64.cspec" id="default"/>
+  </language>
   <language processor="Toy"
             endian="little"
             size="64"

Ghidra/Processors/Toy/data/languages/toy.sinc

@@ -1,5 +1,8 @@
 # Main slaspec must define endianess and alignment
 
+define endian=$(ENDIAN);
+
+
 @ifndef WORDSIZE
 @define WORDSIZE "1"
 @endif
@@ -23,3 +26,22 @@ define register offset=0x1100 size=1 [
        C   Z   N   V
 ];
 
+
+@if SIZE == "4"
+@define HALFSIZE "2"
+@endif
+@if SIZE == "8"
+@define HALFSIZE "4"
+@endif
+
+@if ENDIAN == "little"
+define register offset=0x1000 size=$(HALFSIZE) [
+       r0l  r0h  r1l  r1h  r2l  r2h  r3l  r3h  r4l  r4h  r5l  r5h  r6l  r6h  r7l  r7h
+       r8l  r8h  r9l  r9h r10l r10h r11l r1lh r12l r12h  spl  sph  lrl  lrh  pcl  pch
+];
+@else # ENDIAN == "big"
+define register offset=0x1000 size=$(HALFSIZE) [
+       r0h  r0l  r1h  r1l  r2h  r2l  r3h  r3l  r4h  r4l  r5h  r5l  r6h  r6l  r7h  r7l
+       r8h  r8l  r9h  r9l r10h r10l r11h r1ll r12h r12l  sph  spl  lrh  lrl  pch  pcl
+];
+#endif # ENDIAN

Ghidra/Processors/Toy/data/languages/toy64_be.slaspec

@@ -1,4 +1,4 @@
-define endian=big;
+@define ENDIAN "big"
 
 @define SIZE "8"
 

Ghidra/Processors/Toy/data/languages/toy64_be_harvard.slaspec

@@ -0,0 +1,11 @@
+@define ENDIAN "big"
+
+@define SIZE "8"
+
+@define INSTR_PHASE "" # not used by basic toy language
+
+@include "toy.sinc"
+
+define space data type=ram_space size=$(SIZE) wordsize=$(WORDSIZE);
+
+@include "toyInstructions.sinc"

Ghidra/Processors/Toy/data/languages/toy64_le.slaspec

@@ -1,4 +1,4 @@
-define endian=little;
+@define ENDIAN "little"
 
 @define SIZE "8"
 

Ghidra/Processors/Toy/data/languages/toy_be.slaspec

@@ -1,4 +1,4 @@
-define endian=big;
+@define ENDIAN "big"
 
 @define SIZE "4"
 

Ghidra/Processors/Toy/data/languages/toy_be_posStack.slaspec

@@ -1,4 +1,4 @@
-define endian=big;
+@define ENDIAN "big"
 
 @define SIZE "4"
 

Ghidra/Processors/Toy/data/languages/toy_builder_be.slaspec

@@ -1,4 +1,4 @@
-define endian=big;
+@define ENDIAN "big"
 
 @define SIZE "4"
 

Ghidra/Processors/Toy/data/languages/toy_builder_be_align2.slaspec

@@ -1,4 +1,4 @@
-define endian=big;
+@define ENDIAN "big"
 define alignment=2;
 
 @define SIZE "4"

Ghidra/Processors/Toy/data/languages/toy_builder_le.slaspec

@@ -1,4 +1,4 @@
-define endian=little;
+@define ENDIAN "little"
 
 @define SIZE "4"
 

Ghidra/Processors/Toy/data/languages/toy_builder_le_align2.slaspec

@@ -1,4 +1,4 @@
-define endian=little;
+@define ENDIAN "little"
 define alignment=2;
 
 @define SIZE "4"

Ghidra/Processors/Toy/data/languages/toy_harvard.pspec

@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<processor_spec>
+  <programcounter register="pc"/>
+  <data_space space="data"/>
+</processor_spec>

Ghidra/Processors/Toy/data/languages/toy_le.slaspec

@@ -1,4 +1,4 @@
-define endian=little;
+@define ENDIAN "little"
 
 @define SIZE "4"
 

Ghidra/Processors/Toy/data/languages/toy_wsz_be.slaspec

@@ -1,4 +1,4 @@
-define endian=big;
+@define ENDIAN "big"
 define alignment=2;
 
 @define SIZE "4"

Ghidra/Processors/Toy/data/languages/toy_wsz_le.slaspec

@@ -1,4 +1,4 @@
-define endian=little;
+@define ENDIAN "little"
 define alignment=2;
 
 @define SIZE "4"