| Summary: | [non-stop] Add support for non-stop multi-threaded debugging in GDB. | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Tools] CDT | Reporter: | Pawel Piech <pawel.1.piech> | ||||||||||||||||||||||||
| Component: | cdt-debug-dsf-gdb | Assignee: | Pawel Piech <pawel.1.piech> | ||||||||||||||||||||||||
| Status: | CLOSED FIXED | QA Contact: | |||||||||||||||||||||||||
| Severity: | enhancement | ||||||||||||||||||||||||||
| Priority: | P3 | CC: | cdtdoug, dd.general-inbox, fchouinard, marc.khouzam | ||||||||||||||||||||||||
| Version: | 0 DD 1.1 | Keywords: | plan | ||||||||||||||||||||||||
| Target Milestone: | DD 1.1 | Flags: | cdtdoug:
iplog-
|
||||||||||||||||||||||||
| Hardware: | All | ||||||||||||||||||||||||||
| OS: | All | ||||||||||||||||||||||||||
| Whiteboard: | |||||||||||||||||||||||||||
| Attachments: |
|
||||||||||||||||||||||||||
|
Description
Pawel Piech
Created attachment 107226 [details]
Non-stop GDB Run Control
Summary of changes:
Non-stop mode flag
==================
Non-stop debugging is an option i.e. the "non-stop GDB" can run in either non-stop or all-stop mode. Therefore a flag needs to be added in the launch configuration to indicate which mode to run.
o IGDBLaunchConfigurationConstants
- Added a non-stop flag identifier and a default value (false)
o GdbLaunchDelegate
- Added code to read the non-stop flag from the launch configuration
- Added code to instantiate the correct ServicesFactory
- For now this is very much tied to a specific GDB version :-(
o GdbDebuggerPage
- Added a non-stop checkbox to the debugger pane
- Added code to manage the non-stop flag (in the dialog box and the launch configuration)
- Disabled (grayed) the checkbox to avoid interfere with existing code
- Still some work to do to enable/disable the checkbox based on the GDB selected
o LaunchUIMessages.properties
- Added an entry for the non-stop checkbox label (in the dialog)
o GdbDebugServicesFactoryNS
- Extends GdbDebugServicesFactory
- Non-stop GDB factory (only IRunControl needs to be changed)
Run Control
===========
Unfortunately, the non-stop feature can not be wedged elegantly in the existing MIRunControl without losing some support for legacy GDBs. So a new GDBRunControl and MIRunControl is needed for non-stop (the services factory instantiates the correct one based on GDB capabilities and the non-stop flag in the launch configuration).
To instantiate the "correct" GDB/MIRunControl and keep the rest of the code working requires that new interfaces be defined (IGDBRunControl and IMIRunControl) and that the services be adjusted to use the interface instead of the actual class. The interfaces factorize the corresponding data structures and the abstract the methods.
THIS BREAKS BINARY COMPATIBILITY.
o IRunControl.java
- Fixed a typo in a comment
o IMIRunControl
- New interface so the services can access the right MIRunControl
- Factorized out the data structures and methods signature from MIRunControl
- Introduced ThreadSuspendEvent and ThreadResumedEvent
- Breaks binary compatibility
o IGDBRunControl
- New interface (extends IMIRunControlNS) so the services can access the right GDBRunControl
- Factorized out the data structures and methods signature from GDBRunControl
- Breaks binary compatibility
o MIRunControl
- Factored out IMIRunControl
- Made it implement the new IMIRunControl
o MIRunControlNS
- Non-stop implementation of MIRunControl
- Introduced a map of MIThreadRunState to keep track of individual thread states
- Cleaned-up the code
o GDBRunControl
- Factored out IGDBRunControl
- Made it implement the new IGDBRunControl
- Made it register its service as IMIRunControl and IGDBRunControl (instead of MIRunControl and GDBRunControl)
o GDBRunControlNS
- Non-stop implementation of GDBRunControl
- Use of the new MI thread-info command instead of the crappy CLI "info thread"
- Removed code for legacy GDBs (now irrelevant)
IGDB/MIRunControl Adjustments
=============================
Some classes were modified to use the adequate run control interface instead of the actual class. In fact, this is better design (IMHO).
o GdbThreadFilterEditor
- Made it handle IGDBRunControl instead of GDBRunControl
o ContainerVMNode
- Made it handle IGDBRunControl instead of GDBRunControl
o ThreadVMNode
- Made it handle IGDBRunControl instead of GDBRunControl
o MIBreakpointsManager
- Adjusted the imports for the new IMIRunControl/IGDBRunControl
o MIMemory
- Adjusted the imports for the new IMIRunControl/IGDBRunControl
o MI Events
- Changed references to IMIRunControl/IGDBRunControl to a bunch of events
MI Commands
===========
o MI Commands
- Added an optional "--thread" parameter to a bunch of commands
- Not complete but more than enough for our needs
o CLIInfoThreadsInfo
- Adjusted (and propagated) a class name
- Breaks binary compatibility
o MIThreadInfo/MIThreadInfoInfo
- MI thread-info command that replaces CLI "info thread"
- MI thread-info result parser (new)
Event Handling
==============
o CLIEventProcessor
- Cleaned up the code (not strictly necessary)
o MIRunControlEventProcessor
- Added handling od the "stopped", "running", "thread-created" and "thread-exited" events
- Made it handle IMIRunControl instead of MIRunControl
And voila.
/fc
Created attachment 107235 [details]
MI Stack commands update
The preliminary version of GDB is a bit flaky when handling the thread-select command in non-stop mode: somehow the internal state has to be "primed" by a properly formatted MI command with the --thread option.
This is particularly obvious in the Stack service when selecting a stack frame from a stopped thread after resuming another thread.
This patch:
- Adds the --thread parameter to the MI commands used by the Stack service
- Introduces a non-stop Stack service, identical to the original one but that sets the thread parameter
- Adds the new Stack service (MIStackNS) to GdbDebugServicesFactoryNS
This patch overrides the previous one.
/fc
(In reply to bug 237325 comment #8) > In non-stop RunControl, the thread context is not available but its container > (the process) should still be. The way setContextAvailable() is implemented > right now, setting the thread as unavailable also propagates to its container > and prevents MI thread-info commands to be issued. Logically if a thread within a container is running, then the container is running as well so this seems correct to me. What the problem may be here is that there are different types of commands being issued, where target availability has different meaning. I haven't looked at the patch yet and I have a couple of other urgent issues today, but hopefully I'll have some better insight after I get a chance to understand the problem fully. (In reply to comment #3) > Logically if a thread within a container is running, then the container is > running as well so this seems correct to me. What the problem may be here is > that there are different types of commands being issued, where target > availability has different meaning. I think we are mixing two things here: "running state" and "ability to accept commands" (possibly affected by running state). Anyway, something to discuss Thursday I guess. Regards, /fc Created attachment 107620 [details]
MIRunControlNS with removed CommandCache
I thought about it a little more and realized that non-stop MIRunControl has no use for a CommandCache in the first place:
[1] The only MI command that was sent to the target through the cache was thread-info
[2] Caching thread-info is useless as the cache becomes stale every time a thread is created/terminated (requiring a cache reset each time)
This patch simply removes the command cache from MIRunControlNS and sends the thread-info command directly to the target through fConnection (AbstratcMIControl). MIGDBRunControlNS was adjusted as well.
However, we still have this issue with the Expression service. Working on it.
Best Regards,
/fc
Created attachment 107674 [details]
Handling of Container events
Yet another patch.
Modified MIRunControlNS so that a ContainerResumedEvent/ContainerSuspendedEvent is issued if appropriate (depending on the state of the contained threads.
Changed Memory and Expression so that the state of the context is set according to the state of the container (i.e. the process)".
The main problem is proper testing: the non-stop GDB is still a bit flaky and gets easily into some weird states where recovery is not easy. Also some commands don't seem to be implemented completely for non-stop.
(In reply to comment #1) > THIS BREAKS BINARY COMPATIBILITY. I started looking at your changes today. I think we should reserve the option of breaking backward compatibility as the last resort, so let's see if we could still make these changes in a way that preserves it. > o IMIRunControl > - New interface so the services can access the right MIRunControl > - Factorized out the data structures and methods signature from MIRunControl > - Introduced ThreadSuspendEvent and ThreadResumedEvent > - Breaks binary compatibility Rather than moving the data structures into IMIRunControl, duplicating them should preserve compatibility. > o IGDBRunControl > - New interface (extends IMIRunControlNS) so the services can access the right > GDBRunControl > - Factorized out the data structures and methods signature from GDBRunControl > - Breaks binary compatibility > With Marc's implementation of IProcesses service is this interface needed still? For backward compatibility, we could leave it in GDBRunControl but we could mark them as deprecated and switch to use IProcess instead. > ... > o GDBRunControl > - Factored out IGDBRunControl > - Made it implement the new IGDBRunControl > - Made it register its service as IMIRunControl and IGDBRunControl (instead of > MIRunControl and GDBRunControl) It would be good if the services still registered under the class names in addition to the service names. > ... > o CLIInfoThreadsInfo > - Adjusted (and propagated) a class name > - Breaks binary compatibility I can understand why change the name, but at this point cleaning up is not worth the cost of breaking compatibility. I'd like to try out this patch at some point, is there a public version of GDB somewhere that I could try it out with? (In reply to comment #7) > (In reply to comment #1) > > THIS BREAKS BINARY COMPATIBILITY. > I started looking at your changes today. I think we should reserve the option > of breaking backward compatibility as the last resort, so let's see if we could > still make these changes in a way that preserves it. I would tend to agree but after a lot experimenting, all I got was a big mess. I started wondering if it was worth it to bend over backwards for 1.1. Right now, I would rather consider working on a 2.0 branch. > > o IMIRunControl > > - New interface so the services can access the right MIRunControl > > - Factorized out the data structures and methods signature from MIRunControl > > - Introduced ThreadSuspendEvent and ThreadResumedEvent > > - Breaks binary compatibility > Rather than moving the data structures into IMIRunControl, duplicating them > should preserve compatibility. Not so sure: simply duplicating the structures introduces ambiguity between the all-stop and the non-stop structures. Since most services are compiled with the all-stop structure (through #import declaration), the mix miserably fails at run-time (and it's not a pretty java exception). Of course, I also considered changing/extending the structures name and casting left and right but (again) all I got was a big mess. However, it might be worth another try... > > o IGDBRunControl > > - New interface (extends IMIRunControlNS) so the services can access the right > > GDBRunControl > > - Factorized out the data structures and methods signature from GDBRunControl > > - Breaks binary compatibility > > > With Marc's implementation of IProcesses service is this interface needed > still? For backward compatibility, we could leave it in GDBRunControl but we > could mark them as deprecated and switch to use IProcess instead. Didn't look at Marc's implementation. I didn't follow that thread and he did it after I had that part done. > > ... > > o GDBRunControl > > - Factored out IGDBRunControl > > - Made it implement the new IGDBRunControl > > - Made it register its service as IMIRunControl and IGDBRunControl (instead of > > MIRunControl and GDBRunControl) > It would be good if the services still registered under the class names in > addition to the service names. Why bother? Wouldn't it be a lot cleaner to get the service providing the interface and rely on the service factory to instantiate the appropriate one? > > ... > > o CLIInfoThreadsInfo > > - Adjusted (and propagated) a class name > > - Breaks binary compatibility > I can understand why change the name, but at this point cleaning up is not > worth the cost of breaking compatibility. As I mentioned, this one is not strictly necessary. But in the 2.0 perspective... > I'd like to try out this patch at some point, is there a public version of GDB > somewhere that I could try it out with? All I have a is a buggy version that dates from March/27. If you want I can send you a zip of the source files. As all Linux stuff, it is supremely easy to build. Keep the comments coming! /fc (In reply to comment #8) > ... Right now, I would rather consider working on a 2.0 branch. Keeping backward compatibility is definitely more work, but in the least I think it's a very worthwhile exercise. Large part of Eclipse's success is its focus on backward compatibility, and there's a lot of ways to do it we just need to get familiar with them. > ... > Since most services are compiled with the all-stop structure (through #import > declaration)... All of these structures were declared protected in MIRunControl and I hoped that would be enough to signal that they should not be used outside that class hierarchy. On a quick scan, the only service I saw referencing these is MIBreakpointsManager, which uses MIExecutionDMC and which could easily be converted to use IMIExecutionDMContext. However, this is an indication of an API that's too open. In MIRunControlNS, I would suggest to make these structures private, and if needed to create interfaces for those structures. However for the most part, I don't see what these structures provide that could not be accessed already through the existing interfaces. > Didn't look at Marc's implementation. I didn't follow that thread and he did > it after I had that part done. That's what I figured and that's why I brought it up :-) > Why bother? Wouldn't it be a lot cleaner to get the service providing the > interface and rely on the service factory to instantiate the appropriate one? There may be clients out there which were written to access these services using the class... because the interfaces didn't exist in 1.0. > All I have a is a buggy version that dates from March/27. If you want I can > send you a zip of the source files. As all Linux stuff, it is supremely easy > to build. I'd rather wait for something a little more stable, as next milestone is in six weeks, it should be coming soon :-). > Keep the comments coming! I'll try :-) (In reply to comment #9) > Keeping backward compatibility is definitely more work, but in the least I > think it's a very worthwhile exercise. Large part of Eclipse's success is its > focus on backward compatibility, and there's a lot of ways to do it we just > need to get familiar with them. Let's look at a practical case. Some classes were modified to use IGDBRunControl instead of GDBRunControl for the obvious reason that non-stop and all-stop have different implementations. Also they extend a different MIRunControl (relevant since the client classes perform IMIRunControl calls). The classes that were touched are: o GdbThreadFilterEditor o ContainerVMNode o ThreadVMNode The following GDBRunControl methods were factored out (in IGDBRunControl): o getProcessData(GDBControlDMContext c, DataRequestMonitor<GDBProcessData> drm) o getThreadData(IMIExecutionDMContext c, DataRequestMonitor<GDBThreadData> drm) For obvious reasons, moving the GDBProcessData and GDBThreadData structures from GDBRunControl to IGDBRunControl breaks binary compatibility. Modifying the signatures of the methods would also break compatibility. Since I am in "getting familiar" mode and that there are "lots of ways to do it", would you have an elegant one to recommend? :-) I can think of (at least) one but it sucks. (In reply to comment #10) The simplest solution, which I mentioned before would be to deprecate these and switch to using IProcesses :-) But if that option was not available... I would create interfaces for these structures (IGDBThreadData and IGDBProcessData) and I would make the existing structures implement it. The I would duplicate these structures in the new GDBRunControlNS, but keep them private there. Finally, I would change the existing methods in GDBRunControl to use the interfaces in their signatures. This would not break binary compatibility... since the generics type information is erased at compile time. As a final note though, we kept the services in the GDB plugin in a provisional package, so you're not restricted from changing these.... although I would still do as described above. (In reply to comment #11) > ... since the generics type information is erased at compile time. Didn't know about that. However, on the elegance front... :-) Created attachment 107792 [details]
Updated patch
Marc,
Let me know if this one suits your need. Then I'll check it in.
Ciao,
/fc
(In reply to comment #13) > Let me know if this one suits your need. Then I'll check it in. I went through the patch and it fits quite well with what I am doing. I have some comments, which I can do as part of my patch if it is easier: - I will also change some VMNode classes because for multi-process, they need to use the IProcesses service. So, in the end, I will end up having to do Pawel's other suggestion of comment #11 > The simplest solution, which I mentioned before would be to deprecate these > and switch to using IProcesses Once this is done, it will make IGDBRunControl and GDBRunControlNS not needed at all. - I forgot to tell you that in the meeting yesterday, Pawel suggested that the RunControl service still register itself also with the old names, just in case some other users used those names. So, GDBRunControl should still register with the name MIRunControl. I'm not sure if GDBRunControlNS should do that too... - the MIMemory service does not use the variable fRunControl - I'm hoping we can get rid of MIStackNS once we get a gdb that works better (and before we release this class to the public :-)) - MIThreadInfo's second constructor should take IMIExecutionDMContext as its only parameter. But after extracting the threadId from that context, it should pass the parent Container to the super() call or else we'll have useless thread-select calls... This is a new issue actually, and should get its own bug. -MIRunControlNS.getThreadData call thread-info on all threads but then only keep the result for one of them. I guess it should call -thread-info on a single thread. Looking forward to building on top of this. I also looked through the patch as well. I agree with Marc's comments and I have a couple more: 1) Some of the new constructors in the MI commands have ambiguous signatures. E.g.: MIExecContinue(IExecutionDMContext dmc, boolean allThreads) MIExecContinue(IMIExecutionDMContext dmc, boolean setThread) I've seen bugs in situations like this where the client calls the first constructor while intending to call the second. 2) In general, I think it would be cleaner not to add the --thread to these commands directly, but rether add the smarts to GDBControl or MIControl to do it instead. After all, when using the --thread option, the preceding -thread-select is not needed and may even create confusion when tracking of the proposed =thread-selected event is implemented. However this is an optimization in a way, so given the rush to implement non-stop debugging by next milestone, it may be better to open another bug for this. Created attachment 108117 [details]
Non-stop multi-threading patch
(In reply to comment #14) > - I will also change some VMNode classes because for multi-process, they need > to use the IProcesses service. So, in the end, I will end up having to do > Pawel's other suggestion of comment #11 > > The simplest solution, which I mentioned before would be to deprecate > these > > and switch to using IProcesses > Once this is done, it will make IGDBRunControl and GDBRunControlNS not needed > at all. I am introducing non-stop multi-threading. Feel free to do whatever you like for multi-process. > - I forgot to tell you that in the meeting yesterday, Pawel suggested that the > RunControl service still register itself also with the old names, just in case > some other users used those names. So, GDBRunControl should still register > with the name MIRunControl. I'm not sure if GDBRunControlNS should do that > too... Done for GDBRunControl. Definitely not done for GDBRunControlNS. > - the MIMemory service does not use the variable fRunControl I had left for some reason that I long forgot. Gone. > - I'm hoping we can get rid of MIStackNS once we get a gdb that works better > (and before we release this class to the public :-)) Good luck. > - MIThreadInfo's second constructor should take IMIExecutionDMContext as its > only parameter. But after extracting the threadId from that context, it should > pass the parent Container to the super() call or else we'll have useless > thread-select calls... This is a new issue actually, and should get its own > bug. I disagree because it would force a thread selection which has no meaning for some non-stop commands. > -MIRunControlNS.getThreadData call thread-info on all threads but then only > keep the result for one of them. I guess it should call -thread-info on a > single thread. Done. (In reply to comment #15) > 1) Some of the new constructors in the MI commands have ambiguous signatures. > E.g.: > MIExecContinue(IExecutionDMContext dmc, boolean allThreads) > MIExecContinue(IMIExecutionDMContext dmc, boolean setThread) I used the boolean setThread for symmetry. Removed it. > 2) In general, I think it would be cleaner not to add the --thread to these I don't really agree with that one. I believe that we should use --thread everywhere we can and get rid of the automatic thread selection which was an constant source of problems for non-stop. Patch committed. (In reply to comment #18) > > 2) In general, I think it would be cleaner not to add the --thread to these > > I don't really agree with that one. I believe that we should use --thread > everywhere we can and get rid of the automatic thread selection which was an > constant source of problems for non-stop. Hmm, it really sounds to me like we want the same thing, so I don't understand why we disagree on the implementation. If we get rid of the use of -thread-select completely (at least for the MI commands, since CLI commands still use it), wouldn't it make sense to contain that change within a single module: MIControl? By using the explicit constructor on all these commands, the knowledge of whether --thread is used vs. -thread-select is spread over most of the services. (In reply to comment #19) > Patch committed. Congrats! I'll get started on adding the preliminary multi-process stuff that has already been approved. I was able to make my changes quite well (I think :-)) with the non-stop code. And while doing those, I got to wonder a couple of things: In MIMemory, is it ok to change the EventHandlers signature with respect to API compatibility? e.g., From public void eventDispatched(IRunControl.IResumedDMEvent e) to public void eventDispatched(IRunControl.IContainerResumedDMEvent e) In MIRunControlEventProcessor, we now issue a MIRunningEvent, when getting the "running" event from GDB. However, we still issue MIRunningEvents when getting ^running (also in MIRunControlEventProcessor) and when using the console to make the program run (CLIEventProcessor). This is not a problem for the old GDBs since *running does not exist, but won't it be a problem with the new non-stop GDB? I think we have the same issue in MIRunControlEventProcessor for ThreadCreatedEvent which is now issued with the new =thread-created but also in the CLIEventProcessor when ~"[New Thread 0xb7c76ba0 (LWP 18630)]\n" is seen. ThreadExitedEvent is also affected for all-stop with the new GDB, but not when actually running in non-stop mode, I think. If this is true, we may have to extend the service factory to the EventProcessor classes. Or have a different GDBControl version, which actually instantiates the correct event processors. (In reply to comment #22) > In MIMemory, is it ok to change the EventHandlers signature with respect to API > compatibility? e.g., > From > public void eventDispatched(IRunControl.IResumedDMEvent e) > to > public void eventDispatched(IRunControl.IContainerResumedDMEvent e) Technically yes, although it could be argued that these listener methods were not intended to be overridden. In the future, if that's the case it would be best to add a comment saying so and that would be enough to satisfy the API compatibility issue. As it is, this method should be changed back to satisfy the compatibility requirement. > If this is true, we may have to extend the service factory to the > EventProcessor classes. Or have a different GDBControl version, which actually > instantiates the correct event processors. I thought GDBControl was already being created by the service factory? If not, I think it should be as the protocol changes are quite great and will likely require a new version anyway. Created attachment 108341 [details]
Missing or removed check for threadId 0
The modification to AbstractMIControl did not keep the check for threadId 0. Is that intentional? I guess, we should not make the assumption that thread 0 is not valid, but use the threadId from GDB?
If this was not intentional, then this patch can be committed
(In reply to comment #24) > Created an attachment (id=108341) [details] > Missing or removed check for threadId 0 > > The modification to AbstractMIControl did not keep the check for threadId 0. > Is that intentional? I guess, we should not make the assumption that thread 0 > is not valid, but use the threadId from GDB? > > If this was not intentional, then this patch can be committed > Why do you assume that? How did you reproduce the faulty behavior? Have you tested with GDB 6.6, 6.7, 6.8 and non-stop (like I did)? (In reply to comment #25) > (In reply to comment #24) > > Created an attachment (id=108341) [details] [details] > > Missing or removed check for threadId 0 > > > > The modification to AbstractMIControl did not keep the check for threadId 0. > > Is that intentional? I guess, we should not make the assumption that thread 0 > > is not valid, but use the threadId from GDB? > > > > If this was not intentional, then this patch can be committed > > > > Why do you assume that? How did you reproduce the faulty behavior? Have you > tested with GDB 6.6, 6.7, 6.8 and non-stop (like I did)? > To reproduce, I have a multi-threaded app that I do a local attach launch to. For some reason, thread-list-ids gets called before we have even attached, and at this point, there are no threads, so internally, we create thread 0. Then, when we attach, thread-list-ids is cached with no threads. Bug 241844 is about the fact that an attach should clear the cache. But it is in this case that I can see a thread-select 0, which GDB does not like. That is why I wondered if you removed the check on purpose or not. (In reply to comment #26) > To reproduce, I have a multi-threaded app that I do a local attach launch to. > For some reason, thread-list-ids gets called before we have even attached, and > at this point, there are no threads, so internally, we create thread 0. Then, > when we attach, thread-list-ids is cached with no threads. Bug 241844 is about > the fact that an attach should clear the cache. But it is in this case that I > can see a thread-select 0, which GDB does not like. Who creates that dummy thread 0 and is it really needed? The way MIRunControlNS is designed, the thread list should match exactly what GDB reports. (In reply to comment #27) > Who creates that dummy thread 0 and is it really needed? The way MIRunControlNS > is designed, the thread list should match exactly what GDB reports. > In MIRunControl.makeExecutionDMCs, it creates thread 0 if there is not thread listed. The comment explains that thread-list-ids is empty for single threaded programs, so thread 0 is 'fake'. I guess that is how we avoid sending any thread-select for single threaded programs. Created attachment 108373 [details] Fix for non-thread ID (In reply to comment #28) > (In reply to comment #27) > > Who creates that dummy thread 0 and is it really needed? The way MIRunControlNS > > is designed, the thread list should match exactly what GDB reports. > > > In MIRunControl.makeExecutionDMCs, it creates thread 0 if there is not thread > listed. The comment explains that thread-list-ids is empty for single threaded > programs, so thread 0 is 'fake'. I guess that is how we avoid sending any > thread-select for single threaded programs. Since threadID -1 is the "non-thread" in the rest of the application, I modified MIRunControl accordingly. (In reply to comment #29) > Since threadID -1 is the "non-thread" in the rest of the application, I > modified MIRunControl accordingly. I haven't tried it (my workspace is in flux), but I think this is ok, except that the label of the thread will now show -1 in the debug view, I think. But maybe a check for this is enough. (In reply to comment #30) > I haven't tried it (my workspace is in flux), but I think this is ok, except > that the label of the thread will now show -1 in the debug view, I think. But > maybe a check for this is enough. If thread 0 was not displayed previously, one wonders why thread -1 would be. If you have such a case, I would be interested to know how you got it. (In reply to comment #31) > (In reply to comment #30) > > I haven't tried it (my workspace is in flux), but I think this is ok, except > > that the label of the thread will now show -1 in the debug view, I think. But > > maybe a check for this is enough. > > If thread 0 was not displayed previously, one wonders why thread -1 would be. > If you have such a case, I would be interested to know how you got it. I have to admit, it is a fluke that I noticed this. If you compile a single-threaded application without the -lpthread flag, then GDB does not report any threads, and we use the number 0. You would see this in the debug view. For a single-threaded app compiled with -lpthread GDB does report one thread and its number is 1. And -thread-select is accepted. Created attachment 108457 [details] Default thread ID when GDB doesn't report any (In reply to comment #32) > I have to admit, it is a fluke that I noticed this. If you compile a > single-threaded application without the -lpthread flag, then GDB does not > report any threads, and we use the number 0. You would see this in the debug > view. > > For a single-threaded app compiled with -lpthread GDB does report one thread > and its number is 1. And -thread-select is accepted. Interesting. Here's a patch. Hopefully we won't have cases where there are 2 threads "1". I will commit the patch. (In reply to comment #33) > Created an attachment (id=108457) [details] > Default thread ID when GDB doesn't report any > > (In reply to comment #32) > > I have to admit, it is a fluke that I noticed this. If you compile a > > single-threaded application without the -lpthread flag, then GDB does not > > report any threads, and we use the number 0. You would see this in the debug > > view. > > > > For a single-threaded app compiled with -lpthread GDB does report one thread > > and its number is 1. And -thread-select is accepted. > > Interesting. Here's a patch. Hopefully we won't have cases where there are 2 > threads "1". I also thought that was the way to go, but then I realized that we are going to be doing thread-select on thread 1 in this case, and thread-select is not allowed if you have not compiled with -lpthread. You need to somehow block the thread-select, which I guess is easier to do with the fake threadId being 0. (In reply to comment #34) > I also thought that was the way to go, but then I realized that we are going to > be doing thread-select on thread 1 in this case, and thread-select is not > allowed if you have not compiled with -lpthread. You need to somehow block the > thread-select, which I guess is easier to do with the fake threadId being 0. What do you mean "thread-select is not allowed if you have not compiled with -lpthread"? Where do you see that in GDB's documentation? To reproduce your problem I compiled a sample app without -lpthread. And it worked fine with the default threadId (=1). Would you mind detailing what you did? (In reply to comment #35) > What do you mean "thread-select is not allowed if you have not compiled with > -lpthread"? Where do you see that in GDB's documentation? No documentation :-) I get "Thread ID 1 not known." from GDB > To reproduce your problem I compiled a sample app without -lpthread. And it > worked fine with the default threadId (=1). > > Would you mind detailing what you did? I created a new project in eclipse, with a single threaded program. Compiled it without -lpthread (new project doesn't use pthread by default) and the launched it. I get "Thread ID 1 not known." and things don't work anymore. (In reply to comment #36) > (In reply to comment #35) > I created a new project in eclipse, with a single threaded program. Compiled > it without -lpthread (new project doesn't use pthread by default) and the > launched it. I get "Thread ID 1 not known." and things don't work anymore. Which launch configuration? (In reply to comment #37) > (In reply to comment #36) > > (In reply to comment #35) > > I created a new project in eclipse, with a single threaded program. Compiled > > it without -lpthread (new project doesn't use pthread by default) and the > > launched it. I get "Thread ID 1 not known." and things don't work anymore. > > Which launch configuration? Local Created attachment 108503 [details]
Hacked threadId
Patch committed.
Created attachment 108505 [details] public GDB now supports non-stop The public GDB now supports non-stop for linux according to: http://sourceware.org/ml/gdb-patches/2008-07/msg00174.html This patch fixes the version number we were using and allows the user to click the non-stop checkbox in the launch. Committed. To try it: 1- Checkout the latest GDB HEAD according to http://sourceware.org/gdb/current/ You will get a src/ directory. 2- build it according to <fullpath>/src/gdb/README, which comes down to: mkdir build cd build <fullpath>/src/configure make 3- update your workpace to the latest DSF 4- launch DSF and have a multi-threaded app (our Sanity app is fine). In the local launch pop-up, in the Debugger tab, choose the newly built GDB and check the non-stop checkbox While debugging, you should see threads stopped while others are running. You can then suspend the running threads, etc. (In reply to comment #40) Cool I'll try to get this set up sometime this week. Re-opening to assign for verification Pawel, I know you have a lot of bugs to verify, but since you are the only one besides Francois and I that has tried non-stop, can you mark this as verified (assuming you got it to work :-)) Thanks Re-open to re-assign Re-marking as fixed. Pawel, if you can verify that non-stop works for you? I just did, as I was sanity testing the final 1.1 build. For debugging multi-threaded debugging this is a must have feature IMO, so I hope it'll help us win over some new users when GDB 7 is released. DD 1.1 reelased! |