From 5d71f073f49af9574753413b466ac2059e3985a6 Mon Sep 17 00:00:00 2001 From: Dan <46821332+nsadeveloper789@users.noreply.github.com> Date: Wed, 15 Jan 2025 13:56:40 -0500 Subject: [PATCH] GP-5266: Only track on user click or address change. --- .../api/tracemgr/DebuggerCoordinates.java | 38 ++++++++++++++++++- .../action/DebuggerTrackLocationTrait.java | 37 +++++++++++------- .../gui/listing/DebuggerListingProvider.java | 2 +- .../gui/model/AbstractQueryTablePanel.java | 6 +-- .../debug/gui/model/ObjectsTreePanel.java | 2 +- .../trace/model/time/schedule/Sequence.java | 21 ++++++++++ .../model/time/schedule/TraceSchedule.java | 16 ++++++++ .../time/schedule/TraceScheduleTest.java | 33 ++++++++++++++++ .../GhidraClass/Debugger/A4-MachineState.html | 7 ---- .../GhidraClass/Debugger/A4-MachineState.md | 5 --- 10 files changed, 135 insertions(+), 32 deletions(-) diff --git a/Ghidra/Debug/Debugger-api/src/main/java/ghidra/debug/api/tracemgr/DebuggerCoordinates.java b/Ghidra/Debug/Debugger-api/src/main/java/ghidra/debug/api/tracemgr/DebuggerCoordinates.java index 89ef6ef085..0ac6647f16 100644 --- a/Ghidra/Debug/Debugger-api/src/main/java/ghidra/debug/api/tracemgr/DebuggerCoordinates.java +++ b/Ghidra/Debug/Debugger-api/src/main/java/ghidra/debug/api/tracemgr/DebuggerCoordinates.java @@ -66,7 +66,7 @@ public class DebuggerCoordinates { private static final String KEY_FRAME = "Frame"; private static final String KEY_OBJ_PATH = "ObjectPath"; - public static boolean equalsIgnoreRecorderAndView(DebuggerCoordinates a, + public static boolean equalsIgnoreTargetAndView(DebuggerCoordinates a, DebuggerCoordinates b) { if (!Objects.equals(a.trace, b.trace)) { return false; @@ -417,6 +417,36 @@ public class DebuggerCoordinates { newFrame, newPath); } + /** + * Checks if the given coordinates are the same as this but with an extra or differing patch. + * + * @param that the other coordinates + * @return true if the difference is only in the final patch step + */ + public boolean differsOnlyByPatch(DebuggerCoordinates that) { + if (!Objects.equals(this.trace, that.trace)) { + return false; + } + + if (!Objects.equals(this.platform, that.platform)) { + return false; + } + if (!Objects.equals(this.thread, that.thread)) { + return false; + } + // Consider defaults + if (!Objects.equals(this.getFrame(), that.getFrame())) { + return false; + } + if (!Objects.equals(this.getObject(), that.getObject())) { + return false; + } + if (!this.getTime().differsOnlyByPatch(that.getTime())) { + return false; + } + return true; + } + public DebuggerCoordinates frame(int newFrame) { if (trace == null) { return NOWHERE; @@ -621,7 +651,11 @@ public class DebuggerCoordinates { if (registerContainer != null) { return registerContainer; } - return registerContainer = getObject().findRegisterContainer(getFrame()); + TraceObject object = getObject(); + if (object == null) { + return null; + } + return registerContainer = object.findRegisterContainer(getFrame()); } public synchronized long getViewSnap() { diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/action/DebuggerTrackLocationTrait.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/action/DebuggerTrackLocationTrait.java index 35ff2c3d27..f31c3b4a1b 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/action/DebuggerTrackLocationTrait.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/action/DebuggerTrackLocationTrait.java @@ -4,9 +4,9 @@ * 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. @@ -52,6 +52,10 @@ public class DebuggerTrackLocationTrait { protected static final AutoConfigState.ClassHandler CONFIG_STATE_HANDLER = AutoConfigState.wireHandler(DebuggerTrackLocationTrait.class, MethodHandles.lookup()); + public enum TrackCause { + USER, DB_CHANGE, NAVIGATION, EMU_PATCH, SPEC_CHANGE_API; + } + protected class ForTrackingListener extends TraceDomainObjectListener { public ForTrackingListener() { @@ -68,7 +72,7 @@ public class DebuggerTrackLocationTrait { if (!tracker.affectedByBytesChange(space, range, current)) { return; } - doTrack(); + doTrack(TrackCause.DB_CHANGE); } private void stackChanged(TraceStack stack) { @@ -79,7 +83,7 @@ public class DebuggerTrackLocationTrait { if (!tracker.affectedByStackChange(stack, current)) { return; } - doTrack(); + doTrack(TrackCause.DB_CHANGE); } } @@ -188,11 +192,11 @@ public class DebuggerTrackLocationTrait { public void setSpec(LocationTrackingSpec spec) { if (action == null) { // It might if the client doesn't need a new button, e.g., TraceDiff - doSetSpec(spec); + doSetSpec(spec, TrackCause.SPEC_CHANGE_API); } else if (!hasSpec(spec)) { Msg.warn(this, "No action state for given tracking spec: " + spec); - doSetSpec(spec); + doSetSpec(spec, TrackCause.SPEC_CHANGE_API); } else { action.setCurrentActionStateByUserData(spec); @@ -234,21 +238,21 @@ public class DebuggerTrackLocationTrait { } protected void clickedSpecButton(ActionContext ctx) { - doTrack(); + doTrack(TrackCause.USER); } protected void clickedSpecMenu(ActionState newState, EventTrigger trigger) { - doSetSpec(newState.getUserData()); + doSetSpec(newState.getUserData(), TrackCause.USER); } - protected void doSetSpec(LocationTrackingSpec spec) { + protected void doSetSpec(LocationTrackingSpec spec, TrackCause cause) { if (this.spec != spec) { this.spec = spec; this.tracker = spec.getTracker(); specChanged(spec); } - doTrack(); + doTrack(cause); } protected ProgramLocation computeTrackedLocation() { @@ -282,9 +286,15 @@ public class DebuggerTrackLocationTrait { return spec.getLocationLabel() + " = " + trackedLocation.getByteAddress(); } - protected void doTrack() { + protected void doTrack(TrackCause cause) { try { - trackedLocation = computeTrackedLocation(); + ProgramLocation newLocation = computeTrackedLocation(); + if (Objects.equals(newLocation, trackedLocation)) { + if (cause == TrackCause.DB_CHANGE || cause == TrackCause.EMU_PATCH) { + return; + } + } + trackedLocation = newLocation; locationTracked(); } catch (Throwable ex) { @@ -315,11 +325,12 @@ public class DebuggerTrackLocationTrait { if (doListeners) { removeOldListeners(); } + boolean isPatch = current.differsOnlyByPatch(coordinates); current = coordinates; if (doListeners) { addNewListeners(); } - doTrack(); + doTrack(isPatch ? TrackCause.EMU_PATCH : TrackCause.NAVIGATION); } public void writeConfigState(SaveState saveState) { diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/listing/DebuggerListingProvider.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/listing/DebuggerListingProvider.java index 1341fe7b7c..c2c8e4a842 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/listing/DebuggerListingProvider.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/listing/DebuggerListingProvider.java @@ -235,7 +235,7 @@ public class DebuggerListingProvider extends CodeViewerProvider { super(DebuggerListingProvider.this.tool, DebuggerListingProvider.this.plugin, DebuggerListingProvider.this); - getListingPanel().addIndexMapChangeListener(e -> this.doTrack()); + getListingPanel().addIndexMapChangeListener(e -> this.doTrack(TrackCause.DB_CHANGE)); } @Override diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/model/AbstractQueryTablePanel.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/model/AbstractQueryTablePanel.java index 249aab470a..8130d5a884 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/model/AbstractQueryTablePanel.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/model/AbstractQueryTablePanel.java @@ -4,9 +4,9 @@ * 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. @@ -90,7 +90,7 @@ public abstract class AbstractQueryTablePanel { } return result; } + + public boolean differsOnlyByPatch(Sequence that) { + int size = this.steps.size(); + if (size == that.steps.size()) { + if (size == 0) { + return true; + } + if (!this.steps.subList(0, size - 1).equals(that.steps.subList(0, size - 1))) { + return false; + } + Step thisLast = this.steps.getLast(); + Step thatLast = that.steps.getLast(); + return thisLast.equals(thatLast) || + thisLast instanceof PatchStep && thatLast instanceof PatchStep; + } + if (size == that.steps.size() - 1) { + Step thatLast = that.steps.getLast(); + return thatLast instanceof PatchStep; + } + return false; + } } diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/model/time/schedule/TraceSchedule.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/model/time/schedule/TraceSchedule.java index ea703ad8ea..02c8b3a49a 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/model/time/schedule/TraceSchedule.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/model/time/schedule/TraceSchedule.java @@ -673,4 +673,20 @@ public class TraceSchedule implements Comparable { public TraceSchedule assumeRecorded() { return new TraceSchedule(snap, steps, pSteps, Source.RECORD); } + + public boolean differsOnlyByPatch(TraceSchedule that) { + if (this.snap != that.snap) { + return false; + } + if (this.pSteps.isNop() != that.pSteps.isNop()) { + return false; + } + if (this.pSteps.isNop()) { + return this.steps.differsOnlyByPatch(that.steps); + } + if (!this.steps.equals(that.steps)) { + return false; + } + return this.pSteps.differsOnlyByPatch(that.pSteps); + } } diff --git a/Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/model/time/schedule/TraceScheduleTest.java b/Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/model/time/schedule/TraceScheduleTest.java index a0eb1fbe85..721308b212 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/model/time/schedule/TraceScheduleTest.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/model/time/schedule/TraceScheduleTest.java @@ -470,4 +470,37 @@ public class TraceScheduleTest extends AbstractGhidraHeadlessIntegrationTest { "t0-{r0=0x200000001};t0-{r1l=0x3}", time.toString()); } } + + @Test + public void testDiffersOnlyByPatch() throws Exception { + assertTrue(TraceSchedule.parse("1").differsOnlyByPatch(TraceSchedule.parse("1"))); + assertTrue(TraceSchedule.parse("1:1").differsOnlyByPatch(TraceSchedule.parse("1:1"))); + assertTrue(TraceSchedule.parse("1:1.1").differsOnlyByPatch(TraceSchedule.parse("1:1.1"))); + assertTrue(TraceSchedule.parse("1:1;{r0=1}") + .differsOnlyByPatch(TraceSchedule.parse("1:1;{r0=1}"))); + assertTrue(TraceSchedule.parse("1:1.1;{r0=1}") + .differsOnlyByPatch(TraceSchedule.parse("1:1.1;{r0=1}"))); + + assertFalse(TraceSchedule.parse("1").differsOnlyByPatch(TraceSchedule.parse("1:1"))); + assertFalse(TraceSchedule.parse("1:1").differsOnlyByPatch(TraceSchedule.parse("1"))); + + assertFalse(TraceSchedule.parse("1:1").differsOnlyByPatch(TraceSchedule.parse("1:2"))); + assertFalse(TraceSchedule.parse("1:2").differsOnlyByPatch(TraceSchedule.parse("1:1"))); + + assertFalse(TraceSchedule.parse("1:1").differsOnlyByPatch(TraceSchedule.parse("1:1.1"))); + assertFalse(TraceSchedule.parse("1:1.1").differsOnlyByPatch(TraceSchedule.parse("1:1"))); + + assertTrue(TraceSchedule.parse("1").differsOnlyByPatch(TraceSchedule.parse("1:{r0=1}"))); + assertFalse(TraceSchedule.parse("1:{r0=1}").differsOnlyByPatch(TraceSchedule.parse("1"))); + + assertTrue( + TraceSchedule.parse("1:1").differsOnlyByPatch(TraceSchedule.parse("1:1;{r0=1}"))); + assertFalse( + TraceSchedule.parse("1:1;{r0=1}").differsOnlyByPatch(TraceSchedule.parse("1:1"))); + + assertTrue( + TraceSchedule.parse("1:1.1").differsOnlyByPatch(TraceSchedule.parse("1:1.1;{r0=1}"))); + assertFalse( + TraceSchedule.parse("1:1.1;{r0=1}").differsOnlyByPatch(TraceSchedule.parse("1:1.1"))); + } } diff --git a/GhidraDocs/GhidraClass/Debugger/A4-MachineState.html b/GhidraDocs/GhidraClass/Debugger/A4-MachineState.html index c993f3f8f7..6fc4a3ca2a 100644 --- a/GhidraDocs/GhidraClass/Debugger/A4-MachineState.html +++ b/GhidraDocs/GhidraClass/Debugger/A4-MachineState.html @@ -295,13 +295,6 @@ section of termmines in the Static Listing, the Dynamic Listing will follow along showing you the live values in memory. You can also experiment by placing code units in the Dynamic Listing before committing to them in the Static Listing.

-

NOTE: There’s a known issue with auto-seek obtruding -user navigation in the listings. In most cases, just navigating again -will make it stick. If it becomes a real annoyance, set the -Auto-Track drop-down in the top right of the Dynamic -Listing to Do Not Track while you’re doing static RE. -Be sure to put it back to Track Program Counter when -you are done.

Questions:

    diff --git a/GhidraDocs/GhidraClass/Debugger/A4-MachineState.md b/GhidraDocs/GhidraClass/Debugger/A4-MachineState.md index 122b10aa1d..0ae58a637e 100644 --- a/GhidraDocs/GhidraClass/Debugger/A4-MachineState.md +++ b/GhidraDocs/GhidraClass/Debugger/A4-MachineState.md @@ -137,11 +137,6 @@ Because you are in a dynamic session, you have an example board to work with. As you navigate the `.data` section of `termmines` in the Static Listing, the Dynamic Listing will follow along showing you the live values in memory. You can also experiment by placing code units in the Dynamic Listing before committing to them in the Static Listing. -**NOTE**: There's a known issue with auto-seek obtruding user navigation in the listings. -In most cases, just navigating again will make it stick. -If it becomes a real annoyance, set the **Auto-Track** drop-down in the top right of the Dynamic Listing to **Do Not Track** while you're doing static RE. -Be sure to put it back to **Track Program Counter** when you are done. - #### Questions: 1. How are the cells allocated?