Re: [PATCH v3 1/2] riscv: Introduce support for hardware break/watchpoints

[Himanshu Chauhan]

Hi,

On Tue, Apr 7, 2026 at 7:18 AM liangzhen <liangzhen@xxxxxxxxxxxxxxxxxx> wrote:
>
> I implemented the ptrace and GDB breakpoint support based on this
> patch series. When setting the SIZE field to 0, the test case works
> correctly.
>

Noted. I will be floating the ptrace support immediately after the
revised version of this series. I would request you to test the ptrace
with your GDB breakpoint support. Please let me know when you are
sending patch series for GDB.

Thank you
Regards
Himanshu

>
> On 4/6/2026 12:48 PM, Himanshu Chauhan wrote:
> > Hi Liangzhen,
> >
> > Sorry I was busy with ptrace implementation on this patch series.
> > Which is done now and I will send it after the next revision of this
> > patch.
> >
> > On Fri, Apr 3, 2026 at 1:09 PM liangzhen <liangzhen@xxxxxxxxxxxxxxxxxx> wrote:
> >> Hi,
> >>
> >> Thank you for this patch, I have one question regarding the configuration of the size field:
> >>
> >> On Mon, Feb 23, 2026 at 10:19:17AM +0530, Himanshu Chauhan wrote:
> >>
> >>> +    case HW_BREAKPOINT_LEN_1:
> >>> +        hw->len = 1;
> >>> +        hw->tdata1 = RV_DBTR_SET_MC6_SIZE(hw->tdata1, 1);
> >>> +        break;
> >>> +    case HW_BREAKPOINT_LEN_2:
> >>> +        hw->len = 2;
> >>> +        hw->tdata1 = RV_DBTR_SET_MC6_SIZE(hw->tdata1, 2);
> >>> +        break;
> >>> +    case HW_BREAKPOINT_LEN_4:
> >>> +        hw->len = 4;
> >>> +        hw->tdata1 = RV_DBTR_SET_MC6_SIZE(hw->tdata1, 3);
> >>> +        break;
> >>> +    case HW_BREAKPOINT_LEN_8:
> >>> +        hw->len = 8;
> >>> +        hw->tdata1 = RV_DBTR_SET_MC6_SIZE(hw->tdata1, 5);
> >>> +        break;
> >> GDB's gdbarch_breakpoint_from_pc method returns len=2 for non-aligned addresses, causing size mismatch with hardware triggers.
> >>
> >> A simple test is as follows:
> >>
> >> root@k3:~# cat test.c
> >> #include <stdio.h>
> >>
> >> int a = 0;
> >> int main()
> >> {
> >>         printf("start test\n");
> >>         a = 1;
> >>         printf("a = %d\n", a);
> >>         printf("end test\n"); return 0; } root@k3:~# gcc -march=rv64gc -g test.c -o test root@k3:~# gdb test ... start ... Temporary breakpoint 1, main () at test.c:6 6 printf("start test\n");
> >> (gdb) x/8i $pc
> >> => 0x2aaaaaa6ea <main+8>:       auipc   a0,0x0
> >>    0x2aaaaaa6ee <main+12>:      addi    a0,a0,86
> >>    0x2aaaaaa6f2 <main+16>:      jal     0x2aaaaaa5d0 <puts@plt>
> >>    0x2aaaaaa6f6 <main+20>:      auipc   a5,0x2
> >>    0x2aaaaaa6fa <main+24>:      addi    a5,a5,-1770
> >>    0x2aaaaaa6fe <main+28>:      li      a4,1
> >>    0x2aaaaaa700 <main+30>:      sw      a4,0(a5)
> >>    0x2aaaaaa702 <main+32>:      auipc   a5,0x2
> >> (gdb) hbreak *0x2aaaaaa6f2
> >> Hardware assisted breakpoint 2 at 0x2aaaaaa6f2: file test.c, line 6.
> >> (gdb) c
> >> Continuing.
> >> start test
> >> a = 1
> >> end test
> >> [Inferior 1 (process 1784) exited normally]
> >> (gdb)
> >>
> >> root@k3:~# gcc -march=rv64g -g test.c -o test
> >> root@k3:~# gdb test
> >> ...
> >> start
> >> ...
> >> Temporary breakpoint 1, main () at test.c:6
> >> 6               printf("start test\n");
> >> (gdb) x/8i $pc
> >> => 0x2aaaaaa6f4 <main+16>:      auipc   a0,0x0
> >>    0x2aaaaaa6f8 <main+20>:      addi    a0,a0,100
> >>    0x2aaaaaa6fc <main+24>:      jal     0x2aaaaaa5d0 <puts@plt>
> >>    0x2aaaaaa700 <main+28>:      auipc   a5,0x2
> >>    0x2aaaaaa704 <main+32>:      addi    a5,a5,-1780
> >>    0x2aaaaaa708 <main+36>:      li      a4,1
> >>    0x2aaaaaa70c <main+40>:      sw      a4,0(a5)
> >>    0x2aaaaaa710 <main+44>:      auipc   a5,0x2
> >> (gdb) hbreak *0x2aaaaaa6fc
> >> Hardware assisted breakpoint 2 at 0x2aaaaaa6fc: file test.c, line 6.
> >> (gdb) c
> >> Continuing.
> >>
> >> Breakpoint 2, 0x0000002aaaaaa6fc in main () at test.c:6
> >> 6               printf("start test\n");
> >> (gdb)
> >>
> >> As a result, hardware breakpoints set on 16-bit instruction addresses may fail to trigger due to this size mismatch. So can we consider setting the SIZE field to 0 (match any size), hardware triggers match memory accesses of any size.
> >>
> > I think you have a good test case here. We can set size to 0 if
> > specifically asked or in the default case.
> > Do you have a patch set against GDB to test this out?
> >
> > Thanks
> > Regards
> > Himanshu
> >