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

[Himanshu Chauhan]

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