diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/listing/AbstractBaseDBTraceDefinedUnitsView.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/listing/AbstractBaseDBTraceDefinedUnitsView.java index 5866c6815b..a54ddf2b05 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/listing/AbstractBaseDBTraceDefinedUnitsView.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/listing/AbstractBaseDBTraceDefinedUnitsView.java @@ -20,12 +20,14 @@ import java.util.Map.Entry; import ghidra.program.model.address.*; import ghidra.program.model.util.CodeUnitInsertionException; -import ghidra.trace.database.*; +import ghidra.trace.database.DBTraceCacheForContainingQueries; import ghidra.trace.database.DBTraceCacheForContainingQueries.GetKey; +import ghidra.trace.database.DBTraceCacheForSequenceQueries; import ghidra.trace.database.context.DBTraceRegisterContextSpace; import ghidra.trace.database.map.DBTraceAddressSnapRangePropertyMapAddressSetView; import ghidra.trace.database.map.DBTraceAddressSnapRangePropertyMapSpace; import ghidra.trace.database.map.DBTraceAddressSnapRangePropertyMapTree.TraceAddressSnapRangeQuery; +import ghidra.trace.database.memory.DBTraceMemorySpace; import ghidra.trace.model.*; import ghidra.trace.model.Trace.TraceCodeChangeType; import ghidra.trace.model.listing.TraceBaseDefinedUnitsView; @@ -359,20 +361,28 @@ public abstract class AbstractBaseDBTraceDefinedUnitsView - * The selected lifespan will have the same start snap at that given. The box is the bounding + * The selected lifespan will have the same start snap as that given. The box is the bounding * box of a unit the client is trying to create. * * @param span the lifespan of the box + * @param extending if applicable, the unit whose lifespan is being extended * @param range the address range of the box * @return the selected sub-lifespan * @throws CodeUnitInsertionException if the start snap is contained in an existing unit */ - protected Lifespan truncateSoonestDefined(Lifespan span, AddressRange range) - throws CodeUnitInsertionException { + protected Lifespan truncateSoonestDefined(Lifespan span, AbstractDBTraceCodeUnit extending, + AddressRange range) throws CodeUnitInsertionException { + final Lifespan toScan; + if (extending == null) { + toScan = span; + } + else { + assert span.lmax() > extending.getEndSnap(); + toScan = span.withMin(extending.getEndSnap() + 1); + } T truncateBy = - mapSpace.reduce(TraceAddressSnapRangeQuery.intersecting(range, span) - .starting( - Rectangle2DDirection.BOTTOMMOST)) + mapSpace.reduce(TraceAddressSnapRangeQuery.intersecting(range, toScan) + .starting(Rectangle2DDirection.BOTTOMMOST)) .firstValue(); if (truncateBy == null) { return span; @@ -380,7 +390,25 @@ public abstract class AbstractBaseDBTraceDefinedUnitsView 0) { + buffer.put(byteCache.array(), addressOffset, toCopyFromCache); + } + else { + toCopyFromCache = 0; + } if (byteCache.position() >= end) { return toCopyFromCache; } diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/listing/DBTraceData.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/listing/DBTraceData.java index 5f3d7183ad..75a7b5cd67 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/listing/DBTraceData.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/listing/DBTraceData.java @@ -20,7 +20,8 @@ import java.io.IOException; import db.DBRecord; import ghidra.docking.settings.Settings; import ghidra.program.model.address.AddressSpace; -import ghidra.program.model.data.*; +import ghidra.program.model.data.DataType; +import ghidra.program.model.data.TypeDef; import ghidra.program.model.lang.Language; import ghidra.trace.database.DBTrace; import ghidra.trace.database.DBTraceUtils; diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/listing/DBTraceDefinedDataView.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/listing/DBTraceDefinedDataView.java index 2c690a30b8..94dd85dae3 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/listing/DBTraceDefinedDataView.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/listing/DBTraceDefinedDataView.java @@ -131,21 +131,8 @@ public class DBTraceDefinedDataView extends AbstractBaseDBTraceDefinedUnitsView< Address endAddress = address.addNoWrap(length - 1); AddressRangeImpl createdRange = new AddressRangeImpl(address, endAddress); - // First, truncate lifespan to the next code unit when upper bound is max - if (!lifespan.maxIsFinite()) { - lifespan = space.instructions.truncateSoonestDefined(lifespan, createdRange); - lifespan = space.definedData.truncateSoonestDefined(lifespan, createdRange); - } - - // Second, extend to the next change of bytes in the range within lifespan - // Then, check that against existing code units. - long endSnap = memSpace.getFirstChange(lifespan, createdRange); - if (endSnap == Long.MIN_VALUE) { - endSnap = lifespan.lmax(); - } - else { - endSnap--; - } + // Truncate, then check that against existing code units. + long endSnap = computeTruncatedMax(lifespan, null, createdRange); TraceAddressSnapRange tasr = new ImmutableTraceAddressSnapRange(createdRange, Lifespan.span(startSnap, endSnap)); if (!space.undefinedData.coversRange(tasr)) { @@ -158,13 +145,13 @@ public class DBTraceDefinedDataView extends AbstractBaseDBTraceDefinedUnitsView< } long dataTypeID = space.dataTypeManager.getResolvedID(dataType); - DBTraceData created = space.dataMapSpace.put(tasr, null); + DBTraceData created = mapSpace.put(tasr, null); // TODO: data units with a guest platform created.set(space.trace.getPlatformManager().getHostPlatform(), dataTypeID); // TODO: Explicitly remove undefined from cache, or let weak refs take care of it? - cacheForContaining.notifyNewEntry(lifespan, createdRange, created); - cacheForSequence.notifyNewEntry(lifespan, createdRange, created); + cacheForContaining.notifyNewEntry(tasr.getLifespan(), createdRange, created); + cacheForSequence.notifyNewEntry(tasr.getLifespan(), createdRange, created); space.undefinedData.invalidateCache(); if (dataType instanceof Composite || dataType instanceof Array || diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/listing/DBTraceInstructionsView.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/listing/DBTraceInstructionsView.java index 89a6065a8a..676dccd17b 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/listing/DBTraceInstructionsView.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/listing/DBTraceInstructionsView.java @@ -23,11 +23,11 @@ import ghidra.program.model.address.*; import ghidra.program.model.lang.*; import ghidra.program.model.lang.InstructionError.InstructionErrorType; import ghidra.program.model.listing.*; +import ghidra.program.model.mem.MemoryAccessException; import ghidra.program.model.util.CodeUnitInsertionException; import ghidra.trace.database.context.DBTraceRegisterContextManager; import ghidra.trace.database.context.DBTraceRegisterContextSpace; import ghidra.trace.database.guest.InternalTracePlatform; -import ghidra.trace.database.memory.DBTraceMemorySpace; import ghidra.trace.model.*; import ghidra.trace.model.Trace.TraceCodeChangeType; import ghidra.trace.model.guest.TracePlatform; @@ -59,6 +59,7 @@ public class DBTraceInstructionsView extends AbstractBaseDBTraceDefinedUnitsView private final Address errorAddress; private final InstructionError conflict; private final CodeUnit conflictCodeUnit; + private final boolean overwrite; protected int count = 0; @@ -75,10 +76,11 @@ public class DBTraceInstructionsView extends AbstractBaseDBTraceDefinedUnitsView * @param errorAddress the address of the first error in the block, if any * @param conflict a description of the error, if any * @param conflictCodeUnit if a conflict, the code unit that already exists + * @param overwrite true to overwrite existing defined units */ private InstructionBlockAdder(Set
skipDelaySlots, Lifespan lifespan, InternalTracePlatform platform, InstructionBlock block, Address errorAddress, - InstructionError conflict, CodeUnit conflictCodeUnit) { + InstructionError conflict, CodeUnit conflictCodeUnit, boolean overwrite) { this.skipDelaySlots = skipDelaySlots; this.lifespan = lifespan; this.platform = platform; @@ -86,20 +88,113 @@ public class DBTraceInstructionsView extends AbstractBaseDBTraceDefinedUnitsView this.errorAddress = errorAddress; this.conflict = conflict; this.conflictCodeUnit = conflictCodeUnit; + this.overwrite = overwrite; + } + + protected void truncateOrDelete(TraceInstruction exists) { + if (exists.getStartSnap() < lifespan.lmin()) { + exists.setEndSnap(lifespan.lmin()); + } + else { + exists.delete(); + } + } + + protected boolean isSuitable(Instruction candidate, Instruction protoInstr) { + try { + return candidate.getPrototype().equals(protoInstr.getPrototype()) && + Arrays.equals(candidate.getBytes(), protoInstr.getBytes()) && + candidate.isFallThroughOverridden() == protoInstr.isFallThroughOverridden() && + Objects.equals(candidate.getFallThrough(), protoInstr.getFallThrough()) && + candidate.getFlowOverride() == protoInstr.getFlowOverride(); + } + catch (MemoryAccessException e) { + throw new AssertionError(e); + } + } + + protected Instruction doAdjustExisting(Address address, Instruction protoInstr) + throws AddressOverflowException, CancelledException, CodeUnitInsertionException { + DBTraceInstruction exists = getAt(lifespan.lmin(), address); + if (exists == null || exists.getLength() != protoInstr.getLength()) { + AddressRange range = + new AddressRangeImpl(protoInstr.getAddress(), protoInstr.getLength()); + space.definedUnits.clear(lifespan, range, false, TaskMonitor.DUMMY); + return null; + } + if (!isSuitable(exists, protoInstr)) { + truncateOrDelete(exists); + return null; + } + long curEnd = exists.getEndSnap(); + if (curEnd < lifespan.lmax()) { + AddressRange range = + new AddressRangeImpl(protoInstr.getAddress(), protoInstr.getLength()); + long endSnap = computeTruncatedMax(lifespan, exists, range); + exists.setEndSnap(endSnap); + } + return exists; + } + + /** + * Check the preceding unit and see if it can be extended to "create" the desired one + * + *

+ * For overwrite, the caller should first use + * {@link #doAdjustExisting(Address, InstructionPrototype, Instruction)}. + * + * @param address the starting address of the instruction + * @param protoInstr the prototype instruction + * @return the extended instruction, if it was a match + * @throws AddressOverflowException should never + * @throws CodeUnitInsertionException if there's no room to extend the preceding unit + */ + protected Instruction doExtendPreceding(Address address, Instruction protoInstr) + throws AddressOverflowException, CodeUnitInsertionException { + if (!lifespan.minIsFinite()) { + return null; + } + DBTraceInstruction prec = getAt(lifespan.lmin() - 1, address); + if (prec == null || prec.getLength() != protoInstr.getLength()) { + return null; + } + if (!isSuitable(prec, protoInstr)) { + return null; + } + AddressRange range = + new AddressRangeImpl(protoInstr.getAddress(), protoInstr.getLength()); + long endSnap = computeTruncatedMax(lifespan, prec, range); + if (!lifespan.contains(endSnap)) { + // We can't extend it far enough + return null; + } + prec.setEndSnap(endSnap); + return prec; } /** * Store the given instruction in the database * * @param address the address of the instruction - * @param prototype the instruction prototype * @param protoInstr the parsed (usually pseudo) instruction * @return the created instruction */ - protected Instruction doCreateInstruction(Address address, - InstructionPrototype prototype, Instruction protoInstr) { + protected Instruction doCreateInstruction(Address address, Instruction protoInstr) { try { - Instruction created = doCreate(lifespan, address, platform, prototype, protoInstr); + if (overwrite) { + Instruction exists = doAdjustExisting(address, protoInstr); + if (exists != null) { + return exists; + } + } + + Instruction prec = doExtendPreceding(address, protoInstr); + if (prec != null) { + return prec; + } + + Instruction created = + doCreate(lifespan, address, platform, protoInstr.getPrototype(), protoInstr); // copy override settings to replacement instruction if (protoInstr.isFallThroughOverridden()) { created.setFallThrough(protoInstr.getFallThrough()); @@ -110,9 +205,9 @@ public class DBTraceInstructionsView extends AbstractBaseDBTraceDefinedUnitsView } return created; } - catch (CodeUnitInsertionException | AddressOverflowException e) { - // End address already computed when protoInstr created. - // We've also already checked for conflicts + catch (AddressOverflowException | // End address already checked via protoInstr + CodeUnitInsertionException | // We've already checked for conflicts + CancelledException e) { // There's no monitor to cancel throw new AssertionError(e); } } @@ -160,8 +255,7 @@ public class DBTraceInstructionsView extends AbstractBaseDBTraceDefinedUnitsView } if (!skipDelaySlots.contains(startAddress)) { - InstructionPrototype prototype = protoInstr.getPrototype(); - if (!areDelaySlots && prototype.hasDelaySlots()) { + if (!areDelaySlots && protoInstr.getPrototype().hasDelaySlots()) { // Reverse their order then add them. This ensures pcode can be generated // for the delay-slotted instruction upon its creation. Deque delayed = @@ -172,8 +266,7 @@ public class DBTraceInstructionsView extends AbstractBaseDBTraceDefinedUnitsView lastInstruction = replaceIfNotNull(lastInstruction, doAddInstructions(delayed.iterator(), true)); } - lastInstruction = - doCreateInstruction(startAddress, prototype, protoInstr); + lastInstruction = doCreateInstruction(startAddress, protoInstr); } if (errorAddress != null && conflictCodeUnit == null && errorAddress.compareTo(startAddress) <= 0) { @@ -259,23 +352,8 @@ public class DBTraceInstructionsView extends AbstractBaseDBTraceDefinedUnitsView Address endAddress = address.addNoWrap(prototype.getLength() - 1); AddressRangeImpl createdRange = new AddressRangeImpl(address, endAddress); - // First, truncate lifespan to the next code unit when upper bound is max - if (!lifespan.maxIsFinite()) { - lifespan = space.instructions.truncateSoonestDefined(lifespan, createdRange); - lifespan = space.definedData.truncateSoonestDefined(lifespan, createdRange); - } - - // Second, truncate lifespan to the next change of bytes in the range - // Then, check that against existing code units. - DBTraceMemorySpace memSpace = - space.trace.getMemoryManager().getMemorySpace(space.space, true); - long endSnap = memSpace.getFirstChange(lifespan, createdRange); - if (endSnap == Long.MIN_VALUE) { - endSnap = lifespan.lmax(); - } - else { - endSnap--; - } + // Truncate, then check that against existing code units. + long endSnap = computeTruncatedMax(lifespan, null, createdRange); TraceAddressSnapRange tasr = new ImmutableTraceAddressSnapRange(createdRange, lifespan.withMax(endSnap)); @@ -289,8 +367,8 @@ public class DBTraceInstructionsView extends AbstractBaseDBTraceDefinedUnitsView DBTraceInstruction created = space.instructionMapSpace.put(tasr, null); created.set(platform, prototype, context); - cacheForContaining.notifyNewEntry(lifespan, createdRange, created); - cacheForSequence.notifyNewEntry(lifespan, createdRange, created); + cacheForContaining.notifyNewEntry(tasr.getLifespan(), createdRange, created); + cacheForSequence.notifyNewEntry(tasr.getLifespan(), createdRange, created); space.undefinedData.invalidateCache(); // TODO: Save the context register into the context manager? Flow it? @@ -348,14 +426,19 @@ public class DBTraceInstructionsView extends AbstractBaseDBTraceDefinedUnitsView * @param skipDelaySlots the addresses of delay-slotted instructions to skip * @param platform the instructions' platform (language, compiler) * @param block the block of instructions to add + * @param overwrite true to overwrite existing defined units * @return the adder, or null */ - protected InstructionBlockAdder startAddingBlock(Lifespan lifespan, - Set

skipDelaySlots, InternalTracePlatform platform, InstructionBlock block) { + protected InstructionBlockAdder startAddingBlock(Lifespan lifespan, Set
skipDelaySlots, + InternalTracePlatform platform, InstructionBlock block, boolean overwrite) { + if (overwrite) { + return new InstructionBlockAdder(skipDelaySlots, lifespan, platform, block, null, null, + null, overwrite); + } InstructionError conflict = block.getInstructionConflict(); if (conflict == null) { return new InstructionBlockAdder(skipDelaySlots, lifespan, platform, block, null, null, - null); + null, overwrite); } Address errorAddress = conflict.getInstructionAddress(); if (errorAddress == null) { @@ -363,13 +446,13 @@ public class DBTraceInstructionsView extends AbstractBaseDBTraceDefinedUnitsView } if (!conflict.getInstructionErrorType().isConflict) { return new InstructionBlockAdder(skipDelaySlots, lifespan, platform, block, - errorAddress, conflict, null); + errorAddress, conflict, null, overwrite); } long startSnap = lifespan.lmin(); CodeUnit conflictCodeUnit = space.definedUnits.getAt(startSnap, conflict.getConflictAddress()); return new InstructionBlockAdder(skipDelaySlots, lifespan, platform, block, errorAddress, - conflict, conflictCodeUnit); + conflict, conflictCodeUnit, overwrite); } /** @@ -468,19 +551,14 @@ public class DBTraceInstructionsView extends AbstractBaseDBTraceDefinedUnitsView try (LockHold hold = LockHold.lock(space.lock.writeLock())) { long startSnap = lifespan.lmin(); Set
skipDelaySlots = new HashSet<>(); - if (overwrite) { - for (AddressRange range : instructionSet.getAddressSet()) { - space.definedUnits.clear(lifespan, range, false, TaskMonitor.DUMMY); - } - } - else { + if (!overwrite) { checkInstructionSet(startSnap, instructionSet, skipDelaySlots); } // Add blocks for (InstructionBlock block : instructionSet) { InstructionBlockAdder adder = - startAddingBlock(lifespan, skipDelaySlots, dbPlatform, block); + startAddingBlock(lifespan, skipDelaySlots, dbPlatform, block, overwrite); if (adder == null) { continue; } @@ -496,9 +574,6 @@ public class DBTraceInstructionsView extends AbstractBaseDBTraceDefinedUnitsView } return result; } - catch (CancelledException e) { - throw new AssertionError(e); // No actual monitor - } catch (AddressOverflowException e) { // Better have skipped any delay-slotted instructions whose delays overflowed throw new AssertionError(e); diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/map/DBTraceAddressSnapRangePropertyMapTree.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/map/DBTraceAddressSnapRangePropertyMapTree.java index 14083ae98b..6cef794dda 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/map/DBTraceAddressSnapRangePropertyMapTree.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/map/DBTraceAddressSnapRangePropertyMapTree.java @@ -299,21 +299,28 @@ public class DBTraceAddressSnapRangePropertyMapTree memSpaces = new TreeMap<>(); // Note: can use hash map here. I see no need to order these spaces - protected final Map, M> regSpaces = new HashMap<>(); + protected final Map regSpaces = new HashMap<>(); protected final Map regSpacesByObject = new HashMap<>(); protected final Collection memSpacesView = @@ -130,47 +141,61 @@ public abstract class AbstractDBTraceSpaceBasedManager newRegSpaces = new HashMap<>(); + Map newMemSpaces = new HashMap<>(); + for (TabledSpace ts : getTabledSpaces()) { + if (ts.isRegisterSpace()) { + newRegSpaces.put(ts.frame(), ts); } else { - space = addressFactory.getAddressSpace(ent.spaceName); + newMemSpaces.put(ts.space(), ts); } + } + regSpaces.keySet().retainAll(newRegSpaces.keySet()); + memSpaces.keySet().retainAll(newMemSpaces.keySet()); + for (Entry ent : newRegSpaces.entrySet()) { + if (!regSpaces.containsKey(ent.getKey())) { + regSpaces.put(ent.getKey(), createRegisterSpace(ent.getValue())); + } + } + for (Entry ent : newMemSpaces.entrySet()) { + if (!memSpaces.containsKey(ent.getKey())) { + memSpaces.put(ent.getKey(), createSpace(ent.getValue())); + } + } + } + + protected AddressSpace getSpaceByName(AddressFactory factory, String name) { + if (NO_ADDRESS_SPACE.getName().equals(name)) { + return NO_ADDRESS_SPACE; + } + return factory.getAddressSpace(name); + } + + protected List getTabledSpaces() { + AddressFactory factory = trace.getBaseAddressFactory(); + List result = new ArrayList<>(); + for (DBTraceSpaceEntry ent : spaceStore.asMap().values()) { + AddressSpace space = getSpaceByName(factory, ent.spaceName); if (space == null) { Msg.error(this, "Space " + ent.spaceName + " does not exist in trace (language=" + baseLanguage + ")."); + continue; } - else if (space.isRegisterSpace()) { + if (space.isRegisterSpace()) { if (threadManager == null) { Msg.error(this, "Register spaces are not allowed without a thread manager."); continue; } TraceThread thread = threadManager.getThread(ent.threadKey); - M regSpace; - if (ent.space == null) { - regSpace = createRegisterSpace(space, thread, ent); - } - else { - regSpace = (M) ent.space; - } - regSpaces.put(ImmutablePair.of(thread, ent.getFrameLevel()), regSpace); + result.add(new TabledSpace(ent, space, thread)); } else { - M memSpace; - if (ent.space == null) { - memSpace = createSpace(space, ent); - } - else { - memSpace = (M) ent.space; - } - memSpaces.put(space, memSpace); + result.add(new TabledSpace(ent, space, null)); } } + return result; } protected M getForSpace(AddressSpace space, boolean createIfAbsent) { @@ -210,7 +235,7 @@ public abstract class AbstractDBTraceSpaceBasedManager frame = ImmutablePair.of(thread, frameLevel); + Frame frame = new Frame(thread, frameLevel); if (!createIfAbsent) { try (LockHold hold = LockHold.lock(lock.readLock())) { return regSpaces.get(frame); @@ -340,6 +365,26 @@ public abstract class AbstractDBTraceSpaceBasedManager> creators = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + creators.add(CompletableFuture.supplyAsync(() -> { + try (UndoableTransaction tid = b.startTransaction()) { + b.trace.getCodeManager() + .definedData() + .create(Lifespan.ALL, b.addr(0x4000), IntegerDataType.dataType); + return 0; + } + catch (CodeUnitInsertionException e) { + return 1; + } + })); + } + CompletableFuture.allOf(creators.toArray(CompletableFuture[]::new)).get(); + assertEquals(9, creators.stream() + .mapToInt(c -> c.getNow(null)) + .reduce(Integer::sum) + .orElse(-1)); + } + + @Test + public void testOverlapAllowedAfterAbort() throws Throwable { + try (UndoableTransaction tid = b.startTransaction()) { + b.trace.getCodeManager() + .definedData() + .create(Lifespan.ALL, b.addr(0x4000), IntegerDataType.dataType); + tid.abort(); + } + try (UndoableTransaction tid = b.startTransaction()) { + b.trace.getCodeManager() + .definedData() + .create(Lifespan.ALL, b.addr(0x4000), IntegerDataType.dataType); + } + } + + public void testOverlapErrAfterInvalidate() throws Throwable { + try (UndoableTransaction tid = b.startTransaction()) { + b.trace.getCodeManager() + .definedData() + .create(Lifespan.ALL, b.addr(0x4000), IntegerDataType.dataType); + } + b.trace.undo(); + b.trace.redo(); + try (UndoableTransaction tid = b.startTransaction()) { + b.trace.getCodeManager() + .definedData() + .create(Lifespan.ALL, b.addr(0x4000), IntegerDataType.dataType); + fail(); + } + catch (CodeUnitInsertionException e) { + // pass + } + } + + /** + * This test is interesting because the pointer type def causes an update to the data type + * settings while the unit is still being created. This will invalidate the trace's + * caches. All of them, including the defined data units, which can become the cause of many + * timing issues. + */ + @Test + public void testOverlapErrWithDataTypeSettings() throws Throwable { + AddressSpace space = b.trace.getBaseAddressFactory().getDefaultAddressSpace(); + PointerTypedef type = new PointerTypedef(null, VoidDataType.dataType, 8, null, space); + try (UndoableTransaction tid = b.startTransaction()) { + b.trace.getCodeManager() + .definedData() + .create(Lifespan.ALL, b.addr(0x4000), type); + } + try (UndoableTransaction tid = b.startTransaction()) { + b.trace.getCodeManager() + .definedData() + .create(Lifespan.ALL, b.addr(0x4000), type); + fail(); + } + catch (CodeUnitInsertionException e) { + // pass + } + } + + @Test + public void testOverlapErrAfterSetEndSnap() throws Throwable { + try (UndoableTransaction tid = b.startTransaction()) { + DBTraceDataAdapter data = b.trace.getCodeManager() + .definedData() + .create(Lifespan.ALL, b.addr(0x4000), IntegerDataType.dataType); + assertEquals(Lifespan.before(0), data.getLifespan()); + data.setEndSnap(-10); + assertEquals(Lifespan.before(-9), data.getLifespan()); + } + try (UndoableTransaction tid = b.startTransaction()) { + b.trace.getCodeManager() + .definedData() + .create(Lifespan.ALL, b.addr(0x4000), IntegerDataType.dataType); + fail(); + } + catch (CodeUnitInsertionException e) { + // pass + } + } + protected void assertAllNullFunc(Function, TraceCodeUnit> func) { assertNull(func.apply(manager.codeUnits())); assertNull(func.apply(manager.data())); @@ -1538,7 +1660,8 @@ public class DBTraceCodeManagerTest extends AbstractGhidraHeadlessIntegrationTes coversTwoWays(manager.definedData(), Lifespan.span(0, 5), b.range(0x4000, 0x4007))); assertTrue( coversTwoWays(manager.definedData(), Lifespan.span(0, 9), b.range(0x4001, 0x4003))); - assertFalse(intersectsTwoWays(manager.definedData(), Lifespan.ALL, b.range(0x0000, 0x3fff))); + assertFalse( + intersectsTwoWays(manager.definedData(), Lifespan.ALL, b.range(0x0000, 0x3fff))); assertFalse( intersectsTwoWays(manager.definedData(), Lifespan.ALL, b.range(0x4008, -0x0001))); assertFalse(intersectsTwoWays(manager.definedData(), Lifespan.toNow(-1), all)); diff --git a/Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/database/listing/DBTraceCodeUnitTest.java b/Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/database/listing/DBTraceCodeUnitTest.java index 3f113897b7..b958aa19b5 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/database/listing/DBTraceCodeUnitTest.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/database/listing/DBTraceCodeUnitTest.java @@ -761,6 +761,18 @@ public class DBTraceCodeUnitTest extends AbstractGhidraHeadlessIntegrationTest assertEquals(4, data.getBytes(buf, 0)); assertArrayEquals(b.arr(1, 2, 3, 4), buf.array()); + buf = ByteBuffer.allocate(1); + assertEquals(1, data.getBytes(buf, 4)); + assertArrayEquals(b.arr(0xeb), buf.array()); + + buf = ByteBuffer.allocate(1); + assertEquals(1, data.getBytes(buf, 5)); + assertArrayEquals(b.arr(0xfe), buf.array()); + + buf = ByteBuffer.allocate(4); + assertEquals(4, data.getBytes(buf, 2)); + assertArrayEquals(b.arr(3, 4, 0xeb, 0xfe), buf.array()); + assertArrayEquals(b.arr(1, 2, 3, 4), data.getBytes()); byte[] arr = new byte[6]; diff --git a/Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/database/module/DBTraceModuleManagerTest.java b/Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/database/module/DBTraceModuleManagerTest.java index 9d9977b82b..717f6f1d90 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/database/module/DBTraceModuleManagerTest.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/database/module/DBTraceModuleManagerTest.java @@ -15,9 +15,7 @@ */ package ghidra.trace.database.module; -import static ghidra.lifecycle.Unfinished.TODO; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.*; import java.io.File; import java.util.*; @@ -334,7 +332,6 @@ public class DBTraceModuleManagerTest extends AbstractGhidraHeadlessIntegrationT } @Test - @Ignore("GP-479") public void testUndoIdentitiesPreserved() throws Exception { TraceModule mod1; try (UndoableTransaction tid = b.startTransaction()) { @@ -351,8 +348,7 @@ public class DBTraceModuleManagerTest extends AbstractGhidraHeadlessIntegrationT b.trace.undo(); - assertEquals(mod1, assertOne(moduleManager.getModulesByPath("Modules[first]"))); - TODO(); // TODO: mod1 should still be identical to that in database + assertSame(mod1, assertOne(moduleManager.getModulesByPath("Modules[first]"))); assertTrue(moduleManager.getModulesByPath("Modules[second]").isEmpty()); } diff --git a/Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/database/program/DBTraceDisassemblerIntegrationTest.java b/Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/database/program/DBTraceDisassemblerIntegrationTest.java index e4b786c399..44b3d9b9e2 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/database/program/DBTraceDisassemblerIntegrationTest.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/database/program/DBTraceDisassemblerIntegrationTest.java @@ -19,15 +19,19 @@ import static org.junit.Assert.assertEquals; import java.io.File; import java.io.IOException; +import java.nio.ByteBuffer; +import java.util.List; import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.StreamSupport; import org.junit.*; import ghidra.app.cmd.disassemble.*; +import ghidra.app.plugin.assembler.*; import ghidra.program.database.ProgramBuilder; import ghidra.program.disassemble.Disassembler; -import ghidra.program.model.address.AddressOverflowException; -import ghidra.program.model.address.AddressSet; +import ghidra.program.model.address.*; import ghidra.program.model.lang.*; import ghidra.program.model.listing.CodeUnit; import ghidra.program.model.mem.MemoryBlock; @@ -37,7 +41,8 @@ import ghidra.trace.database.guest.DBTraceGuestPlatform; import ghidra.trace.database.listing.*; import ghidra.trace.database.memory.DBTraceMemoryManager; import ghidra.trace.database.memory.DBTraceMemorySpace; -import ghidra.trace.model.Lifespan; +import ghidra.trace.model.*; +import ghidra.trace.model.listing.TraceCodeUnit; import ghidra.trace.model.memory.TraceMemoryFlag; import ghidra.trace.model.memory.TraceOverlappedRegionException; import ghidra.trace.util.LanguageTestWatcher; @@ -306,4 +311,136 @@ public class DBTraceDisassemblerIntegrationTest extends AbstractGhidraHeadlessIn assertEquals("MOV ECX,EAX", cu2.toString()); } } + + record Repetition(Lifespan lifespan, boolean overwrite) { + } + + protected List toList(Iterable it) { + return StreamSupport.stream(it.spliterator(), false).collect(Collectors.toList()); + } + + protected void runTestCoalesceInstructions(List repetitions) throws Exception { + try (UndoableTransaction tid = b.startTransaction()) { + DBTraceMemoryManager memory = b.trace.getMemoryManager(); + DBTraceCodeManager code = b.trace.getCodeManager(); + + memory.createRegion(".text", 0, b.range(0x00400000, 0x00400fff)); + Assembler asm = Assemblers.getAssembler(b.language); + Address entry = b.addr(0x00400000); + AssemblyBuffer buf = new AssemblyBuffer(asm, entry); + buf.assemble("imm r0, #123"); + buf.assemble("mov r1, r0"); + buf.assemble("ret"); + + long snap = Lifespan.isScratch(repetitions.get(0).lifespan.lmin()) ? Long.MIN_VALUE : 0; + memory.putBytes(snap, entry, ByteBuffer.wrap(buf.getBytes())); + + AddressFactory factory = b.trace.getBaseAddressFactory(); + Disassembler dis = + Disassembler.getDisassembler(b.language, factory, TaskMonitor.DUMMY, null); + InstructionSet set = new InstructionSet(factory); + set.addBlock(dis.pseudoDisassembleBlock(memory.getBufferAt(snap, entry), null, 10)); + + List units = null; + TraceAddressSnapRange all = + new ImmutableTraceAddressSnapRange(b.range(0, -1), Lifespan.ALL); + for (Repetition rep : repetitions) { + code.instructions().addInstructionSet(rep.lifespan, set, rep.overwrite); + if (units == null) { + units = toList(code.definedUnits().getIntersecting(all)); + } + else { + /** + * Technically, getIntersecting makes no guarantee regarding order. + * Nevertheless, the structure shouldn't be perturbed, so I think it's fair to + * expect the same order. + */ + assertEquals(units, toList(code.definedUnits().getIntersecting(all))); + } + } + } + } + + @Test + @TestLanguage(ProgramBuilder._TOY64_BE) + public void testCoalesceInstructionsMinTwiceNoOverwrite() throws Exception { + runTestCoalesceInstructions(List.of( + new Repetition(Lifespan.nowOn(Long.MIN_VALUE), false), + new Repetition(Lifespan.nowOn(Long.MIN_VALUE), false))); + } + + @Test + @TestLanguage(ProgramBuilder._TOY64_BE) + public void testCoalesceInstructionsMinTwiceYesOverwrite() throws Exception { + runTestCoalesceInstructions(List.of( + new Repetition(Lifespan.nowOn(Long.MIN_VALUE), true), + new Repetition(Lifespan.nowOn(Long.MIN_VALUE), true))); + } + + @Test + @TestLanguage(ProgramBuilder._TOY64_BE) + public void testCoalesceInstructionsZeroTwiceYesOverwrite() throws Exception { + runTestCoalesceInstructions(List.of( + new Repetition(Lifespan.nowOn(0), true), + new Repetition(Lifespan.nowOn(0), true))); + } + + @Test + @TestLanguage(ProgramBuilder._TOY64_BE) + public void testCoalesceInstructionsZeroThenOneYesOverwrite() throws Exception { + runTestCoalesceInstructions(List.of( + new Repetition(Lifespan.nowOn(0), true), + new Repetition(Lifespan.nowOn(1), true))); + } + + @Test + @TestLanguage(ProgramBuilder._TOY64_BE) + public void testCoalesceInstructionsZeroOnlyThenOneNoOverwrite() throws Exception { + runTestCoalesceInstructions(List.of( + new Repetition(Lifespan.at(0), false), + new Repetition(Lifespan.nowOn(1), false))); + } + + @Test + @TestLanguage(ProgramBuilder._TOY64_BE) + public void testCoalesceInstructionsZeroOnlyThenOneYesOverwrite() throws Exception { + runTestCoalesceInstructions(List.of( + new Repetition(Lifespan.at(0), true), + new Repetition(Lifespan.nowOn(1), true))); + } + + @Test + public void testNoCoalesceAcrossByteChanges() throws Exception { + try (UndoableTransaction tid = b.startTransaction()) { + DBTraceMemoryManager memory = b.trace.getMemoryManager(); + DBTraceCodeManager code = b.trace.getCodeManager(); + + memory.createRegion(".text", 0, b.range(0x00400000, 0x00400fff)); + Assembler asm = Assemblers.getAssembler(b.language); + Address entry = b.addr(0x00400000); + AssemblyBuffer buf = new AssemblyBuffer(asm, entry); + buf.assemble("imm r0, #123"); + buf.assemble("mov r1, r0"); + buf.assemble("ret"); + + memory.putBytes(-1, entry, ByteBuffer.wrap(buf.getBytes())); + + AddressFactory factory = b.trace.getBaseAddressFactory(); + Disassembler dis = + Disassembler.getDisassembler(b.language, factory, TaskMonitor.DUMMY, null); + InstructionSet set = new InstructionSet(factory); + set.addBlock(dis.pseudoDisassembleBlock(memory.getBufferAt(-1, entry), null, 10)); + + TraceAddressSnapRange all = + new ImmutableTraceAddressSnapRange(b.range(0, -1), Lifespan.ALL); + code.instructions().addInstructionSet(Lifespan.nowOn(-1), set, true); + /** + * This is already a bogus sort of operation: The prototypes may not match the bytes. In + * any case, we should not expect coalescing. + */ + code.instructions().addInstructionSet(Lifespan.nowOn(0), set, true); + List units = toList(code.definedUnits().getIntersecting(all)); + assertEquals(6, units.size()); + } + } } diff --git a/Ghidra/Debug/ProposedUtils/src/main/java/ghidra/pcode/exec/PcodeExecutor.java b/Ghidra/Debug/ProposedUtils/src/main/java/ghidra/pcode/exec/PcodeExecutor.java index 6267adf898..24ae347595 100644 --- a/Ghidra/Debug/ProposedUtils/src/main/java/ghidra/pcode/exec/PcodeExecutor.java +++ b/Ghidra/Debug/ProposedUtils/src/main/java/ghidra/pcode/exec/PcodeExecutor.java @@ -272,7 +272,7 @@ public class PcodeExecutor { throw e; } catch (Exception e) { - throw new PcodeExecutionException("Exception during pcode execution", frame, e); + throw new PcodeExecutionException(e.getMessage(), frame, e); } } diff --git a/Ghidra/Debug/ProposedUtils/src/main/java/ghidra/util/database/DBAnnotatedObject.java b/Ghidra/Debug/ProposedUtils/src/main/java/ghidra/util/database/DBAnnotatedObject.java index 86a3d82876..6165804e61 100644 --- a/Ghidra/Debug/ProposedUtils/src/main/java/ghidra/util/database/DBAnnotatedObject.java +++ b/Ghidra/Debug/ProposedUtils/src/main/java/ghidra/util/database/DBAnnotatedObject.java @@ -326,4 +326,8 @@ public class DBAnnotatedObject extends DatabaseObject { public boolean isDeleted() { return super.isDeleted(adapter.getLock()); } + + public String getTableName() { + return store.getTableName(); + } } diff --git a/Ghidra/Debug/ProposedUtils/src/main/java/ghidra/util/database/DBCachedObjectStore.java b/Ghidra/Debug/ProposedUtils/src/main/java/ghidra/util/database/DBCachedObjectStore.java index 746386f5a4..bd743878d3 100644 --- a/Ghidra/Debug/ProposedUtils/src/main/java/ghidra/util/database/DBCachedObjectStore.java +++ b/Ghidra/Debug/ProposedUtils/src/main/java/ghidra/util/database/DBCachedObjectStore.java @@ -1275,6 +1275,15 @@ public class DBCachedObjectStore implements ErrorHa return builder.toString(); } + /** + * Get the name of the table backing this store + * + * @return the name + */ + public String getTableName() { + return tableName; + } + /** * Invalidate this store's cache * diff --git a/Ghidra/Debug/ProposedUtils/src/main/java/ghidra/util/database/spatial/AbstractConstraintsTree.java b/Ghidra/Debug/ProposedUtils/src/main/java/ghidra/util/database/spatial/AbstractConstraintsTree.java index 835545db40..6954f3a835 100644 --- a/Ghidra/Debug/ProposedUtils/src/main/java/ghidra/util/database/spatial/AbstractConstraintsTree.java +++ b/Ghidra/Debug/ProposedUtils/src/main/java/ghidra/util/database/spatial/AbstractConstraintsTree.java @@ -82,7 +82,7 @@ public abstract class AbstractConstraintsTree< // protected abstract NR createNodeEntry(DBCachedObjectStore store, DBRecord record); protected void init() { - assert root == null; + // assert root == null; root = getOrCreateRoot(); leafLevel = computeLeafLevel(); } @@ -843,6 +843,7 @@ public abstract class AbstractConstraintsTree< // throw new AssertionError( "Leaf node " + n + " cannot contain nodes " + nodeChildren); } + break; } // Check that child count matches by counting over iterator @@ -911,9 +912,6 @@ public abstract class AbstractConstraintsTree< // " out of sync: cache=" + cachedChildren + " db=" + databasedChildren); } } - if (leafLevel != computeLeafLevel()) { - throw new AssertionError("Leaf level is incorrect"); - } visit(null, new TreeRecordVisitor() { @Override protected VisitResult beginNode(NR parent, NR n, QueryInclusion inclusion) { @@ -927,6 +925,9 @@ public abstract class AbstractConstraintsTree< // return VisitResult.NEXT; } }, false); + if (leafLevel != computeLeafLevel()) { + throw new AssertionError("Leaf level is incorrect"); + } } public abstract AbstractConstraintsTreeSpatialMap asSpatialMap(); @@ -945,6 +946,7 @@ public abstract class AbstractConstraintsTree< // cachedNodeChildren.clear(); dataStore.invalidateCache(); nodeStore.invalidateCache(); + init(); } } }