GP-4278: Restrict auto-disassembly to KNOWN memory. Fix offcut re-dis…

…assembly.

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/disassemble/DebuggerDisassemblerPlugin.java

@@ -16,6 +16,7 @@
 package ghidra.app.plugin.core.debug.disassemble;
 
 import java.util.*;
+import java.util.Map.Entry;
 
 import docking.ActionContext;
 import docking.Tool;
@@ -33,15 +34,19 @@
 import ghidra.framework.plugintool.annotation.AutoServiceConsumed;
 import ghidra.framework.plugintool.util.PluginStatus;
 import ghidra.program.model.address.Address;
+import ghidra.program.model.address.AddressSetView;
 import ghidra.program.model.lang.*;
 import ghidra.program.util.DefaultLanguageService;
 import ghidra.program.util.ProgramContextImpl;
-import ghidra.trace.model.Trace;
+import ghidra.trace.model.*;
 import ghidra.trace.model.guest.TraceGuestPlatform;
 import ghidra.trace.model.guest.TracePlatform;
+import ghidra.trace.model.memory.*;
 import ghidra.trace.model.program.TraceProgramView;
 import ghidra.trace.model.target.TraceObject;
 import ghidra.trace.model.thread.TraceThread;
+import ghidra.util.IntersectionAddressSetView;
+import ghidra.util.UnionAddressSetView;
 
 @PluginInfo(
  shortDescription = "Disassemble trace bytes in the debugger",
@@ -100,6 +105,89 @@ public static RegisterValue deriveAlternativeDefaultContext(Language language,
  return result;
  }
 
+ /**
+ * Determine whether the given address is known, or has ever been known in read-only memory, for
+ * the given snapshot
+ * 
+ * <p>
+ * This first examines the memory state. If the current state is {@link TraceMemoryState#KNOWN},
+ * then it returns the snap for the entry. (Because scratch snaps are allowed, the returned snap
+ * may be from an "earlier" snap in the viewport.) Then, it examines the most recent entry. If
+ * one cannot be found, or the found entry's state is <em>not</em>
+ * {@link TraceMemoryState#KNOWN}, it returns null. If the most recent (but not current) entry
+ * is {@link TraceMemoryState#KNOWN}, then it checks whether or not the memory is writable. If
+ * it's read-only, then the snap for that most-recent entry is returned. Otherwise, this check
+ * assumes the memory could have changed since, and so it returns null.
+ * 
+ * @param start the address to check
+ * @param trace the trace whose memory to examine
+ * @param snap the lastest snapshot key, possibly a scratch snapshot, to consider
+ * @return null to indicate the address failed the test, or the defining snapshot key if the
+ * address passed the test.
+ */
+ public static Long isKnownRWOrEverKnownRO(Address start, Trace trace, long snap) {
+ TraceMemoryManager memoryManager = trace.getMemoryManager();
+ Entry<Long, TraceMemoryState> kent = memoryManager.getViewState(snap, start);
+ if (kent != null && kent.getValue() == TraceMemoryState.KNOWN) {
+ return kent.getKey();
+ }
+ Entry<TraceAddressSnapRange, TraceMemoryState> mrent =
+ memoryManager.getViewMostRecentStateEntry(snap, start);
+ if (mrent == null || mrent.getValue() != TraceMemoryState.KNOWN) {
+ // It has never been known up to this snap
+ return null;
+ }
+ TraceMemoryRegion region =
+ memoryManager.getRegionContaining(mrent.getKey().getY1(), start);
+ if (region == null || region.isWrite()) {
+ // It could have changed this snap, so unknown
+ return null;
+ }
+ return mrent.getKey().getY1();
+ }
+
+ /**
+ * Compute a lazy address set for restricting auto-disassembly
+ * 
+ * <p>
+ * The view contains the addresses in {@code known | (readOnly & everKnown)}, where {@code
+ * known} is the set of addresses in the {@link TraceMemoryState#KNOWN} state, {@code readOnly}
+ * is the set of addresses in a {@link TraceMemoryRegion} having
+ * {@link TraceMemoryRegion#isWrite()} false, and {@code everKnown} is the set of addresses in
+ * the {@link TraceMemoryState#KNOWN} state in any previous snapshot.
+ * 
+ * <p>
+ * In plainer English, we want addresses that have freshly read bytes right now, or addresses in
+ * read-only memory that have ever been read. Anything else is either the default 0s (never
+ * read), or could have changed since last read, and so we will refrain from disassembling.
+ * 
+ * <p>
+ * TODO: Is this composition of laziness upon laziness efficient enough? Can experiment with
+ * ordering of address-set-view "expression" to optimize early termination.
+ * 
+ * @param start the intended starting address for disassembly
+ * @param trace the trace whose memory to disassemble
+ * @param snap the current snapshot key, possibly a scratch snapshot
+ * @return the lazy address set
+ */
+ public static AddressSetView computeAutoDisassembleAddresses(Address start, Trace trace,
+ long snap) {
+ Long ks = isKnownRWOrEverKnownRO(start, trace, snap);
+ if (ks == null) {
+ return null;
+ }
+ TraceMemoryManager memoryManager = trace.getMemoryManager();
+ AddressSetView readOnly =
+ memoryManager.getRegionsAddressSetWith(ks, r -> !r.isWrite());
+ AddressSetView everKnown = memoryManager.getAddressesWithState(Lifespan.since(ks),
+ s -> s == TraceMemoryState.KNOWN);
+ AddressSetView roEverKnown = new IntersectionAddressSetView(readOnly, everKnown);
+ AddressSetView known =
+ memoryManager.getAddressesWithState(ks, s -> s == TraceMemoryState.KNOWN);
+ AddressSetView disassemblable = new UnionAddressSetView(known, roEverKnown);
+ return disassemblable;
+ }
+
  @AutoServiceConsumed
  DebuggerTraceManagerService traceManager;
  @AutoServiceConsumed

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/listing/DebuggerListingProvider.java

@@ -48,6 +48,7 @@
 import ghidra.app.plugin.core.clipboard.CodeBrowserClipboardProvider;
 import ghidra.app.plugin.core.codebrowser.CodeViewerProvider;
 import ghidra.app.plugin.core.codebrowser.MarkerServiceBackgroundColorModel;
+import ghidra.app.plugin.core.debug.disassemble.DebuggerDisassemblerPlugin;
 import ghidra.app.plugin.core.debug.disassemble.TraceDisassembleCommand;
 import ghidra.app.plugin.core.debug.gui.DebuggerLocationLabel;
 import ghidra.app.plugin.core.debug.gui.DebuggerResources;
@@ -1197,8 +1198,11 @@ protected void doAutoDisassemble(Address start) {
  if (exists != null) {
  return;
  }
- AddressSpace space = start.getAddressSpace();
- AddressSet set = new AddressSet(space.getMinAddress(), space.getMaxAddress());
+ AddressSetView set = DebuggerDisassemblerPlugin.computeAutoDisassembleAddresses(start,
+ current.getTrace(), current.getViewSnap());
+ if (set == null) {
+ return;
+ }
  TraceDisassembleCommand dis =
  new TraceDisassembleCommand(current.getPlatform(), start, set);
  dis.run(tool, view);

Ghidra/Debug/Debugger/src/test/java/ghidra/app/plugin/core/debug/disassemble/DebuggerDisassemblyTest.java

@@ -53,14 +53,15 @@
 import ghidra.trace.database.listing.DBTraceInstructionsMemoryView;
 import ghidra.trace.database.memory.DBTraceMemoryManager;
 import ghidra.trace.database.memory.DBTraceMemorySpace;
+import ghidra.trace.database.stack.DBTraceStackManager;
 import ghidra.trace.database.target.DBTraceObject;
 import ghidra.trace.database.target.DBTraceObjectManager;
 import ghidra.trace.model.Lifespan;
 import ghidra.trace.model.guest.TraceGuestPlatform;
 import ghidra.trace.model.memory.TraceMemoryFlag;
 import ghidra.trace.model.memory.TraceObjectMemoryRegion;
 import ghidra.trace.model.program.TraceProgramView;
-import ghidra.trace.model.stack.TraceObjectStackFrame;
+import ghidra.trace.model.stack.*;
 import ghidra.trace.model.target.TraceObject.ConflictResolution;
 import ghidra.trace.model.target.TraceObjectKeyPath;
 import ghidra.trace.model.thread.TraceObjectThread;
@@ -130,9 +131,9 @@ public void setUpDisassemblyTest() throws Exception {
  listingProvider.setAutoDisassemble(false);
  }
 
- protected void assertX86Nop(Instruction instruction) {
+ protected void assertMnemonic(String expected, Instruction instruction) {
  assertNotNull(instruction);
- assertEquals("NOP", instruction.getMnemonicString());
+ assertEquals(expected, instruction.getMnemonicString());
  }
 
  protected void enableAutoDisassembly() throws Throwable {
@@ -187,14 +188,23 @@ protected TraceObjectThread createPolyglotTrace(String arch, long offset,
  return thread;
  }
 
+ protected void setLegacyProgramCounter(long offset, TraceThread thread, long snap) {
+ try (Transaction tx = tb.startTransaction()) {
+ DBTraceStackManager manager = tb.trace.getStackManager();
+ TraceStack stack = manager.getStack(thread, snap, true);
+ TraceStackFrame frame = stack.getFrame(0, true);
+ frame.setProgramCounter(Lifespan.nowOn(snap), tb.addr(offset));
+ }
+ }
+
  protected void createLegacyTrace(String langID, long offset,
  Supplier<ByteBuffer> byteSupplier) throws Throwable {
  createAndOpenTrace(langID);
 
  try (Transaction tx = tb.startTransaction()) {
  DBTraceMemoryManager memory = tb.trace.getMemoryManager();
  memory.createRegion("Memory[bin:.text]", 0, tb.range(offset, offset + 0xffff),
- Set.of(TraceMemoryFlag.EXECUTE));
+ Set.of(TraceMemoryFlag.EXECUTE, TraceMemoryFlag.READ));
  ByteBuffer bytes = byteSupplier.get();
  assertEquals(bytes.remaining(), memory.putBytes(0, tb.addr(offset), bytes));
  }
@@ -209,11 +219,44 @@ public void testAutoDisassembleX8664() throws Throwable {
  getSLEIGH_X86_64_LANGUAGE(); // So that the load isn't charged against the time-out
  waitForPass(() -> {
  DBTraceInstructionsMemoryView instructions = tb.trace.getCodeManager().instructions();
- assertX86Nop(instructions.getAt(0, tb.addr(0x00400000)));
- assertX86Nop(instructions.getAt(0, tb.addr(0x00400001)));
- assertX86Nop(instructions.getAt(0, tb.addr(0x00400002)));
- // NB. The auto disassembler will now proceed into "never known" memory.
- // It's too much trouble to prevent it, and it's different behavior than the D key.
+ assertMnemonic("NOP", instructions.getAt(0, tb.addr(0x00400000)));
+ assertMnemonic("NOP", instructions.getAt(0, tb.addr(0x00400001)));
+ assertMnemonic("NOP", instructions.getAt(0, tb.addr(0x00400002)));
+ assertNull(instructions.getAt(0, tb.addr(0x00400003)));
+ });
+ }
+
+ @Test
+ public void testAutoDisasembleReDisasembleOffcut() throws Throwable {
+ enableAutoDisassembly();
+ createLegacyTrace("x86:LE:64:default", 0x00400000, () -> tb.buf(0xeb, 0xff, 0xc0));
+
+ TraceThread thread;
+ try (Transaction tx = tb.startTransaction()) {
+ thread = tb.getOrAddThread("Thread 1", 0);
+ }
+
+ setLegacyProgramCounter(0x00400000, thread, 0);
+
+ waitForPass(() -> {
+ DBTraceInstructionsMemoryView instructions = tb.trace.getCodeManager().instructions();
+ assertMnemonic("JMP", instructions.getAt(0, tb.addr(0x00400000)));
+ /**
+ * Depending on preference for branch or fall-through, the disassembler may or may not
+ * proceed to the following instructions. I don't really care, since the test is the the
+ * JMP gets deleted after the update to PC.
+ */
+ });
+
+ // The jump will advance one byte. Just simulate that by updating the stack and/or regs
+ setLegacyProgramCounter(0x00400001, thread, 1);
+ traceManager.activateSnap(1);
+
+ waitForPass(() -> {
+ DBTraceInstructionsMemoryView instructions = tb.trace.getCodeManager().instructions();
+ assertNull(instructions.getAt(1, tb.addr(0x00400000)));
+ assertMnemonic("INC", instructions.getAt(1, tb.addr(0x00400001)));
+ assertNull(instructions.getAt(1, tb.addr(0x00400003)));
  });
  }