gdb for Riscv, single stepping issue

gdb for Riscv, single stepping issue

[James Becker]

Hello,

I have a RISCV-EL2 core running in a Nexys A7 FPGA board.

I have openocd for riscv running over jtag with a connection by 
riscv-gdb to the openocd instance at port 3333.

Everything works fine, stepping, break points, load, view memory.

But I have one issue: Some of the memory in my design is 4 byte 
aligned.  Its designed for fast instruction fetch, its known as ICCM.

When I have code running in that memory, gdb still works fine for 
breakpoints, but it will not single step.

Looking at the openocd debug files, it appears that gdb is attempting to 
do a 2 byte read as a part of the single stepping procedure.

Since my memory does not support 2 byte reads or writes, this fails.

Is there some way that gdb can be configured to not do any 2-byte word 
reads or writes during single stepping?  I can't seem to find any.

-- 
Jim Becker
CrossFire Technologies Inc
1000 Westgate Dr. Suite 150-I
St. Paul, MN 55114
Mobile: +1 303-507-6399
www.CrossFire-Tech.com Warning: The information contained in this 
electronic mail message (and any attached document) from CrossFire 
Technologies Inc. is intended only for the personal and confidential use 
of the designated recipient(s) named above. This information may be 
considered confidential by CrossFire Technologies Inc, or may be 
privileged or otherwise exempt from disclosure under law. If the reader 
of this message is not the intended recipient or an agent responsible 
for delivering it to the intended recipient, you are hereby notified 
that you have received this message in error and that any review, 
dissemination, distribution, or copying of this message is strictly 
prohibited. If you have received the communication in error, please 
notify us immediately by telephone and e-mail and destroy any and all 
copies of this message (and all attached documents) in your possession 
(whether hard copies or electronically stored copies). This 
communication is covered by the Electronic Communications Privacy Act, 
18 U.S.C. §§ 2510 - 2521. Thank you.

Re: gdb for Riscv, single stepping issue

[John Baldwin]

On 6/27/22 1:40 PM, James Becker wrote:
> Hello,
> 
> I have a RISCV-EL2 core running in a Nexys A7 FPGA board.
> 
> I have openocd for riscv running over jtag with a connection by
> riscv-gdb to the openocd instance at port 3333.
> 
> Everything works fine, stepping, break points, load, view memory.
> 
> But I have one issue: Some of the memory in my design is 4 byte
> aligned.  Its designed for fast instruction fetch, its known as ICCM.
> 
> When I have code running in that memory, gdb still works fine for
> breakpoints, but it will not single step.
> 
> Looking at the openocd debug files, it appears that gdb is attempting to
> do a 2 byte read as a part of the single stepping procedure.
> 
> Since my memory does not support 2 byte reads or writes, this fails.
> 
> Is there some way that gdb can be configured to not do any 2-byte word
> reads or writes during single stepping?  I can't seem to find any.

Hmm, setting breakpoints tries to read 1 byte at a time unless you have
disabled compressed breakpoints.  It looks like as a local hack you could
change use-compressed-breakpoints to just read 4 bytes rather than 2
initially?  Perhaps this is upstreamable if you make it read 4 bytes if
the target address is 4 byte aligned and only fall back to reading 2 bytes
if it isn't?

-- 
John Baldwin

Re: gdb for Riscv, single stepping issue

[Pedro Alves]

On 2022-06-28 16:52, John Baldwin wrote:
> On 6/27/22 1:40 PM, James Becker wrote:
>> Hello,
>>
>> I have a RISCV-EL2 core running in a Nexys A7 FPGA board.
>>
>> I have openocd for riscv running over jtag with a connection by
>> riscv-gdb to the openocd instance at port 3333.
>>
>> Everything works fine, stepping, break points, load, view memory.
>>
>> But I have one issue: Some of the memory in my design is 4 byte
>> aligned.  Its designed for fast instruction fetch, its known as ICCM.
>>
>> When I have code running in that memory, gdb still works fine for
>> breakpoints, but it will not single step.
>>
>> Looking at the openocd debug files, it appears that gdb is attempting to
>> do a 2 byte read as a part of the single stepping procedure.
>>
>> Since my memory does not support 2 byte reads or writes, this fails.
>>
>> Is there some way that gdb can be configured to not do any 2-byte word
>> reads or writes during single stepping?  I can't seem to find any.
> 
> Hmm, setting breakpoints tries to read 1 byte at a time unless you have
> disabled compressed breakpoints.  It looks like as a local hack you could
> change use-compressed-breakpoints to just read 4 bytes rather than 2
> initially?  Perhaps this is upstreamable if you make it read 4 bytes if
> the target address is 4 byte aligned and only fall back to reading 2 bytes
> if it isn't?
> 

