Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 317500

Summary: [processes] The id reported by -list-thread-groups has changed in GDB 7.2
Product: [Tools] CDT Reporter: Marc Khouzam <marc.khouzam>
Component: cdt-debug-dsf-gdbAssignee: Marc Khouzam <marc.khouzam>
Status: RESOLVED FIXED QA Contact: Marc Khouzam <marc.khouzam>
Severity: normal    
Priority: P3 CC: nobody, pawel.1.piech
Version: 7.0Flags: marc.khouzam: review? (nobody)
Target Milestone: 7.0.2   
Hardware: PC   
OS: Linux   
Whiteboard:
Bug Depends on: 326970    
Bug Blocks: 315796    
Attachments:
Description Flags
Fix for HEAD
marc.khouzam: iplog-
Fix for 7_0 marc.khouzam: iplog-

Description Marc Khouzam CLA 2010-06-21 14:18:11 EDT 
The group id reported by -list-thread-groups is officially an opaque string; this implies it does not have meaning outside of GDB and should not be interpreted by the frontend.

However, for GDB 7.0 and 7.1, the id was actually the pid, so I've used it that way.  The reason I did that is that the =thread-group-created event only reports the groupId; therefore, to find the pid would have required an extra call to the backend; it was more efficient to use the groupId directly since it was identical to the pid.

In GDB 7.2, the group id has changed and is no longer the pid.  The good news is that the =thread-group-created (now =thread-group-started) event now explicitly reports the pid.

To support gDB 7.2, we need to modify our code to look for the 'pid' entry of the event.
Comment 1 Marc Khouzam CLA 2010-08-17 11:46:29 EDT 
From bug 322658 comment #20, Mikhail says:
> I made (simple) changes to make "=thread-group-started" work for our GDB that
> already supports 7.2 and was about to submit a patch to CDT but you beat me :)

I'm wondering if you are considering submitting other changes to support GDB 7.2?  The one I believe we need is tracked by this bug.  I haven't started fixing this issue and I didn't want both of us to do the same work :-)
Comment 2 Nobody - feel free to take it CLA 2010-08-17 12:11:36 EDT 
(In reply to comment #1)
> From bug 322658 comment #20, Mikhail says:
> > I made (simple) changes to make "=thread-group-started" work for our GDB that
> > already supports 7.2 and was about to submit a patch to CDT but you beat me :)
> 
> I'm wondering if you are considering submitting other changes to support GDB
> 7.2?  The one I believe we need is tracked by this bug.  I haven't started
> fixing this issue and I didn't want both of us to do the same work :-)

