GP-0: from review

This commit is contained in:
d-millar 2025-01-10 20:50:09 +00:00
parent 4fd092bb73
commit fe71611803
6 changed files with 119 additions and 119 deletions

View file

@ -973,6 +973,8 @@ def ghidra_trace_put_environment():
put_environment() put_environment()
# Detect whether this is supported before defining the command
if hasattr(drgn, 'RelocatableModule'):
def put_regions(): def put_regions():
nproc = util.selected_process() nproc = util.selected_process()
if nproc is None: if nproc is None:
@ -1019,6 +1021,8 @@ def ghidra_trace_put_regions():
# Detect whether this is supported before defining the command
if hasattr(drgn, 'RelocatableModule'):
def put_modules(): def put_modules():
nproc = util.selected_process() nproc = util.selected_process()
if nproc is None: if nproc is None:
@ -1073,6 +1077,7 @@ def ghidra_trace_put_modules():
put_modules() put_modules()
# Detect whether this is supported before defining the command
if hasattr(drgn, 'RelocatableModule'): if hasattr(drgn, 'RelocatableModule'):
def put_sections(m : drgn.RelocatableModule): def put_sections(m : drgn.RelocatableModule):
nproc = util.selected_process() nproc = util.selected_process()

View file

@ -263,6 +263,7 @@ def refresh_locals(node: sch.Schema('LocalsContainer')):
commands.ghidra_trace_put_locals() commands.ghidra_trace_put_locals()
if hasattr(drgn, 'RelocatableModule'):
@REGISTRY.method(action='refresh', display='Refresh Memory') @REGISTRY.method(action='refresh', display='Refresh Memory')
def refresh_mappings(node: sch.Schema('Memory')): def refresh_mappings(node: sch.Schema('Memory')):
"""Refresh the list of memory regions for the process.""" """Refresh the list of memory regions for the process."""
@ -270,6 +271,7 @@ def refresh_mappings(node: sch.Schema('Memory')):
commands.ghidra_trace_put_regions() commands.ghidra_trace_put_regions()
if hasattr(drgn, 'RelocatableModule'):
@REGISTRY.method(action='refresh', display='Refresh Modules') @REGISTRY.method(action='refresh', display='Refresh Modules')
def refresh_modules(node: sch.Schema('ModuleContainer')): def refresh_modules(node: sch.Schema('ModuleContainer')):
""" """

View file

