Community
Participate
Working Groups
Build Identifier: M20100211-1343 We can create a software breakpoint now, but hardware breakpoint is unsupported even though the back end of MIBreakpoint and CDIBreakpoint does support them. My suggestion is that we add a new IToggleBreakpointsTargetFactory that mimics ToggleCBreakpointsTargetFactory and ToggleBreakpointAdapter, but creates a hardware breakpoint instead of the regular ones (using CDIDebugModel.createLineBreakpoint, but using ICBreakpointType.HARDWARE instead). If you think this is a good suggestion, we can create a patch and contribute to Eclipse. Reproducible: Always
The workflow we are currently planning is 1. Add a new breakpoint type in the Run -> Breakpoint Types menu called "Hardware Breakpoint" 2. Subsequent newly created breakpoint (eg. double clicking on the ruler) will be creating the breakpoint via a new IToggleBreakpointTargetFactory, and the ToggleBreakpointAdapter class will be duplicated, and the calls to CDIDebugModel.createLineBreakpoint will be passed in with ICBreakpointType.HARDWARE. 3. This will enable user to create a hardware breakpoint. Please let me know if this workflow is reasonable.
Moving to general debug since those changes will be in general CDT and can be used by any debugger integration.
(In reply to comment #1) > The workflow we are currently planning is > > 1. Add a new breakpoint type in the Run -> Breakpoint Types menu called > "Hardware Breakpoint" > 2. Subsequent newly created breakpoint (eg. double clicking on the ruler) will > be creating the breakpoint via a new IToggleBreakpointTargetFactory, and the > ToggleBreakpointAdapter class will be duplicated, and the calls to > CDIDebugModel.createLineBreakpoint will be passed in with > ICBreakpointType.HARDWARE. I think you are right that you will need to duplicate ToggleBreakpointAdapter. Will you be using an actionSet to hide the concept of Hardware breakpoints, until the user chooses to make them accessible? If so, then you do need a new class that implements IToggleBreakpointTargetFactory. But if you won't use an actionSet, then you can probably simply add a new target within ToggleCBreakpointsTargetFactory instead of creating a new class. > 3. This will enable user to create a hardware breakpoint. > > Please let me know if this workflow is reasonable. Looks good to me. Will you support adding hw breakpoints from the Disassembly view? Then you will need add stuff to org.eclipse.cdt.dsf.debug.internal.ui.ToggleBreakpointsTargetFactory or create a new class for it. You can look at org.eclipse.cdt.dsf.gdb.internal.ui.breakpoints.ToggleTracepointsTargetFactory which supports Tracepoints from the disassembly view.
(In reply to comment #3) > I think you are right that you will need to duplicate ToggleBreakpointAdapter. > > Will you be using an actionSet to hide the concept of Hardware breakpoints, > until the user chooses to make them accessible? If so, then you do need a new > class that implements IToggleBreakpointTargetFactory. But if you won't use an > actionSet, then you can probably simply add a new target within > ToggleCBreakpointsTargetFactory instead of creating a new class. I think we won't need a new actionSet to hide the Hardware breakpoints. I'll look into adding a new target within the ToggleCBreakpointsTargetFactory.
(In reply to comment #4) > I think we won't need a new actionSet to hide the Hardware breakpoints. I'll > look into adding a new target within the ToggleCBreakpointsTargetFactory. +1
(In reply to comment #1) > Please let me know if this workflow is reasonable. This was one of my intended use cases for the toggle target extension point, so I hope it's reasonable :-)
(In reply to comment #5) > (In reply to comment #4) > > I think we won't need a new actionSet to hide the Hardware breakpoints. I'll > > look into adding a new target within the ToggleCBreakpointsTargetFactory. > > +1 I think being able to hide the HW breakpoint option is important. For one, not all target environments support them. Secondly, I think companies have been coming up with custom solutions. Switching to this common approach may not be feasible or desirable. If we can hide the breakpoint type via a Capability, then that's good enough for me. I just don't know off the top of my head if that's currently supported. One way or another, though, we should effectively make this new breakpoint type "avoidable".
(In reply to comment #7) > (In reply to comment #5) > > (In reply to comment #4) > > > I think we won't need a new actionSet to hide the Hardware breakpoints. I'll > > > look into adding a new target within the ToggleCBreakpointsTargetFactory. > > > > +1 > > I think being able to hide the HW breakpoint option is important. For one, not > all target environments support them. Secondly, I think companies have been > coming up with custom solutions. Switching to this common approach may not be > feasible or desirable. If we can hide the breakpoint type via a Capability, > then that's good enough for me. I just don't know off the top of my head if > that's currently supported. One way or another, though, we should effectively > make this new breakpoint type "avoidable". Those are good points. I don't know about Capabilities, but I now the actionSet will work. It can be handled exactly like Tracepoints.
(In reply to comment #7) > (In reply to comment #5) > > (In reply to comment #4) > > > I think we won't need a new actionSet to hide the Hardware breakpoints. I'll > > > look into adding a new target within the ToggleCBreakpointsTargetFactory. > > > > +1 > > I think being able to hide the HW breakpoint option is important. For one, not > all target environments support them. Secondly, I think companies have been > coming up with custom solutions. Switching to this common approach may not be > feasible or desirable. If we can hide the breakpoint type via a Capability, > then that's good enough for me. I just don't know off the top of my head if > that's currently supported. One way or another, though, we should effectively > make this new breakpoint type "avoidable". Hiding and not hiding has their pros and cons IMO. I think the reason behind companies coming up with custom solutions is that currently there's really no way to set hardware breakpoint, and if this kind of changes get integrated into Eclipse, it might even be beneficial to them. I'll be implementing actionSet to allow user to hide them as per your comment.
I'm unable to add a new factory for the Disassembler View in DSF. It currently only picks up the first factory and always uses that for adding a new breakpoint. I'll not add the support for creating a new hardware breakpoint in the Disassembly view in DSF.
(In reply to comment #10) > I'm unable to add a new factory for the Disassembler View in DSF. It currently > only picks up the first factory and always uses that for adding a new > breakpoint. Amazingly you made me realize that setting Tracepoints from the Disassembly view has been broken from the start! I just fixed it in Bug 336847. If you do the same fix for your factory (5 lines) I believe it will work.
Please look at https://bugs.eclipse.org/bugs/show_bug.cgi?id=336875. Would it be helpful for you?
> (In reply to comment #10) > > I'm unable to add a new factory for the Disassembler View in DSF. It currently > > only picks up the first factory and always uses that for adding a new > > breakpoint. > > Amazingly you made me realize that setting Tracepoints from the Disassembly > view has been broken from the start! > > I just fixed it in Bug 336847. > If you do the same fix for your factory (5 lines) I believe it will work. I'll add in the the factory for the Disassembler View and try it out. Thanks for the quick fix. (In reply to comment #12) > Please look at https://bugs.eclipse.org/bugs/show_bug.cgi?id=336875. Would it > be helpful for you? Yeah, it would reduce code duplication. Any ETA on the integration of this patch? I'll just apply the patch locally for now. Thanks for the great work.
(In reply to comment #13) > Yeah, it would reduce code duplication. Any ETA on the integration of this > patch? I'll just apply the patch locally for now. I was just waiting for a feedback from you, now I can commit it. Thanks.
Changes are done, but I haven't got time to create patch and thoroughly verify.
(In reply to comment #15) > Changes are done, but I haven't got time to create patch and thoroughly verify. Can you generate a patch for what you have and post it? Maybe someone else can test it.
Created attachment 191184 [details] HW Breakpoint GUI
(In reply to comment #17) > Created attachment 191184 [details] > HW Breakpoint GUI Is this patch against the HEAD? I am having trouble applying it. Please, check if it works with the latest CDT.
Hi Mikhail, I'll check on this against HEAD in a couple of days.
Created attachment 195828 [details] Patch for the enhancement Attached patch to HEAD.
Hi, Its been a while since the patch is submitted. Would the patch be integrated? Thanks
I'd really like to see this in Eclipse. :) Is there a way I could help (except pinging here)?
I'm in process of expanding various breakpoint creation APIs (see but 360588). You should be able to create your own customized toggle breakpoint adapter given that there is some disagreement here about whether this option should be turned on by default.
This bug may be affected by bug 395404
To help the review, I've pushed the patch to Gerrit with author: Tien Hock Loh. https://git.eclipse.org/r/8945
Created attachment 227302 [details] Patch that adds "Toggle Hardware Breakpoint" action.
Attaching a patch that adds the "Toggle Hardware Breakpoint" action to the C/C++ and ASM editors and the Disassembly view. The patch was requested by Marc during the multi-core call. In my opinion the better way would be to make the "Type" field of the new "Add Breakpoint" dialog editable and support three types "Regular", "Hardware" and "Temporary". Marc, what do you think?
(In reply to comment #27) > Attaching a patch that adds the "Toggle Hardware Breakpoint" action to the > C/C++ and ASM editors and the Disassembly view. The patch was requested by > Marc during the multi-core call. > In my opinion the better way would be to make the "Type" field of the new > "Add Breakpoint" dialog editable and support three types "Regular", > "Hardware" and "Temporary". > Marc, what do you think? I like that. It is less intrusive in the UI and gives access to both hardware and temporary breakpoints. I'm assuming that hardware or temporary breakpoints are not going to be used a lot and therefore that it won't be too annoying to have to manually specify that type for each breakpoint. We could even add the other bp types, such as Tracepoints.
Is this going to be supported in a standard CDT release any time soon?
Created attachment 239738 [details] Disassembly and images Follow-up patch from https://git.eclipse.org/r/#/c/8945/ - Move ToggleCHWBreakpointsTargetFactory/ToggleHWBreakpointAdapter along the rest of breakpoint factories/adapters - Add ToggleHWBreakpointAdapter for disassembly part - Add missing HW break images - Enable breakpoint type field readonly display in property page. I could not upload directly to gerrit as the original patch from Tien Hock Loh does not have CLA. To apply all source to cdt master issue the following commangs: git fetch https://git.eclipse.org/r/cdt/org.eclipse.cdt refs/changes/45/8945/2 && git cherry-pick FETCH_HEAD git apply 0001-Bug-332993_part2.patch where 0001-Bug-332993_part2.patch is attached patch
Mikhail, Do you think it is Ok for me to follow on HW breakpoints enhancements in a separate bug entry? This patch would address the complete lack of the way to set a HW breakpoint and adding incrementally the enhancements.
(In reply to Mikhail Khodjaiants from comment #27) > In my opinion the better way would be to make the "Type" field of the new > "Add Breakpoint" dialog editable and support three types "Regular", > "Hardware" and "Temporary". > Marc, what do you think? I have created Bug 427898 for editable "Type" field to track that work.
(In reply to Teodor Madan from comment #30) > Created attachment 239738 [details] > Disassembly and images > > Follow-up patch from https://git.eclipse.org/r/#/c/8945/ > - Move ToggleCHWBreakpointsTargetFactory/ToggleHWBreakpointAdapter along the > rest of breakpoint factories/adapters > - Add ToggleHWBreakpointAdapter for disassembly part > - Add missing HW break images > - Enable breakpoint type field readonly display in property page. > > I could not upload directly to gerrit as the original patch from Tien Hock > Loh does not have CLA. > I have pushed to a separate gerrit review to be easy to apply/try-out. The new gerrit review link is: https://git.eclipse.org/r/#/c/21827/
Comment on attachment 239738 [details] Disassembly and images Obsolete. The code is pushed into gerrit
Comment on attachment 195828 [details] Patch for the enhancement The patch has been pushed to gerrit