I can't promise it - there are more important issues with our DSF port. Is this only a question of efficiency?
I have noticed some other possible useful changes but need to confirm it first.
Comment 3 Marc Khouzam CLA 2010-08-17 12:21:48 EDT 
(In reply to comment #2)
> (In reply to comment #1)
> > From bug 322658 comment #20, Mikhail says:
> > > I made (simple) changes to make "=thread-group-started" work for our GDB that
> > > already supports 7.2 and was about to submit a patch to CDT but you beat me :)
> > 
> > I'm wondering if you are considering submitting other changes to support GDB
> > 7.2?  The one I believe we need is tracked by this bug.  I haven't started
> > fixing this issue and I didn't want both of us to do the same work :-)
> 
> I can't promise it - there are more important issues with our DSF port. Is this
> only a question of efficiency?

No, I believe the functionality is broken in certain places.  For example, if you run a program with gDB 7.2, you'll notice that the debug view shows a pid of [i1] instead of the real pid.  This is probably much worst in the case of remote/attach sessions.

> I have noticed some other possible useful changes but need to confirm it first.

Nice.  I'm looking forward to them.
Comment 4 Nobody - feel free to take it CLA 2010-08-17 12:29:25 EDT 
(In reply to comment #3)
> No, I believe the functionality is broken in certain places.  For example, if
> you run a program with gDB 7.2, you'll notice that the debug view shows a pid
> of [i1] instead of the real pid.  This is probably much worst in the case of
> remote/attach sessions.
> 
I have noticed that and have no idea what i1 is supposed to mean. So you are saying that we need to display the pid instead because it is useful in case of attach, right?
Comment 5 Marc Khouzam CLA 2010-08-17 12:44:11 EDT 
(In reply to comment #4)
> (In reply to comment #3)
> > No, I believe the functionality is broken in certain places.  For example, if
> > you run a program with gDB 7.2, you'll notice that the debug view shows a pid
> > of [i1] instead of the real pid.  This is probably much worst in the case of
> > remote/attach sessions.
> > 
> I have noticed that and have no idea what i1 is supposed to mean. So you are
> saying that we need to display the pid instead because it is useful in case of
> attach, right?

Well, having the pid shown in the debug view is extra information for the user, so it would be nice if it was correct :-)

-list-thread-groups used to have the groupId be the pid, so I used it like that (even though I realize now that GDB clearly state that the groupId was an opaque-string).  Also, the old =thread-group-created event only gave the groupId.  With GDB 7.2, the groupId has changed to be i1, i2, etc, to mean inferior 1, inferior 2 (multi-process); on the other hand, the =thread-group-started event now has a pid field, which we should use.
Comment 6 Nobody - feel free to take it CLA 2010-08-17 13:06:43 EDT 
(In reply to comment #5)
> Well, having the pid shown in the debug view is extra information for the user,
> so it would be nice if it was correct :-)
> 
> -list-thread-groups used to have the groupId be the pid, so I used it like that
> (even though I realize now that GDB clearly state that the groupId was an
> opaque-string).  Also, the old =thread-group-created event only gave the
> groupId.  With GDB 7.2, the groupId has changed to be i1, i2, etc, to mean
> inferior 1, inferior 2 (multi-process); on the other hand, the
> =thread-group-started event now has a pid field, which we should use.

Yes, I am aware of these changes. I just didn't know what exactly i1 meant and thought it might be a useful information for the users.

BTW, here is output of "list-thread-groups" for our GDB:

518,599 [MI]  10-list-thread-groups
518,601 [MI]  10^done,groups=[{id="i1",type="process",pid="6077",executable="/home/mikhail/runtime-w\
orkspace-i686-pc-linux-gnu/factorial/Debug/factorial",cores=["0"]}]

I am having trouble to build GDB from HEAD and don't know if it's the same as GDB 7.2.
Comment 7 Marc Khouzam CLA 2010-08-18 10:01:24 EDT 
(In reply to comment #6)

> Yes, I am aware of these changes. I just didn't know what exactly i1 meant and
> thought it might be a useful information for the users.

I believe this is just an internal id used between Eclipse and GDB and should not be shown to the user.

> BTW, here is output of "list-thread-groups" for our GDB:
> 
> 518,599 [MI]  10-list-thread-groups
> 518,601 [MI] 
> 10^done,groups=[{id="i1",type="process",pid="6077",executable="/home/mikhail/runtime-w\
> orkspace-i686-pc-linux-gnu/factorial/Debug/factorial",cores=["0"]}]
> 
> I am having trouble to build GDB from HEAD and don't know if it's the same as
> GDB 7.2.

Here is the output from GDB 7.2.  It is the same as yours.  And HEAD yields the same thing.

^done,groups=[{id="i1",type="process",pid="18655",executable="/local/lmckhou/testing/a.out",cores=["1"]}]
Comment 8 Marc Khouzam CLA 2010-10-03 21:58:05 EDT 
Created attachment 180122 [details]
Fix for HEAD

Here is the fix that dissociates groupId from pId.

We will now properly see the pid in the debug view when using GDB 7.2
Comment 9 Marc Khouzam CLA 2010-10-03 22:09:44 EDT 
I committed the solution to HEAD.

The fix has a breaking API change (which I documented in the CDT 8.0 New and Noteworthy), so I can't put it in the 7_0 branch.  I feel that supporting GDB 7.2 is more of a bug fix than a new feature, so I think we should try to adapt this fix to avoid API breakage for the 7_0 branch.
Comment 10 Marc Khouzam CLA 2010-10-03 22:10:24 EDT 
Mikhail, can you review the fix?
Comment 11 CDT Genie CLA 2010-10-03 22:23:03 EDT 
*** cdt cvs genie on behalf of mkhouzam ***
Bug 317500: Dissociates the groupId from the pId, since stating with GDB 7.2, the two ids are no longer the same.

[*] GDBControl_7_0.java 1.23 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_0.java?root=Tools_Project&r1=1.22&r2=1.23
[*] GDBControl.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/gdb/service/command/GDBControl.java?root=Tools_Project&r1=1.18&r2=1.19

[*] MIRunControlEventProcessor.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/MIRunControlEventProcessor.java?root=Tools_Project&r1=1.12&r2=1.13
[*] CLIEventProcessor.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/mi/service/command/CLIEventProcessor.java?root=Tools_Project&r1=1.7&r2=1.8
[*] MIRunControlEventProcessor_7_0.java 1.12 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/MIRunControlEventProcessor_7_0.java?root=Tools_Project&r1=1.11&r2=1.12

[*] MIProcesses.java 1.9 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIProcesses.java?root=Tools_Project&r1=1.8&r2=1.9
[*] IMIProcesses.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/IMIProcesses.java?root=Tools_Project&r1=1.2&r2=1.3

[*] GDBProcesses.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/GDBProcesses.java?root=Tools_Project&r1=1.7&r2=1.8
[*] GDBProcesses_7_0.java 1.27 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.26&r2=1.27
Comment 12 Marc Khouzam CLA 2010-10-03 23:50:07 EDT 
Created attachment 180123 [details]
Fix for 7_0

Here is the fix for 7_0 which duplicates some logic to avoid changing the API.

Committed to the 7_0 branch.
Comment 13 Marc Khouzam CLA 2010-10-03 23:50:24 EDT 
Fixed
Comment 15 Marc Khouzam CLA 2010-10-04 21:34:45 EDT 
I've missed some things this bug. Please see 326970
Comment 16 Nobody - feel free to take it CLA 2010-10-04 22:17:00 EDT 
(In reply to comment #15)
> I've missed some things this bug. Please see 326970

I noticed that the core disappeared but didn't have time to investigate it. 
BTW, can this patch be applied directly to 7.0.0? It didn't work with our CDT fork.
Comment 17 Marc Khouzam CLA 2010-10-04 23:35:03 EDT 
(In reply to comment #16)
> (In reply to comment #15)
> > I've missed some things this bug. Please see 326970
> 
> I noticed that the core disappeared but didn't have time to investigate it. 
> BTW, can this patch be applied directly to 7.0.0? It didn't work with our CDT
> fork.

I believe it should apply to 7.0.  But maybe there has been minor changes that cause some conflicts.  The patch (although I realize now it is not complete) is not very complicated, so the conflicts may be easy to resolve.

Or you could use the patch for the 7_0 branch.
Comment 18 Nobody - feel free to take it CLA 2010-10-04 23:42:48 EDT 
(In reply to comment #17)
> I believe it should apply to 7.0.  But maybe there has been minor changes that
> cause some conflicts.  The patch (although I realize now it is not complete) is
> not very complicated, so the conflicts may be easy to resolve.
> 

I resolved the conflicts and applied the patch, but it broke something. Either some events went missing or our gdb has more changes. I didn't have time to investigate it.

> Or you could use the patch for the 7_0 branch.

I just realized that there was the patch for 7.0.
Comment 19 Marc Khouzam CLA 2010-11-22 11:20:25 EST 
Last issue here was fixed by bug 326970