Bugzilla – Full Text Bug Listing
| Summary: | [breakpoints] Reposition breakpoints when planted on invalid line | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Tools] CDT | Reporter: | Pawel Piech <pawel.1.piech> | ||||||
| Component: | cdt-debug | Assignee: | Pawel Piech <pawel.1.piech> | ||||||
| Status: | RESOLVED FIXED | QA Contact: | Ken Ryall <ken.ryall> | ||||||
| Severity: | enhancement | ||||||||
| Priority: | P3 | CC: | cdtdoug, eostroukhov, kirk.beitz, ling.5.wang, malaperle, marc.khouzam, nobody, pawel.1.piech, pchuong, scott.tepavich | ||||||
| Version: | 8.0 | Flags: | nobody:
review+
pchuong: review+ |
||||||
| Target Milestone: | 8.1.0 | ||||||||
| Hardware: | PC | ||||||||
| OS: | Linux | ||||||||
| Whiteboard: | |||||||||
| Attachments: |
|
||||||||
|
Description
Pawel Piech
adding Ling to the CC: list, because for Nokia, he implemented something like this in the last few months for EDC , and might have comments/pointers to code that could be re-factored into CDT instead. Thanks, Ling please do share any experience you've had implementing this feature. I've recently hooked up CDT breakpoints to our proprietary debugger back end, and got the repositioning working a bit too easily it seemed. So this may be a quick feature. The problem I have, however, is that I rely on our debugger engine to provide the correct line #, but GDB does not seem to give me that information. So I may have to use PDA as the reference implementation, which is not ideal. (In reply to comment #2) > Thanks, Ling please do share any experience you've had implementing this > feature. > > I've recently hooked up CDT breakpoints to our proprietary debugger back end, > and got the repositioning working a bit too easily it seemed. So this may be a > quick feature. The problem I have, however, is that I rely on our debugger > engine to provide the correct line #, but GDB does not seem to give me that > information. So I may have to use PDA as the reference implementation, which > is not ideal. I implemented it for EDC (see bug 323457 for detail). It relies on the symbol reader (the Dwarf parser) in EDC to provide the correct line #. The API defined and used is (see bug 337493 for detail): org.eclipse.cdt.debug.edc.internal.services.dsf.Modules.findClosestLineWithCode() And the code that moves the breakpoint is in org.eclipse.cdt.debug.edc.internal.services.dsf.BreakpointAttributeTranslator.resolveBreakpoint() So my solution is similar to your but not really applicable to GDB. (In reply to comment #0) > Workflow details: > 1) User is able to create breakpoints at any location in the source. This > should remain unchanged to avoid having the tool restrict the user from doing > something non-obvious. > > 2) Once breakpoint is created the debugger integration sends a request to debug > engine to plant a breakpoint at a given location. > > 3) Debug engine comes back with the line number where the breakpoint was > actually planted, this may be different than the requested line. With EDC that has no debugger backend, the symbol reader tells debugger which line in the neighborhood of the original line can have a breakpoint set on it. > > 4) The debugger integration, updates the breakpoint with the planted breakpoint > location. > > 5) Breakpoint is moved in editor to the new location. In breakpoints view, the > breakpoint label shows the original line followed by the actual planted line. > The icon overlay is a small arrow instead of the checkmark (although this stuff > is too small for most people to discern anyway). > > 6) When debug session is terminated. The breakpoint is moved back to the > original location selected by user. The above #5 and #6 are nice to have, but I don't see real value to users. > > Implementation details: > Instead of tracking breakpoint location using the single IMarker.LINE_NUMBER > property, the line number ends up being tracked by three properties: > ATTR_REQUESTED_LINE, ATTR_PLANTED_LINE_LIST, and IMarker.LINE_NUMBER. The list > of planted line locations allows for resolving conflicts between multiple debug > targets. Multiple targets moving a breakpoint to different locations is a rare > and not important in terms of workflow, but it's important technically as it > could lead to dangerous feedback loops. That is a hairy case we thought about but didn't bother to support in EDC as it is really rare to run into the "dangerous feedback loop". To run into it, you have to debug the same code on target A and B simultaneously, and the compilers on A and B have to be different and especially different when generating line info for the same source line(s). Other than that, if the user just debugs on target A and B one after another (which is kind of common in practice), the outcome may not be best (breakpoint may be moved to a line that's not the best candidate) but not unacceptable either. Created attachment 210352 [details]
Proposed patch
Attached is a proposed patch to allow breakpoint relocation.
(In reply to comment #5) > Created attachment 210352 [details] > Proposed patch > > Attached is a proposed patch to allow breakpoint relocation. Hi Scott, I've worked with you to polish this patch, but for everyone else's benefit, could you list the details of what the patch implements? Hey Pawel –
The patch enables the underlying framework/implementation to manage relocating breakpoints (when the backend debugger is capable). It does so without breaking backward compatibility. The patch basically consists of the following:
1.) New ICLineBreakpoint2 and ICBreakpoint 2 interfaces (which extend the existing).
These were created to preserve backward compatibility, while allowing the definition of new attributes:
REQUESTED_LINE – track the user specified line.
REQUESTED_SOURCE_HANDLE – track the user specified source
REQUESTED_CHAR_START/REQUESTED_CHAR_END – track user specified column.
And methods to manage these attributes.
2.) These attributes then allow the existing breakpoint classes, to update usable messages to the user, when their breakpoint has been relocated to another line.
3.) A small update to allow the user to edit a relocated breakpoint to another location.
Thank you very much for the time you spent on the review and feedback.
Hi Mikhail (or anyone else with a vested interested), could you review the API extensions for the ICBreakpoint and ICLineBreakpoint interfaces? The patch itself only enables debugger integrations to reposition the breakpoint safely. It does not change the gdb debugger integration since the gdb debugger does not return the valid source line upon planting the breakpoint. I did update the PDA debugger integration to validate and test the feature and I'll attach that patch as well. Created attachment 210697 [details]
Patch to PDA to support repositioning breakpoints.
In order to validate this API feature I updated the PDA example to support CDT breakpoints and update the install count upon breakpoint install. To do so, I had to migrate PDA to the BreakpointMediator2 implementaiton. I also had to make some changes to the mediator 2 to make it work.
To set the CDT breakpoint, user has to open the PDA source file using the C editor, then double-click. To force a breakpoint to reposition to another line, the PDABreakpointAttributeTranslator.updateBreakpointStatus() needs to be rigged a little (uncomment the commented out block).
I'll plan to commit these changes to the PDA example along with the main patch.
(In reply to comment #9) > ... > breakpoints and update the install count upon breakpoint install. To do so, I > had to migrate PDA to the BreakpointMediator2 implementaiton. I also had to > make some changes to the mediator 2 to make it work. Patrick, I believe you are using the BreakpointMediator2, can you take a look at the changes I made to make sure it doesn't break anything for you? (In reply to comment #8) > Hi Mikhail (or anyone else with a vested interested), could you review the API > extensions for the ICBreakpoint and ICLineBreakpoint interfaces? > I'll look at it. > The patch itself only enables debugger integrations to reposition the > breakpoint safely. It does not change the gdb debugger integration since the > gdb debugger does not return the valid source line upon planting the > breakpoint. Do you mean gdb doesn't reposition a breakpoint set at an invalid line, right? (In reply to comment #11) > Do you mean gdb doesn't reposition a breakpoint set at an invalid line, right? Right... I think :-) It certainly plants the breakpoint on a valid address but it does not echo back what line number that address corresponds to. (In reply to comment #12) > (In reply to comment #11) > > Do you mean gdb doesn't reposition a breakpoint set at an invalid line, right? > Right... I think :-) It certainly plants the breakpoint on a valid address but > it does not echo back what line number that address corresponds to. I had brought this up a while ago to the GDB community. I am not aware of any improvements, although it would be worth trying with GDB 7.4: http://sourceware.org/ml/gdb/2009-04/msg00142.html The API looks good to me. I didn't tried the PDA patch though. Pawel, the patch looks good. I don't use BreakpointMediator for TI breakpoint, however, I do use dsf breakpoint VM* implementations. Trying to fix the review flag. Cool. I committed the changes. Thanks for the review. *** cdt git genie on behalf of Scott Tepavich ***
Bug 360280 - [breakpoints] Reposition breakpoints when planted on invalid line
[*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=b9255e88a54c84de5af5774cecb4bdad0373e5fa
*** cdt git genie on behalf of Scott Tepavich ***
Bug 360280 - [breakpoints] Reposition breakpoints when planted on invalid line
[*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=b9255e88a54c84de5af5774cecb4bdad0373e5fa
*** cdt git genie on behalf of Pawel Piech ***
Bug 360280 - [breakpoints] Reposition breakpoints when planted on invalid line (PDA Example - Added updating of C Breakpoint status to set installed flag and update line number)
[*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=1dbaf3c1baad7a1fd713b8b2c211f13d442eda7a
*** cdt git genie on behalf of Pawel Piech ***
Bug 360280 - [breakpoints] Reposition breakpoints when planted on invalid line (PDA Example - Added updating of C Breakpoint status to set installed flag and update line number)
[*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=1dbaf3c1baad7a1fd713b8b2c211f13d442eda7a
|