| Summary: | [multicore][multi-process] Support for Multi-Process debugging | ||
|---|---|---|---|
| Product: | [Tools] CDT | Reporter: | Marc Khouzam <marc.khouzam> |
| Component: | cdt-debug-dsf-gdb | Assignee: | Marc Khouzam <marc.khouzam> |
| Status: | RESOLVED FIXED | QA Contact: | Marc Khouzam <marc.khouzam> |
| Severity: | enhancement | ||
| Priority: | P3 | CC: | andrew.p.keen, dalexiev, dd.general-inbox, eclipse.sprigogin, eclipse, eostroukhov, malaperle, onurakdemir1, pawel.1.piech |
| Version: | 0 DD 1.1 | Keywords: | plan |
| Target Milestone: | 8.0 | ||
| Hardware: | PC | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Bug Depends on: | 227183, 237308, 239028, 240044, 240507, 241317, 241423, 315796, 335324, 335528 | ||
| Bug Blocks: | |||
| Attachments: | |||
|
Description
Marc Khouzam
I guess there are still some things missing in GDB, which keeps us from completing this feature. (In reply to comment #1) > I guess there are still some things missing in GDB, which keeps us from > completing this feature. Gdb 7.0 will have not only multi-process debugging but multi-exec debugging also. Because of this, there is a good chance that it might affect our support for multi-process. Let's leave this open until GDB 7.0 is out. The needed support for Linux was only added to GDB 7.1. However there are new commands we need to use, so there are some changes needed in DSF-GDB. Created attachment 166478 [details]
Multi-exec support for DSF
(In reply to comment #4) > Created an attachment (id=166478) [details] > Multi-exec support for DSF Thanks Onur for working on this. Just to be clear, this is to support multi-exec on Linux. The patch is a great start. - GDB_7_1_VERSION should be "7.1" instead of "7.1.00" (although the comparison may still work, but I wasn't sure, so let's be safe :-)) - CommandFactory.java, please don't change the imports lists. It is true the list is much smaller using the * format, but the list is generated aautomatically by eclipse using Ctrl-Shft-o, so I'd rather leave it to whatever eclipse puts in there. - I wonder if we can use the IProcesses#isDebugNewProcessSupported and IProcesses#debugNewProcess or isRunNewProcessSupported and runNewProcess for this scenario? Instead of adding a method GDBProcesses_7_1#addInferior? I guess the next step is to update the GdbConnectCommand class. Created attachment 167440 [details]
GDBProcesses_7_1
With this patch , DSF can add new inferior( a Null inferior) . I can see new inferiors in Debug View , but I can not attach each one indiviually. I think all seem same inferior ( address space ) in the background of DSF.
Created attachment 168092 [details]
Multi process Debug DSF
I have implemented add-inferior , info inferior and inferior change commands.
Now I suppose we need to change something in background. Problem seems UI LaunchVMprovider.java has only one containerVMNode , we need one containerNode per inferior(address space ) .
I couldn't test real behaviour of my patch , because I can just attach only one process although I have listed new added processes . All listed processes seem same to the backend.
Created attachment 168093 [details]
Multi exec in action.
I have added add-inferior UI action. I think it is not complete , so I haven't added this code in my patch.
(In reply to comment #7) > Created an attachment (id=168092) [details] > Multi process Debug DSF > > I have implemented add-inferior , info inferior and inferior change commands. > Now I suppose we need to change something in background. Problem seems UI > LaunchVMprovider.java has only one containerVMNode , we need one containerNode > per inferior(address space ) . ContainerVMNode should already handle multi-process. > I couldn't test real behaviour of my patch , because I can just attach only one > process although I have listed new added processes . All listed processes seem > same to the backend. I'll give this patch a try From te cdt dev-list From: cdt-dev-bounces@eclipse.org [mailto:cdt-dev-bounces@eclipse.org] On Behalf Of onur akdemir Sent: Wednesday, May 12, 2010 3:31 AM To: cdt-dev@eclipse.org Subject: [cdt-dev] Multi process debugging Hi , I am working on B237306 bug ( multi process debugging support for linux - remote ) . Gdb 7.1 now fully supports address spaces and multi inferior debugging. what I do is 1) Add-inferior command : I have added CLIAddInferior command and called ContainerStartedDMEvent , Now I can see new processes listed in Debug View. But in the background , I suppose I could not add a new Inferior that I can work on. gdb[0].proc[1].threadGroup[1] I see this identification with new added processes ( all same ). I think this must be left unchanged , Additionally we need a Inferior List in backend. Just creating a new context for new added inferiors not enough. Can you help me about this issue? Thanks. Onur. Tubitak UEKAE-BTE (In reply to comment #10) > 1) Add-inferior command : I have added CLIAddInferior command and called > ContainerStartedDMEvent , Now I can see new processes listed in Debug View. > > But in the background , I suppose I could not add a new Inferior that I can > work on. > > gdb[0].proc[1].threadGroup[1] I see this identification with new added > processes ( all same ). If every entry has the same identification it is probably because the ContainerStartedDMEvent re-uses the original containerDMContext by mistake. A new context must be created, using the groupId that GDB returns. > I think this must be left unchanged , Additionally we need a Inferior List in > backend. Just creating a new context for new added inferiors not enough. I don't understand this. (In reply to comment #11) > (In reply to comment #10) > > > 1) Add-inferior command : I have added CLIAddInferior command and called > > ContainerStartedDMEvent , Now I can see new processes listed in Debug View. > > > > But in the background , I suppose I could not add a new Inferior that I can > > work on. > > > > gdb[0].proc[1].threadGroup[1] I see this identification with new added > > processes ( all same ). > > If every entry has the same identification it is probably because the > ContainerStartedDMEvent re-uses the original containerDMContext by mistake. A > new context must be created, using the groupId that GDB returns. > > > I think this must be left unchanged , Additionally we need a Inferior List in > > backend. Just creating a new context for new added inferiors not enough. > > I don't understand this. Thanks , I see the problem . Also I need to switch between Containers. I have CLISelectActiveInferior command. How can I call this command when selection changes. Can you point me any example. I see some hook methods in Debug.UI , But there is no hook method for selection change. (In reply to comment #13) > Also I need to switch between Containers. I have CLISelectActiveInferior > command. > How can I call this command when selection changes. Can you point me any > example. > I see some hook methods in Debug.UI , But there is no hook method for selection > change. Currently when sending MI commands we use the --tread and --frame MI options. Is there a new one for --threadGroup or something like that in GDB? If not, then you will need to mimic what we do with MIThreadSelect (In reply to comment #14) > (In reply to comment #13) > > Also I need to switch between Containers. I have CLISelectActiveInferior > > command. > > How can I call this command when selection changes. Can you point me any > > example. > > I see some hook methods in Debug.UI , But there is no hook method for selection > > change. > > Currently when sending MI commands we use the --tread and --frame MI options. > Is there a new one for --threadGroup or something like that in GDB? > > If not, then you will need to mimic what we do with MIThreadSelect I meant UI . When I click one of process I need to call CLISelectActiveInferior command ( which uses "inferior infno" command - a new one) I dont know how to call this command when selectionchanges In Debug View UI tree. (In reply to comment #15) > (In reply to comment #14) > > (In reply to comment #13) > > > Also I need to switch between Containers. I have CLISelectActiveInferior > > > command. > > > How can I call this command when selection changes. Can you point me any > > > example. > > > I see some hook methods in Debug.UI , But there is no hook method for selection > > > change. > > > > Currently when sending MI commands we use the --tread and --frame MI options. > > Is there a new one for --threadGroup or something like that in GDB? > > > > If not, then you will need to mimic what we do with MIThreadSelect > > I meant UI . When I click one of process I need to call CLISelectActiveInferior > command ( which uses "inferior infno" command - a new one) > I dont know how to call this command when selectionchanges In Debug View UI > tree. I don't think you need to do anything in the UI. The views will take care of making the calls they need to the services and will include the currently selected debug context (IContainerDMContext in this case) However, when we want to perform an operation in GDB with a new inferior selected, then we need to use CLISelectActiveInferior. This can be done in AbstractMIControl like for MIThreadSelect gdb[0].proc[null].threadGroup[1] proc[] identifies process being debugged. threadGroup is a thread group in this process. Now the problem is how we identify inferiors. As I understand gdb[0] is session ID and all inferiors will work in this session. gbd[0].inferior[0].proc[null].threadGroup[null] Do we need smt like that? Or how can we handle inferior identification. (In reply to comment #17) > gdb[0].proc[null].threadGroup[1] > > proc[] identifies process being debugged. > threadGroup is a thread group in this process. the id for the proc[null] should be the pid. however, I'm not sure it will be easy to find from GDB. in that case you can probably use the same id as threadGroup. There should not be a 'null' in the id. > Now the problem is how we identify inferiors. As I understand gdb[0] is session > ID and all inferiors will work in this session. yes > gbd[0].inferior[0].proc[null].threadGroup[null] > Do we need smt like that? Or how can we handle inferior identification. both proc and threadGroup should have a valid id. threadGroup is the groupId that GDB gives us from -list-thread-groups (In reply to comment #18) > (In reply to comment #17) > > gdb[0].proc[null].threadGroup[1] > > > > proc[] identifies process being debugged. > > threadGroup is a thread group in this process. > > the id for the proc[null] should be the pid. however, I'm not sure it will be > easy to find from GDB. in that case you can probably use the same id as > threadGroup. > > There should not be a 'null' in the id. > > When I run add-inferior command I have an empty inferior waiting to be attached.So proc id null , no id yet. > > Now the problem is how we identify inferiors. As I understand gdb[0] is session > > ID and all inferiors will work in this session. > > yes > > > gbd[0].inferior[0].proc[null].threadGroup[null] > > Do we need smt like that? Or how can we handle inferior identification. > > both proc and threadGroup should have a valid id. threadGroup is the groupId > that GDB gives us from -list-thread-groups I use "info inferiors" command because we need inferior id , -list-thread-groups does not give any info about inferior ids. Created attachment 168510 [details]
Summary of tasks
This is what I have done already ,also plans must be implemented next.
(In reply to comment #20) > Created an attachment (id=168510) [details] > Summary of tasks > > This is what I have done already ,also plans must be implemented next. Can we add an attirubute "inferiorId" to processContext? IMIProcesses IProcessDMContext createProcessContext(ICommandControlDMContext controlDmc, String pid); Two questions about the fix for this bug: 1. Will this fix enable DSF to handle new processes created by the running process when detach-on-fork is set to "off"? These new processes create a gdb output message to the effect of: "[New process 5779]" 2. How far along is the fix? Is it in a state that I could get the source code and try it? Since GDB 7.2 is coming out very soon, and its behavior is different than 7.1, I suggest we start by implementing this feature for 7.2. Later on, if we want to support 7.1, we can try to add it. If we start with 7.1, we will be forced to also implement support for 7.2 right away, since people will expect multi-process to continue working from 7.1 to 7.2. The other benefit of using 7.2 is that is has MI commands for -add-inferior and -remove-inferior, as well as a --thread-group flag to be able to specify an inferior for each other command. This will give us a better implementation in DSF-GDB. The pre-release of GDB 7.2 can be obtained at ftp://sourceware.org/pub/gdb/snapshots/branch/gdb.tar.bz2 I've been thinking about the usecases we may want to support for multi-process on Linux. We should clarify those use cases to be able to define the user-experience. Here are some thoughts, which I haven't actually tested with GDB: 1- for attach sessions, the user should be able to attach/detach from running processes repeatedly. We have two cases: a) local session: in this case, I believe GDB can figure out the binary location automatically when we attach to a process. So, the user could simply press the 'connect' button from the debug view, and we should show the list of running processes; the user would then choose a running process; we then would use IProcesses#attachDebuggerToProcess() to do -add-inferior and then -target-attach. b) remote session: in this case, I believe GDB needs to be told where the binary is on the host. The user could still use the 'connect' button and choose a running process, but then we would need an extra prompt to ask the user where the binary is. I'm not sure we have the proper service call in IProcesses for this. Maybe combination of IProcesses#debugNewProcess() to do -add-inferior <path> followed by IProcesses#attachDebuggerToProcess() to do -target-attach, although that is not totally correct, since debugNewProcess() should probably start the process with -exec-run. We may need a new interface method for this case. 2- for local normal sessions (non-attach), the user should be able to start new processes. We probably need a 'Debug new process' action somewhere that would prompt the user for the location of a binary. We would then use IProcesses#debugNewProcess() to do -add-inferior <path> followed by -exec-run. 3- remote non-attach, would probably work like #2. 4- mix local session: this is a new concept where we mix 1-a and 2; the user could choose to start new processes or to attach to running processes, in any order. I'm not sure we want to provide this. It starts merging the concept of attach session and local session. This may bring the idea of a single type of debugging session, where the user choose to debug a local or a remote machine, and then freely choose attach or startNew. 5- mix remote session: same idea as 4. I think the simplest is 1-a. After that, for 1-b and 2, the extra step is that we must ask the user for a path to a binary. (In reply to comment #24) > 1- for attach sessions, the user should be able to attach/detach from running > processes repeatedly. We have two cases: > > a) local session: in this case, I believe GDB can figure out the binary > location automatically when we attach to a process. I tried and confirmed that GDB (7.2) can figure out the binary automatically. > b) remote session: in this case, I believe GDB needs to be told where the > binary is on the host. I tried and confirmed that GDB (7.2) needs to be told where the binary is. Created attachment 176501 [details]
-add-inferior and -remove-inferior
This patch adds the GDB 7.2 -add-inferior and -remove-inferior MI commands. They are necessary for this solution and didn't require any thought, so I added them to help.
Committed to HEAD.
*** cdt cvs genie on behalf of mkhouzam *** Bug 237306: Added -add-inferior and -remove-inferior commands [+] MIAddInferior.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/commands/MIAddInferior.java?root=Tools_Project&revision=1.1&view=markup [+] MIRemoveInferior.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/commands/MIRemoveInferior.java?root=Tools_Project&revision=1.1&view=markup [*] CommandFactory.java 1.13 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/CommandFactory.java?root=Tools_Project&r1=1.12&r2=1.13 [+] MIAddInferiorInfo.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/output/MIAddInferiorInfo.java?root=Tools_Project&revision=1.1&view=markup Note that to properly use GDB 7.2 with DSF-GDB, we have a dependency on the fix to bug 315796 which already contains a patch. Why we need exec-run , don't we connect to a already running process?After attach , program suspends and we can continue with run or next button. (In reply to comment #29) > Why we need exec-run , don't we connect to a already running process?After > attach , program suspends and we can continue with run or next button. I assume you are referring to > 1-b) remote session: in this case, I believe GDB needs to be told where the > binary is on the host. The user could still use the 'connect' button and > choose a running process, but then we would need an extra prompt to ask the > user where the binary is. I'm not sure we have the proper service call in > IProcesses for this. Maybe combination of IProcesses#debugNewProcess() to do > -add-inferior <path> followed by IProcesses#attachDebuggerToProcess() to do > -target-attach, although that is not totally correct, since debugNewProcess() > should probably start the process with -exec-run. We may need a new interface > method for this case. So, you are right, that we don't want -exec-run in this case. What I was trying to say is that the method IProcesses#debugNewProcess() has a name and javadoc that implies that we will _start_ a new process. This would cover case #2, and will perform an -exec-run for this case. In the case of 1-b), since we don't want to do the -exec-run, the method that is properly named is IProcesses#attachDebuggerToProcess() but it does not take a file name as a parameter, so we can't use it for the -add-inferior <file> command. So, what we can try is to use IProcesses#debugNewProcess() for both cases, but to use its 'attributes' field to tell the method if it should do the -exec-run or if it should not, based on the fact that we are in the 'attach' case or not. (In reply to comment #28) > Note that to properly use GDB 7.2 with DSF-GDB, we have a dependency on the fix > to bug 315796 which already contains a patch. I've committed the patch of bug 315796, so a refresh to CDT HEAD will help use GDB 7.2 Your suggestion sounds good . One more thing is that , we have one inferior process in GDBControl , do we need 1-1 mapping with inferior and processes being debugged or 1-n mapping which one inferior process manages all works. bug 237308 is the answer. (In reply to comment #32) > Your suggestion sounds good . One more thing is that , we have one inferior > process in GDBControl , do we need 1-1 mapping with inferior and processes > being debugged > or 1-n mapping which one inferior process manages all works. I have added a new command " Add Inferior" to Debug UI. Here is multiple debugging flow implemented so far. 1 ) User adds a new empty inferior with "Add İnferior" UI command , which adds uses "add-inferior" CLI command ( we now use MI command anyway) . And UI updates according to this new inferior. 2 ) After new inferior creation , if user clicks this new node "inferior x" command works and underlying gdb swithes active inferior. 3 ) Then one can go with normal attaching procedure. 4 ) After detaching , inferior remains . Now As I understand from Marc's suggestions , we don't need a New UI command(add inferior) . If user hits the connect button anyother place than inferior nodes , new node would be added and process that choosen before attaches this inferior , and active inferior becomes this one. Can you tell me your suggestions please. Thanks. (In reply to comment #34) > I have added a new command " Add Inferior" to Debug UI. Here is multiple > debugging flow implemented so far. > 1 ) User adds a new empty inferior with "Add İnferior" UI command , which > adds uses "add-inferior" CLI command ( we now use MI command anyway) . And UI > updates according to this new inferior. > > 2 ) After new inferior creation , if user clicks this new node "inferior x" > command works and underlying gdb swithes active inferior. > > 3 ) Then one can go with normal attaching procedure. > > 4 ) After detaching , inferior remains . > > Now As I understand from Marc's suggestions , we don't need a New UI > command(add inferior) . If user hits the connect button anyother place than > inferior nodes , new node would be added and process that choosen before > attaches this inferior , and active inferior becomes this one. > > Can you tell me your suggestions please. Yes, I don't think we need "add inferior" in the UI. I don't believe the user needs to know the step of adding an empty inferior. What the user wants is to either attach to a process or to start a new process; for these, we must do add-inferior, but we should hide it from the user, and present unified actions, one for "attach" (we already have the button in the UI) and one for "debug new process", which I would add as a context menu (right-click in the debug view). I will make a copy of GdbConnectCommand (GDBDebugNewProcessCommand). Most of code would be same , but as Marc mentioned before a new prompt for executable needed and last step would be calling "procserv.debugNewProcess(...)". How does it sound? (In reply to comment #36) > I will make a copy of GdbConnectCommand (GDBDebugNewProcessCommand). > Most of code would be same , but as Marc mentioned before a new prompt for > executable needed and last step would be calling > "procserv.debugNewProcess(...)". > > How does it sound? Sounds good. I think the code to GDBDebugNewProcessCommand would actually be much simpler since we don't have to give a list of existing processes to the user, but just a file browser dialog. I have though that GDBDebugNewProcessCommand would be same to GDBConnectCommand , with one more new dialog(File Choose dialog) added. If GDBDebugNewProcessCommand just chooses executable file , How can user attach to this process. When add-inferior command runs? If you mean user first select file ( GDBDebugNewProcessCommand) and than attaches process (GDBConnectCommand) , how can we ensure user would select right process which belongs to file selected before? (In reply to comment #38) > I have though that GDBDebugNewProcessCommand would be same to GDBConnectCommand > , with one more new dialog(File Choose dialog) added. If > GDBDebugNewProcessCommand just chooses executable file , How can user attach to > this process. When add-inferior command runs? When the user wants to attach to a new process, they would click on the 'connect' button that we already have. The GDBConnectCommand would get called, and in the case of remote debugging, we would add a file browser prompt before choosing the process to attach, or maybe we would need a joint prompt asking for both information (file and process to attach to). When the user wants to start a new process (one that is not running at all), they would select the new "Debug new process" command which would trigger GDBDebugNewProcessCommand. This code would not need to attach to a process but only to prompt for the file location, then do -add-inferior followed by -exec-run --threadGroupId (In reply to comment #39) > If you mean user first select file ( GDBDebugNewProcessCommand) and than > attaches process (GDBConnectCommand) , how can we ensure user would select > right process which belongs to file selected before? I don't think we can. If they don't GDB will eventually give an error. We can try to make this as user-friendly as we can, but I don't think we can figure it out early. what about attaching multiple processes remotely? We have to use connect button when attaching to a remote process , if user wants to attach to a "running process" , we need to provide "add-inferior" command which would add a new inferior to be attached. My opinion is that , if we press connect button and there is no selected node , then we would call add-inferior command followed by attach to a process. If a node selected before connect button pressed , than we would just go on with attach command we already have. (In reply to comment #41) > what about attaching multiple processes remotely? Do you mean attaching to multiple processes in one operation? I think that would be nice. We should make the API able to handle that. See bug 293679. > We have to use connect button when attaching to a remote process , if user > wants to attach to a "running process" , we need to provide "add-inferior" > command which would add a new inferior to be attached. I've been using the word 'attach' to mean 'attach to a running process'. If the user wants to debug a new process, it is not the 'attach case anymore. So, -add-inferior just specifies the binary that the inferior will use, but not the actual process to attach to. For 'attach' we need to do -add-inferior and -target-attach > My opinion is that , if we press connect button and there is no selected node what node do you mean? In the debug view? The connect button is grayed out if there is nothing selected in the debug view. If you mean, the node in the process list prompt, then if the user does not select anything, the OK button is grayed out. > then we would call add-inferior command followed by attach to a process. > If a node selected before connect button pressed , than we would just go on > with attach command we already have. Are talking about showing all processes that exist somewhere? We would need a different view for that, I think. There are too many processes. I suggest you ignore the 'inferior' part and try to get the other stuff to work first. Created attachment 178929 [details]
multi process debug(gdbserver)
this patch can do multiple debug with gdbserver. Cant detach right now , there exists lots of groupId/procId conflicts.
I think we need to override attachDebuggerToProcess with file parameter , this is what I do with this patch.
(In reply to comment #43) > Created an attachment (id=178929) [details] > multi process debug(gdbserver) > > this patch can do multiple debug with gdbserver. Cant detach right now , there > exists lots of groupId/procId conflicts. > > I think we need to override attachDebuggerToProcess with file parameter , > this is what I do with this patch. Thanks for the patch, I'll have a detailed look soon. But I suggest: 1- turn on API tooling : http://wiki.eclipse.org/CDT/policy#Using_API_Tooling 2- turn on assertions when you test. That means adding -ea to the VM arguments of your test eclipse (in the Arguments launch tab). Once you turn on assertions, you will notice that the patch causes some :-) Created attachment 179595 [details]
Basic multi-process debug
Thanks Onur for the patch.
I have taken it and simplified it to address the local attach case only. I think we should focus on that case first, to iron out all complex issues; after that is done, the other cases should be more straighforward.
With this patch, we can do a local attach to multiple processes sequentially, using the 'connect' button in the debug view.
Next steps that need to be done:
1- handle the 'switch inferior' aspect of things by using the --thread-group flag to MI commands. This will be tricky. The new '--thread-group' flag should be added in AbstractMIControl like we have done for '--thread' and '-frame'. This new --thread-group flag will indicate which inferior we want to give a command for, without having to explicitely switch the inferior. I have used it in -target-attach directly, although with a proper AbstractMIControl solution, -target-attach should not need to explicitely set --thread-group.
However, I noticed something interesting. When we debug multiple processes, it seems GDB has a unique thread numbering scheme. This means that any MI command that applies to a thread, will properly choose the inferior without even using --thread-group. We will need to confirm this with the GDB folks, but before that, we should test such a theory with multiple processes which have multiple threads (my test was single threaded).
Furthermore, there are most probably some commands that used to be send using ICommandControlDMC but that are now process specific. These commands will need to use an IMIContainerDMC instead of ICommandControlDMC. An example of this is the MIFileExecAndSymbols, which is now process specific.
I have to admit that this change is not simple. I can probably take care of it, since I had to do something similar with the --thread and --frame flags.
2- deal with 'disconnect'. Sadly, there seems to be a bug in GDB 7.2 that makes -target-detach fail when using the groupId. We may be able to work around this using '-target-detach' with the pid instead. Other options maybe to use either '-remove-inferior' (although it technically should only be used for inferiors that have been detached), or the CLI 'detach' command. But I think "-target-detach <pid>" may be enough.
3- Generalize the solution to support multi-process for
a) Local (non-attach) sessions
b) Remote (attach and non-attach) sessions
As you know, both of these cases require to specify the file name of the executable.
But I think we've made some very impressive progress, where we can debug multiple programs on Linux when we attach to them!
Created attachment 179596 [details]
Screenshot of multi-process attach session on Linux
Created attachment 179913 [details] Basic multi-process with disconnect working This patch makes 'disconnect' work properly. (In reply to comment #45) > 2- deal with 'disconnect'. Sadly, there seems to be a bug in GDB 7.2 that makes > -target-detach fail when using the groupId. I've found a better workaround for this bug. -target-detach --thread-group <groupId> works properly. I've added this workaround by overriding detachDebuggerFromProcess in GDBProcesses_7_2. I've posted a fix for GDB. If it gets accepted for GDB 7.2.1, we could remove this workaround. See http://sourceware.org/ml/gdb-patches/2010-09/msg00497.html I've also added the missing @since tags. I've fixed bug 315796, so we can now properly use GDB 7.2 Marc , Are you dealing with first part ( 1 ) ? You made some changes at CVS-HEAD about first part.. (In reply to comment #49) > Marc , > > Are you dealing with first part ( 1 ) ? > You made some changes at CVS-HEAD about first part.. Nothing until November. I'm not sure which changes you are referring to? I don't recall making CVS commits towards part 1 (--thread-group) An update was about groupId to pid mapping , so I thought that you work on first subject.( Actually a workaround as I supposed). I will be dealing with part 1.
> 1-
> However, I noticed something interesting. When we debug multiple processes, it
> seems GDB has a unique thread numbering scheme. This means that any MI command
> that applies to a thread, will properly choose the inferior without even using
> --thread-group. We will need to confirm this with the GDB folks, but before
> that, we should test such a theory with multiple processes which have multiple
> threads (my test was single threaded).
I have tried this with a fork example , it seems that gdb works as you mentioned. There is no problem with stepping , continue , vs. Because all of these commands go directly to thread that gdb knows uniquely , no inferior information needed.
Inferior information needed when we deal with source code operation what I mean that put/clear breakpoint , etc. So I used swith behaviour.
(In reply to comment #52) > > 1- > > However, I noticed something interesting. When we debug multiple processes, it > > seems GDB has a unique thread numbering scheme. This means that any MI command > > that applies to a thread, will properly choose the inferior without even using > > --thread-group. We will need to confirm this with the GDB folks, but before > > that, we should test such a theory with multiple processes which have multiple > > threads (my test was single threaded). > I have tried this with a fork example , it seems that gdb works as you > mentioned. There is no problem with stepping , continue , vs. Because all of > these commands go directly to thread that gdb knows uniquely , no inferior > information needed. > Inferior information needed when we deal with source code operation what I > mean that put/clear breakpoint , etc. So I used swith behaviour. I also tried with multiple threaded program , it is true that gdb successfully selects right inferior which thread-switching to - belongs to. (In reply to comment #53) > I also tried with multiple threaded program , it is true that gdb successfully > selects right inferior which thread-switching to - belongs to. Great. Then I think we don't have to worry about any MI command that takes an IExecutionDMC context (or IFrameDMC or IExpressionDMC) as a context, since they will cause the right thread to be selected, and therefore the right process. We must look at MI commands that take an ICommandControlDMC, IBreakpointsDMC, IMemoryDMC, and if those are process-specific, we will need to pass-in an IContainerDMC instead. We can then base ourselves on the presence of a groupId() from IMIContainerDMC, to add the --thread-group flag, just like we add the --thread flag based on the IMIExecutionDMC. Created attachment 182593 [details]
Call thread-group option
Here is a patch for first milestone that Marc suggested. I have added thread-group option to underlying dsf command call classes.
I have added supportThreadGroupOption as false , because there exists not so much commands affected by this option.
If I did other way ( assign true), I need to code some tricky things .(AbstractMIControl line:597) .
Now I have problem with breakpoints , I want to use IBreakDMContext but I couldnt figure out how associate this with IMIContainerDMContext. I try to make things similar with IExecutionDMContext , but haven't done yet.
(In reply to comment #55) > Created an attachment (id=182593) [details] > Call thread-group option Thanks. > Here is a patch for first milestone that Marc suggested. I have added > thread-group option to underlying dsf command call classes. > I have added supportThreadGroupOption as false , because there exists not so > much commands affected by this option. > If I did other way ( assign true), I need to code some tricky things > .(AbstractMIControl line:597) . I didn't quite understand the issue here, but that is ok for now. We can always change it to true if we find it is better. > Now I have problem with breakpoints , I want to use IBreakDMContext but I > couldnt figure out how associate this with IMIContainerDMContext. I try to make > things similar with IExecutionDMContext , but haven't done yet. When setting a breakpoint in GDB, do we need/want to first set the right inferior? What I mean is: are breakpoints per inferior or for all of GDB? You probably don't want to focus on IBreakDMContext. Instead, you probably want to look at IBreakpointsTargetDMContext. This context is currently part of GDBControlDMContext, but it may need to be moved to the container. Note that other of the GDBControlDMContext interfaces should probably be moved to a container for multi-process support; whichever one GDB uses per inferior, instead of global to all of GDB. (In reply to comment #56) > (In reply to comment #55) > > Created an attachment (id=182593) [details] [details] > > Call thread-group option > > Thanks. > > > Here is a patch for first milestone that Marc suggested. I have added > > thread-group option to underlying dsf command call classes. > > I have added supportThreadGroupOption as false , because there exists not so > > much commands affected by this option. > > If I did other way ( assign true), I need to code some tricky things > > .(AbstractMIControl line:597) . > > I didn't quite understand the issue here, but that is ok for now. We can > always change it to true if we find it is better. > > > Now I have problem with breakpoints , I want to use IBreakDMContext but I > > couldnt figure out how associate this with IMIContainerDMContext. I try to make > > things similar with IExecutionDMContext , but haven't done yet. > > When setting a breakpoint in GDB, do we need/want to first set the right > inferior? What I mean is: are breakpoints per inferior or for all of GDB? > Breakpoints are per inferior , it is visually seems ok with breakpoints with Eclipse , but not true for gdb. Gdb couldn't add breakpoint because source - which is an executable file for another inferior - does not exists with in inferior . > You probably don't want to focus on IBreakDMContext. Instead, you probably > want to look at IBreakpointsTargetDMContext. This context is currently part of > GDBControlDMContext, but it may need to be moved to the container. > Ok , I focused on both of them. > Note that other of the GDBControlDMContext interfaces should probably be moved > to a container for multi-process support; whichever one GDB uses per inferior, > instead of global to all of GDB. (In reply to comment #55) > Created an attachment (id=182593) [details] > Call thread-group option Nice work on the patch, here are comments: I've looked at the GDB code and saw that it is not allowed to specify the --thread-group flag at the same time as the --thread flag. This confirms that for commands that specify the thread, we don't need to worry about --thread-group. But we need to code against putting both in MICommand. MICommand: 1- in constructCommand(groupId, threadId, frameId), can you only set the --thread-group flag if we are _not_ setting the --thread flag 1.5- in constructCommand(groupId, threadId, frameId) can you check the supportsThreadAndFrameOptions() and supportsThreadGroupOption() methods, before adding --thread and --thread-group, respectively? I should have done that from the start. It will avoid having to check it before calling constructCommand(...) from anywhere else (AbstractMIControl). This should greatly simplify the check in TxThread#run AbstractMIControl: 2- can you make the two original construtors call this(session, false, false, new CommandFactory()); and this(session, false, useThreadAndFrameOptions, factory); respectively? 3- I think that if we use the the --thread-group option, we should automatically use the --thread option. In the new constructor can you add this assert: assert useThreadGroupOption ? useThreadAndFrameOptions : true; and can you add a check after having set useThreadAndFrameOptions: if (useThreadGroupOption) useThreadAndFrameOptions = true; 4- Can you add a protected setUseThreadGroupOption(boolean) method 5- in processNextQueuedCommand(), we may need to use the CLI 'inferior' command to select the proper inferior when we cannot use the --thread-group option. However, I'm not sure we will need it, since we won't be doing multi-process for GDB's that don't support --thread-group. You may have left it out for that reason. So, let's not change anything in processNextQueuedCommand()for now, and see if we ever need to. 6- because of point 1.5 and 3, in TxThread#run, you can simply the checks. Something like: if (commandHandle.getCommand() instanceof RawCommand) { } else if (fUseThreadGroupOption) { } else if (fUseThreadAndFrameOptions) { } else { GDBControl_7_0 7- I assume this change was just for testing. We will need a GDBControl_7_2 to call the super() constructor with 'true' for the --thread-group option, and we'll need to instantiate it from GdbDebugServicesFactory FinalLaunchSequence 8- neat trick of using 'null' for the containerCtx. I think this is the simplest solution and good enough. This 'initialContainer' will also need to be used for the MIGDBSetArgs command. So, I suggest creating a global variable fInitialContainerCtx, and initializing it in the stepInitializeFinalLaunchSequence() method. Also, we should create a protected getInitialContainerContext() method to return it. I would also add a comment when creating the context, to explain what we are doing, something like: // We can cheat a little. Since we know GDB starts off focused on the one // groupId it automatically created, we can simply create a container with the 'null' // groupId, which will get ignored and will use the process currently in focus. == How are things going with the breakpoint contexts? On my side, I will try to address part 3: > 3- Generalize the solution to support multi-process for > a) Local (non-attach) sessions > b) Remote (attach and non-attach) sessions (In reply to comment #58) > (In reply to comment #55) > > Created an attachment (id=182593) [details] [details] > > Call thread-group option > > Nice work on the patch, here are comments: > > I've looked at the GDB code and saw that it is not allowed to specify the > --thread-group flag at the same time as the --thread flag. This confirms that > for commands that specify the thread, we don't need to worry about > --thread-group. But we need to code against putting both in MICommand. > > MICommand: > 1- in constructCommand(groupId, threadId, frameId), can you only set the > --thread-group flag if we are _not_ setting the --thread flag > > 1.5- in constructCommand(groupId, threadId, frameId) can you check the > supportsThreadAndFrameOptions() and supportsThreadGroupOption() methods, before > adding --thread and --thread-group, respectively? I should have done that from > the start. It will avoid having to check it before calling > constructCommand(...) from anywhere else (AbstractMIControl). This should > greatly simplify the check in TxThread#run > > AbstractMIControl: > 2- can you make the two original construtors call > this(session, false, false, new CommandFactory()); > and > this(session, false, useThreadAndFrameOptions, factory); > respectively? > > 3- I think that if we use the the --thread-group option, we should > automatically use the --thread option. In the new constructor can you add this > assert: > assert useThreadGroupOption ? useThreadAndFrameOptions : true; > and can you add a check after having set useThreadAndFrameOptions: > if (useThreadGroupOption) useThreadAndFrameOptions = true; > > 4- Can you add a protected setUseThreadGroupOption(boolean) method > > 5- in processNextQueuedCommand(), we may need to use the CLI 'inferior' command > to select the proper inferior when we cannot use the --thread-group option. > However, I'm not sure we will need it, since we won't be doing multi-process > for GDB's that don't support --thread-group. You may have left it out for that > reason. So, let's not change anything in processNextQueuedCommand()for now, > and see if we ever need to. > > 6- because of point 1.5 and 3, in TxThread#run, you can simply the checks. > Something like: > if (commandHandle.getCommand() instanceof RawCommand) { > } else if (fUseThreadGroupOption) { > } else if (fUseThreadAndFrameOptions) { > } else { > > GDBControl_7_0 > 7- I assume this change was just for testing. We will need a GDBControl_7_2 to > call the super() constructor with 'true' for the --thread-group option, and > we'll need to instantiate it from GdbDebugServicesFactory > > FinalLaunchSequence > 8- neat trick of using 'null' for the containerCtx. I think this is the > simplest solution and good enough. This 'initialContainer' will also need to > be used for the MIGDBSetArgs command. So, I suggest creating a global variable > fInitialContainerCtx, and initializing it in the > stepInitializeFinalLaunchSequence() method. Also, we should create a protected > getInitialContainerContext() method to return it. I would also add a comment > when creating the context, to explain what we are doing, something like: > // We can cheat a little. Since we know GDB starts off focused on the > one > // groupId it automatically created, we can simply create a container > with the 'null' > // groupId, which will get ignored and will use the process currently > in focus. > > == > How are things going with the breakpoint contexts? I havent done any working progress about breakpoints , I was busy with other stuffs . I think I can do some useful coding next week. > > On my side, I will try to address part 3: > > 3- Generalize the solution to support multi-process for > > a) Local (non-attach) sessions > > b) Remote (attach and non-attach) sessions (In reply to comment #57) > > When setting a breakpoint in GDB, do we need/want to first set the right > > inferior? What I mean is: are breakpoints per inferior or for all of GDB? > > > Breakpoints are per inferior , it is visually seems ok with breakpoints with > Eclipse , but not true for gdb. Gdb couldn't add breakpoint because source - > which is an executable file for another inferior - does not exists with in > inferior . I looked at breakpoints a bit, and I think they remain global, not per-inferior. If I set a breakpoint in a file, it automatically gets set for all inferiors using that file (see session below). I think this makes sense for Eclipse too. The case you mention is a reality today. If you set a breakpoint in a file that is not part of the binary you are currently debugging, Eclipse will tell GDB to set it, and GDB will fail. This is normal. So, I'm hoping we can leave breakpoints as is. Example session: (gdb) inf inf Num Description Executable * 2 process 4783 /home/lmckhou/testing/loopSecond 1 process 4779 /home/lmckhou/testing/loopFirst = I have two inferiors that use the same loop.cc file = (gdb) interpreter-exec mi "-break-insert --thread-group i1 7" ^done,bkpt={number="3",type="breakpoint",disp="keep",enabled="y",addr="<MULTIPLE>",times="0",original-location="loop.cc:7"} = When I set a breakpoint at line 7, even though I specified --thread-group i1, it gets set to <MULTIPLE> locations = (gdb) info b Num Type Disp Enb Address What 3 breakpoint keep y <MULTIPLE> 3.1 y 0x0804852b in main() at loop.cc:7 inf 2 3.2 y 0x0804852b in main() at loop.cc:7 inf 1 = The breakpoint is on both inferiors automatically = (gdb) interpreter-exec mi "-break-insert --thread-group i1 -p 2 7" ^done,bkpt={number="3",type="breakpoint",disp="keep",enabled="y",addr="<MULTIPLE>",thread="2",thread="2",times="0",original-location="loop.cc:7"} = Even if I use the -p (thread) option for breakpoints, specifying the --thread-group has not impact = (gdb) info b Num Type Disp Enb Address What 3 breakpoint keep y <MULTIPLE> thread 2 stop only in thread 2 3.1 y 0x0804851f in main() at loop.cc:7 inf 2 3.2 y 0x0804851f in main() at loop.cc:7 inf 1 (In reply to comment #60) > (In reply to comment #57) > > > > When setting a breakpoint in GDB, do we need/want to first set the right > > > inferior? What I mean is: are breakpoints per inferior or for all of GDB? > > > > > Breakpoints are per inferior , it is visually seems ok with breakpoints with > > Eclipse , but not true for gdb. Gdb couldn't add breakpoint because source - > > which is an executable file for another inferior - does not exists with in > > inferior . > > I looked at breakpoints a bit, and I think they remain global, not > per-inferior. If I set a breakpoint in a file, it automatically gets set for > all inferiors using that file (see session below). I think this makes sense > for Eclipse too. The case you mention is a reality today. If you set a > breakpoint in a file that is not part of the binary you are currently > debugging, Eclipse will tell GDB to set it, and GDB will fail. This is normal. > > So, I'm hoping we can leave breakpoints as is. Than I think we need to clear breakpoints while start of a new debug session , saved breakpoints are forced to be set but no sutiable source file found.I mean no initialization step for breakpoints . > > Example session: > > (gdb) inf inf > Num Description Executable > * 2 process 4783 /home/lmckhou/testing/loopSecond > 1 process 4779 /home/lmckhou/testing/loopFirst > > = I have two inferiors that use the same loop.cc file = > > (gdb) interpreter-exec mi "-break-insert --thread-group i1 7" > ^done,bkpt={number="3",type="breakpoint",disp="keep",enabled="y",addr="<MULTIPLE>",times="0",original-location="loop.cc:7"} > > = When I set a breakpoint at line 7, even though I specified --thread-group i1, > it gets set to <MULTIPLE> locations = > > (gdb) info b > Num Type Disp Enb Address What > 3 breakpoint keep y <MULTIPLE> > 3.1 y 0x0804852b in main() at loop.cc:7 inf 2 > 3.2 y 0x0804852b in main() at loop.cc:7 inf 1 > > = The breakpoint is on both inferiors automatically = > > (gdb) interpreter-exec mi "-break-insert --thread-group i1 -p 2 7" > ^done,bkpt={number="3",type="breakpoint",disp="keep",enabled="y",addr="<MULTIPLE>",thread="2",thread="2",times="0",original-location="loop.cc:7"} > > = Even if I use the -p (thread) option for breakpoints, specifying the > --thread-group has not impact = > > (gdb) info b > Num Type Disp Enb Address What > 3 breakpoint keep y <MULTIPLE> thread 2 > stop only in thread 2 > 3.1 y 0x0804851f in main() at loop.cc:7 inf 2 > 3.2 y 0x0804851f in main() at loop.cc:7 inf 1 (In reply to comment #61) > > So, I'm hoping we can leave breakpoints as is. > Than I think we need to clear breakpoints while start of a new debug session , > saved breakpoints are forced to be set but no sutiable source file found.I mean > no initialization step for breakpoints . I was wrong about my finding.: they only partially showed what GDB does in the case of breakpoints and multiple inferiors. What I see now is that: 1- gdb will only set breakpoints on files that are in the binary of the current inferior 2- if the file is also in the binary of other inferiors, the breakpoint will be set there too 3- try to set a breakpoint in a file that is for an inferior that is not in focus, will not set the breakpoint, and even if it is pending, it will not become active unless affects the inferior that was in focus. So, we do have to set breakpoints for the right inferior process. And we will need to set them all again whenever we create/attach to a new process. I think GDBControl_7_2 must extend GDBControl_7_0 , but with this change I cant use constructCommand which I add befor into AbstractMIControl. It seems that I have to dublicate GDBControl_7_0 for GDBControl_7_2 . Do you have any suggestions? I think we dont want dublications. (In reply to comment #63) > I think GDBControl_7_2 must extend GDBControl_7_0 , but with this change I cant > use constructCommand which I add befor into AbstractMIControl. It seems that I > have to dublicate GDBControl_7_0 for GDBControl_7_2 . > > Do you have any suggestions? I think we dont want dublications. I don't know what you mean? Where do you use constructCommand()? This is how I implemented GDBControl_7_2 (this is the entire class): public class GDBControl_7_2 extends GDBControl_7_0 implements IGDBControl { public GDBControl_7_2(DsfSession session, ILaunchConfiguration config, CommandFactory factory) { super(session, config, factory); setUseThreadGroupOptions(true); } (In reply to comment #64) > (In reply to comment #63) > > I think GDBControl_7_2 must extend GDBControl_7_0 , but with this change I cant > > use constructCommand which I add befor into AbstractMIControl. It seems that I > > have to dublicate GDBControl_7_0 for GDBControl_7_2 . > > > > Do you have any suggestions? I think we dont want dublications. > > I don't know what you mean? Where do you use constructCommand()? > > > This is how I implemented GDBControl_7_2 (this is the entire class): > > public class GDBControl_7_2 extends GDBControl_7_0 implements IGDBControl { > > public GDBControl_7_2(DsfSession session, ILaunchConfiguration config, > CommandFactory factory) { > super(session, config, factory); > setUseThreadGroupOptions(true); > } you are right , I was mistaken. Thanks. For breakpoints , I think we need to send thread-groupId with breakpoint. If we can add groupId attribute to breakpoints attributes ( or its Marker ). It would be nice . As I see it needs some change in cdt.debug.core .. Otherwise we need to determine active --thread-groupId explicitly . Does it make sense to put an attribute in GDBProcesses_7_2 and update it as active inferior changes. So we can use it wherever needed. What do you think? (In reply to comment #66) > For breakpoints , > > I think we need to send thread-groupId with breakpoint. If we can add groupId > attribute to breakpoints attributes ( or its Marker ). It would be nice . As > I see it needs some change in cdt.debug.core .. I think that when we set a new breakpoint, we should try to set it for every threadGroup. We currently won't have a filter to allow the user to specify which threadGroup. This should probably be a new bug to provide this feature. Until then, it means that we can loop over all existing threadGroups in MIBreakpointsManager#installBreakpoint() maybe. There is currently a loop for threads, so it may be very similar. We may have a problem if we set a breakpoint into a file that is used by more than on threadGroup. In that case we will set the breakpoint for the first threadGroup, but since the source affects other threadGroups, GDB will automatically set them also. Our loop would then also try to set the breakpoint for the second threadGroup, even if GDB already set it. But we can try to deal with that when we get there. With this idea, would you still need to set the groupId in the bp attributes? > Otherwise we need to determine active --thread-groupId explicitly . Does it > make sense to put an attribute in GDBProcesses_7_2 and update it as active > inferior changes. So we can use it wherever needed. > > What do you think? I'm not sure if what I wrote was very clear :-), so just in case: I think we should set the breakpoints for all groupIds, not just the active one. Created attachment 184318 [details] Prototype patch to create a new process Building on Onur's patch (that I modified based on comment 58), this patch builds the GDBProcesses_7_2 service to start a new process. I'm not sure about the best way to trigger it in the UI, so what I did was that for a normal local launch, I enabled the 'connect' button of the debug view. When clicking on it, the first entry is <Create new process>, when selecting that, there is a prompt for a binary file and then we create a brand new process based on that binary. We can use the same connect button to attach to a running process from a local launch. There is lots of cleaning up needed but as a prototype, it works ok. Breakpoints don't work well. Also, disconnect seems to cause GDB to break. Did we already see this issue? I'll look into it tomorrow. Comments welcome. (In reply to comment #67) > (In reply to comment #66) > > For breakpoints , > > > > I think we need to send thread-groupId with breakpoint. If we can add groupId > > attribute to breakpoints attributes ( or its Marker ). It would be nice . As > > I see it needs some change in cdt.debug.core .. > > I think that when we set a new breakpoint, we should try to set it for every > threadGroup. We currently won't have a filter to allow the user to specify > which threadGroup. This should probably be a new bug to provide this feature. > Until then, it means that we can loop over all existing threadGroups in > MIBreakpointsManager#installBreakpoint() maybe. There is currently a loop for > threads, so it may be very similar. > > We may have a problem if we set a breakpoint into a file that is used by more > than on threadGroup. In that case we will set the breakpoint for the first > threadGroup, but since the source affects other threadGroups, GDB will > automatically set them also. Our loop would then also try to set the > breakpoint for the second threadGroup, even if GDB already set it. > > But we can try to deal with that when we get there. > > With this idea, would you still need to set the groupId in the bp attributes? > > > Otherwise we need to determine active --thread-groupId explicitly . Does it > > make sense to put an attribute in GDBProcesses_7_2 and update it as active > > inferior changes. So we can use it wherever needed. > > > > What do you think? > > I'm not sure if what I wrote was very clear :-), so just in case: I think we > should set the breakpoints for all groupIds, not just the active one. it is clear , nop :) it seems we dont need any groupId in bp attributes. (In reply to comment #69) > (In reply to comment #67) > > (In reply to comment #66) > > > For breakpoints , > > > > > > I think we need to send thread-groupId with breakpoint. If we can add groupId > > > attribute to breakpoints attributes ( or its Marker ). It would be nice . As > > > I see it needs some change in cdt.debug.core .. > > > > I think that when we set a new breakpoint, we should try to set it for every > > threadGroup. We currently won't have a filter to allow the user to specify > > which threadGroup. This should probably be a new bug to provide this feature. > > Until then, it means that we can loop over all existing threadGroups in > > MIBreakpointsManager#installBreakpoint() maybe. There is currently a loop for > > threads, so it may be very similar. > > > > We may have a problem if we set a breakpoint into a file that is used by more > > than on threadGroup. In that case we will set the breakpoint for the first > > threadGroup, but since the source affects other threadGroups, GDB will > > automatically set them also. Our loop would then also try to set the > > breakpoint for the second threadGroup, even if GDB already set it. > > > > But we can try to deal with that when we get there. > > > > With this idea, would you still need to set the groupId in the bp attributes? > > > > > Otherwise we need to determine active --thread-groupId explicitly . Does it > > > make sense to put an attribute in GDBProcesses_7_2 and update it as active > > > inferior changes. So we can use it wherever needed. > > > > > > What do you think? > > > > I'm not sure if what I wrote was very clear :-), so just in case: I think we > > should set the breakpoints for all groupIds, not just the active one. > > it is clear , nop :) it seems we dont need any groupId in bp attributes. loop 1..#thread-group loop 1..#thread end end With this solution in installBreakpoints function, It seems we need to visit same threads every time if multiple thread-groups exist. Does it ok? (In reply to comment #70) > loop 1..#thread-group > loop 1..#thread > end > end > > With this solution in installBreakpoints function, > It seems we need to visit same threads every time if multiple thread-groups > exist. Does it ok? Actually, the logic about threads is a bit non-intuitive. A breakpoint is normally applied to every thread in GDB, so it only requires one installation (which will trigger a single -break-insert). If the user wants to restrict a breakpoint to one or more threads, then we need to install the breakpoints for each affected thread (-break-insert -p <threadId> for each thread). So, the normal case is that we don't deal with threads which will give: loop 1..#thread-group loop 0..0 (single threadId 0 meaning the whole process) end end In the case where the breakpoints is affecting specific threads, then we have to do some special logic because a threadId is unique for a process, meaning that threadId X is unique in GDB and maps to a single process. So, I'm not sure how the -break-insert -p command will work in that case; do we need to specify the --thread-group option at all? Probably. So I imagine the loop will be something like: for (String thread : threads) { for (String group : threadGroups) { if (thread == 0) { install breakpoint for group // breakpoint that affects all threads of all processes else if (thread belongs to group) { install breakpoint for group // breakpoint for this thread, and therefore for this process only continue to the next thread id } end end (In reply to comment #71) > (In reply to comment #70) > > > loop 1..#thread-group > > loop 1..#thread > > end > > end > > > > With this solution in installBreakpoints function, > > It seems we need to visit same threads every time if multiple thread-groups > > exist. Does it ok? > > Actually, the logic about threads is a bit non-intuitive. A breakpoint is > normally applied to every thread in GDB, so it only requires one installation > (which will trigger a single -break-insert). If the user wants to restrict a > breakpoint to one or more threads, then we need to install the breakpoints for > each affected thread (-break-insert -p <threadId> for each thread). > > So, the normal case is that we don't deal with threads which will give: > > loop 1..#thread-group > loop 0..0 (single threadId 0 meaning the whole process) > end > end > > In the case where the breakpoints is affecting specific threads, then we have > to do some special logic because a threadId is unique for a process, meaning > that threadId X is unique in GDB and maps to a single process. So, I'm not > sure how the -break-insert -p command will work in that case; do we need to > specify the --thread-group option at all? Probably. So I imagine the loop > will be something like: > > for (String thread : threads) { > for (String group : threadGroups) { > if (thread == 0) { > install breakpoint for group // breakpoint that affects all threads of > all processes > else if (thread belongs to group) { > install breakpoint for group // breakpoint for this thread, and > therefore for this process only > continue to the next thread id > } > end > end I need threadGroup information but I can't access it , what I did so far. 1) used procservice.getProcessesBeingDebugged this does not work beacuse command is being cached so I cant get threadGroup information in sync ( or is there smt that I don't know? ). 2 ) I thought that I can use fGroupToPidMap which is in GDBProcesses_7_0. I need to add a method to IMIProcesses , this seems a good solution , because I already have information locally with fGroupToPidMap! What can I do ? (In reply to comment #72) > I need threadGroup information but I can't access it , what I did so far. > 1) used procservice.getProcessesBeingDebugged this does not work beacuse > command is being cached so I cant get threadGroup information in sync ( or is > there smt that I don't know? ). You should be able to call getProcessesBeingDebugged() before the loop, and stick all the looping code inside the handleSuccess() of the RequestMonitor that you passed to getProcessesBeingDebugged(). Something like (pseudo-code) procService.getProcessesBeingDebugged(ctx, new DataRequestMonitor { protected void handleSuccess() { IDMContext[] groups = getData(); for (thread : threads) { for (group:groups) { ... } } } }); > 2 ) I thought that I can use fGroupToPidMap which is in GDBProcesses_7_0. I > need to add a method to IMIProcesses , this seems a good solution , because I > already have information locally with fGroupToPidMap! The other way is to use IGDBProcesses.getExecutionContexts(containerDmc) which is synchronous. Currently it only returns the list of threads, but you can extend the implementation to accept a null, which would return the list of all containers (groups). Hi Onur, I will start committing parts of the code. The first part will be the patch you submitted called "Call thread-group option". I have modified it according to my comments of comment #58, but it is still your patch. Can you confirm in this bug that: 1- you wrote 100% of the code you submitted 2- you have the right to contribute the code to Eclipse 3- what company your work for so I can add the header in the appropriate files: "Onur Akdemir (<company>) - Multi-process debugging (Bug 237306) Thanks (In reply to comment #74) > Hi Onur, > > I will start committing parts of the code. The first part will be the patch > you submitted called "Call thread-group option". I have modified it according > to my comments of comment #58, but it is still your patch. > > Can you confirm in this bug that: > 1- you wrote 100% of the code you submitted > 2- you have the right to contribute the code to Eclipse > 3- what company your work for so I can add the header in the appropriate files: > "Onur Akdemir (<company>) - Multi-process debugging (Bug 237306) > > Thanks Thanks for your guidance and valuable helps. I wrote the code and have the right to contribute. "Onur Akdemir (BILGEM-ITI) Multi-process debugging (Bug 237306)" (In reply to comment #75) > (In reply to comment #74) > > Hi Onur, > > > > I will start committing parts of the code. The first part will be the patch > > you submitted called "Call thread-group option". I have modified it according > > to my comments of comment #58, but it is still your patch. > > > > Can you confirm in this bug that: > > 1- you wrote 100% of the code you submitted > > 2- you have the right to contribute the code to Eclipse > > 3- what company your work for so I can add the header in the appropriate files: > > "Onur Akdemir (<company>) - Multi-process debugging (Bug 237306) > > > > Thanks > Thanks for your guidance and valuable helps. I wrote the code and have the > right to contribute. > > "Onur Akdemir (BILGEM-ITI) Multi-process debugging (Bug 237306)" I missed smt ,this one better. "Onur Akdemir (TUBITAK BILGEM-ITI) Multi-process debugging (Bug 237306)" Comment on attachment 178929 [details]
multi process debug(gdbserver)
This patch was a very early version and we are not going to use it.
Created attachment 185285 [details] Patch committed corresponding to patch "Call thread-group option" This is the patch I have committed to HEAD. It is Onur's patch called "Call thread-group option" with changes as per comment 58. According to TM's line_count_scripts/lcp utility, the patch has 174 lines of contribution, so I don't need a CQ. In comment #75, we confirm that Onur wrote all the code and has the right to contribute. Also, I confirm that the committed patch contains all the proper license headers. One may notice that my patch has an extra file (GDBProcesses_7_2.java) that I wrote myself, which was not part of Onur's original patch, which is why the license is different on that one file. Committed to HEAD. Thank you Onur for your contribution! We are not done yet though :-) I believe what is left is: 1- breakpoint handling: Onur is working on that 2- Local (non-attach) sessions: this is the patch "Prototype patch to create a new process", but how to handle the UI is not clear yet, which is why I'm not committing it yet. The patch is also incomplete for MI commands that should use a containerDmc instead of a commandControlDmc. 3- Remote (attach and non-attach) sessions: probably handled by #2 (In reply to comment #78) > Created an attachment (id=185285) [details] > Patch committed corresponding to patch "Call thread-group option" > > Also, I confirm that the committed patch contains all the proper license > headers. One may notice that my patch has an extra file > (GDBProcesses_7_2.java) that I wrote myself, which was not part of Onur's > original patch, which is why the license is different on that one file. Typo here, the new file is GDBControl_7_2.java *** cdt cvs genie on behalf of mkhouzam *** Bug 237306: First part of commit for multi-process on Linux (GDB 7.2). It allows for a multi-process session for an attach debug session or a remote attach debug session. [*] MIFileExecAndSymbols.java 1.5 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/commands/MIFileExecAndSymbols.java?root=Tools_Project&r1=1.4&r2=1.5 [*] MITargetDetach.java 1.3 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/commands/MITargetDetach.java?root=Tools_Project&r1=1.2&r2=1.3 [*] MITargetAttach.java 1.3 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/commands/MITargetAttach.java?root=Tools_Project&r1=1.2&r2=1.3 [*] MICommand.java 1.3 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/commands/MICommand.java?root=Tools_Project&r1=1.2&r2=1.3 [*] GdbDebugServicesFactory.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/GdbDebugServicesFactory.java?root=Tools_Project&r1=1.14&r2=1.15 [+] GDBProcesses_7_2.java 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&revision=1.1&view=markup [*] GDBProcesses_7_0.java 1.29 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_0.java?root=Tools_Project&r1=1.28&r2=1.29 [*] CommandFactory.java 1.19 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/CommandFactory.java?root=Tools_Project&r1=1.18&r2=1.19 [*] AbstractMIControl.java 1.16 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/AbstractMIControl.java?root=Tools_Project&r1=1.15&r2=1.16 [+] GDBControl_7_2.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/command/GDBControl_7_2.java?root=Tools_Project&revision=1.1&view=markup Created attachment 185617 [details]
Breakpoint discussion patch
This is not a working patch . I have just done what Marc suggested before for breakpoints.
Initial context being set in FinalLaunchSequence#stepStartTrackingBreakpoints , I set the null containerCtx which has "null groupID". Here is the point , how could we determine active inferior. As we now two option exists.
1) --thread-group
2) switching inferiors(with the help of "CLI inferior command" .
For the former case , We need to supply a setGroupId method -which brokes final , vs. - so we can set this information in MIBreakpiontsManager#installBreakpoint ( at the end of loop ,before calling breakpoint service)
For the latter case , we are supposed to stick to CLI command (inferior infID) , it would work well.
Or another situation from these two , is there a way that breakpoint's contexts avaliable with groupId information setted? But this forces to exactly set breakpoint to just a specific thread-group. With the help of our previous discussions , I think user must be aware of underlying inferiors. So this solution seems bad.
What do you think?
Created attachment 185691 [details] Setting breakpoint prototype (In reply to comment #82) > Created an attachment (id=185617) [details] > Breakpoint discussion patch > > This is not a working patch . I have just done what Marc suggested before for > breakpoints. > Initial context being set in FinalLaunchSequence#stepStartTrackingBreakpoints > , I set the null containerCtx which has "null groupID". Here is the point , how > could we determine active inferior. As we now two option exists. > 1) --thread-group > 2) switching inferiors(with the help of "CLI inferior command" . I think we should only use 1). If we find a case where we really need to use the CLI 'inferior' command, then it will probably mean GDB is missing something. But let's see if we find such a case. When looking at your patch, I realized that I was probably wrong about how we should handle things in the MIBreakpointsManager. It seems right to make MIContainerDMC implement IBreakpointsTargetDMContext, but when that is done, I believe that MIBreakpointsManager can already handle things. What we need to do is call startTrackingBreakpoints() whenever we create/attach to a new process. By doing this, the MIBreakpointsManager will know about each process (IBreakpointsTargetDMContext) and will automatically set new breakpoints in each one of them (see breakpointAdded()). Try out this prototype patch which allows me to set breakpoints in different processes using an attach session. Note that we may have to make MIContainerDMC also be ISourceLookupDMContext. Comment on attachment 185617 [details]
Breakpoint discussion patch
Marc's patch is good.
Created attachment 186266 [details]
done cleanups to Marc's patch , working patch.
This patch build on Marc's patch , removed IBreakpointsTargetDMContext from GDBControlDMContext . It works same as Marc's patch .
(In reply to comment #83) > Note that we may have to make MIContainerDMC also be ISourceLookupDMContext. Source paths are per inferior , but there is no -thread-group option for "environment-directory" MI command yet. Here is an example session part. (gdb) -environment-path ^done,path="/home/onur/help:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games" (gdb) inferior 2 &"inferior 2\n" ~"[Switching to inferior 2 [process 0] (<noexec>)]\n" ^done (gdb) -environment-path ^done,path="/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games" they are different , but with the help of CLI commands. (In reply to comment #86) > (In reply to comment #83) > > Note that we may have to make MIContainerDMC also be ISourceLookupDMContext. > Source paths are per inferior , but there is no -thread-group option for > "environment-directory" MI command yet. The --thread-group option applies to every MI command in GDB. The flag is generic for MI. Did you try it with -environment-directory? (In reply to comment #87) > (In reply to comment #86) > > (In reply to comment #83) > > > Note that we may have to make MIContainerDMC also be ISourceLookupDMContext. > > Source paths are per inferior , but there is no -thread-group option for > > "environment-directory" MI command yet. > > The --thread-group option applies to every MI command in GDB. The flag is > generic for MI. Did you try it with -environment-directory? I was wrong with syntax , you are right , it is ok. Created attachment 187326 [details]
Updated Prototype patch to create a new process for HEAD
This is just an update to HEAD of the code that allows to create a new process. I still need to clean it up before committing.
(In reply to comment #85) > Created attachment 186266 [details] > done cleanups to Marc's patch , working patch. > > This patch build on Marc's patch , removed IBreakpointsTargetDMContext from > GDBControlDMContext . It works same as Marc's patch . Hi Onur, this patch has changes to the disassembly code in dsf.ui. I assume that is a mistake? Created attachment 187357 [details]
Fully enable the user of --thread-group
This patch enables the use of --thread-group for all MI commands, since GDB supports it.
The problem Onur was experiencing with this must have been the following:
613,382 [MI] 34-exec-run --thread-group
613,387 [MI] &"Invalid thread group id\n"
This was because we still use MIProcesses.UNIQUE_GROUP_ID in some places, which gives the empty sting as a group id. This will soon be cleaned up, but until then it was breaking things. What I did was put a check in MICommand to ignore groupId that are empty.
Committed to HEAD.
*** cdt cvs genie on behalf of mkhouzam *** Bug 237306: Enable --thread-group for all MI commands [*] CLICommand.java 1.3 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/commands/CLICommand.java?root=Tools_Project&r1=1.2&r2=1.3 [*] MICommand.java 1.4 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/commands/MICommand.java?root=Tools_Project&r1=1.3&r2=1.4 [*] RawCommand.java 1.3 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/commands/RawCommand.java?root=Tools_Project&r1=1.2&r2=1.3 [*] MITargetDetach.java 1.4 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/commands/MITargetDetach.java?root=Tools_Project&r1=1.3&r2=1.4 [*] MIFileExecAndSymbols.java 1.6 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/commands/MIFileExecAndSymbols.java?root=Tools_Project&r1=1.5&r2=1.6 [*] MITargetAttach.java 1.4 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/command/commands/MITargetAttach.java?root=Tools_Project&r1=1.3&r2=1.4 (In reply to comment #90) > (In reply to comment #85) > > Created attachment 186266 [details] [details] > > done cleanups to Marc's patch , working patch. > > > > This patch build on Marc's patch , removed IBreakpointsTargetDMContext from > > GDBControlDMContext . It works same as Marc's patch . > > Hi Onur, > > this patch has changes to the disassembly code in dsf.ui. I assume that is a > mistake? sorry :( (In reply to comment #93) > (In reply to comment #90) > > (In reply to comment #85) > > > Created attachment 186266 [details] [details] [details] > > > done cleanups to Marc's patch , working patch. > > > > > > This patch build on Marc's patch , removed IBreakpointsTargetDMContext from > > > GDBControlDMContext . It works same as Marc's patch . > > > > Hi Onur, > > > > this patch has changes to the disassembly code in dsf.ui. I assume that is a > > mistake? > sorry :( Hi Onur, can you post this patch (without the disassembly changes) to bug 335324? I would like to fix it in a separate bug so that others can review, since it is an important change. Thanks. Created attachment 190931 [details]
New button in the Attach dialog to create a new process
As discussed in the multicore debug meeting, here is a patch that adds a "New..." button to the bottom of the "Attach to process" dialog. I have some issues to work out for the functionality to work properly but I wanted to get the UI in before API freeze.
The user would first press the 'Connect' button on the debug view toolbar, and then press this new "New..." button which will prompt for a binary. Once the binary is selected, we start the new process using the binary.
The UI is very basic since it does not allow you to configure the new process in any way. But it gives access to the feature which will help figure out the workflow.
The button is disabled unless multi-process is supported.
Committed to HEAD.
*** cdt cvs genie on behalf of mkhouzam *** Bug 237306: Support for Multi-Process debugging. Add a "New..." button to the "Attach dialog" to allow creating a new process from a running session. [*] GdbConnectCommand.java 1.5 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/actions/GdbConnectCommand.java?root=Tools_Project&r1=1.4&r2=1.5 [*] ProcessPrompter.java 1.4 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/launching/ProcessPrompter.java?root=Tools_Project&r1=1.3&r2=1.4 [+] ProcessPrompterDialog.java http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/launching/ProcessPrompterDialog.java?root=Tools_Project&revision=1.1&view=markup [*] LaunchUIMessages.properties 1.8 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/launching/LaunchUIMessages.properties?root=Tools_Project&r1=1.7&r2=1.8 Created attachment 190997 [details]
Prompt for a binary when doing remote attach
For a local attach, we can simply tell GDB the pid of the process. However, for a remote attach, we need to also specify the location of the binary.
The DSF IProcesses interface does not provide a parameter for such a binary path, so I had to add a new method in IGDBProcesses.
Also, in the GDBConnectCommand, we now prompt the user for a binary when we are dealing with a remote session.
Committed to HEAD.
*** cdt cvs genie on behalf of mkhouzam *** Bug 237306: Support for Multi-Process debugging. Prompting the user for the path to the binary when doing a remote attach. [*] ProcessPrompterDialog.java 1.2 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/launching/ProcessPrompterDialog.java?root=Tools_Project&r1=1.1&r2=1.2 [*] GdbConnectCommand.java 1.6 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb.ui/src/org/eclipse/cdt/dsf/gdb/internal/ui/actions/GdbConnectCommand.java?root=Tools_Project&r1=1.5&r2=1.6 [*] GDBProcesses.java 1.21 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.java?root=Tools_Project&r1=1.20&r2=1.21 [*] GDBProcesses_7_2.java 1.6 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.5&r2=1.6 [*] GDBProcesses_7_0.java 1.42 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_0.java?root=Tools_Project&r1=1.41&r2=1.42 [*] IGDBProcesses.java 1.8 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/IGDBProcesses.java?root=Tools_Project&r1=1.7&r2=1.8 Multi-process support has been available since Indigo and I have had no complaints about it. There is one enhancement that I would like to see fixed: supporting starting a new process on a remote target; but this should be another bug. |