GP-2725: Fix misuse of "Bytes" field location when setting breakpoints.

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<TraceBreakpointKind> 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 {

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);

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<Void> changesSettled();
 
+	/**
+	 * Get the address most likely intended by the user for a given location
+	 * 
+	 * <p>
+	 * 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> T programOrTrace(ProgramLocation loc,
 			BiFunction<? super Program, ? super Address, ? extends T> progFunc,
 			BiFunction<? super Trace, ? super Address, ? extends T> 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<LogicalBreakpoint> col) {