@ -230,10 +230,6 @@ public abstract class AbstractDrgnTraceRmiTest extends AbstractGhidraHeadedDebug
protected record PythonAndConnection(ExecInDrgn exec, TraceRmiConnection connection) protected record PythonAndConnection(ExecInDrgn exec, TraceRmiConnection connection)
implements AutoCloseable { implements AutoCloseable {
protected boolean hasMethod(String name) {
return connection.getMethods().get(name) != null;
}
protected RemoteMethod getMethod(String name) { protected RemoteMethod getMethod(String name) {
return Objects.requireNonNull(connection.getMethods().get(name)); return Objects.requireNonNull(connection.getMethods().get(name));
} }

View file

@ -17,6 +17,7 @@ package agent.drgn.rmi;
import static org.hamcrest.Matchers.*; import static org.hamcrest.Matchers.*;
import static org.junit.Assert.*; import static org.junit.Assert.*;
import static org.junit.Assume.*;
import java.util.*; import java.util.*;
@ -200,9 +201,7 @@ public class DrgnMethodsTest extends AbstractDrgnTraceRmiTest {
txCreate(conn, path); txCreate(conn, path);
String out = conn.executeCapture("print(hasattr(drgn, 'RelocatableModule'))").strip(); String out = conn.executeCapture("print(hasattr(drgn, 'RelocatableModule'))").strip();
if (out.equals("False")) { assumeFalse(out.equals("False"));
return;
}
RemoteMethod refreshMappings = conn.getMethod("refresh_mappings"); RemoteMethod refreshMappings = conn.getMethod("refresh_mappings");
try (ManagedDomainObject mdo = openDomainObject(MDO)) { try (ManagedDomainObject mdo = openDomainObject(MDO)) {
@ -227,9 +226,7 @@ public class DrgnMethodsTest extends AbstractDrgnTraceRmiTest {
txCreate(conn, path); txCreate(conn, path);
String out = conn.executeCapture("print(hasattr(drgn, 'RelocatableModule'))").strip(); String out = conn.executeCapture("print(hasattr(drgn, 'RelocatableModule'))").strip();
if (out.equals("False")) { assumeFalse(out.equals("False"));
return;
}
RemoteMethod refreshModules = conn.getMethod("refresh_modules"); RemoteMethod refreshModules = conn.getMethod("refresh_modules");
try (ManagedDomainObject mdo = openDomainObject(MDO)) { try (ManagedDomainObject mdo = openDomainObject(MDO)) {

View file

@ -132,7 +132,7 @@
<h3>Unit tests</h3> <h3>Unit tests</h3>
<p>The hardest part of writing unit tests is almost always getting the first test to run, and the easiest unit tests, as with the Python files, are those for <code>commands.py</code>. For <strong>drgn</strong>, as before, were using <strong>dbgeng</strong> as the pattern, but several elements had to be changed. Because the launchers execute a script, we need to amend the <code>runThrowError</code> logic (and, more specifically, the <code>execInPython</code> logic) in <a href="../../../Ghidra/Test/DebuggerIntegrationTest/src/test.slow/java/agent/drgn/rmi/AbstractDrgnTraceRmiTest.java"><code>AbstractDrgnTraceRmiTest</code></a> with a <code>ProcessBuilder</code> call that takes a script, rather than writing the script to stdin. While there, we can also trim out the unnecessary helper logic around items like breakpoints, watchpoints, etc. from all of the test classes.</p> <p>The hardest part of writing unit tests is almost always getting the first test to run, and the easiest unit tests, as with the Python files, are those for <code>commands.py</code>. For <strong>drgn</strong>, as before, were using <strong>dbgeng</strong> as the pattern, but several elements had to be changed. Because the launchers execute a script, we need to amend the <code>runThrowError</code> logic (and, more specifically, the <code>execInPython</code> logic) in <a href="../../../Ghidra/Test/DebuggerIntegrationTest/src/test.slow/java/agent/drgn/rmi/AbstractDrgnTraceRmiTest.java"><code>AbstractDrgnTraceRmiTest</code></a> with a <code>ProcessBuilder</code> call that takes a script, rather than writing the script to stdin. While there, we can also trim out the unnecessary helper logic around items like breakpoints, watchpoints, etc. from all of the test classes.</p>
<p>JUnits for <code>methods.py</code> follow a similar pattern, but, again, getting the first one to run is often the most difficult. For <strong>drgn</strong>, weve had to override the timeouts in <code>waitForPass</code> and <code>waitForCondition</code>. After starting with hardcoded paths for the test target, we also had to add logic to re-write the <code>PREAMBLE</code> on-the-fly in <code>execInDrgn</code>. Obviously, with no real <code>hooks.py</code> logic, theres no need for <code>DrgnHooksTest</code>.</p> <p>JUnits for <code>methods.py</code> follow a similar pattern, but, again, getting the first one to run is often the most difficult. For <strong>drgn</strong>, weve had to override the timeouts in <code>waitForPass</code> and <code>waitForCondition</code>. After starting with hardcoded paths for the test target, we also had to add logic to re-write the <code>PREAMBLE</code> on-the-fly in <code>execInDrgn</code>. Obviously, with no real <code>hooks.py</code> logic, theres no need for <code>DrgnHooksTest</code>.</p>
<p>Of note, weve used the gdb <code>gcore</code> command to create a core dump for the tests. Both user- and kernel-mode require privileges to run the debugger, and, for testing, thats not ideal. <a href="../../../Ghidra/Test/DebuggerIntegrationTest/build.gradle"><code>build.gradle</code></a> will also need to be modified to include the new debugger package.</p> <p>Of note, weve used the gdb <code>gcore</code> command to create a core dump for the tests. Both user- and kernel-mode require privileges to run the debugger, and, for testing, thats not ideal. <a href="../../../Ghidra/Test/DebuggerIntegrationTest/build.gradle"><code>build.gradle</code></a> for IntegrationTest projext will also need to be modified to include the new debugger package.</p>
</section> </section>
<section id="documentation" class="level3"> <section id="documentation" class="level3">
<h3>Documentation</h3> <h3>Documentation</h3>

View file

@ -211,7 +211,7 @@ Obviously, with no real `hooks.py` logic, there's no need for `DrgnHooksTest`.
Of note, we've used the gdb `gcore` command to create a core dump for the tests. Of note, we've used the gdb `gcore` command to create a core dump for the tests.
Both user- and kernel-mode require privileges to run the debugger, and, for testing, that's not ideal. Both user- and kernel-mode require privileges to run the debugger, and, for testing, that's not ideal.
[`build.gradle`](../../../Ghidra/Test/DebuggerIntegrationTest/build.gradle) will also need to be modified to include the new debugger package. [`build.gradle`](../../../Ghidra/Test/DebuggerIntegrationTest/build.gradle) for IntegrationTest projext will also need to be modified to include the new debugger package.
### Documentation ### Documentation