| Summary: | [debug view][cdi] Thread list in Debug view not updated with non-Linux target | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Tools] CDT | Reporter: | Peter Saunders <pts001> | ||||||||||||||
| Component: | cdt-debug-dsf-gdb | Assignee: | John Cortell <john.cortell> | ||||||||||||||
| Status: | RESOLVED FIXED | QA Contact: | Marc Khouzam <marc.khouzam> | ||||||||||||||
| Severity: | normal | ||||||||||||||||
| Priority: | P3 | CC: | cdtdoug, john.cortell, john, malaperle, marc.khouzam, pawel.1.piech | ||||||||||||||
| Version: | 6.0 | Flags: | marc.khouzam:
review+
|
||||||||||||||
| Target Milestone: | 7.0 | ||||||||||||||||
| Hardware: | PC | ||||||||||||||||
| OS: | Linux-GTK | ||||||||||||||||
| Whiteboard: | |||||||||||||||||
| Attachments: |
|
||||||||||||||||
(In reply to comment #0) > 1. Start debugging an non-Linux executable which creates multiple threads > dynamically using DSF GDB What kind of executable is it? > 701,356 ~"[New Thread 0xb7fd5b90 (LWP 13030)]\n" As you pointed out, we currently count on getting this event. > 860,365 *running,thread-id="1" > 860,365 (gdb) > 860,365 *running,thread-id="all" How come these events are sent in this case, but not in the Linux case. Can we use these events as a hint of a thread creation? > Unlike the DSF GDB debugger, the standard CDI GDB debugger invokes > "info threads" every time execution is suspended Yes, one of the optimizations of using DSF is that commands are only sent when necessary. That is why we -need- the event to indicate a new thread was created. Without it, DSF-GDB thinks the thread list is the same and uses a cached result instead of sending a new -thread-list-ids (In reply to comment #1) > > 701,356 ~"[New Thread 0xb7fd5b90 (LWP 13030)]\n" > > As you pointed out, we currently count on getting this event. > > > 860,365 *running,thread-id="1" > > 860,365 (gdb) > > 860,365 *running,thread-id="all" > > How come these events are sent in this case, but not in the Linux case. Can > we use these events as a hint of a thread creation? Those events simply indicate which threads are now running. Various GDB servers on various operating systems will deliver subtly different sets of events. The GDB documentation indicates that the precise implementation of thread support will vary: http://sourceware.org/gdb/current/onlinedocs/gdb_5.html#SEC28 > > Unlike the DSF GDB debugger, the standard CDI GDB debugger invokes > > "info threads" every time execution is suspended > > Yes, one of the optimizations of using DSF is that commands are only sent when > necessary. That is why we -need- the event to indicate a new thread was > created. Without it, DSF-GDB thinks the thread list is the same and uses a > cached result instead of sending a new -thread-list-ids I welcome the efficiency of DSF-GDB, the user interface is much more responsive than with the CDI GDB debugger. But I expect that a few compromises will be necessary before DSF-GDB is usable across a variety of remote targets. Perhaps DSF-GDB should poll the thread list each time the target is suspended until a "New Thread" event is received. Once a "New Thread" event is received, DSF-GDB can assume it does not need to poll the thread list. The gdbserver process registers to receive thread create events (TD_CREATE) via libthread_db. The current DSF GDB debugger code relies on these thread create events and is therefore not compatible with multi-threaded remote targets which use a lightweight GDB remote stub (rather than using gdbserver). The CDI debugger does not rely on thread create events and therefore _is_ compatible with such targets. If DSF GDB is to be adopted by users of existing deeply embedded multi-threaded operating systems such as eCos, it will need to support polling for new thread information in the way that the CDI debugger does rather than relying on thread create events. Could this be made a configurable option? Created attachment 158286 [details] CLIEventProcessor.java.patch This patch relaxes the match for "New Thread" events, allowing correct operation in non-LWP contexts. See also bug 280607. I believe using GDB 7.0 or later will allow for this problem to go away. Is this a possibility? It doesn't solve the bug but it makes it less important. There are two distinct issues here:
a) The new thread event pushed by GDB is ignored by DSF-GDB if the thread is non-LWP. The thread list in the Debug view is therefore not refreshed and the new thread is not seen. This issue can be observed in many debug scenarios. The attached patch addresses this issue.
b) With some remote debug targets, GDB cannot detect a new thread until either the target becomes suspended in the new thread or an "info threads" ("-thread-info") command is issued. The option to force a refresh of the thread list can therefore be very useful. The refresh could happen on demand (via a "refresh" button) or programatically (via a "refresh on suspend" debugging option) like CDI-GDB.
These issues are observed with GDB 6.8. I do not know if GDB 7.0 makes any difference.
Peter, can you describe or attach a "non-Linux executable" so we can reproduce the issue? It's not clear to me what that is. Marc asked in comment # 1 but didn't get an answer. (In reply to comment #7) > Peter, can you describe or attach a "non-Linux executable" so we can reproduce > the issue? It's not clear to me what that is. Marc asked in comment # 1 but > didn't get an answer. I was looking at debugging a remote multi-threaded eCos RTOS application at the time, but the issue is likely to be seen with other non-POSIX embedded operating systems that provide a lightweight GDB remote stub for debugging. The Linux gdbserver uses libthread_db to plant a thread event breakpoint in the thread creation code. I expect this ensures that GDB notices the new thread immediately. In that case, perhaps the easiest way to demonstrate the reported issue would be to build gdbserver with the call to create_thread_event_breakpoint() in linux-thread-db.c disabled in some way. (In reply to comment #8) > I was looking at debugging a remote multi-threaded eCos RTOS application at the > time, but the issue is likely to be seen with other non-POSIX embedded > operating systems that provide a lightweight GDB remote stub for debugging. > > The Linux gdbserver uses libthread_db to plant a thread event breakpoint in the > thread creation code. I expect this ensures that GDB notices the new thread > immediately. In that case, perhaps the easiest way to demonstrate the reported > issue would be to build gdbserver with the call to > create_thread_event_breakpoint() in linux-thread-db.c disabled in some way. Ah... I didn't get from your description that you were doing remote debugging. Makes sense now. Thanks for the additional info. (In reply to comment #6) > There are two distinct issues here: > > a) The new thread event pushed by GDB is ignored by DSF-GDB if the thread is > non-LWP. The thread list in the Debug view is therefore not refreshed and the > new thread is not seen. This issue can be observed in many debug scenarios. The > attached patch addresses this issue. Peter, I resolved this issue back on March 4th with bug # 304754. Please verify that it works with your scenario. Marc, I think the suggestion to have a preference makes sense. The majority of CDT users don't debug with a gdb that doesn't send thread creation events, so we don't want to impact their step performance. But at the same time we need to allow the ones that do a way to overcome this limitation. So I'm thinking about a DSF-GDB global preference called "Poll thread list on suspend". I don't see online help for that page, so I'll either add it or include a hover tip explaining when a user would want to turn on the option. What do you think? (In reply to comment #6) > b) With some remote debug targets, GDB cannot detect a new thread until either > the target becomes suspended in the new thread or an "info threads" > ("-thread-info") command is issued. The option to force a refresh of the thread > list can therefore be very useful. The refresh could happen on demand (via a > "refresh" button) I should have mentioned that this is already available in DSF-GDB, but you have to turn it on. Window->"Customize perspective..." then choose the tab "Command group availability" and check "Debug Update Modes" > or programatically (via a "refresh on suspend" debugging > option) like CDI-GDB. That would be more user friendly. > These issues are observed with GDB 6.8. I do not know if GDB 7.0 makes any > difference. Not if it is the stub that is not reporting the thread creation. (In reply to comment #11) > Marc, I think the suggestion to have a preference makes sense. ... > So I'm thinking about > a DSF-GDB global preference called "Poll thread list on suspend". +1 > I don't see > online help for that page, so I'll either add it or include a hover tip > explaining when a user would want to turn on the option. What do you think? I'm still new to the help stuff, so it is my fault that there is no help on the option page. Thank you for dealing with it. (In reply to comment #10) >> a) The new thread event pushed by GDB is ignored by DSF-GDB if the thread is >> non-LWP. The thread list in the Debug view is therefore not refreshed and the >> new thread is not seen. This issue can be observed in many debug scenarios. >> The attached patch addresses this issue. > > Peter, I resolved this issue back on March 4th with bug # 304754. Please > verify that it works with your scenario. The patch file looks good to me. I will verify. (In reply to comment #11) > Marc, I think the suggestion to have a preference makes sense. The majority of > CDT users don't debug with a gdb that doesn't send thread creation events, so > we don't want to impact their step performance. But at the same time we need > to allow the ones that do a way to overcome this limitation. So I'm thinking > about a DSF-GDB global preference called "Poll thread list on suspend". I > don't see online help for that page, so I'll either add it or include a hover > tip explaining when a user would want to turn on the option. What do you > think? To be clear, when using a lightweight GDB remote stub for debugging, thread creation events are still raised, but they are not raised immediately. An "info threads" (or MI "-thread-list-ids") command will cause GDB to request the current thread list from the remote target. GDB will then raise thread creation events for each new thread reported by the target. Personally, I don't think "poll thread list on suspend" behaviour should be a global preference. Some users will need to toggle this behaviour frequently as they move between non-Linux remote debugging and other debugging activities. It seems better to configure this in the launch configuration. (In reply to comment #12) >> b) With some remote debug targets, GDB cannot detect a new thread until >> either the target becomes suspended in the new thread or an "info threads" >> ("-thread-info") command is issued. The option to force a refresh of the >> thread list can therefore be very useful. The refresh could happen on demand >> (via a "refresh" button) > > I should have mentioned that this is already available in DSF-GDB, but you > have to turn it on. Window->"Customize perspective..." then choose the tab > "Command group availability" and check "Debug Update Modes" Thanks for the info. I will experiment with this. (In reply to comment #13) > To be clear, when using a lightweight GDB remote stub for debugging, thread > creation events are still raised, but they are not raised immediately. An "info > threads" (or MI "-thread-list-ids") command will cause GDB to request the > current thread list from the remote target. GDB will then raise thread creation > events for each new thread reported by the target. Kind of a useless event by that time :-) Should not bother us to much as long as we check for threads that exist already and allow it. > Personally, I don't think "poll thread list on suspend" behaviour should be a > global preference. Some users will need to toggle this behaviour frequently as > they move between non-Linux remote debugging and other debugging activities. It > seems better to configure this in the launch configuration. Interesting point. Let's see what John thinks. > > I should have mentioned that this is already available in DSF-GDB, but you > > have to turn it on. Window->"Customize perspective..." then choose the tab > > "Command group availability" and check "Debug Update Modes" > > Thanks for the info. I will experiment with this. I also should have said that when you do that, you will get a 'refresh' icon in most view, including the debug view. If you click on it, it should refresh the threads. (In reply to comment #14) > (In reply to comment #13) > > > To be clear, when using a lightweight GDB remote stub for debugging, thread > > creation events are still raised, but they are not raised immediately. An "info > > threads" (or MI "-thread-list-ids") command will cause GDB to request the > > current thread list from the remote target. GDB will then raise thread creation > > events for each new thread reported by the target. > > Kind of a useless event by that time :-) Should not bother us to much as long > as we check for threads that exist already and allow it. Exactly my thought :-) > > Personally, I don't think "poll thread list on suspend" behaviour should be a > > global preference. Some users will need to toggle this behaviour frequently as > > they move between non-Linux remote debugging and other debugging activities. It > > seems better to configure this in the launch configuration. > > Interesting point. Let's see what John thinks. Reasonable enough. Let's go with a launch config setting. (In reply to comment #13) > To be clear, when using a lightweight GDB remote stub for debugging, thread > creation events are still raised, but they are not raised immediately. An "info > threads" (or MI "-thread-list-ids") command will cause GDB to request the > current thread list from the remote target. GDB will then raise thread creation > events for each new thread reported by the target. Peter, are you certain that sending a "-thread-list-ids" to the target will in fact report back the newly created threads? I found the the following comment in CDI's Target.java // HACK/FIXME : gdb/mi thread-list-ids does not // show any newly create thread, we workaround by // issuing "info threads" instead. The comment goes back to rev 1 of the file in 2004. Created attachment 165669 [details]
gdb trace showing -thread-list-ids recognizes new threads
I've done some testing and it appears that the comment in CDI may be outdated. It may refer to a bug in gdb at that time. With a recent gdb (6.8), sending a -thread-list-ids immediately after getting the stopped event from stepping over a thread-creating call has gdb reporting back the newly created thread. This trace was created by rigging dsf-gdb a bit to ensure the -thread-list-ids command is sent after each stopped event. Thus I will continue with a fix assuming this behavior, and ignore the comment in the CDI code.
Created attachment 165681 [details]
Fix
Turns out we currently make a CLI 'info threads' on every suspend in DSF-GDB (as we do in CDI-GDB), except that we cache the command result from one suspend to another in DSF-GDB. Thus subsequent command requests don't actually reach out to gdb. All I had to do was add the launch configuration setting and, when enabled, clear the command cache on every suspend event.
Note: I'm not sure if there are additional considerations for non-stop debugging. I tested that this works with MinGW by disabling the code that reacts to the "[New Thread]" gdb stream record. With this patch, DSF-GDB is able to detect a new thread created by the program (when the new option is enabled).
Created attachment 165683 [details]
Context help I added for this new checkbox
Marc, please review. Keep in mind some of the churn in the launch config GUI code is related to refactoring in order to reduce clutter. (In reply to comment #13) > To be clear, when using a lightweight GDB remote stub for debugging, thread > creation events are still raised, but they are not raised immediately. An "info > threads" (or MI "-thread-list-ids") command will cause GDB to request the > current thread list from the remote target. GDB will then raise thread creation > events for each new thread reported by the target. BTW, it turns out the belated thread created events are quite useful after all. While my patch causes the Debug view to show a newly created thread by sending an 'info threads' on every suspend, the new logic does not concern itself with firing off an MIThreadCreatedEvent, which is what allows all interested parties to find out about the new thread. The firing of that event only happens if we receive the "[New Thread]" message from gdb. So, the fix was made much simpler by the fact that gdb eventually sends the new-thread events when prompted for the thread list. Peter, as I cannot truly reproduce the problematic use case, can you please give this solution a try? (In reply to comment #21) > BTW, it turns out the belated thread created events are quite useful after all. > While my patch causes the Debug view to show a newly created thread by sending > an 'info threads' on every suspend, the new logic does not concern itself with > firing off an MIThreadCreatedEvent, which is what allows all interested parties > to find out about the new thread. The firing of that event only happens if we > receive the "[New Thread]" message from gdb. So, the fix was made much simpler > by the fact that gdb eventually sends the new-thread events when prompted for > the thread list. Nice. I didn't think of that. I'll review now. The patch seems corrupted. The change to IGDBLaunchConfigurationConstants keeps failing for me. I made the change by hand in my workspace. The fix is nice and well contained. I like. 1- Non-stop, if we also don't get thread-created events from GDB, then we have to decide when we want the new threads to be shown. We could leave it up to the user to refresh the view, or we should show any new thread whenever any other thread stops. I think the latter is in line with the current effort, so I suggest moving the clearing of the cache outside of the check of the event type (i.e., do it for all suspended events, not just for containerSuspended) Note that Non-stop is only supported with GDB 7.0 so it is GDBProcesses_7_0 that will run. I guess the comment "// This will happen in non-stop mode" of MIProcesses, was just a copy/paste mistake, as it will never hit. 2- I believe the same change is needed in GDBProcesses_7_0, where we should clear fThreadCommandCache Created attachment 165785 [details]
Updated fix based on Marc's code review
(In reply to comment #25) > Created an attachment (id=165785) [details] > Updated fix based on Marc's code review To be honest, we have code that _could_ send a SuspendedEvent even without non-stop, but I don't know if this code ever runs. You can see when BreakpointHitEvent or SuspendedEvent are being dispatched in MIRunControl. So, I'm not sure if it is safe to really assume it won't happen. I realize I'm that one that said it won't, but it was based on GDB features, but looking at the code, I'm not 100% sure what will happen. I leave the final decision to you. I don't think the risk is very high... (In reply to comment #26) > (In reply to comment #25) > > Created an attachment (id=165785) [details] [details] > > Updated fix based on Marc's code review > > To be honest, we have code that _could_ send a SuspendedEvent even without > non-stop, but I don't know if this code ever runs. You can see when > BreakpointHitEvent or SuspendedEvent are being dispatched in MIRunControl. > > So, I'm not sure if it is safe to really assume it won't happen. I realize I'm > that one that said it won't, but it was based on GDB features, but looking at > the code, I'm not 100% sure what will happen. I leave the final decision to > you. I don't think the risk is very high... If you're talking about the assert in MIProcesses.eventDispatched(ISuspendedDMEvent), then I think it's fine to leave it there. It could only trip *us* up, as end users won't be running with assertions turned on. If it goes off, then, best case, it helped us catch a logic or coding bug. Worst case, someone will confirm that the assert assumes too much and remove it. I'll add a comment next to it documenting some doubt about its validity to reduce confusion if it turns out to be the latter. Committed to HEAD. (In reply to comment #13) > (In reply to comment #10) > >> a) The new thread event pushed by GDB is ignored by DSF-GDB if the thread is > >> non-LWP. The thread list in the Debug view is therefore not refreshed and the > >> new thread is not seen. This issue can be observed in many debug scenarios. > >> The attached patch addresses this issue. > > > > Peter, I resolved this issue back on March 4th with bug # 304754. Please > > verify that it works with your scenario. > > The patch file looks good to me. I will verify. Now verified. Thank you. (In reply to comment #22) > Peter, as I cannot truly reproduce the problematic use case, can you please > give this solution a try? I cannot test the "Force thread list update on suspend" launch option because I need to use the DSF-GDB _hardware_ debugging launcher. Can someone add an equivalent check box to this launcher please? (In reply to comment #29) > (In reply to comment #22) > > > Peter, as I cannot truly reproduce the problematic use case, can you please > > give this solution a try? > > I cannot test the "Force thread list update on suspend" launch option because I > need to use the DSF-GDB _hardware_ debugging launcher. Can someone add an > equivalent check box to this launcher please? I'm on it. Created attachment 165918 [details]
Adds the new checkbox to dsf jtag launch config tab
dsf-jtag patch committed to HEAD. (In reply to comment #31) > Created an attachment (id=165918) [details] > Adds the new checkbox to dsf jtag launch config tab The new "Force thread list update on suspend" launch option works very well. Tested with both thread create and thread delete operations. Thank you. [cdt cvs genie] changed file http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/jtag/org.eclipse.cdt.debug.gdbjtag.ui/src/org/eclipse/cdt/debug/gdbjtag/ui/GDBJtagDSFDebuggerTab.java?root=Tools_Project&r1=1.2&r2=1.3 [cdt cvs genie] changed file http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/jtag/org.eclipse.cdt.debug.gdbjtag.ui/src/org/eclipse/cdt/debug/gdbjtag/ui/JtagUi.properties?root=Tools_Project&r1=1.3&r2=1.4 *** Bug 251229 has been marked as a duplicate of this bug. *** |
Build ID: v200905220803 Steps To Reproduce: 1. Start debugging an non-Linux executable which creates multiple threads dynamically using DSF GDB 2. Observe thread list is correct when execution is first suspended (eg breakpoint on main()) 3. Resume execution so that new threads are created or destroyed 4. Hit a breakpoint or otherwise suspend execution 5. Observe thread list in Debug view has not changed More information: When debugging a local Linux executable, stepping over a pthread_create() call yields the following GDB trace and the thread list is updated correctly: 695,558 (gdb) 701,299 15-exec-next 1 701,354 15^running 701,355 (gdb) 701,356 ~"[New Thread 0xb7fd5b90 (LWP 13030)]\n" 701,357 15*stopped,reason="end-stepping-range",thread-id="1",frame={addr="0x080485a5",func="main",args=[],file="../test.c",fullname="/home/pts/test/pthreads/test.c",line="24"} 701,358 (gdb) 701,406 16-thread-list-ids 701,408 16^done,thread-ids={thread-id="2",thread-id="1"},number-of-threads="2" 701,409 (gdb) When debugging other multi-threaded executables, stepping over a thread creation call yields the following GDB trace and the thread list is not updated: 838,399 (gdb) 859,771 30-exec-next 1 860,362 30^running 860,365 *running,thread-id="1" 860,365 (gdb) 860,365 *running,thread-id="all" 860,365 *stopped,reason="end-stepping-range",thread-id="1",frame={addr="0x001086a9",func="main",args=[],file="../test.c",fullname="/home/pts/test/threads/test.c",line="24"} 860,365 (gdb) Note that GDB is not reporting a new thread and the MI "-thread-list-ids" command is not invoked. Unlike the DSF GDB debugger, the standard CDI GDB debugger invokes "info threads" every time execution is suspended and the thread list is updated correctly.