From 084d2f17357e09812d6f0b3a81b2e3ff0266efe4 Mon Sep 17 00:00:00 2001 From: dev747368 <48332326+dev747368@users.noreply.github.com> Date: Thu, 13 May 2021 16:14:32 -0400 Subject: [PATCH] GP-42 fix result dialog, age text field ui --- .../PDB/src/main/java/pdb/LoadPdbTask.java | 43 ++++++---------- .../PDB/src/main/java/pdb/PdbPlugin.java | 38 ++++++++++++-- .../pdb/symbolserver/ui/LoadPdbDialog.java | 50 +++++++++++++------ .../widgets/textfield/HexOrDecimalInput.java | 21 +++++++- 4 files changed, 103 insertions(+), 49 deletions(-) diff --git a/Ghidra/Features/PDB/src/main/java/pdb/LoadPdbTask.java b/Ghidra/Features/PDB/src/main/java/pdb/LoadPdbTask.java index 4940348759..09ed60a6c9 100644 --- a/Ghidra/Features/PDB/src/main/java/pdb/LoadPdbTask.java +++ b/Ghidra/Features/PDB/src/main/java/pdb/LoadPdbTask.java @@ -19,8 +19,6 @@ import java.io.File; import java.io.IOException; import java.lang.reflect.InvocationTargetException; -import docking.DockingWindowManager; -import docking.widgets.dialogs.MultiLineMessageDialog; import ghidra.app.plugin.core.analysis.*; import ghidra.app.plugin.core.datamgr.archive.DuplicateIdException; import ghidra.app.services.DataTypeManagerService; @@ -32,7 +30,6 @@ import ghidra.app.util.pdb.pdbapplicator.*; import ghidra.framework.options.Options; import ghidra.program.model.address.AddressSetView; import ghidra.program.model.listing.Program; -import ghidra.util.Msg; import ghidra.util.exception.CancelledException; import ghidra.util.task.*; @@ -43,6 +40,8 @@ class LoadPdbTask extends Task { private final boolean useMsDiaParser; private final PdbApplicatorControl control; // PDB Universal Parser only private boolean debugLogging; + private String resultMessages; + private Exception resultException; LoadPdbTask(Program program, File pdbFile, boolean useMsDiaParser, PdbApplicatorControl control, boolean debugLogging, DataTypeManagerService service) { @@ -98,39 +97,27 @@ class LoadPdbTask extends Task { try { AutoAnalysisManager.getAnalysisManager(program).scheduleWorker(worker, null, true, wrappedMonitor); - if (log.hasMessages()) { - MultiLineMessageDialog dialog = new MultiLineMessageDialog("Load PDB File", - "There were warnings/errors loading the PDB file.", log.toString(), - MultiLineMessageDialog.WARNING_MESSAGE, false); - DockingWindowManager.showDialog(null, dialog); - } } - catch (InterruptedException | CancelledException e1) { + catch (InterruptedException | CancelledException e) { // ignore } catch (InvocationTargetException e) { - String message; - - Throwable t = e.getCause(); - - if (t == null) { - message = "Unknown cause"; - } - else { - message = t.getMessage(); - - if (message == null) { - message = t.toString(); - } - } - - message = "Error processing PDB file: " + pdbFile + ".\n" + message; - - Msg.showError(getClass(), null, "Load PDB Failed", message, t); + resultException = e; + } + if (log.hasMessages()) { + resultMessages = log.toString(); } } + String getResultMessages() { + return resultMessages; + } + + Exception getResultException() { + return resultException; + } + private boolean parseWithMsDiaParser(MessageLog log, TaskMonitor monitor) throws IOException, CancelledException { PdbParser parser = new PdbParser(pdbFile, program, service, true, true, monitor); diff --git a/Ghidra/Features/PDB/src/main/java/pdb/PdbPlugin.java b/Ghidra/Features/PDB/src/main/java/pdb/PdbPlugin.java index c3e31a9e13..b3b65364c0 100644 --- a/Ghidra/Features/PDB/src/main/java/pdb/PdbPlugin.java +++ b/Ghidra/Features/PDB/src/main/java/pdb/PdbPlugin.java @@ -17,14 +17,17 @@ package pdb; import java.io.File; import java.io.IOException; +import java.lang.reflect.InvocationTargetException; import java.util.*; import java.util.stream.Collectors; import javax.swing.SwingConstants; +import docking.DockingWindowManager; import docking.action.builder.ActionBuilder; import docking.tool.ToolConstants; import docking.widgets.OptionDialog; +import docking.widgets.dialogs.MultiLineMessageDialog; import ghidra.app.CorePluginPackage; import ghidra.app.context.ProgramActionContext; import ghidra.app.plugin.PluginCategoryNames; @@ -118,12 +121,14 @@ public class PdbPlugin extends Plugin { } } + File pdbFile = null; try { LoadPdbResults loadPdbResults = LoadPdbDialog.choosePdbForProgram(program); if (loadPdbResults == null) { tool.setStatusInfo("Loading PDB was cancelled."); return; } + pdbFile = loadPdbResults.pdbFile; tool.setStatusInfo(""); @@ -138,16 +143,39 @@ public class PdbPlugin extends Plugin { // note: We intentionally use a 0-delay here. Our underlying task may show modal // dialog prompts. We want the task progress dialog to be showing before any // prompts appear. - LoadPdbTask loadPdbTask = new LoadPdbTask(program, loadPdbResults.pdbFile, - loadPdbResults.useMsDiaParser, loadPdbResults.control, - loadPdbResults.debugLogging, dataTypeManagerService); + LoadPdbTask loadPdbTask = + new LoadPdbTask(program, pdbFile, loadPdbResults.useMsDiaParser, + loadPdbResults.control, loadPdbResults.debugLogging, dataTypeManagerService); TaskBuilder.withTask(loadPdbTask) .setStatusTextAlignment(SwingConstants.LEADING) .setLaunchDelay(0); new TaskLauncher(loadPdbTask, null, 0); + + // Check for error messages & exceptions and handle them here + // (previously handled by the task, but dialog parenting issues in a modal + // task cause timing issues) + if (loadPdbTask.getResultException() != null) { + throw loadPdbTask.getResultException(); + } + else if (loadPdbTask.getResultMessages() != null) { + MultiLineMessageDialog dialog = new MultiLineMessageDialog("Load PDB File", + "There were warnings/errors loading PDB file: " + pdbFile, + loadPdbTask.getResultMessages(), + MultiLineMessageDialog.WARNING_MESSAGE, false); + DockingWindowManager.showDialog(null, dialog); + } } - catch (Exception pe) { - Msg.showError(getClass(), null, "Error Loading PDB", pe.getMessage(), pe); + catch (Exception e) { + String message = null; + if (e instanceof InvocationTargetException && e.getCause() != null) { + message = + Objects.requireNonNullElse(e.getCause().getMessage(), e.getCause().toString()); + } + else { + message = Objects.requireNonNullElse(e.getMessage(), e.toString()); + } + Msg.showError(this, null, "Error Loading PDB", + "Error processing PDB file: " + pdbFile + "\n" + message, e); } } diff --git a/Ghidra/Features/PDB/src/main/java/pdb/symbolserver/ui/LoadPdbDialog.java b/Ghidra/Features/PDB/src/main/java/pdb/symbolserver/ui/LoadPdbDialog.java index 00087d802c..f0c01be21a 100644 --- a/Ghidra/Features/PDB/src/main/java/pdb/symbolserver/ui/LoadPdbDialog.java +++ b/Ghidra/Features/PDB/src/main/java/pdb/symbolserver/ui/LoadPdbDialog.java @@ -37,8 +37,8 @@ import docking.widgets.filechooser.GhidraFileChooser; import docking.widgets.filechooser.GhidraFileChooserMode; import docking.widgets.label.GIconLabel; import docking.widgets.label.GLabel; +import docking.widgets.textfield.HexOrDecimalInput; import docking.widgets.textfield.HintTextField; -import docking.widgets.textfield.IntegerTextField; import ghidra.app.util.bin.format.pdb.PdbParser; import ghidra.app.util.pdb.pdbapplicator.PdbApplicatorControl; import ghidra.framework.preferences.Preferences; @@ -92,7 +92,7 @@ public class LoadPdbDialog extends DialogComponentProvider { LoadPdbResults results = new LoadPdbResults(); results.pdbFile = pdbFile; results.control = - (PdbApplicatorControl) choosePdbDlg.restrictionsCombo.getSelectedItem(); + (PdbApplicatorControl) choosePdbDlg.applicatorControlCombo.getSelectedItem(); results.useMsDiaParser = choosePdbDlg.msdiaParserButton.isSelected(); results.debugLogging = choosePdbDlg.debugLoggingCheckbox.isSelected(); return results; @@ -119,7 +119,7 @@ public class LoadPdbDialog extends DialogComponentProvider { private GCheckBox overridePdbPathCheckBox; private JTextField pdbUniqueIdTextField; private GCheckBox overridePdbUniqueIdCheckBox; - private IntegerTextField pdbAgeTextField; + private HexOrDecimalInput pdbAgeTextField; private GCheckBox overridePdbAgeCheckBox; private HintTextField pdbLocationTextField; private GIconLabel exactMatchIconLabel; @@ -138,7 +138,7 @@ public class LoadPdbDialog extends DialogComponentProvider { private JPanel parserOptionsPanel; private JRadioButton universalParserButton; private JRadioButton msdiaParserButton; - private GComboBox restrictionsCombo; + private GComboBox applicatorControlCombo; private GCheckBox debugLoggingCheckbox; /** @@ -327,7 +327,8 @@ public class LoadPdbDialog extends DialogComponentProvider { if (symbolServerService == null || !symbolServerService.isValid()) { return; } - if (pdbAgeTextField.getText().isBlank()) { + if (pdbAgeTextField.getText().isBlank() || + pdbAgeTextField.getValue() > NumericUtilities.MAX_UNSIGNED_INT32_AS_LONG) { Msg.showWarn(this, null, "Bad PDB Age", "Invalid PDB Age value"); return; } @@ -390,7 +391,8 @@ public class LoadPdbDialog extends DialogComponentProvider { } private void buildSymbolFilePanel() { - symbolFilePanel = new SymbolFilePanel(this::searchForPdbs); // panel will be added in layoutAdvanced() + // panel will be added in layoutAdvanced() + symbolFilePanel = new SymbolFilePanel(this::searchForPdbs); symbolFilePanel.getTable() .getSelectionModel() @@ -464,9 +466,8 @@ public class LoadPdbDialog extends DialogComponentProvider { new HelpLocation(PdbPlugin.PDB_PLUGIN_HELP_TOPIC, SymbolFilePanel.SEARCH_OPTIONS_HELP_ANCHOR)); - pdbAgeTextField = new IntegerTextField(8); - pdbAgeTextField.setAllowNegativeValues(false); - pdbAgeTextField.setShowNumberMode(true); + pdbAgeTextField = new BetterNonEditableHexTextField(8); + pdbAgeTextField.setAllowNegative(false); pdbAgeTextField.setHexMode(); pdbAgeTextField.setEditable(false); @@ -502,7 +503,7 @@ public class LoadPdbDialog extends DialogComponentProvider { programPdbPanel.add( join(null, new GLabel("PDB Age:", SwingConstants.RIGHT), overridePdbAgeCheckBox)); - programPdbPanel.add(join(pdbAgeTextField.getComponent(), new JPanel(), null)); + programPdbPanel.add(join(pdbAgeTextField, new JPanel(), null)); return programPdbPanel; } @@ -547,7 +548,7 @@ public class LoadPdbDialog extends DialogComponentProvider { if (universalParserButton.isSelected() && !universalParserButton.isEnabled()) { universalParserButton.setSelected(false); } - restrictionsCombo.setEnabled(universalParserButton.isSelected()); + applicatorControlCombo.setEnabled(universalParserButton.isSelected()); debugLoggingCheckbox.setEnabled(universalParserButton.isSelected()); } @@ -573,8 +574,8 @@ public class LoadPdbDialog extends DialogComponentProvider { radioButtons.add(universalParserButton); radioButtons.add(msdiaParserButton); - restrictionsCombo = new GComboBox<>(PdbApplicatorControl.values()); - restrictionsCombo.setSelectedItem(PdbApplicatorControl.ALL); + applicatorControlCombo = new GComboBox<>(PdbApplicatorControl.values()); + applicatorControlCombo.setSelectedItem(PdbApplicatorControl.ALL); debugLoggingCheckbox = new GCheckBox(); debugLoggingCheckbox.setToolTipText( @@ -591,7 +592,7 @@ public class LoadPdbDialog extends DialogComponentProvider { parserOptionsPanel.add(radioButtons); parserOptionsPanel.add(new GLabel("Control:")); - parserOptionsPanel.add(restrictionsCombo); + parserOptionsPanel.add(applicatorControlCombo); parserOptionsPanel.add(new GLabel("[Dev] PDB Reader/Applicator Debug Logging:")); parserOptionsPanel.add(debugLoggingCheckbox); @@ -928,7 +929,7 @@ public class LoadPdbDialog extends DialogComponentProvider { @Override public Color getBackground() { Container parent = getParent(); - if (parent != null && isEditable() == false) { + if (parent != null && !isEditable()) { Color bg = parent.getBackground(); // mint a new Color object to avoid it being // ignored because the parent handed us a DerivedColor @@ -939,4 +940,23 @@ public class LoadPdbDialog extends DialogComponentProvider { } } + static class BetterNonEditableHexTextField extends HexOrDecimalInput { + + BetterNonEditableHexTextField(int columns) { + super(columns); + } + + @Override + public Color getBackground() { + Container parent = getParent(); + if (parent != null && !isEditable()) { + Color bg = parent.getBackground(); + // mint a new Color object to avoid it being + // ignored because the parent handed us a DerivedColor + // instance + return new Color(bg.getRGB()); + } + return super.getBackground(); + } + } } diff --git a/Ghidra/Framework/Docking/src/main/java/docking/widgets/textfield/HexOrDecimalInput.java b/Ghidra/Framework/Docking/src/main/java/docking/widgets/textfield/HexOrDecimalInput.java index 8d9700c7e9..9614fa62ad 100644 --- a/Ghidra/Framework/Docking/src/main/java/docking/widgets/textfield/HexOrDecimalInput.java +++ b/Ghidra/Framework/Docking/src/main/java/docking/widgets/textfield/HexOrDecimalInput.java @@ -33,8 +33,16 @@ public class HexOrDecimalInput extends JTextField { this(null); } + public HexOrDecimalInput(int columns) { + super(columns); + init(null); + } + public HexOrDecimalInput(Long initialValue) { - super(); + init(initialValue); + } + + private void init(Long initialValue) { currentValue = initialValue; setDocument(new MyDocument()); updateText(); @@ -55,6 +63,17 @@ public class HexOrDecimalInput extends JTextField { return currentValue; } + public int getIntValue() { + if (currentValue == null) { + return 0; + } + return currentValue.intValue(); + } + + public void setValue(int newValue) { + setValue((long) newValue); + } + public void setValue(Long newValue) { if (!allowsNegative && newValue != null && newValue.longValue() < 0) { currentValue = null;