Is that from riscv_breakpoint_kind_from_pc?  That uses target_read_code,
so I'd think that since it goes via the code cache, it would read a whole
cache line at once, meaning 64 bytes (show code-cache, show dcache line-size).

OTOH, riscv_insn::fetch_instruction uses target_read_memory to read 2 bytes,
so I wonder whether this is the access in question:

  /* All insns are at least 16 bits.  */
  status = target_read_memory (addr, buf, 2);
  if (status)
    memory_error (TARGET_XFER_E_IO, addr);

Why is this using target_read_memory instead of target_read_code, though?
If it did that, then the code cache would be involved here too, papering over
the issue, presumably.

Curious that the Riscv stub behaves this way, though.  I mean, failing the
access instead of reading in aligned 4 bytes chunks, and doing read-modify-write,
if necessary, hiding the issue from GDB.  Even inf-ptrace.c handles unaligned
reads/writes and shorter-than-word-sized reads/writes itself, like:

static ULONGEST
inf_ptrace_peek_poke (ptid_t ptid, gdb_byte *readbuf,
		      const gdb_byte *writebuf,
		      ULONGEST addr, ULONGEST len)
{
...
  /* We transfer aligned words.  Thus align ADDR down to a word
     boundary and determine how many bytes to skip at the
     beginning.  */
...
      /* Read the word, also when doing a partial word write.  */
      if (readbuf != NULL || chunk < sizeof (PTRACE_TYPE_RET))
	{


I guess this also affects the "x" command at least (e.g., "x/2b $pc"), since that
doesn't use the code cache either.

Re: gdb for Riscv, single stepping issue

[James Becker]

Hi Pedro and John,

I tried it again, this time with the native gdb attached to riscv-gdb.

I set a breakpoint in the native gdb to break at the function call

target_read_memory () in riscv-gdb.

Then I did a step in the riscv-gdb and hit the breakpoint.  It does have a memory length of 2 for the method

target_read_memory ()

Here is the call stack.  Hope it helps:

Jim

#0  target_read_memory (memaddr=memaddr@entry=340, 
myaddr=myaddr@entry=0x7ffca4c45990 "\360\356\275\315PV",
     len=len@entry=2) at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/target.c:1229
#1  0x00005650cd44f941 in riscv_insn::fetch_instruction 
(gdbarch=gdbarch@entry=0x5650cdbdeef0, addr=340,
     len=len@entry=0x7ffca4c45a10) at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/riscv-tdep.c:1345
#2  0x00005650cd44fa4d in riscv_insn::decode (this=0x7ffca4c45a10, 
gdbarch=0x5650cdbdeef0, pc=<optimized out>)
     at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/riscv-tdep.c:1374
#3  0x00005650cd4520b9 in riscv_next_pc_atomic_sequence 
(next_pc=<synthetic pointer>, pc=340, regcache=0x5650cda77f40)
     at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/riscv-tdep.c:3635
#4  riscv_software_single_step (regcache=0x5650cda77f40)
     at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/riscv-tdep.c:3680
#5  0x00005650cd345288 in gdbarch_software_single_step 
(gdbarch=gdbarch@entry=0x5650cdbdeef0, regcache=0x5650cda77f40)
     at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/gdbarch.c:3280
#6  0x00005650cd256a96 in insert_single_step_breakpoints 
(gdbarch=gdbarch@entry=0x5650cdbdeef0)
     at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/breakpoint.c:14478
#7  0x00005650cd36aa90 in maybe_software_singlestep (pc=<optimized out>, 
gdbarch=0x5650cdbdeef0)
     at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/infrun.c:2127
