GP-994: Improve error reporting when GADP-based models fail to start

This commit is contained in:
Dan 2022-11-08 16:37:40 -05:00
parent 7a92882ba3
commit b4d2cb75ba
14 changed files with 80 additions and 16 deletions

View file

@ -25,6 +25,7 @@ import java.util.concurrent.ExecutionException;
import org.apache.commons.lang3.exception.ExceptionUtils;
import agent.dbgeng.gadp.impl.DbgEngGadpServerImpl;
import ghidra.async.AsyncUtils;
import ghidra.dbg.agent.AgentWindow;
import ghidra.util.Msg;
@ -84,7 +85,8 @@ public interface DbgEngGadpServer extends AutoCloseable {
try (DbgEngGadpServer server = newInstance(bindTo)) {
//TODO: fix/test the debugConnect case when args are passed
server.startDbgEng(dbgengArgs.toArray(new String[] {})).exceptionally(e -> {
Msg.error(this, "Error starting dbgeng/GADP", e);
e = AsyncUtils.unwrapThrowable(e);
Msg.error(this, "Error starting dbgeng/GADP: " + e);
System.exit(-1);
return null;
});

View file

@ -23,6 +23,7 @@ import org.apache.commons.lang3.exception.ExceptionUtils;
import agent.dbgeng.gadp.DbgEngGadpServer;
import agent.dbgmodel.gadp.impl.DbgModelGadpServerImpl;
import ghidra.async.AsyncUtils;
import ghidra.dbg.agent.AgentWindow;
import ghidra.util.Msg;
@ -114,7 +115,8 @@ public interface DbgModelGadpServer extends DbgEngGadpServer {
try (DbgModelGadpServer server = newInstance(bindTo)) {
server.startDbgEng(dbgengArgs.toArray(new String[] {})).exceptionally(e -> {
Msg.error(this, "Error starting dbgeng/GADP", e);
e = AsyncUtils.unwrapThrowable(e);
Msg.error(this, "Error starting dbgeng/GADP: " + e);
System.exit(-1);
return null;
});

View file

@ -23,6 +23,7 @@ import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import agent.frida.gadp.impl.FridaGadpServerImpl;
import ghidra.async.AsyncUtils;
import ghidra.dbg.agent.AgentWindow;
import ghidra.util.Msg;
@ -73,7 +74,8 @@ public interface FridaGadpServer extends AutoCloseable {
try (FridaGadpServer server = newInstance(bindTo)) {
//TODO: fix/test the debugConnect case when args are passed
server.startFrida(fridaArgs.toArray(new String[] {})).exceptionally(e -> {
Msg.error(this, "Error starting Frida/GADP", e);
e = AsyncUtils.unwrapThrowable(e);
Msg.error(this, "Error starting Frida/GADP: " + e);
System.exit(-1);
return null;
});

View file

@ -25,6 +25,7 @@ import java.util.concurrent.ExecutionException;
import org.apache.commons.lang3.exception.ExceptionUtils;
import agent.gdb.gadp.impl.GdbGadpServerImpl;
import ghidra.async.AsyncUtils;
import ghidra.dbg.agent.AgentWindow;
import ghidra.util.Msg;
@ -54,7 +55,8 @@ public interface GdbGadpServer extends AutoCloseable {
try (GdbGadpServer server = newInstance(bindTo)) {
server.startGDB(gdbCmd, gdbArgs.toArray(new String[] {})).exceptionally(e -> {
Msg.error(this, "Error starting GDB/GADP", e);
e = AsyncUtils.unwrapThrowable(e);
Msg.error(this, "Error starting GDB/GADP: " + e);
System.exit(-1);
return null;
});

View file

@ -68,8 +68,6 @@ public class LinuxPtySessionLeader {
LIB_POSIX.execv(subArgs.get(0), subArgs.toArray(new String[0]));
}
catch (Throwable t) {
System.err.println("Error with ptyPath = '" + ptyPath + "', and subArgs = " + subArgs);
t.printStackTrace();
if (bk != -1) {
try {
int bkt = bk;
@ -80,7 +78,8 @@ public class LinuxPtySessionLeader {
System.exit(-1);
}
}
throw t;
System.err.println("Could not execute " + subArgs.get(0) + ": " + t.getMessage());
System.exit(-1);
}
}
}

View file

@ -23,6 +23,7 @@ import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import agent.lldb.gadp.impl.LldbGadpServerImpl;
import ghidra.async.AsyncUtils;
import ghidra.dbg.agent.AgentWindow;
import ghidra.util.Msg;
@ -73,7 +74,8 @@ public interface LldbGadpServer extends AutoCloseable {
try (LldbGadpServer server = newInstance(bindTo)) {
//TODO: fix/test the debugConnect case when args are passed
server.startLLDB(lldbArgs.toArray(new String[] {})).exceptionally(e -> {
Msg.error(this, "Error starting lldb/GADP", e);
e = AsyncUtils.unwrapThrowable(e);
Msg.error(this, "Error starting lldb/GADP: " + e);
System.exit(-1);
return null;
});

View file

@ -138,6 +138,8 @@ public abstract class AbstractGadpLocalDebuggerModelFactory implements DebuggerM
if (line.startsWith(AbstractGadpServer.LISTENING_ON)) {
String[] parts = line.split(":"); // Separates address from port
port = Integer.parseInt(parts[parts.length - 1]);
}
if (AbstractGadpServer.READY.equals(line)) {
ready.complete(null);
}
}
@ -153,7 +155,6 @@ public abstract class AbstractGadpLocalDebuggerModelFactory implements DebuggerM
ready.completeExceptionally(
new RuntimeException("Agent terminated unexpectedly"));
}
}
}
catch (Throwable e) {

View file

@ -18,6 +18,7 @@ package ghidra.dbg.gadp.server;
import java.io.IOException;
import java.net.SocketAddress;
import java.nio.channels.AsynchronousSocketChannel;
import java.util.concurrent.CompletableFuture;
import ghidra.comm.service.AbstractAsyncServer;
import ghidra.dbg.*;
@ -30,6 +31,7 @@ public abstract class AbstractGadpServer
extends AbstractAsyncServer<AbstractGadpServer, GadpClientHandler>
implements DebuggerModelListener {
public static final String LISTENING_ON = "GADP Server listening on ";
public static final String READY = "GADP Server model ready";
protected final DebuggerObjectModel model;
private boolean exitOnClosed = true;
@ -42,6 +44,12 @@ public abstract class AbstractGadpServer
model.addModelListener(this);
}
@Override
public CompletableFuture<Void> launchAsyncService() {
System.out.println(READY);
return super.launchAsyncService();
}
public DebuggerObjectModel getModel() {
return model;
}
@ -70,7 +78,6 @@ public abstract class AbstractGadpServer
@Override
public void modelClosed(DebuggerModelClosedReason reason) {
System.err.println("Model closed: " + reason);
if (exitOnClosed) {
System.exit(0);
}

View file

@ -277,10 +277,12 @@ public class AsyncLock {
/**
* Queue a sequence of actions on this lock
*
* <p>
* The lock will be acquired before executing the first action of the sequence, and the hold
* will be automatically released upon completion, whether normal or exceptional. The first
* action receives a reference to the hold, which may be used to re-enter the lock.
*
* <p>
* If the sequence stalls, i.e., an action never completes, it will cause deadlock.
*
* @param type the type "returned" by the sequence
@ -297,6 +299,7 @@ public class AsyncLock {
/**
* Queue a sequence of actions on this lock
*
* <p>
* Identical to {@link #with(TypeSpec, Hold)} except that the acquired hold is stored into an
* atomic reference rather than passed to the first action.
*

View file

@ -18,7 +18,8 @@ package ghidra.async;
import java.lang.ref.Cleaner.Cleanable;
import java.lang.ref.WeakReference;
import java.util.*;
import java.util.concurrent.*;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.RejectedExecutionException;
import java.util.function.Function;
import java.util.function.Predicate;
@ -375,7 +376,7 @@ public class AsyncReference<T, C> {
}
}
ExecutionException ex = new ExecutionException("Disposed", reason);
DisposedException ex = new DisposedException(reason);
for (CompletableFuture<?> future : toExcept) {
future.completeExceptionally(ex);
}

View file

@ -0,0 +1,22 @@
/* ###
* 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 ghidra.async;
public class DisposedException extends RuntimeException {
public DisposedException(Throwable reason) {
super(reason);
}
}

View file

@ -38,6 +38,7 @@ public class AsyncSequenceWithoutTemp<R> {
/**
* Construct a new sequence without a temporary value
*
* <p>
* Do not call this directly. Please use {@link AsyncUtils#sequence(TypeSpec)}.
*
* @param seqResult the result of the whole sequence, passed to each appended sequence
@ -180,6 +181,7 @@ public class AsyncSequenceWithoutTemp<R> {
/**
* Finish defining this sequence of actions and obtain its future result
*
* <p>
* When an action in the sequence calls {@link AsyncHandlerCanExit#exit(Object, Throwable)}, the
* returned {@link CompletableFuture} is completed. If any action completes exceptionally, the
* returned {@link CompletableFuture} is completed exceptionally. If the final action executes,
@ -197,13 +199,17 @@ public class AsyncSequenceWithoutTemp<R> {
/**
* Register an action to execute on sequence completion
*
* <p>
* All registered actions are submitted for execution simultaneously when an action in the
* sequence calls {@link AsyncHandlerCanExit#exit(Object, Throwable)}. This is useful for
* methods that begin executing sequences "with a context". It is roughly equivalent to a
* {@code finally} block. On-exit actions can be registered before other actions are appended to
* the chain.
*
* An uncaught exception in an on-exit action will simply be logged and ignored.
* <p>
* If the sequence completes exceptionally, that exception is passed to the action. This method
* cannot be used to handle the exception, since this method returns this same sequence, not the
* resulting one. An uncaught exception in an on-exit action will simply be logged and ignored.
*
* @param action the action to execute
*/

View file

@ -87,6 +87,18 @@ public class AsyncLockTest {
}).finish();
}
@Test
public void testWithError() throws Throwable {
AsyncLock l = new AsyncLock();
int result = l.with(TypeSpec.INT, null).then((own, seq) -> {
throw new AssertionError("Blargh");
}).finish().exceptionally(exc -> {
return 0xdead;
}).get(1000, TimeUnit.MILLISECONDS);
assertEquals(0xdead, result);
}
@Test
public void testReentry() {
// This is very contrived. A real use would pass ownership to some method which cannot

View file

@ -20,8 +20,7 @@ import java.util.concurrent.CompletableFuture;
import java.util.concurrent.RejectedExecutionException;
import java.util.function.Predicate;
import ghidra.async.AsyncUtils;
import ghidra.async.TypeSpec;
import ghidra.async.*;
import ghidra.dbg.error.*;
import ghidra.dbg.target.TargetMemory;
import ghidra.dbg.target.TargetObject;
@ -558,12 +557,16 @@ public interface DebuggerObjectModel {
* @param ex the exception
*/
default void reportError(Object origin, String message, Throwable ex) {
Throwable unwrapped = AsyncUtils.unwrapThrowable(ex);
if (ex == null || DebuggerModelTerminatingException.isIgnorable(ex)) {
Msg.warn(origin, message + ": " + ex);
}
else if (AsyncUtils.unwrapThrowable(ex) instanceof RejectedExecutionException) {
else if (unwrapped instanceof RejectedExecutionException) {
Msg.trace(origin, "Ignoring rejection", ex);
}
else if (unwrapped instanceof DisposedException) {
Msg.trace(origin, "Ignoring disposal", ex);
}
else {
Msg.error(origin, message, ex);
}