Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 335324 - [multi-process][breakpoints] Make the container (process) the breakpointTarget context
Summary: [multi-process][breakpoints] Make the container (process) the breakpointTarge...
Status: RESOLVED FIXED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-debug-dsf-gdb (show other bugs)
Version: 8.0   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: 8.0   Edit
Assignee: Marc Khouzam CLA
QA Contact: Marc Khouzam CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 237306
  Show dependency tree
 
Reported: 2011-01-25 10:03 EST by Marc Khouzam CLA
Modified: 2011-02-10 09:12 EST (History)
4 users (show)

See Also:
nobody: review+

Attachments
done cleanups to Marc's patch , working patch. (18.58 KB, patch)
2011-01-27 01:40 EST, Onur Akdemir CLA 		marc.khouzam: iplog+
Details | Diff
Enhanced Onur's contribution (16.91 KB, patch)
2011-02-04 16:29 EST, Marc Khouzam CLA 		marc.khouzam: iplog-
Details | Diff
Prepare JUnit tests for the container being the bpTargetDmc (11.60 KB, patch)
2011-02-04 23:24 EST, Marc Khouzam CLA 		marc.khouzam: iplog-
Details | Diff
Move IBreakpointsTargetDMContext to MIContainerDMC (5.69 KB, patch)
2011-02-08 10:09 EST, Marc Khouzam CLA 		marc.khouzam: iplog-
Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Marc Khouzam CLA 2011-01-25 10:03:22 EST 
To properly support multi-process, breakpoints must be set for each process.  Therefore, the IBreakpointsTargetDMContext must be a container (process) instead of being the commandControl (GDBControlDMContext).

This is a pretty drastic change that may impact extenders of DSF-GDB.

I'm still not sure if I should remove IBreakpointsTargetDMContext from GDBControlDMContext, or leave it there for backwards-compatibility.  Leaving it there will probably only delay finding issues...  We will still have exceptions when the code assumes the breakpointTarget context is a container and uses it as such.

I'm creating this bug so it can be better reviewed by the affected parties.
Comment 1 Onur Akdemir CLA 2011-01-27 01:40:33 EST 
Created attachment 187712 [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 .
Comment 2 Marc Khouzam CLA 2011-02-04 16:29:22 EST 
Created attachment 188369 [details]
Enhanced Onur's contribution

This patch enhances Onur's contribution to adapt it to the latest changes that I did in bug 335528.

I have to run a couple more tests and update the JUnit tests and then I will commit.
Comment 3 Marc Khouzam CLA 2011-02-04 23:24:06 EST 
Created attachment 188384 [details]
Prepare JUnit tests for the container being the bpTargetDmc

This patch makes the JUnit tests ready for the change of bpTargetDmc.  I made them generic so that they work before and after the change.

Committed to HEAD.
Comment 4 Marc Khouzam CLA 2011-02-04 23:36:06 EST 
I've committed "Enhanced Onur's contribution" to HEAD.

The major change is that GDBControlDMContext is no longer an IBreakpointsTargetDMContext.

This means that code that casts the commandControlContext to IBreakpointsTargetDMContext will now fail.  For example:
(IBreakpointsTargetDMContext)fCommandControl.getContext()

I have documented this change in the What's new for CDT 8.0
Comment 5 Marc Khouzam CLA 2011-02-04 23:37:13 EST 
Mikhail, can you review when you have time?

Onur, thank you for you contributions.  We are almost there :-)
Comment 6 Onur Akdemir CLA 2011-02-07 01:54:29 EST 
(In reply to comment #5)
> Mikhail, can you review when you have time?
> 
> Onur, thank you for you contributions.  We are almost there :-)

