Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 335648

Summary: CLI command "hbreak" creates a hardware watchpoint
Product: [Tools] CDT Reporter: Abeer Bagul <abeerbagul>
Component: cdt-debugAssignee: Marc Khouzam <marc.khouzam>
Status: RESOLVED FIXED QA Contact: Ken Ryall <ken.ryall>
Severity: normal    
Priority: P3 CC: cdtdoug, john, marc.khouzam, nobody, pawel.1.piech, pmac
Version: 8.0Flags: nobody: review+
Target Milestone: 8.0   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
This patch creates a watchpoint only if the type description contains string "watchpoint".
marc.khouzam: iplog+
Patch committed to HEAD
marc.khouzam: iplog-
If client sets the "hw" attribute to true, do not automatically set the "watchpoint" attribute
marc.khouzam: iplog+
Second fix for HEAD marc.khouzam: iplog-

Description Abeer Bagul CLA 2011-01-27 23:26:39 EST 
Build Identifier: I20101208-1300

When user types the CLI command "hbreak" in the gdb console, gdb inserts a hardware breakpoint. CDT then sends a command "-break-list" to get the updated list of breakpoints and watchpoints installed in the target. In the reponse, gdb reports the breakpoint inserted thru hbreak as type "hw breakpoint".

Example interaction between CDT and gdb:

[1,293,012,542,777] 177-interpreter-exec console hbreak
[1,293,012,542,792] ~"Hardware assisted breakpoint 4 at 0x60000aa4: file/
build/tree/RD-2010.0-beta4\
_kuma/tools/swtools-MSWin32-x86/xtensa-elf/src/xtos/crt1-sim.S, line 234.\n"
[1,293,012,542,792] 177^done
[1,293,012,542,792] (gdb)
[1,293,012,542,792] 178-break-list
[1,293,012,542,792]
178^done,BreakpointTable={nr_rows="1",nr_cols="6",hdr=[{width="7",alignment="-1"\
,col_name="number",colhdr="Num"},{width="14",alignment="-1",col_name="type",colhdr="Type"},{width="4\
",alignment="-1",col_name="disp",colhdr="Disp"},{width="3",alignment="-1",col_name="enabled",colhdr=\
"Enb"},{width="10",alignment="-1",col_name="addr",colhdr="Address"},{width="40",alignment="2",col_na\
me="what",colhdr="What"}],body=[bkpt={number="4",type="hw
breakpoint",disp="keep",enabled="y",addr="\
0x60000aa4",file="/build/tree/RD-2010.0-beta4_kuma/tools/swtools-MSWin32-x86/xtensa-elf/src/xtos/crt\
1-sim.S",line="234",times="0"}]}
[1,293,012,542,792] (gdb)

In the method, MIBreakpoint.parse(...), any breakpoint whose typename starts with "hw" is treated as a watchpoint. Only those breakpoints whose typename is "hw watchpoint" should be treated as watchpoint.


Reproducible: Always

Steps to Reproduce:
1. Start a debug launch. 
2. In the gdb console in Console view, type the command "hbreak".
3. Check that CDT mistakenly creates a hardware watchpoint instead of hardware breakpoint.
Comment 1 Abeer Bagul CLA 2011-01-27 23:29:14 EST 
Created attachment 187801 [details]
This patch creates a watchpoint only if the type description contains string "watchpoint".
Comment 2 Marc Khouzam CLA 2011-01-28 09:54:24 EST 
Do you have some MI output that shows what a hardware watchpoint looks like in the -break-insert command?

Do you want this in 7.2 or HEAD is enough?
Comment 3 Abeer Bagul CLA 2011-01-31 00:02:08 EST 
MI output for a hardware watchpoint:

[1,296,449,998,740] 20-interpreter-exec console "watch argc"
[1,296,449,998,740] ~"Hardware watchpoint 2: argc\n"
[1,296,449,998,740] 20^done
[1,296,449,998,740] (gdb) 
[1,296,449,998,755] 21-break-list
[1,296,449,998,755] 21^done,BreakpointTable={nr_rows="1",nr_cols="6",hdr=[{width="7",alignment="-1",\
col_name="number",colhdr="Num"},{width="14",alignment="-1",col_name="type",colhdr="Type"},{width="4"\
,alignment="-1",col_name="disp",colhdr="Disp"},{width="3",alignment="-1",col_name="enabled",colhdr="\
Enb"},{width="10",alignment="-1",col_name="addr",colhdr="Address"},{width="40",alignment="2",col_nam\
e="what",colhdr="What"}],body=[bkpt={number="2",type="hw watchpoint",disp="keep",enabled="y",addr=""\
,what="argc",times="0"}]}
[1,296,449,998,755] (gdb) 

It is okay to get the patch in HEAD, since we will be based on Indigo for our product release.
Comment 4 Marc Khouzam CLA 2011-02-01 21:33:49 EST 
Created attachment 188109 [details]
Patch committed to HEAD

I added the copyright notice at the top of both files as:
"Abeer Bagul (Tensilica) - Differentiate between hw breakpoint and watchpoint"

I committed the patch to HEAD.
Comment 5 Marc Khouzam CLA 2011-02-01 21:34:35 EST 
Mikhail, can you review?

Abeer, thanks for the contribution!
Comment 7 Nobody - feel free to take it CLA 2011-02-02 12:37:29 EST 
Looks good to me.
Comment 8 Abeer Bagul CLA 2011-02-11 00:01:34 EST 
A trivial addition to this bug:
There is a method MIBreakpoint.setHardware(boolean hd) which is currently not in use. This method mistakenly marks the given breakpoint as a "watchpoint".
There exists another method MIBreakpoint.setWatchpoint() which the client can call if it wants to mark the breakpoint as a "hw watchpoint".

Example client code to mark a "hw breakpoint" (today):
((MIBreakpoint)bp).setHardware(true);
((MIBreakpoint)bp).setWatchpoint(false);

The client code should be (after patch):
((MIBreakpoint)bp).setHardware(true);

Then client code to set a "hw watchpoint" will be:
((MIBreakpoint)bp).setHardware(true);
((MIBreakpoint)bp).setWatchpoint(true);

Sorry for missing out this bug in the original bug report.
Comment 9 Abeer Bagul CLA 2011-02-11 00:10:24 EST 
Created attachment 188738 [details]
If client sets the "hw" attribute to true, do not automatically set the "watchpoint" attribute
Comment 10 Marc Khouzam CLA 2011-02-15 09:36:03 EST 
Created attachment 188998 [details]
Second fix for HEAD

I committed this patch to HEAD.  It is Abeer's change and the same change for DSF-GDB.
Comment 11 Marc Khouzam CLA 2011-02-15 09:36:41 EST 
(In reply to comment #9)
> Created attachment 188738 [details]
> If client sets the "hw" attribute to true, do not automatically set the
> "watchpoint" attribute

Thanks Abeer.