Re: [PATCH] breakpoint remove fail handle bug fix

[Hui Zhu <hui_zhu@mentor.com>]

On 04/12/12 04:35, Jan Kratochvil wrote:
> On Wed, 11 Apr 2012 11:07:08 +0200, Hui Zhu wrote:
>> (gdb) d
>> Delete all breakpoints? (y or n) y
>> warning: Error removing breakpoint 2
>
> I would propose the attached patch instead.
>
> It needs a testcase, would you write one?
>
> Not sure if gdbserver also needs a fix or not.
>
> No regressions on {x86_64,x86_64-m32,i686}-fedora17-linux-gnu.

Hi Jan,

Thanks for your patch.  And it handle the local debug very well.
But it didn't handle the issue when the target is remote.  I am sorry 
that I didn't talk clear in my mail.  I use the gdb that patch your 
patch and do a test with remote:

(gdb) set target-async on
(reverse-i-search)`target': set ^CQuit-async on
(gdb) disassemble
No frame selected.
(gdb) target remote :1234
Remote debugging using :1234
Reading symbols from /lib64/ld-linux-x86-64.so.2...(no debugging symbols 
found)...done.
Loaded symbols for /lib64/ld-linux-x86-64.so.2
0x00007ffff7ddcaf0 in ?? () from /lib64/ld-linux-x86-64.so.2
(gdb) tb main
Temporary breakpoint 1 at 0x4004c8: file tmp.c, line 4.
(gdb) disassemble
No function contains program counter for selected frame.
(gdb) c
Continuing.

Program received signal SIGINT, Interrupt.
0x00007ffff7ddcaf0 in ?? () from /lib64/ld-linux-x86-64.so.2
(gdb) c
Continuing.

Temporary breakpoint 1, main () at tmp.c:4
4	sleep (20);
(gdb) disas
Dump of assembler code for function main:
    0x00000000004004c4 <+0>:	push   %rbp
    0x00000000004004c5 <+1>:	mov    %rsp,%rbp
=> 0x00000000004004c8 <+4>:	mov    $0x14,%edi
    0x00000000004004cd <+9>:	mov    $0x0,%eax
    0x00000000004004d2 <+14>:	callq  0x4003d0 <sleep@plt>
    0x00000000004004d7 <+19>:	mov    $0x0,%eax
    0x00000000004004dc <+24>:	pop    %rbp
    0x00000000004004dd <+25>:	retq
End of assembler dump.
(gdb) b 6
Breakpoint 2 at 0x4004d7: file tmp.c, line 6.
(gdb) c &
Continuing.
(gdb) d
Delete all breakpoints? (y or n) y
Cannot execute this command while the target is running.
(gdb)
Program received signal SIGTRAP, Trace/breakpoint trap.
0x00000000004004d8 in main () at tmp.c:6
6	return 0;
c
Continuing.

Program received signal SIGSEGV, Segmentation fault.
0x00000000004004d8 in main () at tmp.c:6
6	return 0;
(gdb) c
Continuing.

Program terminated with signal SIGSEGV, Segmentation fault.
The program no longer exists.
(gdb)

Do you have some good idea on this part?


Thanks,
Hui

>
>
> Thanks,
> Jan
>
>
> gdb/
> 2012-04-11  Jan Kratochvil<jan.kratochvil@redhat.com>
>
> 	* linux-nat.c (linux_proc_xfer_partial): Do not check for LEN size and
> 	support also WRITEBUF.
> 	(linux_xfer_partial): Move here the LEN check from
> 	linux_proc_xfer_partial but also call linux_proc_xfer_partial as a last
> 	resort if super_xfer_partial fails.
>
> --- a/gdb/linux-nat.c
> +++ b/gdb/linux-nat.c
> @@ -4494,9 +4494,9 @@ linux_nat_make_corefile_notes (bfd *obfd, int *note_size)
>   }
>
>   /* Implement the to_xfer_partial interface for memory reads using the /proc
> -   filesystem.  Because we can use a single read() call for /proc, this
> -   can be much more efficient than banging away at PTRACE_PEEKTEXT,
> -   but it doesn't support writes.  */
> +   filesystem.  Because we can use a single read or write call for /proc, this
> +   can be much more efficient than banging away at PTRACE_PEEKTEXT or
> +   PTRACE_POKETEXT.  */
>
>   static LONGEST
>   linux_proc_xfer_partial (struct target_ops *ops, enum target_object object,
> @@ -4508,29 +4508,35 @@ linux_proc_xfer_partial (struct target_ops *ops, enum target_object object,
>     int fd;
>     char filename[64];
>
> -  if (object != TARGET_OBJECT_MEMORY || !readbuf)
> -    return 0;
> -
> -  /* Don't bother for one word.  */
> -  if (len<  3 * sizeof (long))
> +  if (object != TARGET_OBJECT_MEMORY)
>       return 0;
>
>     /* We could keep this file open and cache it - possibly one per
>        thread.  That requires some juggling, but is even faster.  */
>     sprintf (filename, "/proc/%d/mem", PIDGET (inferior_ptid));
> -  fd = open (filename, O_RDONLY | O_LARGEFILE);
> +  fd = open (filename, (readbuf ? O_RDONLY : O_WRONLY) | O_LARGEFILE);
>     if (fd == -1)
>       return 0;
>
> -  /* If pread64 is available, use it.  It's faster if the kernel
> +  /* If pread64 or pwrite64 is available, use it.  It's faster if the kernel
>        supports it (only one syscall), and it's 64-bit safe even on
>        32-bit platforms (for instance, SPARC debugging a SPARC64
>        application).  */
> +  if ((readbuf != NULL
> +#ifdef HAVE_PREAD64
> +&&  (pread64 (fd, readbuf, len, offset) != len)
> +#else
> +&&  (lseek (fd, offset, SEEK_SET) == -1 || read (fd, readbuf, len) != len)
> +#endif
> +       )
> +      || (writebuf != NULL
>   #ifdef HAVE_PREAD64
> -  if (pread64 (fd, readbuf, len, offset) != len)
> +	&&  (pwrite64 (fd, writebuf, len, offset) != len)
>   #else
> -  if (lseek (fd, offset, SEEK_SET) == -1 || read (fd, readbuf, len) != len)
> +	&&  (lseek (fd, offset, SEEK_SET) == -1
> +	      || write (fd, writebuf, len) != len)
>   #endif
> +          ))
>       ret = 0;
>     else
>       ret = len;
> @@ -4759,13 +4765,24 @@ linux_xfer_partial (struct target_ops *ops, enum target_object object,
>   	offset&= ((ULONGEST) 1<<  addr_bit) - 1;
>       }
>
> -  xfer = linux_proc_xfer_partial (ops, object, annex, readbuf, writebuf,
> -				  offset, len);
> +  /* Use more expensive linux_proc_xfer_partial only for larger transfers.  */
> +  if (len>= 3 * sizeof (long))
> +    {
> +      xfer = linux_proc_xfer_partial (ops, object, annex, readbuf, writebuf,
> +				      offset, len);
> +      if (xfer != 0)
> +	return xfer;
> +    }
> +
> +  xfer = super_xfer_partial (ops, object, annex, readbuf, writebuf,
> +			     offset, len);
>     if (xfer != 0)
>       return xfer;
>
> -  return super_xfer_partial (ops, object, annex, readbuf, writebuf,
> -			     offset, len);
> +  /* PTRACE_* of super_xfer_partial may not work if the inferior is running.
> +     linux_proc_xfer_partial still may work in such case.  */
> +  return linux_proc_xfer_partial (ops, object, annex, readbuf, writebuf,
> +				  offset, len);
>   }
>
>   static void