GP-0: Fix javodocs. Fix tests. Fix streamSub.

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/breakpoint/PlaceEmuBreakpointActionItem.java

@@ -22,7 +22,6 @@
 import ghidra.async.AsyncUtils;
 import ghidra.dbg.target.*;
 import ghidra.dbg.util.PathMatcher;
-import ghidra.dbg.util.PathPattern;
 import ghidra.program.model.address.*;
 import ghidra.trace.model.Lifespan;
 import ghidra.trace.model.Trace;

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/breakpoint/TraceBreakpointSet.java

@@ -98,7 +98,7 @@ private long getSnap() {
  /**
  * Get the trace
  * 
- * @return
+ * @return the trace
  */
  public Trace getTrace() {
  return trace;
@@ -237,7 +237,7 @@ public Set<TraceBreakpoint> getBreakpoints() {
  * The caller should first call {@link #canMerge(TraceBreakpoint)} to check if the breakpoint
  * "fits."
  * 
- * @param bpt
+ * @param bpt the breakpoint
  * @return true if the set actually changed as a result
  */
  public boolean add(TraceBreakpoint bpt) {

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/breakpoint/DBTraceBreakpoint.java

@@ -408,7 +408,7 @@ public void setEnabled(boolean enabled) {
 
  @Override
  public boolean isEnabled(long snap) {
- // NB. Only object mode support per-snap enablement
+ // NB. Only object mode supports per-snap enablement
  try (LockHold hold = LockHold.lock(space.lock.readLock())) {
  return enabled;
  }
@@ -491,4 +491,11 @@ public String getEmuSleigh() {
  public void delete() {
  space.deleteBreakpoint(this);
  }
+
+ @Override
+ public boolean isValid(long snap) {
+ try (LockHold hold = LockHold.lock(space.lock.readLock())) {
+ return lifespan.contains(snap);
+ }
+ }
 }

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/breakpoint/DBTraceObjectBreakpointLocation.java

@@ -369,6 +369,11 @@ public void delete() {
  }
  }
 
+ @Override
+ public boolean isValid(long snap) {
+ return object.getCanonicalParent(snap) != null;
+ }
+
  @Override
  public TraceObject getObject() {
  return object;

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/breakpoint/DBTraceObjectBreakpointSpec.java

@@ -223,6 +223,11 @@ public void delete() {
  }
  }
 
+ @Override
+ public boolean isValid(long snap) {
+ return object.getCanonicalParent(snap) != null;
+ }
+
  @Override
  public TraceObject getObject() {
  return object;

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/memory/DBTraceMemoryRegion.java

@@ -307,4 +307,11 @@ public Set<TraceMemoryFlag> getFlags() {
  public void delete() {
  space.deleteRegion(this);
  }
+
+ @Override
+ public boolean isValid(long snap) {
+ try (LockHold hold = LockHold.lock(space.lock.readLock())) {
+ return lifespan.contains(snap);
+ }
+ }
 }

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/memory/DBTraceObjectMemoryRegion.java

@@ -386,6 +386,11 @@ public void delete() {
  }
  }
 
+ @Override
+ public boolean isValid(long snap) {
+ return object.getCanonicalParent(snap) != null;
+ }
+
  @Override
  public TraceObject getObject() {
  return object;

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/target/DBTraceObjectValueWriteBehindCache.java

@@ -16,6 +16,7 @@
 package ghidra.trace.database.target;
 
 import java.util.*;
+import java.util.Map.Entry;
 import java.util.concurrent.ExecutionException;
 import java.util.function.Predicate;
 import java.util.stream.Stream;
@@ -210,11 +211,15 @@ public Stream<DBTraceObjectValueBehind> streamParents(DBTraceObject child, Lifes
 
  private Stream<DBTraceObjectValueBehind> streamSub(
  NavigableMap<Long, DBTraceObjectValueBehind> map, Lifespan span, boolean forward) {
- Long floor = map.floorKey(span.min());
- if (floor == null) {
- floor = span.min();
+ long min;
+ Entry<Long, DBTraceObjectValueBehind> floor = map.floorEntry(span.min());
+ if (floor != null && floor.getValue().getLifespan().contains(span.min())) {
+ min = floor.getKey();
  }
- var sub = map.subMap(floor, true, span.max(), true);
+ else {
+ min = span.min();
+ }
+ var sub = map.subMap(min, true, span.max(), true);
  if (!forward) {
  sub = sub.descendingMap();
  }

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/thread/DBTraceObjectThread.java

@@ -175,6 +175,11 @@ public void delete() {
  }
  }
 
+ @Override
+ public boolean isValid(long snap) {
+ return object.getCanonicalParent(snap) != null;
+ }
+
  @Override
  public TraceChangeRecord<?, ?> translateEvent(TraceChangeRecord<?, ?> rec) {
  return translator.translate(rec);

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/thread/DBTraceThread.java

@@ -184,4 +184,11 @@ public String getComment() {
  public void delete() {
  manager.deleteThread(this);
  }
+
+ @Override
+ public boolean isValid(long snap) {
+ try (LockHold hold = LockHold.lock(manager.lock.readLock())) {
+ return lifespan.contains(snap);
+ }
+ }
 }

Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/model/breakpoint/TraceBreakpoint.java

@@ -18,6 +18,8 @@
 import java.util.Collection;
 import java.util.Set;
 
+import ghidra.pcode.emu.DefaultPcodeThread.PcodeEmulationLibrary;
+import ghidra.pcode.exec.SleighUtils;
 import ghidra.program.model.address.Address;
 import ghidra.program.model.address.AddressRange;
 import ghidra.trace.model.*;
@@ -51,6 +53,8 @@ public interface TraceBreakpoint extends TraceUniqueObject {
  * 
  * <p>
  * This should be a name suitable for display on the screen
+ * 
+ * @param name the new name
  */
  void setName(String name);
 
@@ -75,12 +79,18 @@ public interface TraceBreakpoint extends TraceUniqueObject {
  AddressRange getRange();
 
  /**
+ * Get the minimum address in this breakpoint's range
+ * 
  * @see #getRange()
+ * @return the minimum address
  */
  Address getMinAddress();
 
  /**
+ * Get the maximum address in this breakpoint's range
+ * 
  * @see #getRange()
+ * @return the maximum address
  */
  Address getMaxAddress();
 
@@ -109,6 +119,7 @@ public interface TraceBreakpoint extends TraceUniqueObject {
  * Set the cleared snap of this breakpoint
  * 
  * @param clearedSnap the cleared snap, or {@link Long#MAX_VALUE} for "to the end of time"
+ * @throws DuplicateNameException if extending the lifespan would cause a naming collision
  */
  void setClearedSnap(long clearedSnap) throws DuplicateNameException;
 
@@ -236,21 +247,27 @@ public interface TraceBreakpoint extends TraceUniqueObject {
  * Set Sleigh source to replace the breakpointed instruction in emulation
  * 
  * <p>
- * The default is simply "<code>{@link PcodeEmulationLibrary#emu_swi() emu_swi()};
- * {@link PcodeEmulationLibrary#emu_exec_decoded() emu_exec_decoded()};</code>", effectively a
- * non-conditional breakpoint followed by execution of the actual instruction. Modifying this
- * allows clients to create conditional breakpoints or simply override or inject additional
- * logic into an emulated target.
+ * The default is simply:
+ * </p>
  * 
+ * <pre>
+ * {@link PcodeEmulationLibrary#emu_swi() emu_swi()};
+ * {@link PcodeEmulationLibrary#emu_exec_decoded() emu_exec_decoded()};
+ * </pre>
  * <p>
- * <b>NOTE:</b> This current has no effect on access breakpoints, but only execution
+ * That is effectively a non-conditional breakpoint followed by execution of the actual
+ * instruction. Modifying this allows clients to create conditional breakpoints or simply
+ * override or inject additional logic into an emulated target.
+ * 
+ * <p>
+ * <b>NOTE:</b> This currently has no effect on access breakpoints, but only execution
  * breakpoints.
  * 
  * <p>
  * If the specified source fails to compile during emulator set-up, this will fall back to
- * {@link PcodeEmulationLibrary#emu_err
+ * {@link PcodeEmulationLibrary#emu_swi()}
  * 
- * @see #DEFAULT_SLEIGH
+ * @see SleighUtils#UNCONDITIONAL_BREAK
  * @param sleigh the Sleigh source
  */
  void setEmuSleigh(String sleigh);
@@ -266,4 +283,18 @@ public interface TraceBreakpoint extends TraceUniqueObject {
  * Delete this breakpoint from the trace
  */
  void delete();
+
+ /**
+ * Check if the breakpoint is valid at the given snapshot
+ * 
+ * <p>
+ * In object mode, a breakpoint's life may be disjoint, so checking if the snap occurs between
+ * creation and destruction is not quite sufficient. This method encapsulates validity. In
+ * object mode, it checks that the breakpoint object has a canonical parent at the given
+ * snapshot. In table mode, it checks that the lifespan contains the snap.
+ * 
+ * @param snap the snapshot key
+ * @return true if valid, false if not
+ */
+ boolean isValid(long snap);
 }