I hope so :)
Comment 7 Nobody - feel free to take it CLA 2011-02-07 18:56:42 EST 
I am a little bit confused with the context hierarchy and this may be a silly question re.IMIContainerDMContext. It represents the so-called thread groups in GDB/MI. As far as I understand any machine/core/process is a GDB/MI thread group. This patch assumes that any GDB/MI thread group will have the capability of being a breakpoint target. Is this a correct assumption?
Comment 8 Marc Khouzam CLA 2011-02-07 21:42:32 EST 
(In reply to comment #7)
> I am a little bit confused with the context hierarchy and this may be a silly
> question re.IMIContainerDMContext. It represents the so-called thread groups in
> GDB/MI. As far as I understand any machine/core/process is a GDB/MI thread
> group. This patch assumes that any GDB/MI thread group will have the capability
> of being a breakpoint target. Is this a correct assumption?

That is a good point.
Currently, a thread-group is a process, but you are right that GDB has it generic and could use it for things like cores.

I'm not sure if all those concepts would be breakpoint targets.
I was actually hesitant about making IMIContainerDMContext the IBreakpointTarget, but I couldn't come up with a good reason why not.  But you did :-)

I think what I should do is make all MIContainerDMContext (which implement IMIContainerDMContext) be the IBreakpointTarget.

Does that make more sense to you Mikhail?

Thanks for catching this!
Comment 9 Nobody - feel free to take it CLA 2011-02-08 10:06:07 EST 
(In reply to comment #8)

> I think what I should do is make all MIContainerDMContext (which implement
> IMIContainerDMContext) be the IBreakpointTarget.
> 
> Does that make more sense to you Mikhail?

I was going to suggest that too. I think it would be better to use DMContexts.getAncestorOfType instead of castings to IBreakpointTarget in the patch.
Comment 10 Marc Khouzam CLA 2011-02-08 10:09:11 EST 
Created attachment 188521 [details]
Move IBreakpointsTargetDMContext to MIContainerDMC

(In reply to comment #9)
> (In reply to comment #8)
> 
> > I think what I should do is make all MIContainerDMContext (which implement
> > IMIContainerDMContext) be the IBreakpointTarget.
> > 
> > Does that make more sense to you Mikhail?
> 
> I was going to suggest that too. 

This patch does this.

> I think it would be better to use
> DMContexts.getAncestorOfType instead of castings to IBreakpointTarget in the
> patch.

You are right.
I had missed one case where I should have used getAncestorOfType(), which this patch also fixes.

Ok with you?
Comment 11 Nobody - feel free to take it CLA 2011-02-08 10:22:02 EST 
Looks good to me.
Comment 12 Marc Khouzam CLA 2011-02-08 10:25:51 EST 
(In reply to comment #11)
> Looks good to me.

I committed "Move IBreakpointsTargetDMContext to MIContainerDMC" to HEAD.

Thanks for the review.
Comment 13 CDT Genie CLA 2011-02-10 09:12:28 EST 
*** cdt cvs genie on behalf of mkhouzam ***
Bug 335324: Make the container (process) the breakpointTarget context

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

[*] GDBProcesses_7_2.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/gdb/service/GDBProcesses_7_2.java?root=Tools_Project&r1=1.2&r2=1.3
[*] GDBProcesses_7_0.java 1.32 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.31&r2=1.32

[*] MIBreakpointsTest.java 1.20 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/MIBreakpointsTest.java?root=Tools_Project&r1=1.19&r2=1.20
[*] MICatchpointsTest.java 1.7 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/MICatchpointsTest.java?root=Tools_Project&r1=1.6&r2=1.7

[*] SyncUtil.java 1.17 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/framework/SyncUtil.java?root=Tools_Project&r1=1.16&r2=1.17

[*] GDBRemoteTracepointsTest_7_0.java 1.6 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.tests.dsf.gdb/src/org/eclipse/cdt/tests/dsf/gdb/tests/tests_7_0/GDBRemoteTracepointsTest_7_0.java?root=Tools_Project&r1=1.5&r2=1.6

[*] GDBJtagDSFFinalLaunchSequence.java 1.12 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/jtag/org.eclipse.cdt.debug.gdbjtag.core/src/org/eclipse/cdt/debug/gdbjtag/core/GDBJtagDSFFinalLaunchSequence.java?root=Tools_Project&r1=1.11&r2=1.12

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

[*] GDBControlDMContext.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/gdb/service/command/GDBControlDMContext.java?root=Tools_Project&r1=1.3&r2=1.4

[*] StartOrRestartProcessSequence_7_0.java 1.2 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/gdb/service/StartOrRestartProcessSequence_7_0.java?root=Tools_Project&r1=1.1&r2=1.2
[*] GDBProcesses_7_2.java 1.2 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.1&r2=1.2
[*] GDBRunControl_7_0_NS.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/GDBRunControl_7_0_NS.java?root=Tools_Project&r1=1.26&r2=1.27

[*] FinalLaunchSequence.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/launching/FinalLaunchSequence.java?root=Tools_Project&r1=1.18&r2=1.19