Skip to content

Commit

Permalink
Bug 528145 - Breakpoints are not working with remote attach launch (e…
Browse files Browse the repository at this point in the history 
…clipse-cdt#336)

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.

* Revert "Bug 580259: Not all remote session have a connected process"

This reverts commit 96839a0.

* Revert "Bug 528145 - Attach debugger to a gdbserver remote session"

This reverts commit 7bddb5f.
  • Loading branch information
@umairsair
umairsair authored Apr 10, 2023
1 parent 2777c17 commit e8f17be
Show file tree
Hide file tree
Showing 12 changed files with 459 additions and 136 deletions.
1 change: 1 addition & 0 deletions .github/workflows/build-test.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ jobs:
run: |
export DISPLAY=:99
sudo Xvfb -ac :99 -screen 0 1280x1024x24 > /dev/null 2>&1 &
echo 0| sudo tee /proc/sys/kernel/yama/ptrace_scope
mvn \
clean verify -B -V \
-Dmaven.test.failure.ignore=true \
Expand Down
4 changes: 4 additions & 0 deletions NewAndNoteworthy/CDT-11.2.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

The remote connection for attach launch will be moved 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.
Expand Down
8 changes: 8 additions & 0 deletions NewAndNoteworthy/CHANGELOG-API.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -596,3 +596,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

### FinalLaunchSequence.stepRemoteConnection() and FinalLaunchSequence.stepAttachRemoteToDebugger() will be removed

These APIs will be removed and remote connection for attach launch will be moved in the implementation of `IGDBProcesses.attachDebuggerToProcess()`.

See https://github.com/eclipse-cdt/cdt/pull/336
2 changes: 1 addition & 1 deletion dsf-gdb/org.eclipse.cdt.dsf.gdb/META-INF/MANIFEST.MF
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
75 changes: 41 additions & 34 deletions ...db/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/launching/FinalLaunchSequence.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
* Anton Gorenkov - A preference to use RTTI for variable types determination (Bug 377536)
* Xavier Raynaud (Kalray) - Avoid duplicating fields in sub-classes (add protected accessors)
* Marc Khouzam (Ericsson) - Output the version of GDB at startup (Bug 455408)
* Jonathan Tousignant (NordiaSoft) - Remote session breakpoint (Bug 528145)
*******************************************************************************/
package org.eclipse.cdt.dsf.gdb.launching;

Expand All @@ -39,19 +38,18 @@
import org.eclipse.cdt.dsf.concurrent.RequestMonitorWithProgress;
import org.eclipse.cdt.dsf.datamodel.DataModelInitializedEvent;
import org.eclipse.cdt.dsf.datamodel.IDMContext;
import org.eclipse.cdt.dsf.debug.service.IProcesses.IProcessDMContext;
import org.eclipse.cdt.dsf.debug.service.ISourceLookup.ISourceLookupDMContext;
import org.eclipse.cdt.dsf.debug.service.command.ICommandControlService.ICommandControlDMContext;
import org.eclipse.cdt.dsf.gdb.IGDBLaunchConfigurationConstants;
import org.eclipse.cdt.dsf.gdb.IGdbDebugPreferenceConstants;
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;
Expand All @@ -77,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;
Expand Down Expand Up @@ -136,13 +134,13 @@ protected String[] getExecutionOrder(String group) {
//
// "stepSetSourceLookupPath", //$NON-NLS-1$

// For remote-attach launch only
// For remote-attach launch only (deprecated, see javadocs)
"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$
// For remote attach launch only
// For remote attach launch only (deprecated, see javadocs)
"stepAttachRemoteToDebugger", //$NON-NLS-1$
// Global
"stepDataModelInitializationComplete", //$NON-NLS-1$
Expand Down Expand Up @@ -178,7 +176,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$
Expand Down Expand Up @@ -552,13 +550,17 @@ protected void handleError() {
rm.done();
}

@Deprecated(forRemoval = true)
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
*
* When removing, revive/uncomment code in implementations of IGDBProcesses.attachDebuggerToProcess()
*/
@Execute
@Deprecated(forRemoval = true)
public void stepRemoteConnection(final RequestMonitor rm) {
if (fGDBBackend.getSessionType() == SessionType.REMOTE && fGDBBackend.getIsAttachSession()) {
boolean isTcpConnection = CDebugUtils.getAttribute(fAttributes,
Expand Down Expand Up @@ -593,19 +595,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() {
Expand All @@ -623,14 +615,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);
Expand All @@ -639,6 +645,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) {
// Inline following and remove requestMonitor.done() once FinalLaunchSequence.stepAttachRemoteToDebugger() is removed
// stepAttachRemoteToDebugger(requestMonitor);
requestMonitor.done();
} else {
IConnectHandler connectCommand = (IConnectHandler) fSession.getModelAdapter(IConnectHandler.class);
if (connectCommand instanceof IConnect) {
Expand All @@ -656,22 +666,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
*
* When removing, revive/uncomment code in implementations in FinalLaunchSequence.stepAttachToProcess(RequestMonitor)
*/
@Deprecated(forRemoval = true)
@Execute
public void stepAttachRemoteToDebugger(final RequestMonitor requestMonitor) {
if (fGDBBackend.getIsAttachSession() && fGDBBackend.getSessionType() == SessionType.REMOTE) {
DataRequestMonitor<Boolean> rm = new DataRequestMonitor<>(getExecutor(), null);
fProcService.canDetachDebuggerFromProcess(null, rm);

if (rm.getData()) {
IProcessDMContext processContext = fProcService.createProcessContext(fCommandControl.getContext(),
MIProcesses.UNKNOWN_PROCESS_ID);
fProcService.attachDebuggerToProcess(processContext,
new DataRequestMonitor<IDMContext>(getExecutor(), requestMonitor));
return;
}
fProcService.attachDebuggerToProcess(
fProcService.createProcessContext(fCommandControl.getContext(), MIProcesses.UNKNOWN_PROCESS_ID),
getBinary(), new DataRequestMonitor<IDMContext>(getExecutor(), requestMonitor));
} else {
requestMonitor.done();
}
requestMonitor.done();
}

/**
Expand Down
74 changes: 67 additions & 7 deletions dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBProcesses_7_0.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;

/**
Expand All @@ -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
Expand Down Expand Up @@ -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);

Expand All @@ -1240,6 +1238,12 @@ public void execute(RequestMonitor rm) {
new Step() {
@Override
public void execute(RequestMonitor rm) {
if (fBackend.getSessionType() == SessionType.REMOTE && isInitialProcess()) {
// Uncomment following and remove rm.done() once FinalLaunchSequence.stepRemoteConnection() is removed
// connectToTarget(procCtx, rm);
rm.done();
return;
}
// For non-stop mode, we do a non-interrupting attach
// Bug 333284
boolean shouldInterrupt = true;
Expand All @@ -1262,6 +1266,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);
Expand Down Expand Up @@ -1302,6 +1307,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
Expand Down
Loading

0 comments on commit e8f17be

Please sign in to comment.