From b150c557fcf49fe0ba3c2eb7c941bbcd248ba8a3 Mon Sep 17 00:00:00 2001 From: Dan <46821332+nsadeveloper789@users.noreply.github.com> Date: Thu, 29 Apr 2021 15:20:46 -0400 Subject: [PATCH] GP-909: Adjusting how GDB resumes/steps to assure interrupts work. --- .../java/agent/gdb/manager/GdbInferior.java | 4 +- .../java/agent/gdb/manager/GdbManager.java | 43 ++++++++----- .../java/agent/gdb/manager/GdbThread.java | 4 +- .../gdb/manager/impl/GdbInferiorImpl.java | 4 +- .../gdb/manager/impl/GdbManagerImpl.java | 23 ++++++- .../agent/gdb/manager/impl/GdbThreadImpl.java | 4 +- ...ing.java => AbstractLaunchGdbCommand.java} | 4 +- .../manager/impl/cmd/GdbContinueCommand.java | 41 +++--------- .../gdb/manager/impl/cmd/GdbRunCommand.java | 16 +---- .../gdb/manager/impl/cmd/GdbStartCommand.java | 4 +- .../impl/cmd/GdbStartInstructionCommand.java | 2 +- .../gdb/manager/impl/cmd/GdbStepCommand.java | 49 ++++----------- .../impl/cmd/MixinResumeInCliGdbCommand.java | 62 +++++++++++++++++++ .../model/impl/GdbModelTargetInferior.java | 20 +++--- .../gdb/model/impl/GdbModelTargetSession.java | 1 + .../gdb/model/impl/GdbModelTargetThread.java | 20 +++--- .../manager/impl/AbstractGdbManagerTest.java | 6 +- .../AbstractModelForGdbAttacherTest.java | 2 +- .../AbstractModelForGdbInterpreterTest.java | 2 +- .../agent/gdb/model/GdbLinuxSpecimen.java | 4 +- .../InVmModelForGdbInferiorAttacherTest.java | 22 +++++++ .../core/debug/gui/DebuggerResources.java | 11 +++- .../dbg/test/AbstractDebuggerModelTest.java | 2 +- .../java/ghidra/dbg/testutil/DummyProc.java | 4 +- 24 files changed, 210 insertions(+), 144 deletions(-) rename Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/{AbstractGdbCommandExpectRunning.java => AbstractLaunchGdbCommand.java} (91%) create mode 100644 Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/MixinResumeInCliGdbCommand.java diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/GdbInferior.java b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/GdbInferior.java index 1b2ab308ed..670650889c 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/GdbInferior.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/GdbInferior.java @@ -20,7 +20,7 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.CompletableFuture; -import agent.gdb.manager.GdbManager.ExecSuffix; +import agent.gdb.manager.GdbManager.StepCmd; import agent.gdb.manager.impl.GdbMemoryMapping; /** @@ -263,7 +263,7 @@ public interface GdbInferior extends GdbMemoryOperations { * * @return a future that completes once the inferior has stepped */ - CompletableFuture step(ExecSuffix suffix); + CompletableFuture step(StepCmd suffix); /** * Evaluate an expression diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/GdbManager.java b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/GdbManager.java index b8b4c13cab..070114bf03 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/GdbManager.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/GdbManager.java @@ -40,35 +40,34 @@ public interface GdbManager extends AutoCloseable, GdbBreakpointInsertions { public static final String DEFAULT_GDB_CMD = "/usr/bin/gdb"; /** - * Possible values for {@link GdbThread#step(ExecSuffix)} + * Possible values for {@link GdbThread#step(StepCmd)} */ - public enum ExecSuffix { - /** Equivalent to {@code finish} in the CLI */ + public enum StepCmd { FINISH("finish"), - /** Equivalent to {@code next} in the CLI */ NEXT("next"), - /** Equivalent to {@code nexti} in the CLI */ - NEXT_INSTRUCTION("next-instruction"), - /** Equivalent to {@code return} in the CLI */ + NEXTI("nexti", "next-instruction"), RETURN("return"), - /** Equivalent to {@code step} in the CLI */ STEP("step"), - /** Equivalent to {@code stepi} in the CLI */ - STEP_INSTRUCTION("step-instruction"), - /** Equivalent to {@code until} in the CLI */ + STEPI("stepi", "step-instruction"), UNTIL("until"), /** User-defined */ - EXTENDED("until"),; + EXTENDED("echo extended-step?", "???"),; - final String str; + public final String mi2; + public final String cli; - ExecSuffix(String str) { - this.str = str; + StepCmd(String cli, String execSuffix) { + this.cli = cli; + this.mi2 = "-exec-" + execSuffix; + } + + StepCmd(String cli) { + this(cli, cli); } @Override public String toString() { - return str; + return mi2; } } @@ -305,12 +304,23 @@ public interface GdbManager extends AutoCloseable, GdbBreakpointInsertions { * *

* This may be useful if the manager's command queue is stalled because an inferior is running. + * If this doesn't clear the stall, try {@link #cancelCurrentCommand()}. * * @throws IOException if an I/O error occurs * @throws InterruptedException */ void sendInterruptNow() throws IOException; + /** + * Cancel the current command + * + *

+ * Occasionally, a command gets stalled up waiting for an event, which for other reasons, will + * no longer occur. This will free up the queue for other commands to (hopefully) be processed. + * If {@link #sendInterruptNow()} doesn't clear the stall, try this. + */ + void cancelCurrentCommand(); + /** * Get the state of the GDB session * @@ -521,5 +531,4 @@ public interface GdbManager extends AutoCloseable, GdbBreakpointInsertions { * @return the description */ String getPtyDescription(); - } diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/GdbThread.java b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/GdbThread.java index 35755779bb..7dd3cbe328 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/GdbThread.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/GdbThread.java @@ -18,7 +18,7 @@ package agent.gdb.manager; import java.util.List; import java.util.concurrent.CompletableFuture; -import agent.gdb.manager.GdbManager.ExecSuffix; +import agent.gdb.manager.GdbManager.StepCmd; import agent.gdb.manager.breakpoint.GdbBreakpointInsertions; import agent.gdb.manager.impl.GdbThreadInfo; @@ -121,7 +121,7 @@ public interface GdbThread * @param suffix specifies how far to step, or on what conditions stepping ends. * @return a future that completes once the thread is running */ - CompletableFuture step(ExecSuffix suffix); + CompletableFuture step(StepCmd suffix); /** * Detach from the entire process diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/GdbInferiorImpl.java b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/GdbInferiorImpl.java index e52d4dd65e..50ab8a335f 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/GdbInferiorImpl.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/GdbInferiorImpl.java @@ -27,7 +27,7 @@ import java.util.regex.Pattern; import com.google.common.collect.RangeSet; import agent.gdb.manager.*; -import agent.gdb.manager.GdbManager.ExecSuffix; +import agent.gdb.manager.GdbManager.StepCmd; import agent.gdb.manager.impl.cmd.*; import ghidra.async.AsyncLazyValue; import ghidra.lifecycle.Internal; @@ -394,7 +394,7 @@ public class GdbInferiorImpl implements GdbInferior { } @Override - public CompletableFuture step(ExecSuffix suffix) { + public CompletableFuture step(StepCmd suffix) { return execute(new GdbStepCommand(manager, null, suffix)); } diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/GdbManagerImpl.java b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/GdbManagerImpl.java index 495830b9c4..cb01b0ea84 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/GdbManagerImpl.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/GdbManagerImpl.java @@ -735,7 +735,9 @@ public class GdbManagerImpl implements GdbManager { }).exceptionally((exc) -> { pcmd.completeExceptionally(exc); //Msg.debug(this, "ON_EXCEPTION: CURCMD = " + curCmd); - curCmd = null; + synchronized (this) { + curCmd = null; + } //Msg.debug(this, "SET CURCMD = null"); //Msg.debug(this, "RELEASING cmdLock"); Hold hold = cmdLockHold.getAndSet(null); @@ -747,6 +749,23 @@ public class GdbManagerImpl implements GdbManager { return pcmd; } + @Override + public void cancelCurrentCommand() { + GdbPendingCommand curCmd; + synchronized (this) { + curCmd = this.curCmd; + this.curCmd = null; + } + if (curCmd != null) { + Msg.info(this, "Cancelling current command: " + curCmd); + curCmd.cancel(false); + } + Hold hold = cmdLockHold.getAndSet(null); + if (hold != null) { + hold.release(); + } + } + protected PrintWriter getWriter(Interpreter interpreter) { switch (interpreter) { case CLI: @@ -1481,7 +1500,7 @@ public class GdbManagerImpl implements GdbManager { os.write(3); os.flush(); } - if (mi2Thread != null) { + else if (mi2Thread != null) { OutputStream os = mi2Thread.pty.getParent().getOutputStream(); os.write(3); os.flush(); diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/GdbThreadImpl.java b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/GdbThreadImpl.java index 1fba1ce872..56767b9f3f 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/GdbThreadImpl.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/GdbThreadImpl.java @@ -26,7 +26,7 @@ import org.apache.commons.lang3.StringUtils; import com.google.common.collect.RangeSet; import agent.gdb.manager.*; -import agent.gdb.manager.GdbManager.ExecSuffix; +import agent.gdb.manager.GdbManager.StepCmd; import agent.gdb.manager.breakpoint.GdbBreakpointInfo; import agent.gdb.manager.breakpoint.GdbBreakpointType; import agent.gdb.manager.impl.cmd.*; @@ -249,7 +249,7 @@ public class GdbThreadImpl implements GdbThread { } @Override - public CompletableFuture step(ExecSuffix suffix) { + public CompletableFuture step(StepCmd suffix) { return execute(new GdbStepCommand(manager, id, suffix)); } diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/AbstractGdbCommandExpectRunning.java b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/AbstractLaunchGdbCommand.java similarity index 91% rename from Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/AbstractGdbCommandExpectRunning.java rename to Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/AbstractLaunchGdbCommand.java index 2ee0ecc4d1..9bd0b867fe 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/AbstractGdbCommandExpectRunning.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/AbstractLaunchGdbCommand.java @@ -19,9 +19,9 @@ import agent.gdb.manager.GdbThread; import agent.gdb.manager.evt.*; import agent.gdb.manager.impl.*; -public abstract class AbstractGdbCommandExpectRunning extends AbstractGdbCommand { +public abstract class AbstractLaunchGdbCommand extends AbstractGdbCommand { - protected AbstractGdbCommandExpectRunning(GdbManagerImpl manager) { + protected AbstractLaunchGdbCommand(GdbManagerImpl manager) { super(manager); } diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/GdbContinueCommand.java b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/GdbContinueCommand.java index b0f6690ca8..776dc42c82 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/GdbContinueCommand.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/GdbContinueCommand.java @@ -16,62 +16,39 @@ package agent.gdb.manager.impl.cmd; import agent.gdb.manager.GdbInferior; -import agent.gdb.manager.evt.*; import agent.gdb.manager.impl.*; import agent.gdb.manager.impl.GdbManagerImpl.Interpreter; /** * Implementation of {@link GdbInferior#cont()} */ -public class GdbContinueCommand extends AbstractGdbCommandWithThreadId { +public class GdbContinueCommand extends AbstractGdbCommandWithThreadId + implements MixinResumeInCliGdbCommand { public GdbContinueCommand(GdbManagerImpl manager, Integer threadId) { super(manager, threadId); } @Override public Interpreter getInterpreter() { - if (manager.hasCli()) { - return Interpreter.CLI; - } - return Interpreter.MI2; + return getInterpreter(manager); } @Override public String encode(String threadPart) { - switch (getInterpreter()) { - case CLI: - // The significance is the Pty, not so much the actual command - // Using MI2 simplifies event processing (no console output parsing) - return "interpreter-exec mi2 \"-exec-continue" + threadPart + "\""; - case MI2: - return "-exec-continue" + threadPart; - default: - throw new AssertionError(); + if (getInterpreter() == Interpreter.CLI) { + return "continue"; } + return "-exec-continue" + threadPart; } @Override public boolean handle(GdbEvent evt, GdbPendingCommand pending) { - evt = checkErrorViaCli(evt); - if (evt instanceof GdbCommandRunningEvent) { - pending.claim(evt); - return pending.hasAny(GdbRunningEvent.class); - } - else if (evt instanceof AbstractGdbCompletedCommandEvent) { - pending.claim(evt); - return true; // Not the expected Completed event - } - else if (evt instanceof GdbRunningEvent) { - // Event happens no matter which interpreter received the command - pending.claim(evt); - return pending.hasAny(GdbCommandRunningEvent.class); - } - return false; + evt = checkErrorViaCli(evt); // TODO: Deprecated, since that hack can crash GDB + return handleExpectingRunning(evt, pending); } @Override public Void complete(GdbPendingCommand pending) { - pending.checkCompletion(GdbCommandRunningEvent.class); - return null; + return completeOnRunning(pending); } } diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/GdbRunCommand.java b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/GdbRunCommand.java index b4c2ac6352..eafc676018 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/GdbRunCommand.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/GdbRunCommand.java @@ -22,7 +22,7 @@ import agent.gdb.manager.impl.GdbManagerImpl.Interpreter; /** * Implementation of {@link GdbInferior#run()} */ -public class GdbRunCommand extends AbstractGdbCommandExpectRunning { +public class GdbRunCommand extends AbstractLaunchGdbCommand { public GdbRunCommand(GdbManagerImpl manager) { super(manager); @@ -30,23 +30,11 @@ public class GdbRunCommand extends AbstractGdbCommandExpectRunning { @Override public Interpreter getInterpreter() { - if (manager.hasCli()) { - return Interpreter.CLI; - } return Interpreter.MI2; } @Override public String encode() { - switch (getInterpreter()) { - case CLI: - // The significance is the Pty, not so much the actual command - // Using MI2 simplifies event processing (no console output parsing) - return "interpreter-exec mi2 \"-exec-run\""; - case MI2: - return "-exec-run"; - default: - throw new AssertionError(); - } + return "-exec-run"; } } diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/GdbStartCommand.java b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/GdbStartCommand.java index 364f518117..f078a7f617 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/GdbStartCommand.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/GdbStartCommand.java @@ -22,7 +22,7 @@ import agent.gdb.manager.impl.GdbManagerImpl.Interpreter; /** * Implementation of {@link GdbInferior#start()} */ -public class GdbStartCommand extends AbstractGdbCommandExpectRunning { +public class GdbStartCommand extends AbstractLaunchGdbCommand { public GdbStartCommand(GdbManagerImpl manager) { super(manager); @@ -35,6 +35,6 @@ public class GdbStartCommand extends AbstractGdbCommandExpectRunning { @Override public String encode() { - return "-interpreter-exec console start"; + return "-exec-run --start"; } } diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/GdbStartInstructionCommand.java b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/GdbStartInstructionCommand.java index d166c5a015..4bff920060 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/GdbStartInstructionCommand.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/GdbStartInstructionCommand.java @@ -22,7 +22,7 @@ import agent.gdb.manager.impl.GdbManagerImpl.Interpreter; /** * Implementation of {@link GdbInferior#starti()} */ -public class GdbStartInstructionCommand extends AbstractGdbCommandExpectRunning { +public class GdbStartInstructionCommand extends AbstractLaunchGdbCommand { public GdbStartInstructionCommand(GdbManagerImpl manager) { super(manager); diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/GdbStepCommand.java b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/GdbStepCommand.java index 4aaf63003d..04c56c357e 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/GdbStepCommand.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/GdbStepCommand.java @@ -15,67 +15,44 @@ */ package agent.gdb.manager.impl.cmd; -import agent.gdb.manager.GdbManager.ExecSuffix; +import agent.gdb.manager.GdbManager.StepCmd; import agent.gdb.manager.GdbThread; -import agent.gdb.manager.evt.*; import agent.gdb.manager.impl.*; import agent.gdb.manager.impl.GdbManagerImpl.Interpreter; /** * Implementation of {@link GdbThread#stepInstruction()} */ -public class GdbStepCommand extends AbstractGdbCommandWithThreadId { - protected final ExecSuffix suffix; +public class GdbStepCommand extends AbstractGdbCommandWithThreadId + implements MixinResumeInCliGdbCommand { + protected final StepCmd cmd; - public GdbStepCommand(GdbManagerImpl manager, Integer threadId, ExecSuffix suffix) { + public GdbStepCommand(GdbManagerImpl manager, Integer threadId, StepCmd cmd) { super(manager, threadId); - this.suffix = suffix; + this.cmd = cmd; } @Override public Interpreter getInterpreter() { - if (manager.hasCli()) { - return Interpreter.CLI; - } - return Interpreter.MI2; + return getInterpreter(manager); } @Override protected String encode(String threadPart) { - String mi2Cmd = "-exec-" + suffix + threadPart; - switch (getInterpreter()) { - case CLI: - // The significance is the Pty, not so much the actual command - // Using MI2 simplifies event processing (no console output parsing) - return "interpreter-exec mi2 \"" + mi2Cmd + "\""; - case MI2: - return mi2Cmd; - default: - throw new AssertionError(); + if (getInterpreter() == Interpreter.CLI) { + return cmd.cli; } + return cmd.mi2 + threadPart; } @Override public boolean handle(GdbEvent evt, GdbPendingCommand pending) { - evt = checkErrorViaCli(evt); - if (evt instanceof GdbCommandRunningEvent) { - pending.claim(evt); - return pending.hasAny(GdbRunningEvent.class); - } - else if (evt instanceof AbstractGdbCompletedCommandEvent) { - pending.claim(evt); - return true; // Not the expected Completed event - } - else if (evt instanceof GdbRunningEvent) { - pending.claim(evt); - return pending.hasAny(GdbCommandRunningEvent.class); - } - return false; + evt = checkErrorViaCli(evt); // TODO: Deprecated, since that hack can crash GDB + return handleExpectingRunning(evt, pending); } @Override public Void complete(GdbPendingCommand pending) { - pending.checkCompletion(GdbCommandRunningEvent.class); - return null; + return completeOnRunning(pending); } } diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/MixinResumeInCliGdbCommand.java b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/MixinResumeInCliGdbCommand.java new file mode 100644 index 0000000000..874292fd2f --- /dev/null +++ b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/MixinResumeInCliGdbCommand.java @@ -0,0 +1,62 @@ +/* ### + * 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 agent.gdb.manager.impl.cmd; + +import agent.gdb.manager.evt.*; +import agent.gdb.manager.impl.*; +import agent.gdb.manager.impl.GdbManagerImpl.Interpreter; + +/** + * A marker and mixin for dealing with commands where resuming in a secondary (mi2) ui seems + * problematic. + * + *

+ * I'm not sure if it's a bug in GDB or Linux, or what, but if I attach to a target that doesn't + * have a tty then resume (i.e., continue or step) from the mi2 interpreter, it seems I cannot use + * "interrupt" from the primary (console) interpreter. So, for these resumes, I need to issue the + * command from the console, allowing ^C to work. + */ +public interface MixinResumeInCliGdbCommand extends GdbCommand { + + default Interpreter getInterpreter(GdbManagerImpl manager) { + if (manager.hasCli()) { + return Interpreter.CLI; + } + return Interpreter.MI2; + } + + default boolean handleExpectingRunning(GdbEvent evt, GdbPendingCommand pending) { + if (evt instanceof GdbCommandRunningEvent) { + pending.claim(evt); + return pending.hasAny(GdbRunningEvent.class); + } + else if (evt instanceof AbstractGdbCompletedCommandEvent) { + pending.claim(evt); + return true; // Not the expected Completed event + } + else if (evt instanceof GdbRunningEvent) { + // Event happens no matter which interpreter received the command + pending.claim(evt); + return pending.hasAny(GdbCommandRunningEvent.class); + } + return false; + } + + default Void completeOnRunning(GdbPendingCommand pending) { + pending.checkCompletion(GdbCommandRunningEvent.class); + return null; + } +} diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelTargetInferior.java b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelTargetInferior.java index 4c1437db9c..81680f9011 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelTargetInferior.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelTargetInferior.java @@ -19,7 +19,7 @@ import java.util.*; import java.util.concurrent.CompletableFuture; import agent.gdb.manager.*; -import agent.gdb.manager.GdbManager.ExecSuffix; +import agent.gdb.manager.GdbManager.StepCmd; import agent.gdb.manager.impl.cmd.GdbStateChangeRecord; import agent.gdb.manager.reason.*; import ghidra.async.AsyncFence; @@ -160,24 +160,24 @@ public class GdbModelTargetInferior return impl.gateFuture(inferior.cont()); } - protected ExecSuffix convertToGdb(TargetStepKind kind) { + protected StepCmd convertToGdb(TargetStepKind kind) { switch (kind) { case FINISH: - return ExecSuffix.FINISH; + return StepCmd.FINISH; case INTO: - return ExecSuffix.STEP_INSTRUCTION; + return StepCmd.STEPI; case LINE: - return ExecSuffix.STEP; + return StepCmd.STEP; case OVER: - return ExecSuffix.NEXT_INSTRUCTION; + return StepCmd.NEXTI; case OVER_LINE: - return ExecSuffix.NEXT; + return StepCmd.NEXT; case RETURN: - return ExecSuffix.RETURN; + return StepCmd.RETURN; case UNTIL: - return ExecSuffix.UNTIL; + return StepCmd.UNTIL; case EXTENDED: - return ExecSuffix.EXTENDED; + return StepCmd.EXTENDED; default: throw new AssertionError(); } diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelTargetSession.java b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelTargetSession.java index 2057c6d821..26c2b37241 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelTargetSession.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelTargetSession.java @@ -229,6 +229,7 @@ public class GdbModelTargetSession extends DefaultTargetModelRoot //return impl.gdb.interrupt(); try { impl.gdb.sendInterruptNow(); + impl.gdb.cancelCurrentCommand(); } catch (IOException e) { Msg.error(this, "Could not interrupt", e); diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelTargetThread.java b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelTargetThread.java index 73a8b61ac5..8e7cd2cfd8 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelTargetThread.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelTargetThread.java @@ -20,7 +20,7 @@ import java.util.Map; import java.util.concurrent.CompletableFuture; import agent.gdb.manager.*; -import agent.gdb.manager.GdbManager.ExecSuffix; +import agent.gdb.manager.GdbManager.StepCmd; import agent.gdb.manager.impl.GdbFrameInfo; import agent.gdb.manager.impl.GdbThreadInfo; import agent.gdb.manager.impl.cmd.GdbStateChangeRecord; @@ -183,24 +183,24 @@ public class GdbModelTargetThread } } - protected ExecSuffix convertToGdb(TargetStepKind kind) { + protected StepCmd convertToGdb(TargetStepKind kind) { switch (kind) { case FINISH: - return ExecSuffix.FINISH; + return StepCmd.FINISH; case INTO: - return ExecSuffix.STEP_INSTRUCTION; + return StepCmd.STEPI; case LINE: - return ExecSuffix.STEP; + return StepCmd.STEP; case OVER: - return ExecSuffix.NEXT_INSTRUCTION; + return StepCmd.NEXTI; case OVER_LINE: - return ExecSuffix.NEXT; + return StepCmd.NEXT; case RETURN: - return ExecSuffix.RETURN; + return StepCmd.RETURN; case UNTIL: - return ExecSuffix.UNTIL; + return StepCmd.UNTIL; case EXTENDED: - return ExecSuffix.EXTENDED; + return StepCmd.EXTENDED; default: throw new AssertionError(); } diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/test/java/agent/gdb/manager/impl/AbstractGdbManagerTest.java b/Ghidra/Debug/Debugger-agent-gdb/src/test/java/agent/gdb/manager/impl/AbstractGdbManagerTest.java index 1f2db9f917..9d05ca3c32 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/test/java/agent/gdb/manager/impl/AbstractGdbManagerTest.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/test/java/agent/gdb/manager/impl/AbstractGdbManagerTest.java @@ -33,7 +33,7 @@ import org.junit.*; import com.google.common.collect.*; import agent.gdb.manager.*; -import agent.gdb.manager.GdbManager.ExecSuffix; +import agent.gdb.manager.GdbManager.StepCmd; import agent.gdb.manager.breakpoint.GdbBreakpointInfo; import agent.gdb.pty.PtyFactory; import ghidra.async.AsyncReference; @@ -259,7 +259,7 @@ public abstract class AbstractGdbManagerTest extends AbstractGhidraHeadlessInteg assertTrue("Didn't stop at syscall", out.contains("syscall")); // Now the real test - waitOn(mgr.currentInferior().step(ExecSuffix.STEP_INSTRUCTION)); + waitOn(mgr.currentInferior().step(StepCmd.STEPI)); CompletableFuture stopped = mgr.waitForState(GdbState.STOPPED); Thread.sleep(100); // NB: Not exactly reliable, but verify we're waiting assertFalse(stopped.isDone()); @@ -400,7 +400,7 @@ public abstract class AbstractGdbManagerTest extends AbstractGhidraHeadlessInteg GdbThread thread = waitOn(mgr.currentInferior().run()); waitOn(mgr.waitForState(GdbState.STOPPED)); //waitOn(mgr.waitForPrompt()); - waitOn(thread.step(ExecSuffix.NEXT_INSTRUCTION)); + waitOn(thread.step(StepCmd.NEXTI)); waitOn(mgr.waitForState(GdbState.STOPPED)); assertNull(mgr.currentInferior().getExitCode()); } diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/test/java/agent/gdb/model/AbstractModelForGdbAttacherTest.java b/Ghidra/Debug/Debugger-agent-gdb/src/test/java/agent/gdb/model/AbstractModelForGdbAttacherTest.java index 4e03e2123c..6c6423dc82 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/test/java/agent/gdb/model/AbstractModelForGdbAttacherTest.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/test/java/agent/gdb/model/AbstractModelForGdbAttacherTest.java @@ -33,7 +33,7 @@ public abstract class AbstractModelForGdbAttacherTest extends AbstractDebuggerMo @Override public DebuggerTestSpecimen getAttachSpecimen() { - return GdbLinuxSpecimen.DD; + return GdbLinuxSpecimen.SLEEP; } @Override diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/test/java/agent/gdb/model/AbstractModelForGdbInterpreterTest.java b/Ghidra/Debug/Debugger-agent-gdb/src/test/java/agent/gdb/model/AbstractModelForGdbInterpreterTest.java index d2840e7343..eda1037391 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/test/java/agent/gdb/model/AbstractModelForGdbInterpreterTest.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/test/java/agent/gdb/model/AbstractModelForGdbInterpreterTest.java @@ -54,7 +54,7 @@ public abstract class AbstractModelForGdbInterpreterTest @Override public DebuggerTestSpecimen getAttachSpecimen() { - return GdbLinuxSpecimen.DD; + return GdbLinuxSpecimen.SLEEP; } @Override diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/test/java/agent/gdb/model/GdbLinuxSpecimen.java b/Ghidra/Debug/Debugger-agent-gdb/src/test/java/agent/gdb/model/GdbLinuxSpecimen.java index 0c57e922fe..88cad6a897 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/test/java/agent/gdb/model/GdbLinuxSpecimen.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/test/java/agent/gdb/model/GdbLinuxSpecimen.java @@ -28,10 +28,10 @@ import ghidra.dbg.testutil.DummyProc; import ghidra.dbg.util.ShellUtils; public enum GdbLinuxSpecimen implements DebuggerTestSpecimen, DebuggerModelTestUtils { - DD { + SLEEP { @Override String getCommandLine() { - return "dd"; + return "sleep 100000"; } }, FORK_EXIT { diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/test/java/agent/gdb/model/invm/InVmModelForGdbInferiorAttacherTest.java b/Ghidra/Debug/Debugger-agent-gdb/src/test/java/agent/gdb/model/invm/InVmModelForGdbInferiorAttacherTest.java index 0a8d1d1c55..f44e47a463 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/test/java/agent/gdb/model/invm/InVmModelForGdbInferiorAttacherTest.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/test/java/agent/gdb/model/invm/InVmModelForGdbInferiorAttacherTest.java @@ -15,9 +15,14 @@ */ package agent.gdb.model.invm; +import static org.junit.Assume.assumeFalse; + import org.junit.Ignore; +import org.junit.Test; import agent.gdb.model.AbstractModelForGdbInferiorAttacherTest; +import ghidra.util.Msg; +import ghidra.util.SystemUtilities; public class InVmModelForGdbInferiorAttacherTest extends AbstractModelForGdbInferiorAttacherTest { @Override @@ -30,4 +35,21 @@ public class InVmModelForGdbInferiorAttacherTest extends AbstractModelForGdbInfe public void testAttachableContainerIsWhereExpected() throws Throwable { // nop } + + /** + * Run a dummy process without a tty. It seems when GDB (I tested with 8.0.1) attaches to such a + * process, it is unable to interrupt it from the opposite interpreter that resumed it. + */ + @Test + @Ignore("Not a real test") + public void testRunADummy() throws Throwable { + assumeFalse(SystemUtilities.isInTestingBatchMode()); + + DebuggerTestSpecimen specimen = getAttachSpecimen(); + dummy = specimen.runDummy(); + + Msg.info(this, "Dummy pid: " + dummy.pid); + dummy.process.waitFor(); + Msg.info(this, "Dummy terminated"); + } } diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/DebuggerResources.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/DebuggerResources.java index 1e810d5be2..286f0d61fa 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/DebuggerResources.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/DebuggerResources.java @@ -19,6 +19,7 @@ import java.awt.Color; import java.awt.Component; import java.awt.event.*; import java.util.*; +import java.util.concurrent.CancellationException; import java.util.function.Function; import javax.swing.*; @@ -45,11 +46,13 @@ import ghidra.app.plugin.core.debug.gui.watch.DebuggerWatchesPlugin; import ghidra.app.plugin.core.debug.service.model.launch.DebuggerProgramLaunchOffer; import ghidra.app.services.DebuggerTraceManagerService.BooleanChangeAdapter; import ghidra.app.services.MarkerService; +import ghidra.async.AsyncUtils; import ghidra.framework.plugintool.Plugin; import ghidra.framework.plugintool.util.PluginUtils; import ghidra.program.database.ProgramContentHandler; import ghidra.trace.model.Trace; import ghidra.util.*; +import ghidra.util.exception.CancelledException; import resources.MultiIcon; import resources.ResourceManager; import resources.icons.RotateIcon; @@ -1546,7 +1549,13 @@ public interface DebuggerResources { static Function showError(Component parent, String message) { return e -> { - Msg.showError(parent, parent, DebuggerPluginPackage.NAME, message, e); + Throwable t = AsyncUtils.unwrapThrowable(e); + if (t instanceof CancelledException || t instanceof CancellationException) { + Msg.error(parent, "Cancelled: " + message); + } + else { + Msg.showError(parent, parent, DebuggerPluginPackage.NAME, message, e); + } return null; }; } diff --git a/Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/test/AbstractDebuggerModelTest.java b/Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/test/AbstractDebuggerModelTest.java index b0e2150480..e2b54a9ff0 100644 --- a/Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/test/AbstractDebuggerModelTest.java +++ b/Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/test/AbstractDebuggerModelTest.java @@ -324,7 +324,7 @@ public abstract class AbstractDebuggerModelTest extends AbstractGhidraHeadedInte assertEquals(TargetExecutionState.RUNNING, stateful.getExecutionState()); }, List.of(AssertionError.class)); } - // NB. Never have to wait to interrupt. (Hmmmm, do we believe this?) + // NB. Never have to waitAcc to interrupt. It's likely inaccessible, anyway. waitOn(interruptible.interrupt()); if (stateful != null) { retryVoid(() -> { diff --git a/Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/testutil/DummyProc.java b/Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/testutil/DummyProc.java index ba172292c2..07a4638d23 100644 --- a/Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/testutil/DummyProc.java +++ b/Ghidra/Debug/Framework-Debugging/src/test/java/ghidra/dbg/testutil/DummyProc.java @@ -60,7 +60,9 @@ public class DummyProc implements AutoCloseable { DummyProc(String... args) throws IOException { args[0] = which(args[0]); - process = new ProcessBuilder(args).start(); + process = new ProcessBuilder(args) + .inheritIO() + .start(); pid = process.pid(); }