Merge remote-tracking branch 'origin/GP-5999_dev747368_fix_coff_NPE'

'origin/GP-5995_ghidra1_DynamicPointerLabels' into patch (Closes #8510)

Ghidra/Configurations/Public_Release/src/global/docs/WhatsNew.md

@@ -153,10 +153,12 @@ concrete emulator, effectively constructing what is commonly called a "concolic"
 symbolic emulator creates Z3 expressions and branching constraints, but it only follows the path 
 determined by concrete emulation. This is most easily accessed by installing the "SymbolicSummaryZ3"
 extension (**File** → **Install Extensions**) and then enabling the `Z3SummaryPlugin` in the 
-Debugger or Emulator tool, which includes a GUI for viewing and sorting through the results. Before 
-using the Z3 emulator, you must download and install z3-4.13.0 from https://github.com/Z3Prover/z3. 
-Depending on your platform, you may need to build it from source. Other versions may work, but our 
-current test configuration uses 4.13.0.
+Debugger or Emulator tool, which includes a GUI for viewing and sorting through the results. The Z3
+emulator requires z3-4.13.0, available from https://github.com/Z3Prover/z3. Other versions may work,
+but our current test configuration uses 4.13.0. Depending on the release and your platform, the
+required libraries may be missing or incompatible. If this is the case, you will need to download
+Z3, or build it from source with Java bindings, and install the libraries into 
+`Ghidra/Extensions/SymbolicSummaryZ3/os/<platform>/`.
 
 ## Emulation API
 The `PcodeEmulator` and related API has undergone substantial changes in preparation for integrating

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

@@ -481,22 +481,23 @@ public class CodeViewerProvider extends NavigatableComponentProviderAdapter
 		action = new GotoNextFunctionAction(tool, plugin.getName());
 		tool.addAction(action);
 
-		toggleVariablesAction = new ToggleActionBuilder("Show Function Variables", plugin.getName())
-				.popupMenuPath("Show/Hide All Variables")
+		toggleVariablesAction =
+			new ToggleActionBuilder("Show All Function Variables", plugin.getName())
+					.popupMenuPath("Function", "Show/Hide All Variables")
 					.popupMenuGroup("Variables")
 					.helpLocation(new HelpLocation("CodeBrowserPlugin", "Show_All_Variables"))
 					.selected(true)
 					.withContext(ProgramLocationActionContext.class)
-				.enabledWhen(this::isInFunctionArea)
 					.onAction(c -> showVariablesForAllFunctions(toggleVariablesAction.isSelected()))
 					.buildAndInstallLocal(this);
 
 		new ActionBuilder("Toggle Show Function Variables", plugin.getName())
-				.popupMenuPath("Show/Hide Variables")
+				.popupMenuPath("Function", "Show/Hide Variables")
 				.popupMenuGroup("Variables")
 				.helpLocation(new HelpLocation("CodeBrowserPlugin", "Show_Variables"))
 				.keyBinding("SPACE")
 				.withContext(ProgramLocationActionContext.class)
+				.validWhen(this::isInFunctionArea)
 				.enabledWhen(this::isInFunctionArea)
 				.onAction(c -> toggleShowVariables(c.getAddress()))
 				.buildAndInstallLocal(this);
@@ -508,7 +509,7 @@ public class CodeViewerProvider extends NavigatableComponentProviderAdapter
 	private void toggleShowVariables(Address address) {
 		ListingModel model = listingPanel.getListingModel();
 		boolean open = model.areFunctionVariablesOpen(address);
-		model.setAllFunctionVariablesOpen(!open);
+		model.setFunctionVariablesOpen(address, !open);
 		setLocation(new VariablesOpenCloseLocation(program, address));
 	}
 

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

@@ -42,7 +42,8 @@ public class CollapseAllDataAction extends ProgramLocationContextAction {
 		super("Collapse All Data", provider.getOwner());
 		this.provider = provider;
 
-		setPopupMenuData(new MenuData(new String[] { "Collapse All Data" }, null, "Structure"));
+		setPopupMenuData(
+			new MenuData(new String[] { "Data", "Collapse All Data" }, null, "BasicData2"));
 
 		setHelpLocation(new HelpLocation("CodeBrowserPlugin", "ExpandCollapseActions"));
 

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

@@ -39,7 +39,8 @@ public class ExpandAllDataAction extends ProgramLocationContextAction {
 		super("Expand All Data", provider.getOwner());
 		this.provider = provider;
 
-		setPopupMenuData(new MenuData(new String[] { "Expand All Data" }, null, "Structure"));
+		setPopupMenuData(
+			new MenuData(new String[] { "Data", "Expand All Data" }, null, "BasicData2"));
 		setDescription("Open all data recursively from the current location downward.");
 
 		setHelpLocation(new HelpLocation("CodeBrowserPlugin", "ExpandCollapseActions"));
@@ -83,11 +84,11 @@ public class ExpandAllDataAction extends ProgramLocationContextAction {
 
 	private void updatePopupMenuName(boolean hasSelection) {
 		if (hasSelection) {
-			getPopupMenuData().setMenuPath(new String[] { "Expand All Data In Selection" });
+			getPopupMenuData().setMenuPath(new String[] { "Data", "Expand All Data In Selection" });
 			setDescription("Open all data recursively in the current selection.");
 		}
 		else {
-			getPopupMenuData().setMenuPath(new String[] { "Expand All Data" });
+			getPopupMenuData().setMenuPath(new String[] { "Data", "Expand All Data" });
 			setDescription("Open all data recursively from the current location downward.");
 		}
 	}

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

@@ -42,7 +42,8 @@ public class ToggleExpandCollapseDataAction extends ProgramLocationContextAction
 		this.provider = provider;
 
 		setPopupMenuData(
-			new MenuData(new String[] { "Toggle Expand/Collapse Data" }, null, "Structure"));
+			new MenuData(new String[] { "Data", "Toggle Expand/Collapse Data" }, null,
+				"BasicData2"));
 		setKeyBindingData(new KeyBindingData(' ', 0));
 
 		setHelpLocation(new HelpLocation("CodeBrowserPlugin", "ExpandCollapseActions"));

Ghidra/Features/Base/src/main/java/ghidra/app/util/bin/format/coff/archive/CoffArchiveHeader.java

@@ -16,8 +16,7 @@
 package ghidra.app.util.bin.format.coff.archive;
 
 import java.io.IOException;
-import java.util.ArrayList;
-import java.util.List;
+import java.util.*;
 
 import ghidra.app.util.bin.*;
 import ghidra.app.util.bin.format.coff.CoffException;
@@ -42,13 +41,14 @@ public final class CoffArchiveHeader implements StructConverter {
 	 * Returns true if the data contained in the {@link ByteProvider provider} contains
 	 * a COFF Archive file.
 	 * 
-	 * @param provider
-	 * @return
-	 * @throws IOException
+	 * @param provider {@link ByteProvider} stream
+	 * @return boolean true if stream contains a CoffArchiveHeader at position 0
+	 * @throws IOException if error reading
 	 */
 	public static boolean isMatch(ByteProvider provider) throws IOException {
-		return (provider.length() > CoffArchiveConstants.MAGIC_LEN) && CoffArchiveConstants.MAGIC
-				.equals(new String(provider.readBytes(0, CoffArchiveConstants.MAGIC_LEN)));
+		return (provider.length() >= CoffArchiveConstants.MAGIC_LEN) &&
+			Arrays.equals(CoffArchiveConstants.MAGIC_BYTES,
+				provider.readBytes(0, CoffArchiveConstants.MAGIC_LEN));
 	}
 
 	/**
@@ -75,9 +75,7 @@ public final class CoffArchiveHeader implements StructConverter {
 
 		CoffArchiveHeader cah = new CoffArchiveHeader();
 
-		long eofPos = reader.length() - CoffArchiveMemberHeader.CAMH_MIN_SIZE;
-
-		while (reader.getPointerIndex() < eofPos) {
+		while (reader.hasNext(CoffArchiveMemberHeader.CAMH_MIN_SIZE)) {
 			if (monitor.isCancelled()) {
 				break;
 			}

Ghidra/Features/Base/src/main/java/ghidra/app/util/viewer/format/FieldFormatModel.java

@@ -83,7 +83,7 @@ public class FieldFormatModel {
 	}
 
 	/**
-	 * Returns the formatMgr that is managing this model.
+	 * {@return the FormatManager managing this format.}
 	 */
 	public FormatManager getFormatManager() {
 		return formatMgr;
@@ -109,6 +109,7 @@ public class FieldFormatModel {
 	/**
 	 * Adds new empty row at the given position.  The position must be in the
 	 * interval [0,numRows].
+	 * @param index the index to add a new row
 	 * @exception IllegalArgumentException thrown if the position is outside the
 	 * interval [0,numRows].
 	 */
@@ -186,21 +187,23 @@ public class FieldFormatModel {
 	}
 
 	/**
-	 * Returns the number of rows in the model.
+	 * {@return the number of rows in this format}
 	 */
 	public int getNumRows() {
 		return rows.size();
 	}
 
 	/**
-	 * Returns the name of this format model.
+	 * {@return the name of this format model.}
 	 */
 	public String getName() {
 		return name;
 	}
 
 	/**
-	 * Returns the number of FieldFactorys on any given row.
+	 * Returns the number of factories on the given row.
+	 * @param row the row to get the number of factories for
+	 * @return the number of factories on the given row
 	 */
 	public int getNumFactorys(int row) {
 		if ((row < 0) || (row >= rows.size())) {
@@ -210,14 +213,16 @@ public class FieldFormatModel {
 	}
 
 	/**
-	 * Returns the FieldFactorys on a given row.
+	 * Returns the factories on the given row.
+	 * @param row the row to factories for
+	 * @return the FieldFactorys for a given row
 	 */
 	public FieldFactory[] getFactorys(int row) {
 		return (rows.get(row)).getFactorys();
 	}
 
 	/**
-	 * Returns the list factories valid for this format.
+	 * {@return the list of factories used in this format.}
 	 */
 	public FieldFactory[] getFactorys() {
 		return factories.clone();
@@ -251,7 +256,7 @@ public class FieldFormatModel {
 	}
 
 	/**
-	 * Returns the width of this model
+	 * {@return the width of this format.}
 	 */
 	public int getWidth() {
 		return width;
@@ -280,6 +285,7 @@ public class FieldFormatModel {
 
 	/**
 	 * Saves this format to XML.
+	 * @return the XML element for the saved format
 	 */
 	public Element saveToXml() {
 		Element root = new Element("FORMAT");

Ghidra/Features/Base/src/main/java/ghidra/app/util/viewer/format/FormatManager.java

@@ -959,6 +959,9 @@ public class FormatManager implements OptionsChangeListener {
 		for (int i = 0; i < NUM_MODELS; i++) {
 			if (saveState.hasValue(models[i].getName())) {
 				models[i].restoreFromXml(saveState.getXmlElement(models[i].getName()));
+				// hack to make sure the new open/close variables field is present
+				// If missing, we are just going to reset it to the default format
+				checkForMissingOpenCloseField(models[i]);
 			}
 			else {
 				models[i].restoreFromXml(getDefaultModel(i));
@@ -968,6 +971,27 @@ public class FormatManager implements OptionsChangeListener {
 		modelChanged(null);
 	}
 
+	// This is a hack to make sure the new variables open/close field is present.
+	// This was added in version 12.0 and can probably be removed in a few releases.
+	private void checkForMissingOpenCloseField(FieldFormatModel model) {
+		if (!model.getName().equals("Variable")) {
+			return;
+		}
+		if (!hasField(model, "+")) {
+			model.restoreFromXml(getDefaultVariableFormat());
+		}
+	}
+
+	private boolean hasField(FieldFormatModel model, String fieldName) {
+		FieldFactory[] unusedFactories = model.getUnusedFactories();
+		for (FieldFactory fieldFactory : unusedFactories) {
+			if (fieldFactory.getFieldName().equals("+")) {
+				return false;
+			}
+		}
+		return true;
+	}
+
 	public ServiceProvider getServiceProvider() {
 		return serviceProvider;
 	}

Ghidra/Features/Base/src/main/java/ghidra/features/base/quickfix/QuickFixTableProvider.java

@@ -47,7 +47,7 @@ import ghidra.util.task.TaskMonitor;
  * Component Provider for displaying lists of {@link QuickFix}s and the actions to execute them
  * in bulk or individually. 
  */
-public class QuckFixTableProvider extends ComponentProvider {
+public class QuickFixTableProvider extends ComponentProvider {
 	private static final Icon EXECUTE_ICON = new GIcon("icon.base.plugin.quickfix.done");
 	private JComponent component;
 	private QuickFixTableModel tableModel;
@@ -57,7 +57,7 @@ public class QuckFixTableProvider extends ComponentProvider {
 	private ToggleDockingAction toggleAutoDeleteAction;
 	private boolean autoDelete;
 
-	public QuckFixTableProvider(PluginTool tool, String title, String owner, Program program,
+	public QuickFixTableProvider(PluginTool tool, String title, String owner, Program program,
 			TableDataLoader<QuickFix> loader) {
 		super(tool, title, owner);
 		setIcon(new GIcon("icon.plugin.table.service"));
@@ -168,7 +168,7 @@ public class QuckFixTableProvider extends ComponentProvider {
 			if (e.getValueIsAdjusting()) {
 				return;
 			}
-			dockingTool.contextChanged(QuckFixTableProvider.this);
+			dockingTool.contextChanged(QuickFixTableProvider.this);
 		});
 
 		table.setActionsEnabled(true);
@@ -314,7 +314,7 @@ public class QuckFixTableProvider extends ComponentProvider {
 //==================================================================================================
 	private class QuickFixActionContext extends DefaultActionContext {
 		QuickFixActionContext() {
-			super(QuckFixTableProvider.this, table);
+			super(QuickFixTableProvider.this, table);
 		}
 
 		public int getSelectedRowCount() {

Ghidra/Features/Base/src/main/java/ghidra/features/base/replace/SearchAndReplaceProvider.java

@@ -26,10 +26,10 @@ import ghidra.util.HelpLocation;
 import ghidra.util.Msg;
 
 /**
- * Subclass of the {@link QuckFixTableProvider} that customizes it specifically for search and replace
+ * Subclass of the {@link QuickFixTableProvider} that customizes it specifically for search and replace
  * operations.
  */
-public class SearchAndReplaceProvider extends QuckFixTableProvider {
+public class SearchAndReplaceProvider extends QuickFixTableProvider {
 
 	private SearchAndReplacePlugin plugin;
 	private SearchAndReplaceQuery query;

Ghidra/Features/Base/src/test.slow/java/ghidra/program/model/symbol/SymbolUtilities2Test.java

@@ -130,6 +130,56 @@ public class SymbolUtilities2Test extends AbstractGhidraHeadedIntegrationTest {
 		assertEquals("s__CODE_0200", symbol.getName());
 	}
 
+	@Test
+	public void testDynamicPTRLabel() throws Exception {
+		// 50 -> 100(pointer) -> 200(pointer) -> 300(pointer) -> 400(byte)XYZ
+		listing.createData(addr(0x100), PointerDataType.dataType);
+		listing.createData(addr(0x200), PointerDataType.dataType);
+		listing.createData(addr(0x300), PointerDataType.dataType);
+		listing.createData(addr(0x400), ByteDataType.dataType);
+		symbolTable.createLabel(addr(0x400), "XYZ", SourceType.USER_DEFINED);
+		refMgr.addMemoryReference(addr(0x50), addr(0x100), RefType.DATA, SourceType.USER_DEFINED,
+			0);
+		refMgr.addMemoryReference(addr(0x100), addr(0x200), RefType.DATA, SourceType.USER_DEFINED,
+			0);
+		refMgr.addMemoryReference(addr(0x200), addr(0x300), RefType.DATA, SourceType.USER_DEFINED,
+			0);
+		refMgr.addMemoryReference(addr(0x300), addr(0x400), RefType.DATA, SourceType.USER_DEFINED,
+			0);
+
+		Symbol symbol = symbolTable.getPrimarySymbol(addr(0x100));
+		assertEquals("PTR_PTR_CODE_0100", symbol.getName());
+		symbol = symbolTable.getPrimarySymbol(addr(0x200));
+		assertEquals("PTR_PTR_CODE_0200", symbol.getName());
+		symbol = symbolTable.getPrimarySymbol(addr(0x300));
+		assertEquals("PTR_XYZ_CODE_0300", symbol.getName());
+	}
+
+	@Test
+	public void testDynamicPTRLOOP1Label() throws Exception {
+		// 100(pointer) -> 100(pointer)
+		listing.createData(addr(0x100), PointerDataType.dataType);
+		refMgr.addMemoryReference(addr(0x100), addr(0x100), RefType.READ, SourceType.USER_DEFINED,
+			0);
+		Symbol symbol = symbolTable.getPrimarySymbol(addr(0x100));
+		assertEquals("PTR_LOOP_CODE_0100", symbol.getName());
+	}
+
+	@Test
+	public void testDynamicPTRLOOP2Label() throws Exception {
+		// 100(pointer) -> 200(pointer) -> 100(pointer)
+		listing.createData(addr(0x100), PointerDataType.dataType);
+		listing.createData(addr(0x200), PointerDataType.dataType);
+		refMgr.addMemoryReference(addr(0x100), addr(0x200), RefType.READ, SourceType.USER_DEFINED,
+			0);
+		refMgr.addMemoryReference(addr(0x200), addr(0x100), RefType.READ, SourceType.USER_DEFINED,
+			0);
+		Symbol symbol = symbolTable.getPrimarySymbol(addr(0x100));
+		assertEquals("PTR_LOOP_CODE_0100", symbol.getName());
+		symbol = symbolTable.getPrimarySymbol(addr(0x200));
+		assertEquals("PTR_LOOP_CODE_0200", symbol.getName());
+	}
+
 	@Test
 	public void testParseDynamicName() {
 		assertEquals(addr(0x100),

Ghidra/Framework/Docking/src/main/java/docking/actions/SharedActionRegistry.java

@@ -18,6 +18,7 @@ package docking.actions;
 import docking.*;
 import docking.action.DockingActionIf;
 import docking.tool.ToolConstants;
+import docking.widgets.filechooser.GhidraFileChooser;
 import docking.widgets.table.GTable;
 import docking.widgets.tree.GTree;
 
@@ -36,11 +37,11 @@ public class SharedActionRegistry {
 	 */
 	public static void installSharedActions(Tool tool, ToolActions toolActions) {
 		GTable.createSharedActions(tool, toolActions, ToolConstants.SHARED_OWNER);
-
 		GTree.createSharedActions(tool, toolActions, ToolConstants.SHARED_OWNER);
 
 		DialogComponentProvider.createSharedActions(tool, toolActions, ToolConstants.SHARED_OWNER);
-
 		DockingWindowManager.createSharedActions(tool, toolActions, ToolConstants.SHARED_OWNER);
+
+		GhidraFileChooser.registerSharedActions(tool, toolActions);
 	}
 }

Ghidra/Framework/Docking/src/main/java/docking/widgets/filechooser/GhidraFileChooser.java

@@ -40,7 +40,7 @@ import docking.*;
 import docking.action.DockingAction;
 import docking.action.DockingActionIf;
 import docking.action.builder.ActionBuilder;
-import docking.actions.KeyBindingUtils;
+import docking.actions.*;
 import docking.menu.DockingToolBarUtils;
 import docking.widgets.*;
 import docking.widgets.combobox.GComboBox;
@@ -87,6 +87,10 @@ public class GhidraFileChooser extends ReusableDialogComponentProvider implement
 	 */
 	private static final int BIG_DATA_THRESHOLD = 200;
 
+	private static final String ACTION_NAME_BACK = "Last Folder Visited";
+	private static final String ACTION_NAME_FORWARD = "Previous Folder Visited";
+	private static final String ACTION_NAME_UP = "Up One Level";
+
 	static final String UP_BUTTON_NAME = "UP_BUTTON";
 	private static final Color FOREROUND_COLOR = new GColor("color.fg.filechooser");
 	private static final Color BACKGROUND_COLOR = new GColor("color.bg.filechooser");
@@ -285,16 +289,19 @@ public class GhidraFileChooser extends ReusableDialogComponentProvider implement
 
 		String owner = getClass().getSimpleName();
 		upAction = new ActionBuilder("Up One Level", owner)
+				.sharedKeyBinding()
 				.keyBinding("Alt Up")
 				.onAction(c -> goUp())
 				.build();
 
 		backAction = new ActionBuilder("Last Folder Visited", owner)
+				.sharedKeyBinding()
 				.keyBinding("Alt Left")
 				.onAction(c -> goBack())
 				.build();
 
 		forwardAction = new ActionBuilder("Previous Folder Visited", owner)
+				.sharedKeyBinding()
 				.keyBinding("Alt Right")
 				.onAction(c -> goForward())
 				.build();
@@ -310,6 +317,18 @@ public class GhidraFileChooser extends ReusableDialogComponentProvider implement
 		updateNavigationButtonToolTips();
 	}
 
+	public static void registerSharedActions(Tool tool, ToolActions toolActions) {
+
+		toolActions.registerSharedActionPlaceholder(
+			new GfcActionPlaceholder(ACTION_NAME_BACK, "Alt Left"));
+
+		toolActions.registerSharedActionPlaceholder(
+			new GfcActionPlaceholder(ACTION_NAME_FORWARD, "Alt Right"));
+
+		toolActions.registerSharedActionPlaceholder(
+			new GfcActionPlaceholder(ACTION_NAME_UP, "Alt Up"));
+	}
+
 	private JComponent buildWorkPanel() {
 		buildWaitPanel();
 
@@ -2530,4 +2549,30 @@ public class GhidraFileChooser extends ReusableDialogComponentProvider implement
 
 	}
 
+	// A class that allows us to register actions and keybindings before this dialog is instantiated
+	private static class GfcActionPlaceholder implements SharedDockingActionPlaceholder {
+
+		private String name;
+		private String keyBinding;
+
+		GfcActionPlaceholder(String name, String keyBinding) {
+			this.name = name;
+			this.keyBinding = keyBinding;
+		}
+
+		@Override
+		public String getName() {
+			return name;
+		}
+
+		@Override
+		public KeyStroke getKeyBinding() {
+			return KeyBindingUtils.parseKeyStroke(keyBinding);
+		}
+
+		@Override
+		public String getOwner() {
+			return GhidraFileChooser.class.getSimpleName();
+		}
+	}
 }

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

@@ -259,7 +259,7 @@ public class PointerDataType extends BuiltIn implements Pointer {
 		while (ref != null && ref.isMemoryReference()) {
 			Address toAddr = ref.getToAddress();
 			if (!refAddrs.add(toAddr)) {
-				break;
+				return PointerReferenceClassification.LOOP;
 			}
 			if (++depth > 2) {
 				return PointerReferenceClassification.DEEP;