eclipse-cdt-cloud / cdt-gdb-adapter

CDT GDB Debug Adapter
Eclipse Public License 2.0
28 stars 40 forks source link

Support set Hardware Breakpoint when debugging #218

Closed thamho0902 closed 1 year ago

thamho0902 commented 1 year ago

Root-cause: VS Code does not have option to set hardware breakpoint when debugging. => Solution: Add option for selecting use hardware breakpoint or software breakpoint by config in launch.json file

eclipse-cdt-bot commented 1 year ago

Can one of the admins verify this patch?

jonahgraham commented 1 year ago

This comment is design decision question, I would appreciate your thoughts on this before we merge it.

I'm not sure this is the correct solution to HW breakpoints. It may be suitable for now, but in particular I think hardware-breakpoint-limit should be configurable and set along with turning on hardware breakpoints.

IIRC the other way to get gdb to do this is to set hardware-breakpoint-limit and set all memory to be ro and let gdb manage it automatically.

Please share your comments on the above before I consider merging.

bangdang1411 commented 1 year ago

Do you know what -f does? It may be important in some circumstances, but it is likely just as important for software as hardware breakpoints. If you need -f, the pending in requests should be used.

I see all breakpoint types in e2 studio always have option '-f'. If the decision is using '-f' option, I will set option pending is true when calling the function sendBreakInsert() or sendBreakFunctionInsert(). Note: I have added option pending and set it true, but the integration test is running failed in case set breakpoint unknown file. Could you please give me your idea?

Yes. Always, for all tickets. If for some reason you can't, you should document that fact in the PR somewhere. Non-exhaustive list of possible reasons to not include a new test:

I will write the integration test if possible. Or not, I will update document in Jira/Confluence ticket. Thank you.

bangdang1411 commented 1 year ago

This comment is design decision question, I would appreciate your thoughts on this before we merge it.

I'm not sure this is the correct solution to HW breakpoints. It may be suitable for now, but in particular I think hardware-breakpoint-limit should be configurable and set along with turning on hardware breakpoints.

IIRC the other way to get gdb to do this is to set hardware-breakpoint-limit and set all memory to be ro and let gdb manage it automatically.

I am trying to use command '-gdb-set breakpoint auto-hw on' in launch process but it seems not to affect the type of breakpoint.

It is also for option 'hardware-breakpoint-limit', I am not sure if we have the special config for it or not but the command sent to Renesas GDB server has an error (cannot set option limit or unlimited).

So, I think using the current solution can be helpful, The extension has a limit in UI (has only the option "Add breakpoint" but cannot choose the type of breakpoint). I think the exact way is to improve the breakpoint widget to have an option "Add hardware breakpoint", but it seems not available for now.

I think my solution is fixing temporarily, if users do not care type of breakpoint, it is not configured in the launch file and software breakpoint is used for default. Thank you.

jonahgraham commented 1 year ago

I will write the integration test if possible. Or not, I will update document in Jira/Confluence ticket.

Great. Please also document the reasoning here. This is an open source project and we should try not to put cdt-gdb-adapater specific information in systems that are not visible to everyone. It is complicated because I have access to the JIRA, but imagine the reviewer of these PRs didn't.

jonahgraham commented 1 year ago

If you click details

FE18E95D-E50F-4306-8426-689BA43CC624

And then go to the summary, you can download the logs of the tests to see hopefully enough details

A2868968-8D09-4C91-8B67-368AF5226CB8

thamho0902 commented 1 year ago

Sorry, I have a mistake. Please help me check again.