From a17eb15006d799d6b3bad8f0edbae20a49e51f81 Mon Sep 17 00:00:00 2001 From: Dan <46821332+nsadeveloper789@users.noreply.github.com> Date: Wed, 19 Oct 2022 16:36:59 -0400 Subject: [PATCH 1/3] GP-2726: Default to 1 when GDB cannot evaluate sizeof() a watchpoint --- .../manager/breakpoint/GdbBreakpointInfo.java | 4 +-- .../GdbModelTargetBreakpointLocation.java | 32 +++++++++++++++---- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/breakpoint/GdbBreakpointInfo.java b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/breakpoint/GdbBreakpointInfo.java index 2dc308def0..4618850521 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/breakpoint/GdbBreakpointInfo.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/manager/breakpoint/GdbBreakpointInfo.java @@ -149,8 +149,8 @@ public class GdbBreakpointInfo { else { locs = List.of(); } - return new GdbBreakpointInfo(number, type, typeName, GdbBreakpointDisp.KEEP, null, null, - origLoc, origLoc, null, true, 0, locs); + return new GdbBreakpointInfo(number, type, typeName, GdbBreakpointDisp.KEEP, null, origLoc, + null, origLoc, null, true, 0, locs); } /** diff --git a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelTargetBreakpointLocation.java b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelTargetBreakpointLocation.java index 0a2949c17b..864459db3c 100644 --- a/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelTargetBreakpointLocation.java +++ b/Ghidra/Debug/Debugger-agent-gdb/src/main/java/agent/gdb/model/impl/GdbModelTargetBreakpointLocation.java @@ -18,11 +18,13 @@ package agent.gdb.model.impl; import java.util.List; import java.util.Map; import java.util.concurrent.CompletableFuture; +import java.util.function.Function; import agent.gdb.manager.breakpoint.GdbBreakpointLocation; import agent.gdb.manager.parsing.GdbCValueParser; import agent.gdb.manager.parsing.GdbParsingUtils.GdbParseError; import generic.Unique; +import ghidra.async.AsyncUtils; import ghidra.dbg.agent.DefaultTargetObject; import ghidra.dbg.target.TargetBreakpointLocation; import ghidra.dbg.target.TargetObject; @@ -98,7 +100,7 @@ public class GdbModelTargetBreakpointLocation int iid = Unique.assertOne(loc.getInferiorIds()); GdbModelTargetInferior inf = impl.session.inferiors.getTargetInferior(iid); String addrSizeExp = String.format("{(long long)&(%s), (long long)sizeof(%s)}", exp, exp); - return inf.inferior.evaluate(addrSizeExp).thenAccept(result -> { + return inf.inferior.evaluate(addrSizeExp).thenApply(result -> { List vals; try { vals = GdbCValueParser.parseArray(result).expectLongs(); @@ -112,13 +114,29 @@ public class GdbModelTargetBreakpointLocation range = makeRange(impl.space.getAddress(vals.get(0)), vals.get(1).intValue()); doChangeAttributes("Initialized"); + return AsyncUtils.NIL; }).exceptionally(ex -> { - Msg.warn(this, "Could not evaluated breakpoint location and/or size: " + ex); - Address addr = impl.space.getAddress(0); - range = new AddressRangeImpl(addr, addr); - doChangeAttributes("Defaulted for eval/parse error"); - return null; - }); + CompletableFuture secondTry = + inf.inferior.evaluate(String.format("(long long)&(%s)", exp)); + return secondTry.thenAccept(result -> { + long addr; + try { + addr = GdbCValueParser.parseValue(result).expectLong(); + } + catch (GdbParseError e) { + throw new AssertionError("Unexpected result type: " + result, e); + } + range = makeRange(impl.space.getAddress(addr), 1); + doChangeAttributes("Initialized, but defaulted length=1"); + }).exceptionally(ex2 -> { + Msg.warn(this, + "Could not evaluated breakpoint location and/or size: " + ex2); + Address addr = impl.space.getAddress(0); + range = new AddressRangeImpl(addr, addr); + doChangeAttributes("Defaulted for eval/parse error"); + return null; + }); + }).thenCompose(Function.identity()); } protected String computeDisplay() { From b610c697895b94622c76be9130650433877bc97b Mon Sep 17 00:00:00 2001 From: Dan <46821332+nsadeveloper789@users.noreply.github.com> Date: Thu, 20 Oct 2022 10:38:58 -0400 Subject: [PATCH 2/3] GP-2736: Fix NPEs when restoring the Debugger with open traces --- .../app/plugin/core/debug/DebuggerCoordinates.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/DebuggerCoordinates.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/DebuggerCoordinates.java index c987db62c9..d9f23b0f2d 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/DebuggerCoordinates.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/DebuggerCoordinates.java @@ -351,7 +351,7 @@ public class DebuggerCoordinates { throw new IllegalArgumentException("Cannot change trace"); } if (newThread == null) { - newThread = resolveThread(recorder, getTime()); + newThread = resolveThread(trace, recorder, getTime()); } Trace newTrace = trace != null ? trace : newThread.getTrace(); TracePlatform newPlatform = platform != null ? platform : resolvePlatform(newTrace); @@ -407,6 +407,13 @@ public class DebuggerCoordinates { newObject); } + public DebuggerCoordinates frame(Integer newFrame) { + if (newFrame == null) { + return this; + } + return frame(newFrame.intValue()); + } + private DebuggerCoordinates replaceView(TraceProgramView newView) { return new DebuggerCoordinates(trace, platform, recorder, thread, newView, time, frame, object); From 5b052319a2e58a1e0548b837a8110619df7d5cd8 Mon Sep 17 00:00:00 2001 From: Dan <46821332+nsadeveloper789@users.noreply.github.com> Date: Thu, 20 Oct 2022 12:19:58 -0400 Subject: [PATCH 3/3] GP-2725: Fix misuse of "Bytes" field location when setting breakpoints. --- .../DebuggerPlaceBreakpointDialog.java | 50 +++++++++++++++++-- ...ebuggerLogicalBreakpointServicePlugin.java | 2 +- .../DebuggerLogicalBreakpointService.java | 26 ++++++++-- 3 files changed, 69 insertions(+), 9 deletions(-) diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/breakpoint/DebuggerPlaceBreakpointDialog.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/breakpoint/DebuggerPlaceBreakpointDialog.java index 3d0800351f..e091bce6c2 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/breakpoint/DebuggerPlaceBreakpointDialog.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/breakpoint/DebuggerPlaceBreakpointDialog.java @@ -22,6 +22,8 @@ import java.util.Set; import javax.swing.*; import javax.swing.border.EmptyBorder; +import javax.swing.event.DocumentEvent; +import javax.swing.event.DocumentListener; import docking.DialogComponentProvider; import ghidra.app.plugin.core.debug.gui.DebuggerResources.AbstractSetBreakpointAction; @@ -29,6 +31,7 @@ import ghidra.app.services.DebuggerLogicalBreakpointService; import ghidra.async.AsyncUtils; import ghidra.framework.plugintool.PluginTool; import ghidra.program.model.address.Address; +import ghidra.program.model.listing.Instruction; import ghidra.program.model.listing.Program; import ghidra.program.util.ProgramLocation; import ghidra.trace.model.breakpoint.TraceBreakpointKind; @@ -56,6 +59,22 @@ public class DebuggerPlaceBreakpointDialog extends DialogComponentProvider { populateComponents(); } + protected boolean validateAddress() { + address = program.getAddressFactory().getAddress(fieldAddress.getText()); + if (address == null) { + setStatusText("Invalid address: " + fieldAddress.getText()); + return false; + } + Instruction instruction = program.getListing().getInstructionContaining(address); + if (instruction != null && !address.equals(instruction.getAddress())) { + setStatusText("Warning: breakpoint is offset within an instruction."); + } + else { + clearStatusText(); + } + return true; + } + protected void populateComponents() { JPanel panel = new JPanel(new PairLayout(5, 5)); @@ -66,6 +85,29 @@ public class DebuggerPlaceBreakpointDialog extends DialogComponentProvider { panel.add(labelAddress); panel.add(fieldAddress); + fieldAddress.setInputVerifier(new InputVerifier() { + @Override + public boolean verify(JComponent input) { + return validateAddress(); + } + }); + fieldAddress.getDocument().addDocumentListener(new DocumentListener() { + @Override + public void insertUpdate(DocumentEvent e) { + validateAddress(); + } + + @Override + public void removeUpdate(DocumentEvent e) { + validateAddress(); + } + + @Override + public void changedUpdate(DocumentEvent e) { + validateAddress(); + } + }); + JLabel labelLength = new JLabel("Length"); fieldLength = new JTextField(); panel.add(labelLength); @@ -99,7 +141,7 @@ public class DebuggerPlaceBreakpointDialog extends DialogComponentProvider { ProgramLocation loc, long length, Collection kinds, String name) { this.service = service; this.program = loc.getProgram(); - this.address = loc.getAddress(); // byte address can be confusing here. + this.address = DebuggerLogicalBreakpointService.addressFromLocation(loc); this.length = length; this.kinds = Set.copyOf(kinds); this.name = name; @@ -109,7 +151,7 @@ public class DebuggerPlaceBreakpointDialog extends DialogComponentProvider { this.fieldKinds.setSelectedItem(TraceBreakpointKindSet.encode(kinds)); this.fieldName.setText(""); - clearStatusText(); + validateAddress(); setTitle(title); tool.showDialog(this); @@ -117,9 +159,7 @@ public class DebuggerPlaceBreakpointDialog extends DialogComponentProvider { @Override protected void okCallback() { - address = program.getAddressFactory().getAddress(fieldAddress.getText()); - if (address == null) { - setStatusText("Invalid address: " + fieldAddress.getText()); + if (!validateAddress()) { return; } try { diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/breakpoint/DebuggerLogicalBreakpointServicePlugin.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/breakpoint/DebuggerLogicalBreakpointServicePlugin.java index db59ba4989..13b6d6c7cd 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/breakpoint/DebuggerLogicalBreakpointServicePlugin.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/breakpoint/DebuggerLogicalBreakpointServicePlugin.java @@ -1108,7 +1108,7 @@ public class DebuggerLogicalBreakpointServicePlugin extends Plugin } MappedLogicalBreakpoint lb = new MappedLogicalBreakpoint(staticLocation.getProgram(), - staticLocation.getAddress(), length, kinds); + staticLocation.getByteAddress(), length, kinds); lb.setTraceAddress(recorder, address); lb.enableForProgramWithName(name); return lb.enableForTrace(trace); diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/services/DebuggerLogicalBreakpointService.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/services/DebuggerLogicalBreakpointService.java index dfd8947e6b..34a67e54ed 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/services/DebuggerLogicalBreakpointService.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/services/DebuggerLogicalBreakpointService.java @@ -25,7 +25,7 @@ import ghidra.app.services.LogicalBreakpoint.State; import ghidra.framework.plugintool.ServiceInfo; import ghidra.program.model.address.Address; import ghidra.program.model.listing.Program; -import ghidra.program.util.ProgramLocation; +import ghidra.program.util.*; import ghidra.trace.model.Trace; import ghidra.trace.model.breakpoint.TraceBreakpoint; import ghidra.trace.model.breakpoint.TraceBreakpointKind; @@ -158,15 +158,35 @@ public interface DebuggerLogicalBreakpointService { */ CompletableFuture changesSettled(); + /** + * Get the address most likely intended by the user for a given location + * + *

+ * Program locations always have addresses at the start of a code unit, no matter how the + * location was produced. This attempts to interpret the context a bit deeper to discern the + * user's intent. At the moment, it seems reasonable to check if the location includes a code + * unit. If so, take its min address, i.e., the location's address. If not, take the location's + * byte address. + * + * @param loc the location + * @return the address + */ + static Address addressFromLocation(ProgramLocation loc) { + if (loc instanceof CodeUnitLocation) { + return loc.getAddress(); + } + return loc.getByteAddress(); + } + static T programOrTrace(ProgramLocation loc, BiFunction progFunc, BiFunction traceFunc) { Program progOrView = loc.getProgram(); if (progOrView instanceof TraceProgramView) { TraceProgramView view = (TraceProgramView) progOrView; - return traceFunc.apply(view.getTrace(), loc.getByteAddress()); + return traceFunc.apply(view.getTrace(), addressFromLocation(loc)); } - return progFunc.apply(progOrView, loc.getByteAddress()); + return progFunc.apply(progOrView, addressFromLocation(loc)); } default State computeState(Collection col) {