Community
Participate
Working Groups
Build Identifier: Build id: M20100909-0800 I noticed that DFS command handlers DsfResumeCommand DsfStepIntoCommand DsfStepOverCommand DsfStepReturnCommand DsfSuspendCommand do not allow multiple selection? Reproducible: Always Steps to Reproduce: Select mutiple nodes in the debug view and see that the Step In, Step out, Resume and Halt toolbars are disabled.
I think it should be the services that decide if multiple selection is supported instead of the commandHandlers. That way it will be up to the debugger integration to decide to implement this feature or not. I would like to see this supported for DSF-GDB.
I agree with you. As part of multi-context debugging we should introduce a component that describe some debugger functionality. Here I see different policy a debugger can have in terms of stepping/recursive containers: 1. Do not allow debug commands on multiple selection. ( current GDB, debugging single process) 2. Allow debug commands on multiple selection and use to each top parent container of the selected execution contexts. ( not sure, but probably a GDB that debugs multiple processes, correct me if I a wrong). 3. Allow debug commands on multiple selection and issue them on each selected execution context. ( Java debugger). 4. Allow debug commands on multiple selection, but instead of issuing them on each execution context create a dynamic container on the fly, add the selected execution contexts to it and issue the command to the dynamic container (TI debugger). I believe in order to accommodate these cases there should be a service component that describe the debug command policy. I don’t know if there is a need for the command policy to be dynamic. Can’t see a debugger switching it on the fly, or being a user preference either. I also think the stepping controller has to consult this component.
(In reply to comment #2) > I agree with you. > > As part of multi-context debugging we should introduce a component that > describe some debugger functionality. > > Here I see different policy a debugger can have in terms of stepping/recursive > containers: > 1. Do not allow debug commands on multiple selection. ( current GDB, > debugging single process) > 2. Allow debug commands on multiple selection and use to each top parent > container of the selected execution contexts. ( not sure, but probably a GDB > that debugs multiple processes, correct me if I a wrong). Not yet supported by DSF-GDB. Multiple selections would have to be done by sending the command to each execution context. > 3. Allow debug commands on multiple selection and issue them on each > selected execution context. ( Java debugger). I think this would be the way to do it for DSF-GDB. > 4. Allow debug commands on multiple selection, but instead of issuing them > on each execution context create a dynamic container on the fly, add the > selected execution contexts to it and issue the command to the dynamic > container (TI debugger). Yes, that is a good option as well. GDB may some day also provide such a feature. > I believe in order to accommodate these cases there should be a service > component that describe the debug command policy. > I don’t know if there is a need for the command policy to be dynamic. Can’t see > a debugger switching it on the fly, or being a user preference either. > I also think the stepping controller has to consult this component. It does sound like having a service for the 'debug command policy' would be a good thing. To be honest though, I'm not entirely sure why we couldn't hard-code the policy in the existing services? Since GDB does not support option #4, we can hard-code the use of option #3. For TI, you would override the service and change the relevant places to use option #4. It would be nice to contribute that code to open-source, but how will we be able to run it without support from GDB?
Hi Dobrin, Did you have time to look at this bug? I'm asking because it is very applicable to the multicore visualizer (bug 335027) which is coming to CDT very soon. Thanks for any info you may have.
Hi Mark, I didn’t have cycles for this. Go ahead with whatever makes sense for the visualizer. Whatever patch gets submitted in this bug I’ll make sure it fits our debugger too. Regards Dobrin
I'm going to focus on non-stop mode for this feature. The first part is to clean-up GDBRunControl_7_0_NS with some minor refactoring. I've pushed a commit to Gerrit: https://git.eclipse.org/r/#/c/5613/ As this is just a cleanup that does not change any APIs, I have committed it right away to master, but I still pushed it to Gerrit to allow for a review. I will soon post an API patch to support multiple selection which will need a review before being committed.
Created attachment 214147 [details] Prototype for resume and suspend Here is a prototype of the feature for the resume and suspend operations. It makes changes in both DSF and DSF-GDB. I'm looking for comments on the general approach. I suggest adding to DSF a new service interface called IMultiRunControl. This interface will allow passing multiple IExecutionDMContexts to the service. For example: void canResumeSome(IExecutionDMContext[], DataRequestMonitor<Boolean>); void canResumeAll(IExecutionDMContext[], DataRequestMonitor<Boolean>); void resume(IExecutionDMContext[], RequestMonitor); Because it is an interface, I thought it would be worth being a little future proof, so I added can*Some() and can*All() methods. These different set of methods would allow for a policy to decide if a multi-selection should require every element to support the operation or only one element. For example, if the user selects one running thread and one suspended thread together, canResumeAll() would return false, but canResumeSome() would return true. In the prototype, I use the can*Some() policy, which I think is more user-friendly. Also, I made isSuspended() and isStepping() asynchronous, as it will be needed for the Grouping features, as stated by Dobrin here: http://wiki.eclipse.org/CDT/MultiCoreDebugWorkingGroup/DebugElementsGrouping#Design_bits_and_pieces I also feel that we need run control events that report changes in multiple contexts but I didn't quite figure out how to implement such events yet. Stepping is also a bigger challenge because the steps go through the SteppingController, which will also need a new Multi interface or something. Comments welcome. Let me know if you'd like to see it on Gerrit...
Hi Marc, I think the interfaces are fine. Stepping will be the real challenge since as it was pointed out before, the viewer does not have the capability to make a multi-selection. Still you could consider that an edge case, and just having the ability to resume multiple contexts is a big plus.
(In reply to comment #8) > Hi Marc, > I think the interfaces are fine. Thanks Pawel. > Stepping will be the real challenge since as > it was pointed out before, the viewer does not have the capability to make a > multi-selection. Sorry, I don't remember when this was discussed. Right now, when I select multiple elements in the debug view, the DsfStepIntoCommand.canExecute() method is called with the multiple selection. Passing it to the SteppingController becomes the problem. What do you mean by the 'viewer'? > Still you could consider that an edge case, and just having the ability > to resume multiple contexts is a big plus. Absolutely, thanks for suggesting this. I will in fact start with this implementation or else I won't make it for M7.
Dobrin, I wonder if you have an opinion on the new IMultiRunControl interface?
(In reply to comment #9) > > Stepping will be the real challenge since as > > it was pointed out before, the viewer does not have the capability to make a > > multi-selection. > > Sorry, I don't remember when this was discussed. Right now, when I select > multiple elements in the debug view, the DsfStepIntoCommand.canExecute() method > is called with the multiple selection. Passing it to the SteppingController > becomes the problem. What do you mean by the 'viewer'? Sorry, I wasn't very clear. After a step or suspend event, the debugger generates a delta to cause the viewer to select the suspended context. Currently the viewer only supports a delta that makes a single selection. I.e. a delta cannot select multiple elements. This means that you can multi-select and step, but after the step is finished, your multi-selection will be reduced to a single selection and further stepping will act only on that one selected context.
(In reply to comment #11) > Sorry, I wasn't very clear. After a step or suspend event, the debugger > generates a delta to cause the viewer to select the suspended context. > Currently the viewer only supports a delta that makes a single selection. I.e. > a delta cannot select multiple elements. This means that you can multi-select > and step, but after the step is finished, your multi-selection will be reduced > to a single selection and further stepping will act only on that one selected > context. Ah yes, Mikhail had mentioned that the resulting selection after a multi-select operation would not behave nicely. Thanks for the clarification. I think that for resume/suspend, we can live with the single resulting selection.
(In reply to comment #7) > Created attachment 214147 [details] > Prototype for resume and suspend I've cleaned up the prototype. The only thing missing are JUnit tests, which I will work on now. I have pushed the current feature to Gerrit if someone wants to review: https://git.eclipse.org/r/#/c/5650 I won't commit until I have the JUnit tests. Things to note: - Non-stop only. I will post a patch for all-stop but it does not add any functional value, so I suggest we don't use it (more details on that when I post that patch) - New interface in DSF: IMultiRunControl accepting an array of IExecutionDMContexts. Run control events that report changes in multiple contexts can be added in the future without breaking the API, so I'm not going to add them now, as I don't know exactly what format they will take - Minor API extension to DsfCommandRunnable to handle multiple contexts - API extension in GDBRunControl_7_0_NS.java to support the new IMultiRunControl - I will notify the cdt-dev list about the change (new only) in API
Created attachment 214418 [details] Patch for all-stop recommended not to use Here is a patch that would add multi-select support to all-stop. However, I suggest we don't use it as it adds not value and had a limitation. In all-stop mode, when suspending one thread, all other threads will also be suspended. Therefore, being able to select more than one thread won't allow the user to achieve anything new. The same goes for resuming, where resuming a thread will cause all other threads to also resume. There is one caveat for the resume case however. GDB has a feature that allows to 'lock the scheduler' so that if one thread is resumed, the other threads are kept stopped. This is not the same thing as non-stop, because only one thread can run at a time. See http://sourceware.org/gdb/onlinedocs/gdb/All_002dStop-Mode.html for details. We don't officially support 'scheduler locking' in CDT at this time but the user can still add the command to their .gdbinit file or type it in the console. In that case, for example, the debug view will show that all threads are resumed, when in fact, only one thread is actually executing. If we ever support this feature properly (bug 339882), supporting multi-selection resume will be a problem. In that case, if the user selects two threads, and presses the resume button, which one should be resumed? The behavior of the program will be different depending on which one we choose, since only that one thread will actually be resumed. I feel the user must make that choice and therefore we will need to turn off mutli-select in that case. Because of all this, I won't add multi-selection support for all-stop, or at least until someone finds a reason do add it.
Mark I commented in the Gerrit review ( hope I did everything right there, my first time with Gerrit ). W We did the same thing in our product ( just not in DSF-GDB, timing was such we could not put it in ). Anyway, you take the approach if any can perform the operation, then go ahead and enable the operation. We took the approach, that all must support it to be enabled, to avoid sending commands which would fail down below. By what I saw in the code you would issue commands for those which do not support the operation individually, generating an error. Unless I misread the code, which is a good possibility. Both ways are valid, just depends on your style. Randy
(In reply to comment #15) > Mark > > I commented in the Gerrit review ( hope I did everything right there, my > first time with Gerrit ). W Thanks Randy for looking at this. (Looks good on Gerrit) > We did the same thing in our product ( just not in DSF-GDB, timing was > such we could not put it in ). Anyway, you take the approach if any can > perform the operation, then go ahead and enable the operation. Is there something in the interface that you feel should be changed? Does it match what you have? I'm talking about the new IMultiRunControl DSF interface. > We took the approach, that all must support it to be enabled, to avoid > sending commands which would fail down below. I hesitated for this. As you can see, the IMultiRunControl interface is prepared for both approaches (e.g., canResumeAll() or canResumeSome()). Because of the way it has to be implemented in GDB at this time, which is to send individual commands for each thread, I thought it would be more user-friendly to accept threads that would be ignored in the operation. But that was my own opinion. If others chime in and the consensus is the other method, we can change it. > By what I saw in the code > you would issue commands for those which do not support the operation > individually, generating an error. Unless I misread the code, which is > a good possibility. So, the DsfResumeCommand/DsfSuspendCommand will use all selected threads as long as one can be resumed/suspended. However, in the MultiRunControl service implementation (GDBRunControl_7_0_NS), any thread that cannot be resumed/suspended (based on canResume()/canSuspend()) will be ignored. Therefore, only commands that are expected to succeed will be sent to GDB. A further detail is that if a process is selected, along with some of its threads, the process will be resumed, and no extra action will be taken for the threads. Did you implement this for all-stop? I couldn't figure out any value in doing that. Do you have support for the stepping case? Thanks
I've written 62 new JUnit tests to cover the new IMultiRunControl service. I may have gone a little overboard :) The tests cover the entire interface for many different cases such as selecting one thread, two threads, a process, a process and a thread, a process and two threads. I have pushed a new patch set to Gerrit with contains the orignal patch plus the JUnit tests: https://git.eclipse.org/r/#/c/5650/ (patch set 2) The JUnit tests for GDB 7.4 all pass with this patch. I will commit to master from Gerrit but that does not mean it is too late for a review :)
Mark I went back and re-reviewed the second patch. Looks good. Also I see where you are testing for acceptability before actually issuing the operation. Also, we only do this for RUN/STOP operations. Multiple context stepping workflow just never seemed to make sense to us or to our customer base. Randy
(In reply to comment #18) > Mark > > I went back and re-reviewed the second patch. Looks good. Thanks! > Also, we only do this for RUN/STOP operations. That is our 'non-stop' right? > Multiple context > stepping workflow just never seemed to make sense to us or to our > customer base. That is good to know. I'll therefore close this bug and if someone does make the case for a stepping multi-select, we'll treat it as another bug. Thanks Randy.
I pushed a small fix to the JUnit tests to properly set the GDB version when starting the tests: https://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=bde03774f293e5c5de443cd32104d43542f412be
I've updated the N&N to describe this feature: http://wiki.eclipse.org/CDT/User/NewIn81#Multi-select_Resume.2FSuspend_operations And I've described it also in the FAQ: http://wiki.eclipse.org/CDT/User/FAQ#How_does_the_multi-select_Resume.2FSuspend_operations_behave.3F
*** cdt git genie on behalf of Marc Khouzam *** Bug 330974: Clean up non-stop runControl services. Change-Id: I6e1eff14072080ba2689b1af12ec4a1f45877241 [*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=0e6e83e1cbbf8ea77962bfd0803500a978933b93
*** cdt git genie on behalf of Marc Khouzam *** Bug 330974: Need to set the version of GDB for the new tests. [*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=bde03774f293e5c5de443cd32104d43542f412be
