Bugzilla – Full Text Bug Listing
| Summary: | [remote] Cannot start a new process on a remote target with extended-remote | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Tools] CDT | Reporter: | Marc Khouzam <marc.khouzam> | ||||||||
| Component: | cdt-debug-dsf-gdb | Assignee: | Nobody - feel free to take it <nobody> | ||||||||
| Status: | RESOLVED FIXED | QA Contact: | Marc Khouzam <marc.khouzam> | ||||||||
| Severity: | enhancement | ||||||||||
| Priority: | P3 | CC: | cdtdoug, IngedevTeam.fr, nobody, pawel.1.piech, vladimir.prus | ||||||||
| Version: | 8.0 | Flags: | nobody:
iplog-
|
||||||||
| Target Milestone: | 8.2 | ||||||||||
| Hardware: | PC | ||||||||||
| OS: | Linux | ||||||||||
| Whiteboard: | |||||||||||
| Attachments: |
|
||||||||||
|
Description
Marc Khouzam
Created attachment 194877 [details]
Small fix
In preparation for this feature, I found a small problem where we send an -exec-continue when attempting to start a new process remotely. We should use -exec-run instead.
This patch fixes this small issue. Committed to HEAD.
I don't plan to work on the rest of this feature right now, since it is too late for Indigo.
Created attachment 194878 [details]
Disable the "New..." button for remote targets
Since we don't support starting a new process on a remote target, the user should not have access to the "New..." button within the 'Connect' command.
This patch disables it until we support the feature.
Committed to HEAD.
*** cdt cvs genie on behalf of mkhouzam *** Bug 344890: Once we support starting a new process on a remote target, we should not use -exec-continue for that case, we must use -exec-run [*] StartOrRestartProcessSequence_7_0.java 1.10 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/StartOrRestartProcessSequence_7_0.java?root=Tools_Project&r1=1.9&r2=1.10 [*] GDBProcesses_7_2.java 1.15 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/GDBProcesses_7_2.java?root=Tools_Project&r1=1.14&r2=1.15 Isn't it better to simply add a new menu "New Executable" or something like that instead of combining it with "Connect"? I had been under impression that starting a new process wasn't supported even for local session until I looked at the code. I always thought the "New" button meant running a program outside of GDB and attaching to it. In any case for remote sessions we would need more complex UI under "New" - we need to specify the paths of the executable on the host and on the target. (In reply to comment #4) > Isn't it better to simply add a new menu "New Executable" or something like > that instead of combining it with "Connect"? I had been under impression that > starting a new process wasn't supported even for local session until I looked > at the code. I always thought the "New" button meant running a program outside > of GDB and attaching to it. Yeah, that UI might not be so good. I didn't have any input for the UI part, so I went with the least intrusive change I could think of. I don't mind a new UI for this, especially since you found that the current one didn't make it obvious what you could actually do. > In any case for remote sessions we would need more complex UI under "New" - we > need to specify the paths of the executable on the host and on the target. Yes, and I didn't want to go there when I was the only one deciding what that UI would look like (UI is not my strength). Now that you are looking at it, we can at least be two commenting on the UI. (In reply to comment #0) > Currently, we only support attaching to a running process, but we could allow > the user to create a new process. Note that this is supported for older GDBs > as well. I've tried it with GDB 7.0 successfully. Why the 'add inferior' step is added to DebugNewProcessSequence_7_2 instead of DebugNewProcessSequence_7_0? (In reply to comment #6) > (In reply to comment #0) > > Currently, we only support attaching to a running process, but we could allow > > the user to create a new process. Note that this is supported for older GDBs > > as well. I've tried it with GDB 7.0 successfully. > > Why the 'add inferior' step is added to DebugNewProcessSequence_7_2 instead of > DebugNewProcessSequence_7_0? -add-inferior is a new command in GDB 7.2 (In reply to comment #7) > -add-inferior is a new command in GDB 7.2 BTW, we only support multi-process on Linux starting with GDB 7.2 since that was the GDB that had the most complete foundation. We could use 7.1, but it was missing some MI commands and we'd have to use the CLI equivalent. (In reply to comment #8) > (In reply to comment #7) > > > -add-inferior is a new command in GDB 7.2 > > BTW, we only support multi-process on Linux starting with GDB 7.2 since that > was the GDB that had the most complete foundation. We could use 7.1, but it > was missing some MI commands and we'd have to use the CLI equivalent. Thanks Marc. I'll implement this functionality in *_7_2 services which I should add some new extensions for the *_7_0 services. Since gdbserver needs to be started in the daemon mode I have a couple of questions: Wouldn't it better to implement a service around gdbserver? It would make the shutdown cleaner. Also we can add wrappers for the monitor commands for gdbserver. How it would affect the tracepoint support? (In reply to comment #9) > Since gdbserver needs to be started in the daemon mode I have a couple of > questions: > Wouldn't it better to implement a service around gdbserver? It would make the > shutdown cleaner. Also we can add wrappers for the monitor commands for > gdbserver. > How it would affect the tracepoint support? I'm not sure what you have in mind so it is hard for me to have an opinion. Could you explain how you imagine such a service? I haven't experienced too much complexity around gdbserver, although what you are working on now could complicate things enough to warrant some cleanup. Hard to say from where I stand. (In reply to comment #10) > (In reply to comment #9) > > > Since gdbserver needs to be started in the daemon mode I have a couple of > > questions: > > Wouldn't it better to implement a service around gdbserver? It would make the > > shutdown cleaner. Also we can add wrappers for the monitor commands for > > gdbserver. > > How it would affect the tracepoint support? > > I'm not sure what you have in mind so it is hard for me to have an opinion. > Could you explain how you imagine such a service? > > I haven't experienced too much complexity around gdbserver, although what you > are working on now could complicate things enough to warrant some cleanup. > Hard to say from where I stand. I don't like the way how gdbserver is started in the 'RemoteGdbLaunchDelegate', but on the second thought I decided that implementing gdbserver support as a service is not a good idea. That part should be implemented as a step in the final launch sequence, so gdbserver is started after all services are initialized. I'll try to come up with a solution. The other issue that I have come across to is 'StartOrRestartProcessSequence_7_0.useContinueCommand()' the current implementation depends on 'GDBBackend.isAttachSession()'. If we have multiple processes some of them can be started in the 'run' mode while others in the 'attach' mode. I think we need a new method 'isAttachMode(IContainerDMContext)' in GDBProcesses that would return the mode for each process being debugged. What do you think? (In reply to comment #11) > I don't like the way how gdbserver is started in the 'RemoteGdbLaunchDelegate', Ah, you mean the 'automatic' remote. This one is really a separate plugin and it was not developed along with DSF-GDB, but came in much later as an adaptation from the CDI remote launch from RSE. > The other issue that I have come across to is > 'StartOrRestartProcessSequence_7_0.useContinueCommand()' the current > implementation depends on 'GDBBackend.isAttachSession()'. If we have multiple > processes some of them can be started in the 'run' mode while others in the > 'attach' mode. I think we need a new method 'isAttachMode(IContainerDMContext)' > in GDBProcesses that would return the mode for each process being debugged. > What do you think? You are right that GDBBackend does not account for multi-process. You are probably right that this stuff belongs in GDBProcesses, while GDBBackend would give info about the launch. Be aware that other parts of GDBBackend also would make sense to use for multi-process... (In reply to comment #12) > (In reply to comment #11) > > > I don't like the way how gdbserver is started in the 'RemoteGdbLaunchDelegate', > > Ah, you mean the 'automatic' remote. This one is really a separate plugin and > it was not developed along with DSF-GDB, but came in much later as an > adaptation from the CDI remote launch from RSE. > Yes, I am using the 'automatic' remote. I couldn't find the manual launcher until you pointed me to it. But the manual remote also needs adjustments. In the manual mode it is up to the user to start gdbserver either in the daemon mode (--multi) or with the executable specified. Maybe we should add a check box to the "Connection" tab to allow users to specify the mode in which gdbserver is started. We need to know it because the "--multi" mode requires "-target-select extended-remote" and an extra command at the end of session ("monitor exit") to stop gdbserver. The problem with manual vs automatic is that with RSE we can do much more, i.e. browse the executables on the target or download an executable from the host to the target. This means the "Debug New Executable" dialog should be somehow adjustable. (In reply to comment #13) > Yes, I am using the 'automatic' remote. I couldn't find the manual launcher > until you pointed me to it. > But the manual remote also needs adjustments. In the manual mode it is up to > the user to start gdbserver either in the daemon mode (--multi) or with the > executable specified. > Maybe we should add a check box to the "Connection" tab > to allow users to specify the mode in which gdbserver is started. We need to > know it because the "--multi" mode requires "-target-select extended-remote" > and an extra command at the end of session ("monitor exit") to stop gdbserver. That makes sense. But it also make me wonder if you wanted to always use --multi mode? I'm not sure if there is anything we cannot do when using --multi mode. Originally, you could only attach with --multi so we needed the other mode to debug a process from startup. We may not need this anymore. I think you could hack the launch to do that pretty easily and then see how things go without needing any UI changes. (In reply to comment #14) > That makes sense. But it also make me wonder if you wanted to always use > --multi mode? I'm not sure if there is anything we cannot do when using > --multi mode. Originally, you could only attach with --multi so we needed the > other mode to debug a process from startup. We may not need this anymore. > > I think you could hack the launch to do that pretty easily and then see how > things go without needing any UI changes. I'll try it. If it works we also need to use "extended-remote". Would that be a problem? Would the tracepoints work in this scenario? (In reply to comment #15) > I'll try it. If it works we also need to use "extended-remote". Would that be a > problem? Would the tracepoints work in this scenario? Good question. I don't remember. We'll have to try it out. But now that you mention it, I realize that some targets may have a gdbserver that only supports the "remote" protocol and not the "extended-remote" one, so we need to keep that option available for the manual launch. I think the automatic launch could use only "extended-remote" if tracepoints are ok with that. I pushed the first version of the proposed patch to Gerrit: https://git.eclipse.org/r/7438. In this version the new "Debug New Executable" command is added to the toolbar. It can be implemented as part of the "Connect" command or both commands can be removed from the toolbar and be accessible from the context menu only. The latter is my preference - users don't use these commands frequently but this is something that we need to discuss. Marc, could you please review it. (In reply to comment #17) > both commands can be removed from the toolbar > and be accessible from the context menu only. I'm leaning towards this also. But I suggest asking the cdt-dev list for opinions on this as it will remove a button that has been there for a couple of years. (In reply to comment #18) > (In reply to comment #17) > > both commands can be removed from the toolbar > > and be accessible from the context menu only. > > I'm leaning towards this also. But I suggest asking the cdt-dev list for > opinions on this as it will remove a button that has been there for a couple > of years. Thanks, will do. BTW, did you receive the Gerrit review request? I am not trying to put a pressure on you, just not sure if it went through :) Created attachment 220414 [details]
Transparent icon
Mikhail, I made the new icon without a background. What do you think?
(In reply to comment #19) > Thanks, will do. BTW, did you receive the Gerrit review request? I am not > trying to put a pressure on you, just not sure if it went through :) Looking at things now :) (In reply to comment #20) > Created attachment 220414 [details] > Transparent icon > > Mikhail, I made the new icon without a background. What do you think? Thanks. I learned yesterday how to remove the background in gimp. Will update the icon for the next patch. Mikhail, the approach to starting a new binary looks very good and I think that if we factored it out into its own patch it would be ready to go in very soon. On the other hand, the handling of the 'extended-remote' vs 'remote' case is more complicated and I think needs more discussion. Do you want to split the two improvements or do you prefer to keep the two together? In the mean time I'll post my concerns about the protocol handling. I'm having trouble with the solution with respect to the handling of remote sessions. Currently, the solution (before your changes) for remote debug goes like this: 1- The "C/C++ Attach to Application" launch offers a local aspect but also a remote one. The remote one is selected by choosing the 'gdbserver' debugger in the Debugger tab. This launch is meant to be used to connect to a remote target and let the user attach to one or more processes on the remote. The launch uses "extended-remote" and requires gdbserver to be running with --multi on the remote target. When the connection is made to the target, no process is automatically debugged; instead the user must press 'Connect' and attach to one or more processes. In the code I refer to this as "remote-attach" debug session and it satisfies the check: IGDBBackend.getSessionType() == SessionType.REMOTE && IGDBBackend.getIsAttachSession() == true 2- The "C/C++ Remote Application" launch offers an automatic delegate when RSE is used, and a manual delegate when RSE is not used. Let's focus on the manual case, since it is used by the automatic case also. I refer to this debug session as "remote-non-attach" and it satisfies the check: IGDBBackend.getSessionType() == SessionType.REMOTE && IGDBBackend.getIsAttachSession() == false This launch currently allows the user to start and debug a single process on a remote target. The GDB "remote" protocol is used and requires a gdbserver started without --multi. It also requires the user to specify the binary to debug and any arguments when starting gdbserver manually. === Currently, from the user's point-of-view, remote-attach should be used to debug processes already running on the target, while remote-non-attach should be used to debug a process from the very start, on the remote target. Thanks to your proposed improvement, a remote-attach session will be able to debug a new process from the start. So we need to decide what will be the difference between those two launches, or even if we can merge the two into a single one. I think there are still two cases that the user will probably want to launch differently: A) Connect to the target and immediately debug the process specified in the launch B) Connect to the target and don't start a process, so as to allow to attach instead We cannot do away with B) since there is no other way to handle that 'attach' scenario. On the other hand, A) could be merged into B) by requiring the user to press the "Debug new executable" button after the launch is finished; that would not be so friendly though since the user would always have to fill in the binary path in the prompt (unless we pre-fill the dialog with what was specified in the launch...). So, if we start with improving the situation as you are doing, without trying to merge the two launches, I think we should keep the two launches geared towards their current reason for being. a) 'remote-attach' should not automatically start a binary but should let the user either press 'connect' or 'debug new executable'. I don't see a reason (but I could be wrong) why this launch should allow to use the 'remote' protocol; I believe 'extended-remote' should always be used in that case, without bothering the user about it. Note that if a target uses the 'remote' protocol, then there is no need for the 'remote-attach' launch to be used, and the 'remote-non-attach' can be used instead. So, in this case, I think the UI should not be changed in the Debugger tab and the user should not be asked about '--multi'; in fact the current UI change break the scenario in question because when choosing --multi, the user is asked for a remote binary, which should not be the case here. b) 'remote-non-attach' should allow to start debugging without requiring the user to press 'connect' or 'debug new executable'. The tricky question is if we use 'extended-remote' or 'remote'. Although 'extended-remote' is better and should probably be used most of the time, I know that some targets still use 'remote' and therefore we must keep that option available. So, your new UI asking about '--multi' makes sense in this case. What are your thoughts about this? There is another aspect of the code that will need some attention, if we support both 'extended-remote' and 'remote' in the remote-non-attach launch. When implementing the multi-process support, I found out that we must handle the 'remote' case slightly differently than the 'extended-remote'. For example, in the case of 'remote' the 'file' command must be sent to GDB _before_ we connect to the target. Because of this, you will notice that for remote-attach the connection is done in FinalLaunchSequence.stepRemoteConnection() while for remote-non-attach, the connection is made in DebugNewProcessSequence.stepRemoteConnection(). Up-to-now the code assumed that a remote-attach session was using 'extended-remote' and a remote-non-attach was using 'remote'. Therefore, I could use IGDBBackend.getIsAttachSession() to know which protocol was being used. If we support both 'extended-remote' and 'remote' in the remote-non-attach launch, this assumption is wrong and we will need a way to know which protocol is used. We'll need something new like IGDBBackend.isExtendedRemote() or at least something providing us with this information. You could of course decide to simply focus on the "Debug new executable" command and dialog, and only use it for the remote-attach case. If so, you won't need to change any of the launch stuff nor it's UI. This entirely depends on how much extra value you feel there is in using 'extended-remote' for the remote-non-attach launch. Marc, thanks for the comments. I don't have answers right now and need time to digest it. (In reply to comment #26) > Marc, thanks for the comments. I don't have answers right now and need time > to digest it. No problem. I am continuing the Gerrit review and I will post specific comments on most things. I may ignore a file or two until we resolve the questions I posted on this bug, but at least it will help not slow you down as much. Marc, I have a quick comment about the proposed 'IGDBBackend.isExtendedRemote()' method. Right now, documentation for IGDBBackend say it's managing the GDB process. It would seem that 'is this extended remote' property is a bit separate from how we run and communicate with GDB. In fact, some existing methods, like IGDBBackend.getProgramPath() also have more to do with target than with management of GDB process. Can we move towards a design where properties of target are separated in a different interface? (In reply to comment #28) > Marc, > > I have a quick comment about the proposed 'IGDBBackend.isExtendedRemote()' > method. Right now, documentation for IGDBBackend say it's managing the GDB > process. It would seem that 'is this extended remote' property is a bit > separate from how we run and communicate with GDB. In fact, some existing > methods, like > IGDBBackend.getProgramPath() also have more to do with target than with > management of GDB process. > > Can we move towards a design where properties of target are separated in a > different interface? Yes, that would be better. I just wrote IGDBBackend.isExtendedRemote() to explain what we needed, but it does not have to be that specifically. In fact, I saw that in Mikhails patch he reads this value from the launch attributes. We can keep doing that if it satisfies all the cases we need it for. (In reply to comment #29) > I just wrote IGDBBackend.isExtendedRemote() to explain what we needed, but > it does not have to be that specifically. In fact, I saw that in Mikhails > patch he reads this value from the launch attributes. We can keep doing > that if it satisfies all the cases we need it for. Initially I added a new extension to IGDBBackend interface to provide access to some target and gdbserver properties. But after Vladimir pointed me to the java-doc for IGDBBackend I decided to use launch configuration instead, at least for now. I need to debug on a remote target using gdbserver in multi mode. Can I use DSF for such a task ? We already use CDT (GDB-MI command) to debug with gdb in multi mode but we are very interested in moving to DSF. Please let us know if it is possible, or when it is planned. (In reply to comment #31) > I need to debug on a remote target using gdbserver in multi mode. Can I use > DSF for such a task ? > > We already use CDT (GDB-MI command) to debug with gdb in multi mode but we > are very interested in moving to DSF. Please let us know if it is possible, > or when it is planned. Sure: http://wiki.eclipse.org/CDT/User/FAQ#How_do_I_debug_a_remote_application.3F and look at the Remote Attach Launcher part. The missing aspect(which is not part of CDI either) is starting (not attaching to) a new process on the remote target. This is what this bug is about. (In reply to comment #24) > b) 'remote-non-attach' should allow to start debugging without requiring the > user to press 'connect' or 'debug new executable'. The tricky question is > if we use 'extended-remote' or 'remote'. Although 'extended-remote' is > better and should probably be used most of the time, I know that some > targets still use 'remote' and therefore we must keep that option available. > So, your new UI asking about '--multi' makes sense in this case. > > What are your thoughts about this? What if I just add the UI for '--multi' only for 'remote-non-attach'. That would answer your concerns from https://bugs.eclipse.org/bugs/show_bug.cgi?id=344890#c25. I have modified the patch according to your comments except for the UI part and if you agree with my proposal I will submit a new version of the patch by the end of the week. (In reply to comment #33) > (In reply to comment #24) > > b) 'remote-non-attach' should allow to start debugging without requiring the > > user to press 'connect' or 'debug new executable'. The tricky question is > > if we use 'extended-remote' or 'remote'. Although 'extended-remote' is > > better and should probably be used most of the time, I know that some > > targets still use 'remote' and therefore we must keep that option available. > > So, your new UI asking about '--multi' makes sense in this case. > > > > What are your thoughts about this? > > What if I just add the UI for '--multi' only for 'remote-non-attach'. It sounds like a good way to go, but I'd like to play around with it to be more confident. I'm concerned about adding UI complexity if we are not really sure it is needed. Can you describe when the user would benefit from 'remote-non-attach' with '--multi'? > That would answer your concerns from comment 25. Does it? We will still have to deal with a case that could be using the remote or extended-remote protocol. > I have modified > the patch according to your comments except for the UI part and if you agree > with my proposal I will submit a new version of the patch by the end of the > week. I agree with not changing the UI for the remote-attach case ;) That is easy to agree with. Having the new UI for the remote-non-attach case, could be valuable, but I'm not very sure yet. I'd like to try it out if possible. (In reply to comment #34) > (In reply to comment #33) > > (In reply to comment #24) > > > b) 'remote-non-attach' should allow to start debugging without requiring the > > > user to press 'connect' or 'debug new executable'. The tricky question is > > > if we use 'extended-remote' or 'remote'. Although 'extended-remote' is > > > better and should probably be used most of the time, I know that some > > > targets still use 'remote' and therefore we must keep that option available. > > > So, your new UI asking about '--multi' makes sense in this case. > > > > > > What are your thoughts about this? > > > > What if I just add the UI for '--multi' only for 'remote-non-attach'. > > It sounds like a good way to go, but I'd like to play around with it to be > more confident. I'm concerned about adding UI complexity if we are not > really sure it is needed. > > Can you describe when the user would benefit from 'remote-non-attach' with > '--multi'? > The ability to start gdbserver with '--multi' would allow users to debug multiple processes, right? Now it is not possible for this launch configuration mode. > > That would answer your concerns from comment 25. > > Does it? We will still have to deal with a case that could be using the > remote or extended-remote protocol. > I assume we can use 'extended-remote' if gdbserver is started in '--multi' mode and 'remote' otherwise. Another options is to specify the protocol in the UI along with '--multi' but it would make the UI even more complicated. > > I have modified > > the patch according to your comments except for the UI part and if you agree > > with my proposal I will submit a new version of the patch by the end of the > > week. > > I agree with not changing the UI for the remote-attach case ;) That is easy > to agree with. Having the new UI for the remote-non-attach case, could be > valuable, but I'm not very sure yet. I'd like to try it out if possible. I can submit a new patch. (In reply to comment #35) > > Can you describe when the user would benefit from 'remote-non-attach' with > > '--multi'? > > > > The ability to start gdbserver with '--multi' would allow users to debug > multiple processes, right? Now it is not possible for this launch > configuration mode. So, the value would be to have the automatic launch support multi-process? Yes, that makes sense. However, after reading the GDB doc and trying out a couple of test with --multi/remote/extended-remote, I get the impression the situation is different than we originally thought. It seems that multi-process support is triggered by using 'extended-remote'. It is not related to --multi. Furthermore, gdbserver automatically exiting is already related to 'remote' vs 'extended-remote' and not to --multi. This made me wonder why we don't always use --multi? We need to allow the user to choose between 'remote' and 'extended-remote' because some target only support 'remote', so maybe that should be the option in the UI? But then we use --multi for both (in the automatic case, while the manual case will leave it up to the user)? > > > That would answer your concerns from comment 25. > > > > Does it? We will still have to deal with a case that could be using the > > remote or extended-remote protocol. > > > > I assume we can use 'extended-remote' if gdbserver is started in '--multi' > mode and 'remote' otherwise. Yes, but knowing if it was started with --multi will be done differently that how we do it now. But it is still possible, as you have shown. > Another options is to specify the protocol in the UI along with '--multi' > but it would make the UI even more complicated. Maybe the protocol is the option we should show, instead of --multi. See above. > > > I have modified > > > the patch according to your comments except for the UI part and if you agree > > > with my proposal I will submit a new version of the patch by the end of the > > > week. > > > > I agree with not changing the UI for the remote-attach case ;) That is easy > > to agree with. Having the new UI for the remote-non-attach case, could be > > valuable, but I'm not very sure yet. I'd like to try it out if possible. > > I can submit a new patch. Thanks, that will prove useful. Marc, I decided to support only 'remote-attach', at least for now. The launch UI for 'remote-non-attach' was getting too complicated, let's just get the first part checked in first. Before I pushed the new patch I need to clarify the following comments in GDBProcesses_7_2.doIsDebuggerAttachSupported(): // Multi-process does not work for all-stop right now // NOTE: when we support multi-process in all-stop mode, // we will need to interrupt the target to when doing the attach. Adding a new executable seems to be working in the 'all-stop' mode. What exactly is the problem? (In reply to comment #37) > Marc, I decided to support only 'remote-attach', at least for now. The > launch UI for 'remote-non-attach' was getting too complicated, let's just > get the first part checked in first. > Before I pushed the new patch I need to clarify the following comments in > GDBProcesses_7_2.doIsDebuggerAttachSupported(): > > // Multi-process does not work for all-stop right now > > // NOTE: when we support multi-process in all-stop mode, > // we will need to interrupt the target to when doing the attach. > > Adding a new executable seems to be working in the 'all-stop' mode. What > exactly is the problem? The problem is not in creating the process but in running it. Since we don't use GDB's target-async, when an inferior is running, we cannot interact with GDB. Therefore, if you have two processes and you resume one of them, you cannot talk to GDB anymore to control the second process. If you interrupt the running process, you can then control either one. That meant you could only have a single process running at a time. I asked about this on the GDB mailing list. See: http://sourceware.org/ml/gdb/2011-04/msg00125.html I pushed a new patch to Gerrit (https://git.eclipse.org/r/#/c/9688/). It's a new review because I forgot to add the change id to the patch. The patch adds a new command "Debug New Executable" to the context menu of the Debug view. I commented the part of plugin.xml that adds this command to the view's toolbar. The command is enabled for 'local' and 'remote-attach' sessions only. I didn't know what to do with the "New" button of the "Attach to process" dialog. Simple solution is to remove it and use the "Debug New Executable" command instead. Another solution is to replace the dialog it opens with the dialog from "Debug New Executable", the latter has extra fields as 'Arguments' and 'Binary on target'. Marc, please take a look. (In reply to comment #39) > I pushed a new patch to Gerrit (https://git.eclipse.org/r/#/c/9688/). It's a > new review because I forgot to add the change id to the patch. What I recommend is to amend your commit to add the changeId, and push again to Gerrit, to update the original review. If that works and the review is restored from the abandoned state, then you can abandon the new review. Since the comments and previous commit are in the old review, it would be easier to continue with that one. > The patch adds a new command "Debug New Executable" to the context menu of > the Debug view. I commented the part of plugin.xml that adds this command to > the view's toolbar. Are you no longer aiming to add it to the toolbar (or was it me that didn't like it? :)) > The command is enabled for 'local' and 'remote-attach' > sessions only. > I didn't know what to do with the "New" button of the "Attach to process" > dialog. Simple solution is to remove it and use the "Debug New Executable" > command instead. That was my first thought. > Another solution is to replace the dialog it opens with the > dialog from "Debug New Executable", the latter has extra fields as > 'Arguments' and 'Binary on target'. > Marc, please take a look. I think that is better. At least for the next release. I've seen other situations that did the same, which is to keep an older UI along with the new one for one release, and then removing it. What do you think? (In reply to comment #40) > (In reply to comment #39) > > I pushed a new patch to Gerrit (https://git.eclipse.org/r/#/c/9688/). It's a > > new review because I forgot to add the change id to the patch. > > What I recommend is to amend your commit to add the changeId, and push again > to Gerrit, to update the original review. If that works and the review is > restored from the abandoned state, then you can abandon the new review. > Since the comments and previous commit are in the old review, it would be > easier to continue with that one. > It doesn't seem to work - keep getting "rejected non-fast-forward". Moreover, it looks like I have broken the second review. Should I create a new review? > > > The patch adds a new command "Debug New Executable" to the context menu of > > the Debug view. I commented the part of plugin.xml that adds this command to > > the view's toolbar. > > Are you no longer aiming to add it to the toolbar (or was it me that didn't > like it? :)) > It was you :) I prefer removing the "Connect" from the toolbar as well. > > The command is enabled for 'local' and 'remote-attach' > > sessions only. > > I didn't know what to do with the "New" button of the "Attach to process" > > dialog. Simple solution is to remove it and use the "Debug New Executable" > > command instead. > > That was my first thought. > > > Another solution is to replace the dialog it opens with the > > dialog from "Debug New Executable", the latter has extra fields as > > 'Arguments' and 'Binary on target'. > > Marc, please take a look. > > I think that is better. At least for the next release. I've seen other > situations that did the same, which is to keep an older UI along with the > new one for one release, and then removing it. What do you think? I'll update the patch and push it when we figure out what to do with Gerrit. (In reply to comment #41) > (In reply to comment #40) > > (In reply to comment #39) > > > I pushed a new patch to Gerrit (https://git.eclipse.org/r/#/c/9688/). It's a > > > new review because I forgot to add the change id to the patch. > > > > What I recommend is to amend your commit to add the changeId, and push again > > to Gerrit, to update the original review. If that works and the review is > > restored from the abandoned state, then you can abandon the new review. > > Since the comments and previous commit are in the old review, it would be > > easier to continue with that one. > > > > It doesn't seem to work - keep getting "rejected non-fast-forward". You press the "restore review" button and put a restore comment. But when do you get this 'rejected' error? > Moreover, it looks like I have broken the second review. Should I create a > new review? If we can't restore, then yes :) (In reply to comment #42) > (In reply to comment #41) > > (In reply to comment #40) > > > (In reply to comment #39) > > > > I pushed a new patch to Gerrit (https://git.eclipse.org/r/#/c/9688/). It's a > > > > new review because I forgot to add the change id to the patch. > > > > > > What I recommend is to amend your commit to add the changeId, and push again > > > to Gerrit, to update the original review. If that works and the review is > > > restored from the abandoned state, then you can abandon the new review. > > > Since the comments and previous commit are in the old review, it would be > > > easier to continue with that one. > > > > > > > It doesn't seem to work - keep getting "rejected non-fast-forward". > > You press the "restore review" button and put a restore comment. But when > do you get this 'rejected' error? > Sorry, I wasn't clear. The abandon/restore for a change in Gerrit works fine. The problem is that my old git repository was broken when my computer crashed. I recreated the patch and amended its change id but I am getting 'rejected' error when trying to push the patch to Gerrit. I don't think it is fixable. > > Moreover, it looks like I have broken the second review. Should I create a > > new review? > > If we can't restore, then yes :) I'll go through your comments in the old review and copy the relevant ones to the new review. (In reply to comment #43) > Sorry, I wasn't clear. The abandon/restore for a change in Gerrit works > fine. The problem is that my old git repository was broken when my computer > crashed. I recreated the patch and amended its change id but I am getting > 'rejected' error when trying to push the patch to Gerrit. I don't think it > is fixable. Can you restore the review again? I can then try to push your change for you and see if that works. (In reply to comment #44) > Can you restore the review again? I can then try to push your change for > you and see if that works. I have restored the old review (https://git.eclipse.org/r/#/c/7438/) and abandoned the newer one (https://git.eclipse.org/r/#/c/9688/). (In reply to comment #45) > (In reply to comment #44) > > Can you restore the review again? I can then try to push your change for > > you and see if that works. > > I have restored the old review (https://git.eclipse.org/r/#/c/7438/) and > abandoned the newer one (https://git.eclipse.org/r/#/c/9688/). I've pushed the patch from 9688 to 7438. You can fetch that patch into your Git repo and I'm hoping things will work again. (In reply to comment #46) > I've pushed the patch from 9688 to 7438. You can fetch that patch into your > Git repo and I'm hoping things will work again. Thanks. How did you do that? By applying the patch manually to the 7438 branch that you fetched from Gerrit and then push it back? (In reply to comment #47) > (In reply to comment #46) > > I've pushed the patch from 9688 to 7438. You can fetch that patch into your > > Git repo and I'm hoping things will work again. > > Thanks. How did you do that? By applying the patch manually to the 7438 > branch that you fetched from Gerrit and then push it back? I fetched the code from new review and then amended the commit and changed the ChangeId to the one from the old review (which can be found as the top line on the left-hand side in the Gerrit review webpage). Then I pushed to Gerrit. Since the changeId was the one of the old review, it pushed the code to that one. (In reply to comment #48) > I fetched the code from new review and then amended the commit and changed > the ChangeId to the one from the old review (which can be found as the top > line on the left-hand side in the Gerrit review webpage). Then I pushed to > Gerrit. Since the changeId was the one of the old review, it pushed the > code to that one. Oh, and I snuck in a rebase before amending the commit. (In reply to comment #46) > I fetched the code from new review and then amended the commit and changed > the ChangeId to the one from the old review (which can be found as the top > line on the left-hand side in the Gerrit review webpage). Then I pushed to > Gerrit. Since the changeId was the one of the old review, it pushed the > code to that one. I am stuck. Can't push any of new changes back to Gerrit. The change id seems to be set correctly but it doesn't seem to find the right review and tries to create a new branch. I don't know where this new branch is getting created. (In reply to comment #50) > (In reply to comment #46) > > I fetched the code from new review and then amended the commit and changed > > the ChangeId to the one from the old review (which can be found as the top > > line on the left-hand side in the Gerrit review webpage). Then I pushed to > > Gerrit. Since the changeId was the one of the old review, it pushed the > > code to that one. > > I am stuck. Can't push any of new changes back to Gerrit. The change id > seems to be set correctly but it doesn't seem to find the right review and > tries to create a new branch. I don't know where this new branch is getting > created. Can you copy/paste the exact text of the commit you are pushing? (In reply to comment #51) > > I am stuck. Can't push any of new changes back to Gerrit. The change id > > seems to be set correctly but it doesn't seem to find the right review and > > tries to create a new branch. I don't know where this new branch is getting > > created. > > Can you copy/paste the exact text of the commit you are pushing? Until we figure this out, you could attach the patch to the bug and I'll push it for you. At commit time, you can still do the commit from Gerrit in your name. BTW, if someone else runs into this, with platform 4.3M4, the context menu buttons are no longer visible (Connect, Start/Stop tracing, and the from this feature: Debug new executable). I tried with the most recent I-build, I20130115, and the buttons have returned as expected. Nice work Mikhail. It think it is a nice addition that blends in very cleanly. And I think it is great that you added the handling of the arguments! This was missing for the local case also, so great stuff. I put a couple of minor comments in Gerrit. One thing I wanted to discuss with you is if we should support "Debug new executable" for earlier GDB's than 7.2. Without multi-process support, we could still allow the user to connect to the remote target and start a single process instead of attaching. I tried it with GDB 7.1 and it looks like it works. I think the changes to your patch would be minor, but I wonder if we should spent the time for such old GDB's... Oh, and I think it is fine to leave the "Debug new executable" in the context menu of the view and not in the toolbar. I'm still hesitant to remove the connect button from the toolbar. Did we ever ask on the mailing list? (In reply to comment #54) > Nice work Mikhail. It think it is a nice addition that blends in very > cleanly. And I think it is great that you added the handling of the > arguments! This was missing for the local case also, so great stuff. > > I put a couple of minor comments in Gerrit. > I modified the patch according your comments and replaced the old dialog for the "New" button with the new one (Patch Set #5). > One thing I wanted to discuss with you is if we should support "Debug new > executable" for earlier GDB's than 7.2. Without multi-process support, we > could still allow the user to connect to the remote target and start a > single process instead of attaching. I tried it with GDB 7.1 and it looks > like it works. I think the changes to your patch would be minor, but I > wonder if we should spent the time for such old GDB's... > So this is only for "remote-attach" sessions, right? > Oh, and I think it is fine to leave the "Debug new executable" in the > context menu of the view and not in the toolbar. I'm still hesitant to > remove the connect button from the toolbar. Did we ever ask on the mailing > list? I think we did, but can be wrong. BTW, we still need to address the enablement issue for both commands. Should we create a separate bug for it? (In reply to comment #55) > I modified the patch according your comments and replaced the old dialog for > the "New" button with the new one (Patch Set #5). Nice. I like how you hooked the "New..." button with the new dialog. We can talk about removing that button in the next release, once you new command is out there for a while. > > One thing I wanted to discuss with you is if we should support "Debug new > > executable" for earlier GDB's than 7.2. Without multi-process support, we > > could still allow the user to connect to the remote target and start a > > single process instead of attaching. I tried it with GDB 7.1 and it looks > > like it works. I think the changes to your patch would be minor, but I > > wonder if we should spent the time for such old GDB's... > > > > So this is only for "remote-attach" sessions, right? Yes, that's right. > > Oh, and I think it is fine to leave the "Debug new executable" in the > > context menu of the view and not in the toolbar. I'm still hesitant to > > remove the connect button from the toolbar. Did we ever ask on the mailing > > list? > > I think we did, but can be wrong. After thinking about it, I feel that removing 'Connect' from the toolbar will force people to search the context menu, which will help them notice your new "Debug New Executable" command. So, although I'm hesitant, I'm ok with it. If you prefer to remove it, can you just mention it on the list? > BTW, we still need to address the > enablement issue for both commands. Should we create a separate bug for it? Yes please. (In reply to comment #56) > > > One thing I wanted to discuss with you is if we should support "Debug new > > > executable" for earlier GDB's than 7.2. Without multi-process support, we > > > could still allow the user to connect to the remote target and start a > > > single process instead of attaching. I tried it with GDB 7.1 and it looks > > > like it works. I think the changes to your patch would be minor, but I > > > wonder if we should spent the time for such old GDB's... > > > > > > > So this is only for "remote-attach" sessions, right? > > Yes, that's right. > What is the lowest version of GDB for this feature? 7.1 or 7.0? > > > Oh, and I think it is fine to leave the "Debug new executable" in the > > > context menu of the view and not in the toolbar. I'm still hesitant to > > > remove the connect button from the toolbar. Did we ever ask on the mailing > > > list? > > > > I think we did, but can be wrong. > > After thinking about it, I feel that removing 'Connect' from the toolbar > will force people to search the context menu, which will help them notice > your new "Debug New Executable" command. So, although I'm hesitant, I'm ok > with it. If you prefer to remove it, can you just mention it on the list? > I am fine with leaving 'Connect' in the toolbar, at least for now. There is https://bugs.eclipse.org/bugs/show_bug.cgi?id=376283. > > BTW, we still need to address the > > enablement issue for both commands. Should we create a separate bug for it? > > Yes please. Done, see https://bugs.eclipse.org/bugs/show_bug.cgi?id=398913. (In reply to comment #57) > (In reply to comment #56) > > > > One thing I wanted to discuss with you is if we should support "Debug new > > > > executable" for earlier GDB's than 7.2. Without multi-process support, we > > > > could still allow the user to connect to the remote target and start a > > > > single process instead of attaching. I tried it with GDB 7.1 and it looks > > > > like it works. I think the changes to your patch would be minor, but I > > > > wonder if we should spent the time for such old GDB's... > > > > > > > > > > So this is only for "remote-attach" sessions, right? > > > > Yes, that's right. > > > > What is the lowest version of GDB for this feature? 7.1 or 7.0? It seems that --multi was added with GDB 6.8. > Done, see https://bugs.eclipse.org/bugs/show_bug.cgi?id=398913. Thanks I checked in the patch. In regard to supporting the earlier versions I am not sure it is worth to spend time on it now. Let's leave this bug open and see if we can do it in the future. (In reply to comment #59) > I checked in the patch. > In regard to supporting the earlier versions I am not sure it is worth to > spend time on it now. Let's leave this bug open and see if we can do it in > the future. I've looked into supporting earlier version of GDB and it is more complicated that I thought. Things work well for the first process, but if we kill/detach the process and then start a new one, things don't work anymore. The problem is that before GDB 7.2, the groupId which characterizes the containerDMC is the pid of the process. However, we don't know the pid before we start the process. So, what happens is that the MIBreakpointsManager gets confused because we try to set breakpoints for a process that has and empty pid, but that process seems to already have breakpoints due to the previous process. This is a problem I ran into for multi-process and I couldn't come up with a clean solution. I then decided to forget about making this work for older GDBs. Therefore, I suggest we forget about older GDBs in this case as well. Mikhail, if you agree, how about we resolve the bug? Could you add a note in the N&N about this nice improvement? Thanks! (In reply to comment #60) > Therefore, I suggest we forget about older GDBs in this case as well. > > Mikhail, if you agree, how about we resolve the bug? > > Could you add a note in the N&N about this nice improvement? > > Thanks! Thank you. Will do. (In reply to comment #62) > N&N: > http://wiki.eclipse.org/CDT/User/ > NewIn82#Debugging_multiple_processes_within_one_debug_session Thanks! |