From a5f02d7ebe8224862943f72924046b28db924f33 Mon Sep 17 00:00:00 2001 From: ghidra1 Date: Fri, 1 Sep 2023 12:45:56 -0400 Subject: [PATCH 1/2] GP-0 Updated patch Ghidra version to 10.3.4 --- Ghidra/application.properties | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Ghidra/application.properties b/Ghidra/application.properties index 83f1d8a159..f82c0d0543 100644 --- a/Ghidra/application.properties +++ b/Ghidra/application.properties @@ -1,5 +1,5 @@ application.name=Ghidra -application.version=10.3.3 +application.version=10.3.4 application.release.name=DEV application.layout.version=1 application.gradle.min=7.3 From 2a2393184064ec31e51cbf089b41b9f399248a3d Mon Sep 17 00:00:00 2001 From: Dan <46821332+nsadeveloper789@users.noreply.github.com> Date: Thu, 7 Sep 2023 13:06:20 -0400 Subject: [PATCH 2/2] GP-3690: Anticipate changes in upcoming gdb-14. --- .../gdb/manager/impl/GdbPendingCommand.java | 4 ++ .../manager/impl/cmd/AbstractGdbCommand.java | 34 +---------- .../impl/cmd/AbstractLaunchGdbCommand.java | 1 - .../manager/impl/cmd/GdbContinueCommand.java | 1 - .../gdb/manager/impl/cmd/GdbStepCommand.java | 1 - .../impl/cmd/MixinResumeInCliGdbCommand.java | 56 +++++++++++++++---- .../manager/impl/AbstractGdbManagerTest.java | 27 ++++----- .../SpawnedMi2GdbHomeLocalManagerTest.java | 5 -- 8 files changed, 64 insertions(+), 65 deletions(-) diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/GdbPendingCommand.java b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/GdbPendingCommand.java index dfba0017c5..27933b4bce 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/GdbPendingCommand.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/GdbPendingCommand.java @@ -26,6 +26,7 @@ import agent.gdb.manager.impl.cmd.GdbCommandError; /** * A command queued on the GDB manager * + *

* A {@link GdbCommand} is queued by wrapping it in a {@link GdbPendingCommand} and submitting it to * the manager implementation's executor. This object also keep track of claimed/stolen events and * provides convenience methods for sifting through them. @@ -92,6 +93,7 @@ public class GdbPendingCommand extends CompletableFuture implements GdbCau /** * Claim an event * + *

* This stores the event for later retrieval and processing. * * @param evt the event @@ -104,6 +106,7 @@ public class GdbPendingCommand extends CompletableFuture implements GdbCau /** * Steal an event * + *

* This stores the event for later retrieval and processing. * * @param evt the event @@ -205,6 +208,7 @@ public class GdbPendingCommand extends CompletableFuture implements GdbCau /** * Check that the command completed with one of the given results * + *

* {@link GdbCommandErrorEvent} need not be listed. This method will handle it as a special case * already. To avoid the special treatment, list it explicitly. * diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/AbstractGdbCommand.java b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/AbstractGdbCommand.java index a69e1fe815..e0bc6378c5 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/AbstractGdbCommand.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/AbstractGdbCommand.java @@ -66,37 +66,6 @@ public abstract class AbstractGdbCommand implements GdbCommand { return Interpreter.MI2; } - /** - * Check for an error reported in MI2 syntax via the CLI - * - *

- * This must be used in the {@link #handle(GdbEvent, GdbPendingCommand)} callback when the - * command is encoded as a MI2 command (using {@code interpreter-exec mi2}) but issued via the - * CLI. Depending on the GDB version and the outcome of the command, the result may be reported - * via the CLI, but in MI2 syntax. As of yet, this has only been observed for {@code ^error} - * results. - * - * @param evt the event to check - * @return the decoded error event, if applicable, or the original unmodified event. - */ - protected GdbEvent checkErrorViaCli(GdbEvent evt) { - if (evt instanceof GdbConsoleOutputEvent) { - GdbConsoleOutputEvent outEvt = (GdbConsoleOutputEvent) evt; - // This is quirky in 8.0.1. - // I don't know to what other version(s) it applies. - String out = outEvt.getOutput(); - if (out.startsWith("^error")) { - try { - return GdbCommandErrorEvent.fromMi2(out.split(",", 2)[1].trim()); - } - catch (GdbParseError e) { - Msg.error(this, "Could not parse error result", e); - } - } - } - return evt; - } - @Override public boolean handle(GdbEvent evt, GdbPendingCommand pending) { /** @@ -106,7 +75,8 @@ public abstract class AbstractGdbCommand implements GdbCommand { * spurious {@code ^running} command-completion events actually complete any command, except * ones where we expect that result. This seems a bug in GDB to me. * - * UPDATE: It looks like this will be fixed in gdb-14. + * UPDATE: It looks like this will be fixed in gdb-14. Despite the fix we leave this + * workaround in place while we still support older gdb versions. */ if (evt instanceof GdbCommandRunningEvent) { return false; diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/AbstractLaunchGdbCommand.java b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/AbstractLaunchGdbCommand.java index 220938a109..3cb9665397 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/AbstractLaunchGdbCommand.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/impl/cmd/AbstractLaunchGdbCommand.java @@ -41,7 +41,6 @@ public abstract class AbstractLaunchGdbCommand extends AbstractGdbCommand evt, GdbPendingCommand pending) { - evt = checkErrorViaCli(evt); if (evt instanceof GdbThreadCreatedEvent) { pending.claim(evt); } 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 d8e073b7bb..ed03a67f27 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 @@ -43,7 +43,6 @@ public class GdbContinueCommand extends AbstractGdbCommandWithThreadId @Override public boolean handle(GdbEvent evt, GdbPendingCommand pending) { - evt = checkErrorViaCli(evt); // TODO: Deprecated, since that hack can crash GDB return handleExpectingRunning(evt, pending); } 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 5b8c86ad2a..2eceb8c5cb 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 @@ -47,7 +47,6 @@ public class GdbStepCommand extends AbstractGdbCommandWithThreadId @Override public boolean handle(GdbEvent evt, GdbPendingCommand pending) { - evt = checkErrorViaCli(evt); // TODO: Deprecated, since that hack can crash GDB return handleExpectingRunning(evt, 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 index a12e97f38d..9d55a01cad 100644 --- 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 @@ -39,23 +39,55 @@ public interface MixinResumeInCliGdbCommand extends GdbCommand { } default boolean handleExpectingRunning(GdbEvent evt, GdbPendingCommand pending) { - if (evt instanceof GdbCommandRunningEvent) { - pending.claim(evt); - return pending.hasAny(GdbRunningEvent.class); + if (getInterpreter() == Interpreter.CLI) { + /** + * As of gdb-14 (anticipated based on build from git commit b6ac461a), the MI2 console + * will no longer receive ^running from commands issued to the CLI that resume the + * target. I suspect only the console that issues the command is meant to receive the + * ^running result, and that the behavior we had been taking advantage of was in fact a + * bug. Thus, we should only expect the *running event. For the sake of older versions, + * when we receive ^running, we'll still claim it, so long as it comes before *running. + */ + if (evt instanceof GdbRunningEvent) { + pending.claim(evt); + return true; + } + else if (evt instanceof AbstractGdbCompletedCommandEvent) { + pending.claim(evt); + return false; + } } - 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); + else { + /** + * This situation should only occur with versions before about 8.0, anyway, since those + * supporting new-ui should have the CLI. I guess a user could still pass -i mi2 to a + * modern version, which might induce this. In any case, if we're issuing the command + * from MI2, we should expect both the *running event and the ^running result. + */ + 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); + // See comments in handleExpectingRunning + if (getInterpreter() == Interpreter.CLI) { + pending.findSingleOf(GdbRunningEvent.class); + } + else { // MI2 + pending.checkCompletion(GdbCommandRunningEvent.class); + } } } 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 5f05295c77..d97a876071 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 @@ -26,10 +26,12 @@ import java.math.BigInteger; import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.util.*; -import java.util.concurrent.*; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; import java.util.function.Predicate; import java.util.stream.Collectors; +import org.hamcrest.Matchers; import org.junit.*; import agent.gdb.manager.*; @@ -52,7 +54,7 @@ public abstract class AbstractGdbManagerTest extends AbstractGhidraHeadlessInteg protected File gdbBin; protected File findGdbBin() { - return new File(GdbManager.DEFAULT_GDB_CMD); + return new File(System.getProperty("test.gdbmanager.path", GdbManager.DEFAULT_GDB_CMD)); } @Before @@ -73,12 +75,7 @@ public abstract class AbstractGdbManagerTest extends AbstractGhidraHeadlessInteg } protected T waitOn(CompletableFuture future) throws Throwable { - try { - return future.get(TIMEOUT_MILLISECONDS, TimeUnit.MILLISECONDS); - } - catch (ExecutionException e) { - throw e.getCause(); - } + return future.get(TIMEOUT_MILLISECONDS, TimeUnit.MILLISECONDS); } @After @@ -164,9 +161,16 @@ public abstract class AbstractGdbManagerTest extends AbstractGhidraHeadlessInteg @Test public void testListModulesAndSections() throws Throwable { try (GdbManager mgr = GdbManager.newInstance(getPtyFactory())) { + // See testStartInterrupt + LibraryWaiter libcLoaded = new LibraryWaiter(name -> name.contains("libc")); + mgr.addEventsListener(libcLoaded); waitOn(startManager(mgr)); GdbInferior inferior = mgr.currentInferior(); waitOn(inferior.fileExecAndSymbols("/usr/bin/echo")); + waitOn(inferior.start()); // listModules now requires info proc mappings + waitOn(libcLoaded); + Thread.sleep(100); // TODO: Why? + mgr.sendInterruptNow(); Map modules = waitOn(inferior.listModules(false)); GdbModule modEcho = modules.get("/usr/bin/echo"); assertNotNull(modEcho); @@ -287,6 +291,7 @@ public abstract class AbstractGdbManagerTest extends AbstractGhidraHeadlessInteg Msg.debug(this, "Interrupting"); mgr.sendInterruptNow(); Msg.debug(this, "Verifying at syscall"); + Thread.sleep(100); // TODO: Actually wait for *stopped event String out = waitOn(mgr.consoleCapture("x/1i $pc-2")); // TODO: This is x86-specific assertTrue("Didn't stop at syscall", out.contains("syscall")); @@ -317,16 +322,12 @@ public abstract class AbstractGdbManagerTest extends AbstractGhidraHeadlessInteg } } - protected String getExpectedDefaultArgsVar() { - return null; - } - @Test public void testSetVarGetVar() throws Throwable { try (GdbManager mgr = GdbManager.newInstance(getPtyFactory())) { waitOn(startManager(mgr)); String val = waitOn(mgr.currentInferior().getVar("args")); - assertEquals(getExpectedDefaultArgsVar(), val); + assertThat(val, Matchers.emptyOrNullString()); waitOn(mgr.currentInferior().setVar("args", "test")); val = waitOn(mgr.currentInferior().getVar("args")); assertEquals("test", val); diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/test/java/agent/gdb/manager/impl/SpawnedMi2GdbHomeLocalManagerTest.java b/Ghidra/Debug/Debugger-agent-gdb/src/test/java/agent/gdb/manager/impl/SpawnedMi2GdbHomeLocalManagerTest.java index 558cb63503..932c4426f2 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/test/java/agent/gdb/manager/impl/SpawnedMi2GdbHomeLocalManagerTest.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/test/java/agent/gdb/manager/impl/SpawnedMi2GdbHomeLocalManagerTest.java @@ -41,9 +41,4 @@ public class SpawnedMi2GdbHomeLocalManagerTest extends AbstractGdbManagerTest { throw new AssertionError(e); } } - - @Override - protected String getExpectedDefaultArgsVar() { - return ""; - } }