#8  maybe_software_singlestep (gdbarch=gdbarch@entry=0x5650cdbdeef0, pc=340)
     at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/infrun.c:2121
#9  0x00005650cd371d48 in resume_1 (sig=GDB_SIGNAL_0)
     at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/infrun.c:2477
#10 resume (sig=GDB_SIGNAL_0) at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/infrun.c:2630
#11 0x00005650cd3728c4 in keep_going_pass_signal (ecs=0x7ffca4c45ca0)
     at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/infrun.c:7876
--Type <RET> for more, q to quit, c to continue without paging--
#12 0x00005650cd372bed in start_step_over () at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/infrun.c:2028
#13 0x00005650cd3789f0 in proceed (addr=addr@entry=18446744073709551615, 
siggnal=siggnal@entry=GDB_SIGNAL_DEFAULT)
     at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/infrun.c:3079
#14 0x00005650cd362691 in step_1 (skip_subroutines=0, 
single_inst=<optimized out>, count_string=<optimized out>)
     at /home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/infcmd.c:900
#15 0x00005650cd27f14a in cmd_func (cmd=<optimized out>, args=<optimized 
out>, from_tty=<optimized out>)
     at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/cli/cli-decode.c:2181
#16 0x00005650cd4c9c28 in execute_command (p=<optimized out>, 
p@entry=0x5650cdb65910 "step", from_tty=1)
     at /home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/top.c:668
#17 0x00005650cd326685 in command_handler (command=0x5650cdb65910 "step")
     at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/event-top.c:588
#18 0x00005650cd327551 in command_line_handler (rl=...)
     at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/event-top.c:773
#19 0x00005650cd326efc in gdb_rl_callback_handler (rl=0x5650ce306580 
"step") at /usr/include/c++/9/bits/unique_ptr.h:153
#20 0x00005650cd536f88 in rl_callback_read_char ()
     at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/readline/readline/callback.c:281
#21 0x00005650cd3261d6 in gdb_rl_callback_read_char_wrapper_noexcept ()
     at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/event-top.c:177
#22 0x00005650cd326db5 in gdb_rl_callback_read_char_wrapper 
(client_data=<optimized out>)
     at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/event-top.c:193
#23 0x00005650cd326108 in stdin_event_handler (error=<optimized out>, 
client_data=0x5650cd903ef0)
     at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/event-top.c:516
#24 0x00005650cd5d61d6 in gdb_wait_for_event (block=block@entry=1)
--Type <RET> for more, q to quit, c to continue without paging--
     at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdbsupport/event-loop.cc:673
#25 0x00005650cd5d643b in gdb_wait_for_event (block=1)
     at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdbsupport/event-loop.cc:569
#26 gdb_do_one_event () at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdbsupport/event-loop.cc:215
#27 0x00005650cd3a5035 in start_event_loop () at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/main.c:356
#28 captured_command_loop () at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/main.c:416
#29 0x00005650cd3a6ec5 in captured_main (data=<optimized out>)
     at /home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/main.c:1253
#30 gdb_main (args=<optimized out>) at 
/home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/main.c:1268
#31 0x00005650cd1f1f50 in main (argc=<optimized out>, argv=<optimized out>)
     at /home/crossfire/SweRV/riscv-gnu-toolchain/riscv-gdb/gdb/gdb.c:32


