[RFA/commit,gdbserver/lynx] spurious failure to write in inferior memory
Commit Message
Hello,
We noticed the following error on ppc-lynx178, using just about
any program:
(gdb) tar remote mytarget:4444
Remote debugging using mytarget:4444
0x000100c8 in _start ()
(gdb) b try
Breakpoint 1 at 0x10844: file try.adb, line 11.
(gdb) cont
Continuing.
!!!-> Cannot remove breakpoints because program is no longer writable.
!!!-> Further execution is probably impossible.
Breakpoint 1, try () at try.adb:11
11 Local : Integer := 18;
And, of course, trying to continue yielded the expected outcome:
(gdb) c
Continuing.
warning: Error removing breakpoint 1
Cannot remove breakpoints because program is no longer writable.
Further execution is probably impossible.
It turns out that the problem is caused by an intentional test
against a variable with an undefined value. After GDB receives
notification of the inferior stopping, it tries to remove the
breakpoint by sending a memory-write packet ("X10844,4:9 ").
This leads us to lynx_write_memory, where it tries to split
the memory-write into chunks of 4 bytes. And, in order to handle
writes which are not aligned on word boundaries, we have the
following code:
if (skip > 0 || truncate > 0)
/* We need to read the memory at this address in order to preserve
the data that we are not overwriting. */
lynx_read_memory (addr, (unsigned char *) &buf, xfer_size);
if (errno)
return errno;
(the comment explains what the code is about).
Unfortunately, the not-so-glaring error that we've made here is
that we're checking ERRNO regardless of whether we've called
lynx_read_memory. In our case, because we are writing 4 bytes
aligned on a word boundary, we do not call lynx_read_memory and
therefore test an ERRNO with an undefined value.
gdb/gdbserver/ChangeLog:
* lynx-low.c (lynx_write_memory): Put lynx_read_memory and
corresponding ERRNO check in same block.
One wonders how we got lucky for so long. In any case, tested on
ppc-lynx5 and ppc-lynx178 using AdaCore's testsuite. It's a fairly
obvious fix to me, but I will let it sit for a few days... or an
approval ;-).
Thank you,
Comments
On 11/14/2014 04:51 PM, Joel Brobecker wrote:
>
> gdb/gdbserver/ChangeLog:
>
> * lynx-low.c (lynx_write_memory): Put lynx_read_memory and
> corresponding ERRNO check in same block.
Eh.
> One wonders how we got lucky for so long. In any case, tested on
> ppc-lynx5 and ppc-lynx178 using AdaCore's testsuite. It's a fairly
> obvious fix to me, but I will let it sit for a few days... or an
> approval ;-).
Looks obvious to me.
Thanks,
Pedro Alves
> > gdb/gdbserver/ChangeLog:
> >
> > * lynx-low.c (lynx_write_memory): Put lynx_read_memory and
> > corresponding ERRNO check in same block.
>
> Eh.
>
> > One wonders how we got lucky for so long. In any case, tested on
> > ppc-lynx5 and ppc-lynx178 using AdaCore's testsuite. It's a fairly
> > obvious fix to me, but I will let it sit for a few days... or an
> > approval ;-).
>
> Looks obvious to me.
Thanks for taking a look, Pedro. I just pushed the patch.
@@ -688,11 +688,13 @@ lynx_write_memory (CORE_ADDR memaddr, const unsigned char *myaddr, int len)
if (addr + xfer_size > memaddr + len)
truncate = addr + xfer_size - memaddr - len;
if (skip > 0 || truncate > 0)
- /* We need to read the memory at this address in order to preserve
- the data that we are not overwriting. */
- lynx_read_memory (addr, (unsigned char *) &buf, xfer_size);
- if (errno)
- return errno;
+ {
+ /* We need to read the memory at this address in order to preserve
+ the data that we are not overwriting. */
+ lynx_read_memory (addr, (unsigned char *) &buf, xfer_size);
+ if (errno)
+ return errno;
+ }
memcpy ((gdb_byte *) &buf + skip, myaddr + (addr - memaddr) + skip,
xfer_size - skip - truncate);
errno = 0;