Bugzilla – Full Text Bug Listing
| Summary: | MIBreakpointsManager convertToTargetBreakpoint didn't convert the breakpoint type attribute | ||
|---|---|---|---|
| Product: | [Tools] CDT | Reporter: | Tien Hock Loh <thloh> |
| Component: | cdt-debug-dsf-gdb | Assignee: | Marc Khouzam <marc.khouzam> |
| Status: | RESOLVED FIXED | QA Contact: | Marc Khouzam <marc.khouzam> |
| Severity: | normal | ||
| Priority: | P3 | CC: | cdtdoug, marc.khouzam, pawel.1.piech |
| Version: | 8.0 | ||
| Target Milestone: | 7.0.2 | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
| Attachments: | |||
|
Description
Tien Hock Loh
From looking at the code it seems you are right. The reason it was never noticed is that we never set Hardware breakpoints. Same goes for temporary breakpoints. So, MIBreakpointsManager#convertToTargetBreakpoint should convert both: MIBreakpointDMData.IS_TEMPORARY MIBreakpointDMData.IS_HARDWARE for breakpoints, catchpoints and tracepoints, so maybe it is best to set them all the time in convertToTargetBreakpoint() Thanks for reporting this. Yes I noticed that HW Breakpoints cannot be set using the UI of CDI, but we're actually trying to add the feature to Eclipse so that user can set the hardware breakpoint (currently prototyping, will be contributing to Eclipse if possible) Is this considered a high priority bug? We can provide a patch if it helps. (In reply to comment #2) > Yes I noticed that HW Breakpoints cannot be set using the UI of CDI, but we're > actually trying to add the feature to Eclipse so that user can set the hardware > breakpoint (currently prototyping, will be contributing to Eclipse if possible) Sounds great! > Is this considered a high priority bug? We can provide a patch if it helps. I wouldn't say high-priority, but it would be nice to fix. As you know the fix is easy, so if you can provide a patch, I'll commit it. Thanks Sure, I'll submit a patch when I've got it fixed. Created attachment 185400 [details]
Patch to MIBreakpointsManager::convertToTargetBreakpoints so that hardware/temporary breakpoints are set from CDIBreakpoints
This patch is against a old version. Can you update to the latest please? Do I need to get the latest bleeding edge source code or just the latest released 3.6 version will do? How do I get the bleeding edge source code if I need to patch against that? (In reply to comment #7) > Do I need to get the latest bleeding edge source code or just the latest > released 3.6 version will do? > How do I get the bleeding edge source code if I need to patch against that? Yes you would need the latest code. You can look at http://wiki.eclipse.org/CDT/contributing You would also need the code under org.eclipse.cdt/dsf and org.eclipse.cdt/dsf-gdb However, this patch is small enough that I can migrate your changes to the latest code and commit it. But if you want to 'practice' for future contributions, I welcome you to do it. Just let me know what you prefer. I'll do the patch. We'll likely be contributing to Eclipse features in near future. Thanks. Created attachment 185897 [details]
Adds hardware/temporary breakpoint property when converting CDI breakpoint to DSF breakpoint
Hi Marc, Do you have an ETA when you can get this integrated? Thanks (In reply to comment #10) > Created attachment 185897 [details] > Adds hardware/temporary breakpoint property when converting CDI breakpoint to > DSF breakpoint A hardware breakpoint and a hardware watchpoints are two different things, right? I've never actually seen how GDB sets either one. I'm asking because MIBreakpoint#parse sets both isHdw and isWpt together. Is this right? This was copied from the same class in CDI. (In reply to comment #11) > Hi Marc, > Do you have an ETA when you can get this integrated? > Thanks ((Integer) attributes.get(ICBreakpointType.TYPE) & ICBreakpointType.HARDWARE) attributes.get could return null, which would lead to an NPE here. Can you check for null? (In reply to comment #12) > (In reply to comment #10) > > Created attachment 185897 [details] [details] > > Adds hardware/temporary breakpoint property when converting CDI breakpoint to > > DSF breakpoint > > A hardware breakpoint and a hardware watchpoints are two different things, > right? I've never actually seen how GDB sets either one. I'm asking because > MIBreakpoint#parse sets both isHdw and isWpt together. Is this right? This > was copied from the same class in CDI. Yes they should be. The thing is, I'm haven't been reading a lot of watch point's code, thus I'm not including hardware property when breakpoint is an instance of ICWatchPoint. There's currently no way to set the hardware breakpoint, but we're going to make a patch/plug-in to support it and hopefully integrate back to Eclipse. (In reply to comment #13) > (In reply to comment #11) > > Hi Marc, > > Do you have an ETA when you can get this integrated? > > Thanks > > ((Integer) attributes.get(ICBreakpointType.TYPE) & ICBreakpointType.HARDWARE) > > attributes.get could return null, which would lead to an NPE here. Can you > check for null? Looks like I've missed out an important detail here, will update the patch. Thanks. (In reply to comment #14) > Looks like I've missed out an important detail here, will update the patch. > Thanks. If you can have the patch before Monday, I can include it for the RC1 build. Created attachment 186891 [details]
MIBreakpointsManager patch to include hardware and temporary breakpoints attributes
Committed to both HEAD and the 7_0 branch. Thanks for the patch. *** cdt cvs genie on behalf of mkhouzam *** Bug 332605: MIBreakpointsManager convertToTargetBreakpoint didn't convert the breakpoint type attribute [*] MIBreakpointsManager.java 1.21 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIBreakpointsManager.java?root=Tools_Project&r1=1.20&r2=1.21 [*] MIBreakpointsManager.java 1.18.2.2 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt/dsf-gdb/org.eclipse.cdt.dsf.gdb/src/org/eclipse/cdt/dsf/mi/service/MIBreakpointsManager.java?root=Tools_Project&r1=1.18.2.1&r2=1.18.2.2 |