From 7bd553d55dd4d0bd8727946e1e462d7aa6201716 Mon Sep 17 00:00:00 2001
From: dragonmacher <48328597+dragonmacher@users.noreply.github.com>
Date: Tue, 8 Apr 2025 14:26:12 -0400
Subject: [PATCH] GP-5572 - Decompiler - Updated actions to not show warning
dialog if executed while the Decompiler is busy
---
.../decompile/DecompilerActionContext.java | 62 -------------------
.../actions/AbstractDecompilerAction.java | 11 ++--
.../DecompilerStructureVariableAction.java | 55 ++++++++--------
.../FindReferencesToAddressAction.java | 18 ++----
.../FindReferencesToDataTypeAction.java | 20 +-----
.../TaintAbstractDecompilerAction.java | 8 +--
.../java/docking/DockingKeyBindingAction.java | 9 +++
.../KeyBindingOverrideKeyEventDispatcher.java | 14 +++--
.../docking/action/MultipleKeyAction.java | 44 ++++++-------
9 files changed, 79 insertions(+), 162 deletions(-)
diff --git a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/DecompilerActionContext.java b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/DecompilerActionContext.java
index ce6c51c881..57f0cf48fd 100644
--- a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/DecompilerActionContext.java
+++ b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/DecompilerActionContext.java
@@ -15,10 +15,6 @@
*/
package ghidra.app.plugin.core.decompile;
-import java.util.function.Supplier;
-
-import docking.ActionContext;
-import docking.action.DockingActionIf;
import ghidra.app.context.NavigatableActionContext;
import ghidra.app.context.RestrictedAddressSetContext;
import ghidra.app.decompiler.*;
@@ -28,9 +24,7 @@ import ghidra.framework.plugintool.PluginTool;
import ghidra.program.model.address.Address;
import ghidra.program.model.listing.Function;
import ghidra.program.model.pcode.HighFunction;
-import ghidra.util.Msg;
import ghidra.util.UndefinedFunction;
-import utility.function.Callback;
public class DecompilerActionContext extends NavigatableActionContext
implements RestrictedAddressSetContext {
@@ -168,60 +162,4 @@ public class DecompilerActionContext extends NavigatableActionContext
return null;
}
-
- /**
- * The companion method of {@link #checkActionEnablement(Supplier)}.
- *
- *
- * Decompiler actions must call this method from their
- * {@link DockingActionIf#actionPerformed(ActionContext)} if they require state from the
- * Decompiler.
- *
- * @param actionCallback the action's code to execute
- */
- public void performAction(Callback actionCallback) {
-
- if (isDecompiling) {
- Msg.showInfo(getClass(), getComponentProvider().getComponent(),
- "Decompiler Action Blocked",
- "You cannot perform Decompiler actions while the Decompiler is busy");
- return;
- }
-
- actionCallback.call();
- }
-
- /**
- * The companion method of {@link #performAction(Callback)}.
- *
- *
- * Decompiler actions must call this method from their
- * {@link DockingActionIf#isEnabledForContext(ActionContext)} if they require state from the
- * Decompiler.
- *
- * @param actionBooleanSupplier the action's code to verify its enablement
- * @return true if the action should be considered enabled
- */
- public boolean checkActionEnablement(Supplier actionBooleanSupplier) {
-
- //
- // Unusual Code: actions will call this method when their 'isEnabledForContext()' is
- // called. If the decompiler is still working, we return true here so
- // the action is considered enabled. This allows any key bindings registered
- // for the action to get consumed. If we did not returned false when
- // the decompiler was still working, then the key binding would not match and
- // the system would pass the key binding up to the global action system,
- // which we do not want.
- //
- // Each action that needs state from the decompiler must call this method
- // from 'isEnabledForContext()'. Also, each action must call
- // 'performAction()' on this class, which will skip the action's work and
- // show an message if the decompiler is busy.
- //
- if (isDecompiling()) {
- return true;
- }
-
- return actionBooleanSupplier.get();
- }
}
diff --git a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/AbstractDecompilerAction.java b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/AbstractDecompilerAction.java
index 11c5bf1bf9..8815b32993 100644
--- a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/AbstractDecompilerAction.java
+++ b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/AbstractDecompilerAction.java
@@ -62,17 +62,16 @@ public abstract class AbstractDecompilerAction extends DockingAction {
@Override
public boolean isEnabledForContext(ActionContext context) {
DecompilerActionContext decompilerContext = (DecompilerActionContext) context;
- return decompilerContext.checkActionEnablement(() -> {
- return isEnabledForDecompilerContext(decompilerContext);
- });
+ if (decompilerContext.isDecompiling()) {
+ return false;
+ }
+ return isEnabledForDecompilerContext(decompilerContext);
}
@Override
public void actionPerformed(ActionContext context) {
DecompilerActionContext decompilerContext = (DecompilerActionContext) context;
- decompilerContext.performAction(() -> {
- decompilerActionPerformed(decompilerContext);
- });
+ decompilerActionPerformed(decompilerContext);
}
/**
diff --git a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/DecompilerStructureVariableAction.java b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/DecompilerStructureVariableAction.java
index 9316429f70..7c666d92f6 100644
--- a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/DecompilerStructureVariableAction.java
+++ b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/DecompilerStructureVariableAction.java
@@ -4,9 +4,9 @@
* 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.
@@ -40,37 +40,32 @@ public class DecompilerStructureVariableAction extends CreateStructureVariableAc
return false;
}
- DecompilerActionContext decompilerContext = (DecompilerActionContext) context;
- return decompilerContext.checkActionEnablement(() -> {
+ Function function = controller.getFunction();
+ if (function == null || function instanceof UndefinedFunction) {
+ return false;
+ }
- Function function = controller.getFunction();
- if (function == null || function instanceof UndefinedFunction) {
- return false;
- }
+ DataType dt = null;
+ boolean isThisParam = false;
- DataType dt = null;
- boolean isThisParam = false;
+ // get the data type at the location and see if it is OK
+ DecompilerPanel decompilerPanel = controller.getDecompilerPanel();
+ ClangToken tokenAtCursor = decompilerPanel.getTokenAtCursor();
+ if (tokenAtCursor == null) {
+ return false;
+ }
+ int maxPointerSize = controller.getProgram().getDefaultPointerSize();
+ HighVariable var = tokenAtCursor.getHighVariable();
+ if (var != null && !(var instanceof HighConstant)) {
+ dt = var.getDataType();
+ isThisParam = DecompilerUtils.isThisParameter(var, function);
+ }
- // get the data type at the location and see if it is OK
- DecompilerPanel decompilerPanel = controller.getDecompilerPanel();
- ClangToken tokenAtCursor = decompilerPanel.getTokenAtCursor();
- if (tokenAtCursor == null) {
- return false;
- }
- int maxPointerSize = controller.getProgram().getDefaultPointerSize();
- HighVariable var = tokenAtCursor.getHighVariable();
- if (var != null && !(var instanceof HighConstant)) {
- dt = var.getDataType();
- isThisParam = DecompilerUtils.isThisParameter(var, function);
- }
+ if (dt == null || dt.getLength() > maxPointerSize) {
+ return false;
+ }
- if (dt == null || dt.getLength() > maxPointerSize) {
- return false;
- }
-
- adjustCreateStructureMenuText(dt, isThisParam);
- return true;
-
- });
+ adjustCreateStructureMenuText(dt, isThisParam);
+ return true;
}
}
diff --git a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/FindReferencesToAddressAction.java b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/FindReferencesToAddressAction.java
index 7973ef74b0..9c54ad4569 100644
--- a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/FindReferencesToAddressAction.java
+++ b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/FindReferencesToAddressAction.java
@@ -4,9 +4,9 @@
* 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.
@@ -54,18 +54,8 @@ public class FindReferencesToAddressAction extends AbstractFindReferencesToAddre
}
DecompilerActionContext decompilerContext = (DecompilerActionContext) context;
- return decompilerContext.checkActionEnablement(() -> {
- updateMenuName(decompilerContext.getAddress());
- return super.isEnabledForContext(context);
- });
- }
-
- @Override
- public void actionPerformed(ActionContext context) {
- DecompilerActionContext decompilerContext = (DecompilerActionContext) context;
- decompilerContext.performAction(() -> {
- super.actionPerformed(context);
- });
+ updateMenuName(decompilerContext.getAddress());
+ return super.isEnabledForContext(context);
}
private void updateMenuName(Address addr) {
diff --git a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/FindReferencesToDataTypeAction.java b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/FindReferencesToDataTypeAction.java
index b81a514126..fe0fec1b49 100644
--- a/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/FindReferencesToDataTypeAction.java
+++ b/Ghidra/Features/Decompiler/src/main/java/ghidra/app/plugin/core/decompile/actions/FindReferencesToDataTypeAction.java
@@ -82,23 +82,9 @@ public class FindReferencesToDataTypeAction extends AbstractFindReferencesDataTy
return false;
}
- DecompilerActionContext decompilerContext = (DecompilerActionContext) context;
- return decompilerContext.checkActionEnablement(() -> {
-
- DataType dataType = getDataType(context);
- updateMenuName(dataType);
-
- return super.isEnabledForContext(context);
- });
- }
-
- @Override
- public void actionPerformed(ActionContext context) {
-
- DecompilerActionContext decompilerContext = (DecompilerActionContext) context;
- decompilerContext.performAction(() -> {
- super.actionPerformed(context);
- });
+ DataType dataType = getDataType(context);
+ updateMenuName(dataType);
+ return super.isEnabledForContext(context);
}
private void updateMenuName(DataType type) {
diff --git a/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/plugin/core/decompiler/taint/actions/TaintAbstractDecompilerAction.java b/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/plugin/core/decompiler/taint/actions/TaintAbstractDecompilerAction.java
index 63051e16a5..0ecbc85d26 100644
--- a/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/plugin/core/decompiler/taint/actions/TaintAbstractDecompilerAction.java
+++ b/Ghidra/Features/DecompilerDependent/src/main/java/ghidra/app/plugin/core/decompiler/taint/actions/TaintAbstractDecompilerAction.java
@@ -130,17 +130,13 @@ public abstract class TaintAbstractDecompilerAction extends DockingAction {
@Override
public boolean isEnabledForContext(ActionContext context) {
DecompilerActionContext decompilerContext = (DecompilerActionContext) context;
- return decompilerContext.checkActionEnablement(() -> {
- return isEnabledForDecompilerContext(decompilerContext);
- });
+ return isEnabledForDecompilerContext(decompilerContext);
}
@Override
public void actionPerformed(ActionContext context) {
DecompilerActionContext decompilerContext = (DecompilerActionContext) context;
- decompilerContext.performAction(() -> {
- decompilerActionPerformed(decompilerContext);
- });
+ decompilerActionPerformed(decompilerContext);
}
protected Symbol getSymbol(DecompilerActionContext context) {
diff --git a/Ghidra/Framework/Docking/src/main/java/docking/DockingKeyBindingAction.java b/Ghidra/Framework/Docking/src/main/java/docking/DockingKeyBindingAction.java
index fdf7f5cc49..bdc600b9ad 100644
--- a/Ghidra/Framework/Docking/src/main/java/docking/DockingKeyBindingAction.java
+++ b/Ghidra/Framework/Docking/src/main/java/docking/DockingKeyBindingAction.java
@@ -15,6 +15,7 @@
*/
package docking;
+import java.awt.Toolkit;
import java.awt.event.ActionEvent;
import java.util.List;
@@ -46,6 +47,14 @@ public abstract class DockingKeyBindingAction extends AbstractAction {
return true; // always enable; this is a internal action that cannot be disabled
}
+ public void reportNotEnabled() {
+ String name = dockingAction.getName();
+ String ksText = KeyBindingUtils.parseKeyStroke(keyStroke);
+ String message = "Action '%s' (%s) not currently enabled".formatted(name, ksText);
+ tool.setStatusInfo(message, true);
+ Toolkit.getDefaultToolkit().beep();
+ }
+
public abstract KeyBindingPrecedence getKeyBindingPrecedence();
public boolean isSystemKeybindingPrecedence() {
diff --git a/Ghidra/Framework/Docking/src/main/java/docking/KeyBindingOverrideKeyEventDispatcher.java b/Ghidra/Framework/Docking/src/main/java/docking/KeyBindingOverrideKeyEventDispatcher.java
index 3c4a9f0741..62931a9aad 100644
--- a/Ghidra/Framework/Docking/src/main/java/docking/KeyBindingOverrideKeyEventDispatcher.java
+++ b/Ghidra/Framework/Docking/src/main/java/docking/KeyBindingOverrideKeyEventDispatcher.java
@@ -144,12 +144,14 @@ public class KeyBindingOverrideKeyEventDispatcher implements KeyEventDispatcher
KeyBindingPrecedence keyBindingPrecedence = getValidKeyBindingPrecedence(action);
if (keyBindingPrecedence == null) {
- // Odd Code: we use the precedence as a signal to say that, when it is null, there
- // are no valid bindings to be processed. We used to have a isValidContext()
- // method on the action, but it repeated the involved work that is done
- // in getKeyBindingPrecedence(), which is needed below. So, just use the
- // one call for both purposes.
- return false;
+ // Note: we used to return false here. Returning false allows Java to handle a given
+ // key stroke when our actions are disabled. We have decided it is simpler to
+ // always consume a given key stroke when we have actions registered. This
+ // prevents inconsistent action firing between Ghidra and Java, depending upon
+ // Ghidra's action enablement. If we find a case that is broken by this change,
+ // then we will need a more robust solution here.
+ action.reportNotEnabled();
+ return true;
}
// Process the key event in precedence order.
diff --git a/Ghidra/Framework/Docking/src/main/java/docking/action/MultipleKeyAction.java b/Ghidra/Framework/Docking/src/main/java/docking/action/MultipleKeyAction.java
index 0bc8585e15..9e0ea4121c 100644
--- a/Ghidra/Framework/Docking/src/main/java/docking/action/MultipleKeyAction.java
+++ b/Ghidra/Framework/Docking/src/main/java/docking/action/MultipleKeyAction.java
@@ -15,10 +15,10 @@
*/
package docking.action;
-import java.awt.Component;
-import java.awt.Window;
+import java.awt.*;
import java.awt.event.ActionEvent;
import java.util.*;
+import java.util.List;
import javax.swing.*;
@@ -154,6 +154,7 @@ public class MultipleKeyAction extends DockingKeyBindingAction {
else {
String name = (String) getValue(Action.NAME);
tool.setStatusInfo("Action (" + name + ") not valid in this context!", true);
+ Toolkit.getDefaultToolkit().beep();
}
}
@@ -216,28 +217,29 @@ public class MultipleKeyAction extends DockingKeyBindingAction {
// 3) Check for default context actions
//
for (ActionData actionData : actions) {
- if (actionData.isGlobalAction()) {
- // When looking for context matches, we prefer local context, even though this
- // is a 'global' action. This allows more specific context to be used when
- // available
- if (isValidAndEnabled(actionData, localContext)) {
- list.add(new ExecutableAction(actionData.action, localContext));
- continue;
- }
+ if (!actionData.isGlobalAction()) {
+ continue;
+ }
- // this happens if we are in a dialog, default context is not used
- if (contextMap == null) {
- continue;
- }
+ // When looking for context matches, we prefer local context, even though this
+ // is a 'global' action. This allows more specific context to be used when available
+ if (isValidAndEnabled(actionData, localContext)) {
+ list.add(new ExecutableAction(actionData.action, localContext));
+ continue;
+ }
- if (!actionData.supportsDefaultContext()) {
- continue;
- }
+ // this happens if we are in a dialog, default context is not used
+ if (contextMap == null) {
+ continue;
+ }
- ActionContext defaultContext = contextMap.get(actionData.getContextType());
- if (isValidAndEnabled(actionData, defaultContext)) {
- list.add(new ExecutableAction(actionData.action, defaultContext));
- }
+ if (!actionData.supportsDefaultContext()) {
+ continue;
+ }
+
+ ActionContext defaultContext = contextMap.get(actionData.getContextType());
+ if (isValidAndEnabled(actionData, defaultContext)) {
+ list.add(new ExecutableAction(actionData.action, defaultContext));
}
}
return list;