diff --git a/Ghidra/Framework/Pty/src/main/java/ghidra/pty/PtySession.java b/Ghidra/Framework/Pty/src/main/java/ghidra/pty/PtySession.java index f0281ce74d..0695eb6901 100644 --- a/Ghidra/Framework/Pty/src/main/java/ghidra/pty/PtySession.java +++ b/Ghidra/Framework/Pty/src/main/java/ghidra/pty/PtySession.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. @@ -47,9 +47,12 @@ public interface PtySession { void destroyForcibly(); /** - * Get a human-readable description of the session - * - * @return the description + * {@return a human-readable description of the session} */ String description(); + + /** + * {@return the process handle for the session leader} + */ + ProcessHandle handle(); } diff --git a/Ghidra/Framework/Pty/src/main/java/ghidra/pty/local/LocalProcessPtySession.java b/Ghidra/Framework/Pty/src/main/java/ghidra/pty/local/LocalProcessPtySession.java index 20bf77a767..cbb6f70270 100644 --- a/Ghidra/Framework/Pty/src/main/java/ghidra/pty/local/LocalProcessPtySession.java +++ b/Ghidra/Framework/Pty/src/main/java/ghidra/pty/local/LocalProcessPtySession.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. @@ -57,4 +57,9 @@ public class LocalProcessPtySession implements PtySession { public String description() { return "process " + process.pid() + " on " + ptyName; } + + @Override + public ProcessHandle handle() { + return process.toHandle(); + } } diff --git a/Ghidra/Framework/Pty/src/main/java/ghidra/pty/local/LocalWindowsNativeProcessPtySession.java b/Ghidra/Framework/Pty/src/main/java/ghidra/pty/local/LocalWindowsNativeProcessPtySession.java index 4df5657b76..a69178618d 100644 --- a/Ghidra/Framework/Pty/src/main/java/ghidra/pty/local/LocalWindowsNativeProcessPtySession.java +++ b/Ghidra/Framework/Pty/src/main/java/ghidra/pty/local/LocalWindowsNativeProcessPtySession.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. @@ -15,6 +15,7 @@ */ package ghidra.pty.local; +import java.util.Optional; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; @@ -96,4 +97,15 @@ public class LocalWindowsNativeProcessPtySession implements PtySession { public String description() { return "process " + pid + " on " + ptyName; } + + @Override + public ProcessHandle handle() { + IntByReference lpExitCode = new IntByReference(); + Kernel32.INSTANCE.GetExitCodeProcess(processHandle.getNative(), lpExitCode); + Optional result = ProcessHandle.of(pid); + if (lpExitCode.getValue() != WinBase.STILL_ACTIVE) { + return null; + } + return result.orElseThrow(() -> new AssertionError()); + } } diff --git a/Ghidra/Framework/Pty/src/test/java/ghidra/pty/windows/ConPtyTest.java b/Ghidra/Framework/Pty/src/test/java/ghidra/pty/windows/ConPtyTest.java index d85ab6bd8a..0f1d29ffee 100644 --- a/Ghidra/Framework/Pty/src/test/java/ghidra/pty/windows/ConPtyTest.java +++ b/Ghidra/Framework/Pty/src/test/java/ghidra/pty/windows/ConPtyTest.java @@ -15,13 +15,12 @@ */ package ghidra.pty.windows; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.fail; +import static org.junit.Assert.*; import static org.junit.Assume.assumeTrue; import java.io.*; import java.lang.ProcessBuilder.Redirect; +import java.util.concurrent.TimeUnit; import org.junit.Before; import org.junit.Test; @@ -31,8 +30,14 @@ import com.sun.jna.LastErrorException; import ghidra.framework.OperatingSystem; import ghidra.pty.*; import ghidra.pty.testutil.DummyProc; +import ghidra.util.Msg; public class ConPtyTest extends AbstractPtyTest { + public static final int JOIN_TIMEOUT_MS = 3000; + + public static final String CMD = DummyProc.which("cmd.exe"); + public static final String GDB = DummyProc.which("gdb.exe"); + public static final String NOTEPAD = DummyProc.which("notepad.exe"); @Before public void checkWindows() { @@ -40,11 +45,11 @@ public class ConPtyTest extends AbstractPtyTest { } @Test - public void testSessionCmd() throws IOException, InterruptedException { + public void testSessionCmd() throws Exception { try (Pty pty = ConPtyFactory.INSTANCE.openpty()) { - PtySession cmd = pty.getChild().session(new String[] { DummyProc.which("cmd") }, null); + PtySession cmd = pty.getChild().session(new String[] { CMD }, null); pty.getParent().getOutputStream().write("exit\r\n".getBytes()); - assertEquals(0, cmd.waitExited()); + assertEquals(0, cmd.waitExited(JOIN_TIMEOUT_MS, TimeUnit.MILLISECONDS)); } } @@ -60,12 +65,12 @@ public class ConPtyTest extends AbstractPtyTest { } @Test - public void testSessionCmdEchoTest() throws IOException, InterruptedException { + public void testSessionCmdEchoTest() throws Exception { try (Pty pty = ConPtyFactory.INSTANCE.openpty()) { PtyParent parent = pty.getParent(); PrintWriter writer = new PrintWriter(parent.getOutputStream()); BufferedReader reader = loggingReader(parent.getInputStream()); - PtySession cmd = pty.getChild().session(new String[] { DummyProc.which("cmd") }, null); + PtySession cmd = pty.getChild().session(new String[] { CMD }, null); runExitCheck(3, cmd); writer.println("echo test"); @@ -77,38 +82,37 @@ public class ConPtyTest extends AbstractPtyTest { do { line = reader.readLine(); } - while (!"test".equals(line)); - } catch (IOException e) { - // TODO Auto-generated catch block - e.printStackTrace(); + while (!"test".equals(line)); + } + catch (IOException e) { + Msg.info(this, "done reading"); } }); - + t.setDaemon(true); t.start(); - + writer.println("exit 3"); writer.flush(); - assertEquals(3, cmd.waitExited()); - assertFalse("Content read successfully from the cmd", t.isAlive()); + assertEquals(3, cmd.waitExited(JOIN_TIMEOUT_MS, TimeUnit.MILLISECONDS)); + t.join(JOIN_TIMEOUT_MS); } } @Test - public void testSessionGdbLineLength() throws IOException, InterruptedException { + public void testSessionGdbLineLength() throws Exception { try (Pty pty = ConPtyFactory.INSTANCE.openpty()) { PtyParent parent = pty.getParent(); PrintWriter writer = new PrintWriter(parent.getOutputStream()); BufferedReader reader = loggingReader(parent.getInputStream()); - PtySession gdb = - pty.getChild().session(new String[] { "C:\\msys64\\mingw64\\bin\\gdb.exe" }, null); + PtySession gdb = pty.getChild().session(new String[] { GDB }, null); writer.println( "echo This line is cleary much, much, much, much, much, much, much, much, much " + " longer than 80 characters"); writer.flush(); - + // set up reading cmd output on a thread since "readLine" is blocking Thread t = new Thread(() -> { String line; @@ -116,87 +120,145 @@ public class ConPtyTest extends AbstractPtyTest { do { line = reader.readLine(); } - while (!"test".equals(line)); - } catch (IOException e) { - // TODO Auto-generated catch block - e.printStackTrace(); + while (!"test".equals(line)); + } + catch (IOException e) { + Msg.info(this, "done reading"); } }); - + t.setDaemon(true); t.start(); - + writer.println("exit 3"); writer.flush(); - assertEquals(3, gdb.waitExited()); - assertFalse("Content read successfully from the cmd", t.isAlive()); + assertEquals(3, gdb.waitExited(JOIN_TIMEOUT_MS, TimeUnit.MILLISECONDS)); + t.join(JOIN_TIMEOUT_MS); } } - @Test + /** + * Verifies that the ConPty is actually necessary to send interrupts to child processes. + *

+ * Sending char 3 down the stdin is not sufficient, as demonstrated in this experiment. GDB does + * not receive the interrupt, and so the target process remains running, and none of the + * subsequent gdb commands are processed. Thus, the target and gdb are still running by the time + * we get to the {@link Process#waitFor(long, TimeUnit)} call. It will return false, thus + * causing the expected {@link AssertionError}. + * + * @throws Exception + * 'tis a test + */ + @Test(expected = AssertionError.class) public void testGdbInterruptPlain() throws Exception { - ProcessBuilder builder = new ProcessBuilder("C:\\msys64\\mingw64\\bin\\gdb.exe"); - builder.redirectOutput(Redirect.PIPE); - builder.redirectInput(Redirect.PIPE); - builder.redirectErrorStream(true); - Process gdb = builder.start(); + boolean terminated = false; + Process gdb = null; + try { + ProcessBuilder builder = new ProcessBuilder(GDB); + builder.redirectOutput(Redirect.PIPE); + builder.redirectInput(Redirect.PIPE); + builder.redirectErrorStream(true); - PrintWriter writer = new PrintWriter(gdb.getOutputStream()); - pump(gdb.getInputStream(), System.err); + gdb = builder.start(); - System.out.println("Testing"); - writer.println("echo test"); - writer.println("set new-console on"); - System.out.println("Launching notepad"); - writer.println("file C:\\\\Windows\\\\notepad.exe"); - writer.println("run"); - writer.flush(); - System.out.println("Waiting"); - Thread.sleep(3000); - System.out.println("Interrupting"); - writer.write(3); - writer.println(); - writer.flush(); - System.out.println("Killing"); - writer.println("kill"); - writer.flush(); - writer.println("y"); - writer.flush(); - } + PrintWriter writer = new PrintWriter(gdb.getOutputStream()); + pump(gdb.getInputStream(), System.out); - @Test - public void testGdbInterruptConPty() throws Exception { - try (Pty pty = ConPtyFactory.INSTANCE.openpty()) { - PtyParent parent = pty.getParent(); - PrintWriter writer = new PrintWriter(parent.getOutputStream()); - //BufferedReader reader = loggingReader(parent.getInputStream()); - PtySession gdb = - pty.getChild().session(new String[] { "C:\\msys64\\mingw64\\bin\\gdb.exe" }, null); - - pump(parent.getInputStream(), System.err); - - System.out.println("Testing"); + Msg.info(this, "Testing"); writer.println("echo test"); writer.println("set new-console on"); - System.out.println("Launching notepad"); - writer.println("file C:\\\\Windows\\\\notepad.exe"); + Msg.info(this, "Launching notepad"); + writer.println("file %s".formatted(NOTEPAD.replace("\\", "\\\\"))); writer.println("run"); writer.flush(); - System.out.println("Waiting"); + Msg.info(this, "Waiting"); Thread.sleep(3000); - System.out.println("Interrupting"); + Msg.info(this, "Interrupting"); writer.write(3); writer.println(); writer.flush(); - System.out.println("Killing"); + Thread.sleep(1000); + Msg.info(this, "Killing"); writer.println("kill"); writer.flush(); writer.println("y"); writer.flush(); + writer.println("quit"); + writer.flush(); - Thread.sleep(100000); + terminated = gdb.waitFor(JOIN_TIMEOUT_MS, TimeUnit.MILLISECONDS); + assertTrue("Gdb did not terminate", terminated); + } + finally { + if (gdb != null && !terminated) { + for (ProcessHandle child : gdb.descendants().toList()) { + Msg.info(this, "Killing descendant process: %d".formatted(child.pid())); + child.destroyForcibly(); + } + Msg.info(this, "Killing gdb: %d".formatted(gdb.pid())); + gdb.destroyForcibly(); + } + } + } + + /** + * STRANGENESS: This test will fail if Eclipse was started from git-bash on Windows. I can only + * guess this is because of some strange interaction between ConPty (under test here) and the + * WinPty hack that git-bash still uses? Specifically, the interrupt (char 3) is not causing the + * signal to actually get sent to gdb. I haven't the slightest idea where it goes instead, if + * anywhere. + * + * @throws Exception + * 'tis a test + */ + @Test + public void testGdbInterruptConPty() throws Exception { + PtySession gdb = null; + try (Pty pty = ConPtyFactory.INSTANCE.openpty()) { + PtyParent parent = pty.getParent(); + PrintWriter writer = new PrintWriter(parent.getOutputStream()); + gdb = pty.getChild().session(new String[] { GDB }, null); + + pump(parent.getInputStream(), System.err); + + Msg.info(this, "Testing"); + writer.println("echo test"); + writer.println("set new-console on"); + Msg.info(this, "Launching notepad"); + writer.println("file %s".formatted(NOTEPAD.replace("\\", "\\\\"))); + writer.println("run"); + writer.flush(); + Msg.info(this, "Waiting"); + Thread.sleep(3000); + + Msg.info(this, "Interrupting"); + writer.write(3); + writer.println(); + writer.flush(); + Thread.sleep(1000); + + Msg.info(this, "Killing"); + writer.println("kill"); + writer.flush(); + writer.println("y"); + writer.flush(); + writer.println("quit"); + writer.flush(); + + gdb.waitExited(JOIN_TIMEOUT_MS, TimeUnit.MILLISECONDS); + } + finally { + ProcessHandle handle = gdb.handle(); + if (handle != null) { + for (ProcessHandle child : handle.descendants().toList()) { + Msg.info(this, "Killing descendant process: %d".formatted(child.pid())); + child.destroyForcibly(); + } + Msg.info(this, "Killing gdb: %d".formatted(handle.pid())); + gdb.destroyForcibly(); + } } } @@ -207,8 +269,7 @@ public class ConPtyTest extends AbstractPtyTest { PrintWriter writer = new PrintWriter(parent.getOutputStream()); //BufferedReader reader = loggingReader(parent.getInputStream()); PtySession gdb = pty.getChild() - .session(new String[] { "C:\\msys64\\mingw64\\bin\\gdb.exe", "-i", "mi2" }, - null); + .session(new String[] { GDB, "-i", "mi2" }, null); InputStream inputStream = parent.getInputStream(); inputStream = new AnsiBufferedInputStream(inputStream); @@ -218,8 +279,7 @@ public class ConPtyTest extends AbstractPtyTest { writer.println("-interpreter-exec console \"quit\""); writer.flush(); - gdb.waitExited(); - //System.out.println("Exited"); + gdb.waitExited(JOIN_TIMEOUT_MS, TimeUnit.MILLISECONDS); } } } diff --git a/Ghidra/Test/DebuggerIntegrationTest/src/test/java/ghidra/app/plugin/core/debug/gui/tracermi/launcher/ScriptTraceRmiLaunchOfferTest.java b/Ghidra/Test/DebuggerIntegrationTest/src/test/java/ghidra/app/plugin/core/debug/gui/tracermi/launcher/ScriptTraceRmiLaunchOfferTest.java index d1986336ac..fd1c6b0667 100644 --- a/Ghidra/Test/DebuggerIntegrationTest/src/test/java/ghidra/app/plugin/core/debug/gui/tracermi/launcher/ScriptTraceRmiLaunchOfferTest.java +++ b/Ghidra/Test/DebuggerIntegrationTest/src/test/java/ghidra/app/plugin/core/debug/gui/tracermi/launcher/ScriptTraceRmiLaunchOfferTest.java @@ -224,6 +224,11 @@ public class ScriptTraceRmiLaunchOfferTest extends AbstractGhidraHeadedDebuggerT public String description() { return null; } + + @Override + public ProcessHandle handle() { + return null; + } } record MockPty() implements Pty {