Bug 528145 - Breakpoints are not working with remote attach launch

Looking at the logs, it seems that the regression is caused at 8bec791
where support for multi-process was added. We removed breakpoints
tracking support from final launch sequence and moved it to debug new
process and attach to process logic but none of these are run for remote
attach launch, hence breakpoint tracking is not started for remote
attach launch.

To fix the problem, IGDBProcesses.attachDebuggerToProcess(..) is updated
to handle remote attach launch as well instead of final launch sequence
handling it.

This commit is created after reverting 7bddb5f and 96839a0 which is the
older fix done to fix this issue and the other commit was to fix the
regression caused by the old fix.

The problem with older fix was that for non-stop mode, attach to process
was not working for remote launches when there is already a process
being debugged. Note that to use this feature, gdbserver should be
started with --multi option.

Author: umairsair

NewAndNoteworthy/CDT-11.2.md

@@ -12,6 +12,10 @@ This is the New & Noteworthy page for CDT 11.2 which is part of Eclipse 2023-06
 
 Please see [CHANGELOG-API](CHANGELOG-API.md) for details on the breaking API changes in this release as well as future planned API changes.
 
+## `FinalLaunchSequence.stepRemoteConnection()` and `FinalLaunchSequence.stepAttachRemoteToDebugger()` are deprecated and no more called
+
+The remote connection for attach launch is now handled in the implementation of `IGDBProcesses.attachDebuggerToProcess()`
+
 # Noteworthy Issues and Pull Requests
 
 See [Noteworthy issues and PRs](https://github.com/eclipse-cdt/cdt/issues?q=is%3Aclosed+label%3Anoteworthy+milestone%3A11.2.0) for this release in the issue/PR tracker.

NewAndNoteworthy/CHANGELOG-API.md

@@ -665,3 +665,11 @@ The class BuiltinDetctionArgsGeneric will be removed. Use the correctly
 spelled BuiltinDetectionArgsGeneric instead.
 
 - org.eclipse.cdt.jsoncdb.core.participant.Arglets.BuiltinDetctionArgsGeneric
+
+## API Removals after June 2025
+
+### <span id="FinalLaunchSequence">FinalLaunchSequence.stepRemoteConnection() and FinalLaunchSequence.stepAttachRemoteToDebugger() will be removed</span>
+
+These APIs will be removed as they are no more used.
+
+See https://github.com/eclipse-cdt/cdt/pull/336

dsf-gdb/org.eclipse.cdt.dsf.gdb/META-INF/MANIFEST.MF

@@ -3,7 +3,7 @@ Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-Vendor: %providerName
 Bundle-SymbolicName: org.eclipse.cdt.dsf.gdb;singleton:=true
-Bundle-Version: 7.0.0.qualifier
+Bundle-Version: 7.1.0.qualifier
 Bundle-Activator: org.eclipse.cdt.dsf.gdb.internal.GdbPlugin
 Bundle-Localization: plugin
 Require-Bundle: org.eclipse.core.runtime,

dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/launching/FinalLaunchSequence.java

@@ -45,11 +45,12 @@
 import org.eclipse.cdt.dsf.gdb.actions.IConnect;
 import org.eclipse.cdt.dsf.gdb.internal.GdbPlugin;
 import org.eclipse.cdt.dsf.gdb.service.IGDBBackend;
+import org.eclipse.cdt.dsf.gdb.service.IGDBProcesses;
 import org.eclipse.cdt.dsf.gdb.service.IGDBSourceLookup;
 import org.eclipse.cdt.dsf.gdb.service.SessionType;
 import org.eclipse.cdt.dsf.gdb.service.command.IGDBControl;
 import org.eclipse.cdt.dsf.mi.service.CSourceLookup;
-import org.eclipse.cdt.dsf.mi.service.IMIProcesses;
+import org.eclipse.cdt.dsf.mi.service.MIProcesses;
 import org.eclipse.cdt.dsf.mi.service.command.CommandFactory;
 import org.eclipse.cdt.dsf.mi.service.command.output.MIGDBVersionInfo;
 import org.eclipse.cdt.dsf.mi.service.command.output.MIInfo;
@@ -74,7 +75,7 @@ public class FinalLaunchSequence extends ReflectionSequence {
 
 	private IGDBControl fCommandControl;
 	private IGDBBackend fGDBBackend;
-	private IMIProcesses fProcService;
+	private IGDBProcesses fProcService;
 	private CommandFactory fCommandFactory;
 
 	private DsfServicesTracker fTracker;
@@ -133,11 +134,9 @@ protected String[] getExecutionOrder(String group) {
 					//
 					// "stepSetSourceLookupPath",   //$NON-NLS-1$
 
-					// For remote-attach launch only
-					"stepRemoteConnection", //$NON-NLS-1$
 					// For all launches except attach ones
 					"stepNewProcess", //$NON-NLS-1$
-					// For local attach launch only
+					// For all attach launch only
 					"stepAttachToProcess", //$NON-NLS-1$
 					// Global
 					"stepDataModelInitializationComplete", //$NON-NLS-1$
@@ -173,7 +172,7 @@ public void stepInitializeFinalLaunchSequence(RequestMonitor requestMonitor) {
 
 		fCommandFactory = fCommandControl.getCommandFactory();
 
-		fProcService = fTracker.getService(IMIProcesses.class);
+		fProcService = fTracker.getService(IGDBProcesses.class);
 		if (fProcService == null) {
 			requestMonitor.setStatus(
 					new Status(IStatus.ERROR, GdbPlugin.PLUGIN_ID, -1, "Cannot obtain process service", null)); //$NON-NLS-1$
@@ -547,37 +546,15 @@ protected void handleError() {
 		rm.done();
 	}
 
-	private static final String INVALID = "invalid"; //$NON-NLS-1$
-
 	/**
 	 * If we are dealing with a remote-attach debugging session, connect to the target.
 	 * @since 4.0
+	 * this is no more added in execution steps
 	 */
 	@Execute
+	@Deprecated(forRemoval = true)
 	public void stepRemoteConnection(final RequestMonitor rm) {
-		if (fGDBBackend.getSessionType() == SessionType.REMOTE && fGDBBackend.getIsAttachSession()) {
-			boolean isTcpConnection = CDebugUtils.getAttribute(fAttributes,
-					IGDBLaunchConfigurationConstants.ATTR_REMOTE_TCP, false);
-
-			if (isTcpConnection) {
-				String remoteTcpHost = CDebugUtils.getAttribute(fAttributes, IGDBLaunchConfigurationConstants.ATTR_HOST,
-						INVALID);
-				String remoteTcpPort = CDebugUtils.getAttribute(fAttributes, IGDBLaunchConfigurationConstants.ATTR_PORT,
-						INVALID);
-
-				fCommandControl.queueCommand(fCommandFactory.createMITargetSelect(fCommandControl.getContext(),
-						remoteTcpHost, remoteTcpPort, true), new ImmediateDataRequestMonitor<MIInfo>(rm));
-			} else {
-				String serialDevice = CDebugUtils.getAttribute(fAttributes, IGDBLaunchConfigurationConstants.ATTR_DEV,
-						INVALID);
-
-				fCommandControl.queueCommand(
-						fCommandFactory.createMITargetSelect(fCommandControl.getContext(), serialDevice, true),
-						new ImmediateDataRequestMonitor<MIInfo>(rm));
-			}
-		} else {
-			rm.done();
-		}
+		rm.done();
 	}
 
 	/**
@@ -588,19 +565,9 @@ public void stepRemoteConnection(final RequestMonitor rm) {
 	@Execute
 	public void stepNewProcess(final RequestMonitor rm) {
 		if (!fGDBBackend.getIsAttachSession()) {
-			boolean noBinarySpecified = CDebugUtils.getAttribute(fAttributes,
-					IGDBLaunchConfigurationConstants.ATTR_DEBUGGER_USE_SOLIB_SYMBOLS_FOR_APP,
-					IGDBLaunchConfigurationConstants.DEBUGGER_USE_SOLIB_SYMBOLS_FOR_APP_DEFAULT);
-
-			String binary = null;
-			final IPath execPath = fGDBBackend.getProgramPath();
-			if (!noBinarySpecified && execPath != null && !execPath.isEmpty()) {
-				binary = execPath.toString();
-			}
-
 			// Even if binary is null, we must call this to do all the other steps
 			// necessary to create a process.  It is possible that the binary is not needed
-			fProcService.debugNewProcess(fCommandControl.getContext(), binary, fAttributes,
+			fProcService.debugNewProcess(fCommandControl.getContext(), getBinary(), fAttributes,
 					new DataRequestMonitor<IDMContext>(getExecutor(), rm) {
 						@Override
 						protected void handleCancel() {
@@ -618,14 +585,28 @@ protected void handleCancel() {
 	}
 
 	/**
-	 * If we are dealing with an local attach session, perform the attach.
-	 * For a remote attach session, we don't attach during the launch; instead
-	 * we wait for the user to manually do the attach.
+	 * @since 7.1
+	 */
+	protected String getBinary() {
+		boolean noBinarySpecified = CDebugUtils.getAttribute(fAttributes,
+				IGDBLaunchConfigurationConstants.ATTR_DEBUGGER_USE_SOLIB_SYMBOLS_FOR_APP,
+				IGDBLaunchConfigurationConstants.DEBUGGER_USE_SOLIB_SYMBOLS_FOR_APP_DEFAULT);
+
+		String binary = null;
+		final IPath execPath = fGDBBackend.getProgramPath();
+		if (!noBinarySpecified && execPath != null && !execPath.isEmpty()) {
+			binary = execPath.toString();
+		}
+		return binary;
+	}
+
+	/**
+	 * If we are dealing with an attach session, perform the attach.
 	 * @since 4.0
 	 */
 	@Execute
 	public void stepAttachToProcess(final RequestMonitor requestMonitor) {
-		if (fGDBBackend.getIsAttachSession() && fGDBBackend.getSessionType() != SessionType.REMOTE) {
+		if (fGDBBackend.getIsAttachSession()) {
 			// Is the process id already stored in the launch?
 			int pid = CDebugUtils.getAttribute(fAttributes, ICDTLaunchConfigurationConstants.ATTR_ATTACH_PROCESS_ID,
 					-1);
@@ -634,6 +615,10 @@ public void stepAttachToProcess(final RequestMonitor requestMonitor) {
 				fProcService.attachDebuggerToProcess(
 						fProcService.createProcessContext(fCommandControl.getContext(), Integer.toString(pid)),
 						new DataRequestMonitor<IDMContext>(getExecutor(), requestMonitor));
+			} else if (fGDBBackend.getSessionType() == SessionType.REMOTE) {
+				fProcService.attachDebuggerToProcess(
+						fProcService.createProcessContext(fCommandControl.getContext(), MIProcesses.UNKNOWN_PROCESS_ID),
+						getBinary(), new DataRequestMonitor<IDMContext>(getExecutor(), requestMonitor));
 			} else {
 				IConnectHandler connectCommand = (IConnectHandler) fSession.getModelAdapter(IConnectHandler.class);
 				if (connectCommand instanceof IConnect) {
@@ -647,6 +632,19 @@ public void stepAttachToProcess(final RequestMonitor requestMonitor) {
 		}
 	}
 
+	/**
+	 * If we are dealing with an remote attach session, perform the attach to debugger.
+	 * Bug 528145
+	 * @since 6.6
+	 *
+	 * this is no more added in execution steps
+	 */
+	@Execute
+	@Deprecated(forRemoval = true)
+	public void stepAttachRemoteToDebugger(final RequestMonitor requestMonitor) {
+		requestMonitor.done();
+	}
+
 	/**
 	 * Indicate that the Data Model has been filled.  This will trigger the Debug view to expand.
 	 * @since 4.0

dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBProcesses_7_0.java

@@ -39,6 +39,7 @@
 import org.eclipse.cdt.core.CCorePlugin;
 import org.eclipse.cdt.core.IProcessInfo;
 import org.eclipse.cdt.core.IProcessList;
+import org.eclipse.cdt.debug.core.CDebugUtils;
 import org.eclipse.cdt.dsf.concurrent.DataRequestMonitor;
 import org.eclipse.cdt.dsf.concurrent.DsfExecutor;
 import org.eclipse.cdt.dsf.concurrent.DsfRunnable;
@@ -75,6 +76,7 @@
 import org.eclipse.cdt.dsf.gdb.IGdbDebugConstants;
 import org.eclipse.cdt.dsf.gdb.IGdbDebugPreferenceConstants;
 import org.eclipse.cdt.dsf.gdb.internal.GdbPlugin;
+import org.eclipse.cdt.dsf.gdb.launching.GDBRemoteTCPLaunchTargetProvider;
 import org.eclipse.cdt.dsf.gdb.launching.InferiorRuntimeProcess;
 import org.eclipse.cdt.dsf.gdb.service.command.IGDBControl;
 import org.eclipse.cdt.dsf.mi.service.IMICommandControl;
@@ -113,6 +115,8 @@
 import org.eclipse.debug.core.DebugPlugin;
 import org.eclipse.debug.core.ILaunch;
 import org.eclipse.debug.core.model.IProcess;
+import org.eclipse.launchbar.core.target.ILaunchTarget;
+import org.eclipse.launchbar.core.target.launch.ITargetedLaunch;
 import org.osgi.framework.BundleContext;
 
 /**
@@ -126,6 +130,7 @@ public class GDBProcesses_7_0 extends AbstractDsfService implements IGDBProcesse
 	 * Each one is shown in the debug view.
 	 */
 	private final static int MAX_NUMBER_EXITED_PROCESS = 5;
+	private final static String INVALID = "invalid"; //$NON-NLS-1$
 
 	// Below is the context hierarchy that is implemented between the
 	// MIProcesses service and the MIRunControl service for the MI
@@ -1216,13 +1221,6 @@ public void attachDebuggerToProcess(final IProcessDMContext procCtx, final Strin
 						new Step() {
 							@Override
 							public void execute(RequestMonitor rm) {
-
-								if (isInitialProcess()) {
-									// To be proper, set the initialProcess variable to false
-									// it may be necessary for a class that extends this class
-									setIsInitialProcess(false);
-								}
-
 								// There is no groupId until we attach, so we can use the default groupId
 								fContainerDmc = createContainerContext(procCtx, MIProcesses.UNIQUE_GROUP_ID);
 
@@ -1240,6 +1238,10 @@ public void execute(RequestMonitor rm) {
 						new Step() {
 							@Override
 							public void execute(RequestMonitor rm) {
+								if (fBackend.getSessionType() == SessionType.REMOTE && isInitialProcess()) {
+									connectToTarget(procCtx, rm);
+									return;
+								}
 								// For non-stop mode, we do a non-interrupting attach
 								// Bug 333284
 								boolean shouldInterrupt = true;
@@ -1262,6 +1264,7 @@ public void execute(RequestMonitor rm) {
 
 								// Store the fully formed container context so it can be returned to the caller.
 								dataRm.setData(fContainerDmc);
+								setIsInitialProcess(false);
 
 								// Initialize memory data for this process.
 								IGDBMemory memory = getServicesTracker().getService(IGDBMemory.class);
@@ -1302,6 +1305,61 @@ public Step[] getSteps() {
 		}
 	}
 
+	/**
+	 * @since 7.1
+	 */
+	protected void connectToTarget(IProcessDMContext procCtx, RequestMonitor rm) {
+		ILaunch launch = procCtx.getAdapter(ILaunch.class);
+		assert launch != null;
+		if (launch != null) {
+			Map<String, Object> attributes = new HashMap<>();
+			try {
+				attributes.putAll(launch.getLaunchConfiguration().getAttributes());
+			} catch (CoreException e) {
+				rm.done(e.getStatus());
+				return;
+			}
+
+			if (launch instanceof ITargetedLaunch) {
+				ILaunchTarget target = ((ITargetedLaunch) launch).getLaunchTarget();
+				if (target != null) {
+					attributes.putAll(target.getAttributes());
+					String tcp = target.getAttribute(IGDBLaunchConfigurationConstants.ATTR_REMOTE_TCP, ""); //$NON-NLS-1$
+					if (!tcp.isEmpty()) {
+						attributes.put(IGDBLaunchConfigurationConstants.ATTR_REMOTE_TCP, Boolean.parseBoolean(tcp));
+					} else {
+						attributes.put(IGDBLaunchConfigurationConstants.ATTR_REMOTE_TCP,
+								target.getTypeId().equals(GDBRemoteTCPLaunchTargetProvider.TYPE_ID));
+					}
+				}
+			}
+
+			boolean isTcpConnection = CDebugUtils.getAttribute(attributes,
+					IGDBLaunchConfigurationConstants.ATTR_REMOTE_TCP, false);
+
+			if (isTcpConnection) {
+				String remoteTcpHost = CDebugUtils.getAttribute(attributes, IGDBLaunchConfigurationConstants.ATTR_HOST,
+						INVALID);
+				String remoteTcpPort = CDebugUtils.getAttribute(attributes, IGDBLaunchConfigurationConstants.ATTR_PORT,
+						INVALID);
+
+				fCommandControl.queueCommand(fCommandFactory.createMITargetSelect(fCommandControl.getContext(),
+						remoteTcpHost, remoteTcpPort, true), new ImmediateDataRequestMonitor<MIInfo>(rm));
+			} else {
+				String serialDevice = CDebugUtils.getAttribute(attributes, IGDBLaunchConfigurationConstants.ATTR_DEV,
+						INVALID);
+
+				fCommandControl.queueCommand(
+						fCommandFactory.createMITargetSelect(fCommandControl.getContext(), serialDevice, true),
+						new ImmediateDataRequestMonitor<MIInfo>(rm));
+			}
+		} else {
+			rm.setStatus(new Status(IStatus.ERROR, GdbPlugin.PLUGIN_ID, INTERNAL_ERROR, "Cannot reconnect to target.", //$NON-NLS-1$
+					null));
+			rm.done();
+		}
+	}
+
 	/** @since 5.0 */
 	protected void doReverseDebugStep(final IProcessDMContext procCtx, RequestMonitor rm) {
 		// Turn on reverse debugging if it was enabled as a launch option