Merge remote-tracking branch 'origin/GP-2243_Dan_breakpointToggleErrorMessages'

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/breakpoint/DebuggerBreakpointMarkerPlugin.java

@@ -532,7 +532,12 @@ public class DebuggerBreakpointMarkerPlugin extends Plugin
 			}
 			ProgramLocation location = getLocationFromContext(context);
 			Set<LogicalBreakpoint> col = breakpointService.getBreakpointsAt(location);
-			breakpointService.enableAll(col, getTraceFromContext(context)).exceptionally(ex -> {
+			Trace trace = getTraceFromContext(context);
+			String status = breakpointService.generateStatusEnable(col, trace);
+			if (status != null) {
+				tool.setStatusInfo(status, true);
+			}
+			breakpointService.enableAll(col, trace).exceptionally(ex -> {
 				breakpointError(NAME, "Could not enable breakpoint", ex);
 				return null;
 			});
@@ -869,6 +874,10 @@ public class DebuggerBreakpointMarkerPlugin extends Plugin
 		if (loc == null) {
 			return;
 		}
+		String status = breakpointService.generateStatusToggleAt(loc);
+		if (status != null) {
+			tool.setStatusInfo(status, true);
+		}
 		breakpointService.toggleBreakpointsAt(loc, () -> {
 			Set<TraceBreakpointKind> supported = getSupportedKindsFromContext(context);
 			if (supported.isEmpty()) {

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/breakpoint/DebuggerBreakpointsProvider.java

@@ -267,7 +267,12 @@ public class DebuggerBreakpointsProvider extends ComponentProviderAdapter
 				DebuggerLogicalBreakpointsActionContext ctx =
 					(DebuggerLogicalBreakpointsActionContext) context;
 				Collection<LogicalBreakpoint> sel = ctx.getBreakpoints();
-				breakpointService.enableAll(sel, null).exceptionally(ex -> {
+				Trace trace = isFilterByCurrentTrace() ? currentTrace : null;
+				String status = breakpointService.generateStatusEnable(sel, trace);
+				if (status != null) {
+					tool.setStatusInfo(status, true);
+				}
+				breakpointService.enableAll(sel, trace).exceptionally(ex -> {
 					breakpointError("Enable Breakpoints", "Could not enable breakpoints", ex);
 					return null;
 				});
@@ -309,7 +314,12 @@ public class DebuggerBreakpointsProvider extends ComponentProviderAdapter
 		@Override
 		public void actionPerformed(ActionContext context) {
 			Set<LogicalBreakpoint> all = breakpointService.getAllBreakpoints();
-			breakpointService.enableAll(all, null).exceptionally(ex -> {
+			Trace trace = isFilterByCurrentTrace() ? currentTrace : null;
+			String status = breakpointService.generateStatusEnable(all, trace);
+			if (status != null) {
+				tool.setStatusInfo(status, true);
+			}
+			breakpointService.enableAll(all, trace).exceptionally(ex -> {
 				breakpointError("Enable All Breakpoints", "Could not enable breakpoints", ex);
 				return null;
 			});
@@ -996,7 +1006,12 @@ public class DebuggerBreakpointsProvider extends ComponentProviderAdapter
 			new DebuggerBreakpointStateTableCellEditor<>(breakpointFilterPanel) {
 				@Override
 				protected State getToggledState(LogicalBreakpointRow row, State current) {
-					return current.getToggled(row.isMapped());
+					boolean mapped = row.isMapped();
+					if (!mapped) {
+						tool.setStatusInfo(
+							"Breakpoint has no locations. Only toggling its bookmark.", true);
+					}
+					return current.getToggled(mapped);
 				}
 			});
 		bptEnCol.setMaxWidth(24);

Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/breakpoint/LogicalBreakpointRow.java

@@ -60,8 +60,14 @@ public class LogicalBreakpointRow {
 	}
 
 	public void setEnabled(boolean enabled) {
+		boolean filter = provider.isFilterByCurrentTrace();
 		if (enabled) {
-			CompletableFuture<Void> future = provider.isFilterByCurrentTrace()
+			String status = lb.generateStatusEnable(filter ? provider.currentTrace : null);
+			if (status != null) {
+				provider.getTool().setStatusInfo(status, true);
+			}
+			lb.enableForProgram();
+			CompletableFuture<Void> future = filter
 					? lb.enableForTrace(provider.currentTrace)
 					: lb.enable();
 			future.exceptionally(ex -> {
@@ -70,7 +76,8 @@ public class LogicalBreakpointRow {
 			});
 		}
 		else {
-			CompletableFuture<Void> future = provider.isFilterByCurrentTrace()
+			lb.disableForProgram();
+			CompletableFuture<Void> future = filter
 					? lb.disableForTrace(provider.currentTrace)
 					: lb.disable();
 			future.exceptionally(ex -> {
@@ -139,6 +146,9 @@ public class LogicalBreakpointRow {
 	 */
 	public boolean isMapped() {
 		if (provider.isFilterByCurrentTrace()) {
+			if (provider.currentTrace == null) {
+				return false;
+			}
 			return lb.getMappedTraces().contains(provider.currentTrace);
 		}
 		return !lb.getMappedTraces().isEmpty();

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

@@ -1131,15 +1131,25 @@ public class DebuggerLogicalBreakpointServicePlugin extends Plugin
 			if (trace == null || participants.isEmpty() || participants.equals(Set.of(trace))) {
 				consumerForProgram.accept(lb);
 			}
-			if (!(lb instanceof LogicalBreakpointInternal)) {
+			if (!(lb instanceof LogicalBreakpointInternal lbi)) {
 				continue;
 			}
-			LogicalBreakpointInternal lbi = (LogicalBreakpointInternal) lb;
 			consumerForTarget.accept(actions, lbi);
 		}
 		return actions.execute();
 	}
 
+	@Override
+	public String generateStatusEnable(Collection<LogicalBreakpoint> col, Trace trace) {
+		String message;
+		for (LogicalBreakpoint lb : col) {
+			if ((message = lb.generateStatusEnable(trace)) != null) {
+				return message;
+			}
+		}
+		return null;
+	}
+
 	@Override
 	public CompletableFuture<Void> enableAll(Collection<LogicalBreakpoint> col, Trace trace) {
 		return actOnAll(col, trace, LogicalBreakpoint::enableForProgram,
@@ -1205,6 +1215,32 @@ public class DebuggerLogicalBreakpointServicePlugin extends Plugin
 		});
 	}
 
+	@Override
+	public String generateStatusToggleAt(ProgramLocation loc) {
+		Set<LogicalBreakpoint> bs = getBreakpointsAt(loc);
+		if (bs == null || bs.isEmpty()) {
+			return null;
+		}
+		State state = computeState(bs, loc);
+		Trace trace =
+			DebuggerLogicalBreakpointService.programOrTrace(loc, (p, a) -> null, (t, a) -> t);
+		/**
+		 * TODO: If we have a trace here, then there are mapped breakpoints, no? We should never
+		 * expect a status message in that case. I don't suppose it hurts to check, though, since
+		 * the rules could change later.
+		 */
+		boolean mapped = anyMapped(bs, trace);
+		if (!mapped) {
+			return "No breakpoint at this location is mapped to a live trace. " +
+				"Cannot toggle on target. Is there a target? Check your module map.";
+		}
+		State toggled = state.getToggled(mapped);
+		if (!toggled.isEnabled()) {
+			return null;
+		}
+		return generateStatusEnable(bs, trace);
+	}
+
 	@Override
 	public CompletableFuture<Set<LogicalBreakpoint>> toggleBreakpointsAt(ProgramLocation loc,
 			Supplier<CompletableFuture<Set<LogicalBreakpoint>>> placer) {

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

@@ -211,6 +211,14 @@ public class LoneLogicalBreakpoint implements LogicalBreakpointInternal {
 		breaks.planEnable(actions, length, kinds);
 	}
 
+	@Override
+	public String generateStatusEnable(Trace trace) {
+		if (trace == null || trace == breaks.getTrace()) {
+			return null;
+		}
+		return "A breakpoint is not in this trace. Is there a target? Check your module map.";
+	}
+
 	@Override
 	public CompletableFuture<Void> enable() {
 		BreakpointActionSet actions = new BreakpointActionSet();

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

@@ -192,6 +192,25 @@ public class MappedLogicalBreakpoint implements LogicalBreakpointInternal {
 		return enableForTraces();
 	}
 
+	@Override
+	public String generateStatusEnable(Trace trace) {
+		if (trace == null) {
+			for (TraceBreakpointSet breaks : traceBreaks.values()) {
+				if (!breaks.isEmpty()) {
+					return null;
+				}
+			}
+			return "A breakpoint is not mapped to any live trace. Cannot enable it on target. " +
+				"Is there a target? Check your module map.";
+		}
+		TraceBreakpointSet breaks = traceBreaks.get(trace);
+		if (breaks != null && !breaks.isEmpty()) {
+			return null;
+		}
+		return "A breakpoint is not mapped to the trace, or the trace is not live. " +
+			"Cannot enable it on target. Is there a target? Check your module map.";
+	}
+
 	@Override
 	public CompletableFuture<Void> enable() {
 		enableForProgram();

Ghidra/Debug/Debugger/src/main/java/ghidra/app/services/DebuggerLogicalBreakpointService.java

@@ -292,6 +292,23 @@ public interface DebuggerLogicalBreakpointService {
 	CompletableFuture<Void> placeBreakpointAt(ProgramLocation loc, long length,
 			Collection<TraceBreakpointKind> kinds, String name);
 
+	/**
+	 * Generate an informational status message when enabling the selected breakpoints
+	 * 
+	 * <p>
+	 * Breakpoint enabling may fail for a variety of reasons. Some of those reasons deal with the
+	 * trace database and GUI rather than with the target. When enabling will not likely behave in
+	 * the manner expected by the user, this should provide a message explaining why. For example,
+	 * if a breakpoint has no locations on a target, then we already know "enable" will not work.
+	 * This should explain the situation to the user. If enabling is expected to work, then this
+	 * should return null.
+	 * 
+	 * @param col the collection we're about to enable
+	 * @param trace a trace, if the command will be limited to the given trace
+	 * @return the status message, or null
+	 */
+	String generateStatusEnable(Collection<LogicalBreakpoint> col, Trace trace);
+
 	/**
 	 * Enable a collection of logical breakpoints on target, if applicable
 	 * 
@@ -353,6 +370,21 @@ public interface DebuggerLogicalBreakpointService {
 	 */
 	CompletableFuture<Void> deleteLocs(Collection<TraceBreakpoint> col);
 
+	/**
+	 * Generate an informational message when toggling the breakpoints at the given location
+	 * 
+	 * <p>
+	 * This works in the same manner as {@link #generateStatusEnable(Collection)}, except it is for
+	 * toggling breakpoints at a given location. If there are no breakpoints at the location, this
+	 * should return null, since the usual behavior in that case is to prompt to place a new
+	 * breakpoint.
+	 * 
+	 * @see #generateStatusEnable(Collection)
+	 * @param loc the location
+	 * @return the status message, or null
+	 */
+	String generateStatusToggleAt(ProgramLocation loc);
+
 	/**
 	 * Toggle the breakpoints at the given location
 	 * 

Ghidra/Debug/Debugger/src/main/java/ghidra/app/services/LogicalBreakpoint.java

@@ -790,6 +790,18 @@ public interface LogicalBreakpoint {
 	 */
 	CompletableFuture<Void> deleteForTrace(Trace trace);
 
+	/**
+	 * Generate a status message for enabling this breakpoint
+	 * 
+	 * <p>
+	 * If this breakpoint has no locations in the given trace, then the status message should
+	 * explain that it cannot actually enable the breakpoint.
+	 * 
+	 * @param trace optional to limit scope of message to locations in the given trace
+	 * @return the status message, or null
+	 */
+	String generateStatusEnable(Trace trace);
+
 	/**
 	 * Enable (or create) this breakpoint everywhere in the tool.
 	 *