On 6/29/22 04:34, Pedro Alves wrote:
> On 2022-06-28 16:52, John Baldwin wrote:
>> On 6/27/22 1:40 PM, James Becker wrote:
>>> Hello,
>>>
>>> I have a RISCV-EL2 core running in a Nexys A7 FPGA board.
>>>
>>> I have openocd for riscv running over jtag with a connection by
>>> riscv-gdb to the openocd instance at port 3333.
>>>
>>> Everything works fine, stepping, break points, load, view memory.
>>>
>>> But I have one issue: Some of the memory in my design is 4 byte
>>> aligned.  Its designed for fast instruction fetch, its known as ICCM.
>>>
>>> When I have code running in that memory, gdb still works fine for
>>> breakpoints, but it will not single step.
>>>
>>> Looking at the openocd debug files, it appears that gdb is attempting to
>>> do a 2 byte read as a part of the single stepping procedure.
>>>
>>> Since my memory does not support 2 byte reads or writes, this fails.
>>>
>>> Is there some way that gdb can be configured to not do any 2-byte word
>>> reads or writes during single stepping?  I can't seem to find any.
>> Hmm, setting breakpoints tries to read 1 byte at a time unless you have
>> disabled compressed breakpoints.  It looks like as a local hack you could
>> change use-compressed-breakpoints to just read 4 bytes rather than 2
>> initially?  Perhaps this is upstreamable if you make it read 4 bytes if
>> the target address is 4 byte aligned and only fall back to reading 2 bytes
>> if it isn't?
>>
> Is that from riscv_breakpoint_kind_from_pc?  That uses target_read_code,
> so I'd think that since it goes via the code cache, it would read a whole
> cache line at once, meaning 64 bytes (show code-cache, show dcache line-size).
>
> OTOH, riscv_insn::fetch_instruction uses target_read_memory to read 2 bytes,
> so I wonder whether this is the access in question:
>
>    /* All insns are at least 16 bits.  */
>    status = target_read_memory (addr, buf, 2);
>    if (status)
>      memory_error (TARGET_XFER_E_IO, addr);
>
> Why is this using target_read_memory instead of target_read_code, though?
> If it did that, then the code cache would be involved here too, papering over
> the issue, presumably.
>
> Curious that the Riscv stub behaves this way, though.  I mean, failing the
> access instead of reading in aligned 4 bytes chunks, and doing read-modify-write,
> if necessary, hiding the issue from GDB.  Even inf-ptrace.c handles unaligned
> reads/writes and shorter-than-word-sized reads/writes itself, like:
>
> static ULONGEST
> inf_ptrace_peek_poke (ptid_t ptid, gdb_byte *readbuf,
> 		      const gdb_byte *writebuf,
> 		      ULONGEST addr, ULONGEST len)
> {
> ...
>    /* We transfer aligned words.  Thus align ADDR down to a word
>       boundary and determine how many bytes to skip at the
>       beginning.  */
> ...
>        /* Read the word, also when doing a partial word write.  */
>        if (readbuf != NULL || chunk < sizeof (PTRACE_TYPE_RET))
> 	{
>
>
> I guess this also affects the "x" command at least (e.g., "x/2b $pc"), since that
> doesn't use the code cache either.
-- 
Jim Becker
CrossFire Technologies Inc
1000 Westgate Dr. Suite 150-I
St. Paul, MN 55114
Mobile: +1 303-507-6399
www.CrossFire-Tech.com Warning: The information contained in this 
electronic mail message (and any attached document) from CrossFire 
Technologies Inc. is intended only for the personal and confidential use 
of the designated recipient(s) named above. This information may be 
considered confidential by CrossFire Technologies Inc, or may be 
privileged or otherwise exempt from disclosure under law. If the reader 
of this message is not the intended recipient or an agent responsible 
for delivering it to the intended recipient, you are hereby notified 
that you have received this message in error and that any review, 
dissemination, distribution, or copying of this message is strictly 
prohibited. If you have received the communication in error, please 
notify us immediately by telephone and e-mail and destroy any and all 
copies of this message (and all attached documents) in your possession 
(whether hard copies or electronically stored copies). This 
communication is covered by the Electronic Communications Privacy Act, 
18 U.S.C. §§ 2510 - 2521. Thank you.

Re: gdb for Riscv, single stepping issue

[John Baldwin]

On 6/29/22 2:34 AM, Pedro Alves wrote:
> On 2022-06-28 16:52, John Baldwin wrote:
>> On 6/27/22 1:40 PM, James Becker wrote:
>>> Hello,
>>>
>>> I have a RISCV-EL2 core running in a Nexys A7 FPGA board.
>>>
>>> I have openocd for riscv running over jtag with a connection by
>>> riscv-gdb to the openocd instance at port 3333.
>>>
>>> Everything works fine, stepping, break points, load, view memory.
>>>
>>> But I have one issue: Some of the memory in my design is 4 byte
>>> aligned.  Its designed for fast instruction fetch, its known as ICCM.
>>>
>>> When I have code running in that memory, gdb still works fine for
>>> breakpoints, but it will not single step.
>>>
>>> Looking at the openocd debug files, it appears that gdb is attempting to
>>> do a 2 byte read as a part of the single stepping procedure.
>>>
>>> Since my memory does not support 2 byte reads or writes, this fails.
>>>
>>> Is there some way that gdb can be configured to not do any 2-byte word
>>> reads or writes during single stepping?  I can't seem to find any.
>>
>> Hmm, setting breakpoints tries to read 1 byte at a time unless you have
>> disabled compressed breakpoints.  It looks like as a local hack you could
>> change use-compressed-breakpoints to just read 4 bytes rather than 2
>> initially?  Perhaps this is upstreamable if you make it read 4 bytes if
>> the target address is 4 byte aligned and only fall back to reading 2 bytes
>> if it isn't?
>>
> 
> Is that from riscv_breakpoint_kind_from_pc?  That uses target_read_code,
> so I'd think that since it goes via the code cache, it would read a whole
> cache line at once, meaning 64 bytes (show code-cache, show dcache line-size).
> 
> OTOH, riscv_insn::fetch_instruction uses target_read_memory to read 2 bytes,
> so I wonder whether this is the access in question:
> 
>    /* All insns are at least 16 bits.  */
>    status = target_read_memory (addr, buf, 2);
>    if (status)
>      memory_error (TARGET_XFER_E_IO, addr);
> 
> Why is this using target_read_memory instead of target_read_code, though?
> If it did that, then the code cache would be involved here too, papering over
> the issue, presumably.
> 
> Curious that the Riscv stub behaves this way, though.  I mean, failing the
> access instead of reading in aligned 4 bytes chunks, and doing read-modify-write,
> if necessary, hiding the issue from GDB.  Even inf-ptrace.c handles unaligned
> reads/writes and shorter-than-word-sized reads/writes itself, like:
> 
> static ULONGEST
> inf_ptrace_peek_poke (ptid_t ptid, gdb_byte *readbuf,
> 		      const gdb_byte *writebuf,
> 		      ULONGEST addr, ULONGEST len)
> {
> ...
>    /* We transfer aligned words.  Thus align ADDR down to a word
>       boundary and determine how many bytes to skip at the
>       beginning.  */
> ...
>        /* Read the word, also when doing a partial word write.  */
>        if (readbuf != NULL || chunk < sizeof (PTRACE_TYPE_RET))
> 	{
> 
> 
> I guess this also affects the "x" command at least (e.g., "x/2b $pc"), since that
> doesn't use the code cache either.

Yes, I had reworked my e-mail part way through and I really meant to talk about
this code, not the breakpoint code.  I agree that target_read_code is probably
more correct in fetch_instruction and would in this case hide the odd behavior
of the stub:

diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index 69f2123dcdb..09b2599e958 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -1661,7 +1661,7 @@ riscv_insn::fetch_instruction (struct gdbarch *gdbarch,
    int instlen, status;
  
    /* All insns are at least 16 bits.  */
-  status = target_read_memory (addr, buf, 2);
+  status = target_read_code (addr, buf, 2);
    if (status)
      memory_error (TARGET_XFER_E_IO, addr);
  
@@ -1672,7 +1672,7 @@ riscv_insn::fetch_instruction (struct gdbarch *gdbarch,
  
    if (instlen > 2)
      {
-      status = target_read_memory (addr + 2, buf + 2, instlen - 2);
+      status = target_read_code (addr + 2, buf + 2, instlen - 2);
        if (status)
  	memory_error (TARGET_XFER_E_IO, addr + 2);
      }


-- 
John Baldwin

Re: gdb for Riscv, single stepping issue

[Pedro Alves]

Hi!

Jim, could you check whether the patch below addresses the issue for you?
I imagine it does, but it'd be good to hear just in case we're missing some
more target_read_memory calls somewhere or some such.

Pedro Alves

On 2022-06-29 18:54, John Baldwin wrote:
> On 6/29/22 2:34 AM, Pedro Alves wrote:

>> OTOH, riscv_insn::fetch_instruction uses target_read_memory to read 2 bytes,
>> so I wonder whether this is the access in question:
>>
>>    /* All insns are at least 16 bits.  */
>>    status = target_read_memory (addr, buf, 2);
>>    if (status)
>>      memory_error (TARGET_XFER_E_IO, addr);
>>
>> Why is this using target_read_memory instead of target_read_code, though?
>> If it did that, then the code cache would be involved here too, papering over
>> the issue, presumably.
>>
> Yes, I had reworked my e-mail part way through and I really meant to talk about
> this code, not the breakpoint code.  I agree that target_read_code is probably
> more correct in fetch_instruction and would in this case hide the odd behavior
> of the stub:
> 
> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> index 69f2123dcdb..09b2599e958 100644
> --- a/gdb/riscv-tdep.c
> +++ b/gdb/riscv-tdep.c
> @@ -1661,7 +1661,7 @@ riscv_insn::fetch_instruction (struct gdbarch *gdbarch,
>    int instlen, status;
>  
>    /* All insns are at least 16 bits.  */
> -  status = target_read_memory (addr, buf, 2);
> +  status = target_read_code (addr, buf, 2);
>    if (status)
>      memory_error (TARGET_XFER_E_IO, addr);
>  
> @@ -1672,7 +1672,7 @@ riscv_insn::fetch_instruction (struct gdbarch *gdbarch,
>  
>    if (instlen > 2)
>      {
> -      status = target_read_memory (addr + 2, buf + 2, instlen - 2);
> +      status = target_read_code (addr + 2, buf + 2, instlen - 2);
>        if (status)
>      memory_error (TARGET_XFER_E_IO, addr + 2);
>      }
> 
> 

Re: gdb for Riscv, single stepping issue

[James Becker]

Hi Pedro,

I did make the change and recompiled, but I have unfortunately lost my hardware program to my client for testing.

So it will be a week or two until I can test.

Thanks for your help.

Jim
________________________________
From: Pedro Alves <pedro@palves.net>
Sent: Thursday, June 30, 2022 4:40 AM
To: John Baldwin <jhb@FreeBSD.org>; James Becker <james.becker@crossfire-tech.com>; gdb@sourceware.org <gdb@sourceware.org>
Subject: Re: gdb for Riscv, single stepping issue

Hi!

Jim, could you check whether the patch below addresses the issue for you?
I imagine it does, but it'd be good to hear just in case we're missing some
more target_read_memory calls somewhere or some such.

Pedro Alves

On 2022-06-29 18:54, John Baldwin wrote:
> On 6/29/22 2:34 AM, Pedro Alves wrote:

>> OTOH, riscv_insn::fetch_instruction uses target_read_memory to read 2 bytes,
>> so I wonder whether this is the access in question:
>>
>>    /* All insns are at least 16 bits.  */
>>    status = target_read_memory (addr, buf, 2);
>>    if (status)
>>      memory_error (TARGET_XFER_E_IO, addr);
>>
>> Why is this using target_read_memory instead of target_read_code, though?
>> If it did that, then the code cache would be involved here too, papering over
>> the issue, presumably.
>>
> Yes, I had reworked my e-mail part way through and I really meant to talk about
> this code, not the breakpoint code.  I agree that target_read_code is probably
> more correct in fetch_instruction and would in this case hide the odd behavior
> of the stub:
>
> diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
> index 69f2123dcdb..09b2599e958 100644
> --- a/gdb/riscv-tdep.c
> +++ b/gdb/riscv-tdep.c
> @@ -1661,7 +1661,7 @@ riscv_insn::fetch_instruction (struct gdbarch *gdbarch,
>    int instlen, status;
>
>    /* All insns are at least 16 bits.  */
> -  status = target_read_memory (addr, buf, 2);
> +  status = target_read_code (addr, buf, 2);
>    if (status)
>      memory_error (TARGET_XFER_E_IO, addr);
>
> @@ -1672,7 +1672,7 @@ riscv_insn::fetch_instruction (struct gdbarch *gdbarch,
>
>    if (instlen > 2)
>      {
> -      status = target_read_memory (addr + 2, buf + 2, instlen - 2);
> +      status = target_read_code (addr + 2, buf + 2, instlen - 2);
>        if (status)
>      memory_error (TARGET_XFER_E_IO, addr + 2);
>      }
>
>