This is the mail archive of the gdb-patches@sources.redhat.com mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [RFA] Revised patch: fix crasher bug in child_xfer_memory


Michael Snyder wrote:
> 
> I've revised the patch so that it will not use alloca for a buffer
> greater than 4K.  I've also added a call to do_cleanups, so that if
> xmalloc is used, its memory will be freed as soon as possible.
> And I've added FIXME comments to the four other modules that use
> clones of this code (in case they also run into problems with
> large alloca buffers).

Committed.

> 
> 2001-12-30  Michael Snyder  <msnyder@redhat.com>
> 
>         * infptrace.c (GDB_MAX_ALLOCA): New define.
>         (child_xfer_memory): Use xmalloc/xfree instead of alloca if the
>         size of the buffer exceeds GDB_MAX_ALLOCA (default 1 megabyte,
>         can be overridden with whatever value is appropriate to the host).
>         * infttrace.c (child_xfer_memory): Add FIXME warning about use of
>         alloca to allocate potentially large buffer.
>         * rs6000-nat.c (child_xfer_memory): Ditto.
>         * symm-nat.c (child_xfer_memory): Ditto.
>         * x86-64-linux-nat.c (child_xfer_memory): Ditto.
> 
> 2001-12-30  Michael Snyder  <msnyder@redhat.com>
> 
>         * gdb.base/huge.exp: New test.  Print a very large target data object.
>         (skip_huge_test): New test variable.  Define if you want to skip this
>         test.  The test reads an 8 megabyte data object from the target, so it
>         might be very time consuming on remote targets with a slow connection.
>         * gdb.base/huge.c: New file.  Test case for above.
> 
> Index: infptrace.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infptrace.c,v
> retrieving revision 1.19
> diff -p -r1.19 infptrace.c
> *** infptrace.c 2001/11/19 23:59:55     1.19
> --- infptrace.c 2002/01/03 20:26:08
> *************** store_inferior_registers (int regno)
> *** 480,485 ****
> --- 480,490 ----
>   #endif /* !defined (FETCH_INFERIOR_REGISTERS).  */
> 
> 
> + /* Set an upper limit on alloca.  */
> + #ifndef GDB_MAX_ALLOCA
> + #define GDB_MAX_ALLOCA 0x1000
> + #endif
> +
>   #if !defined (CHILD_XFER_MEMORY)
>   /* NOTE! I tried using PTRACE_READDATA, etc., to read and write memory
>      in the NEW_SUN_PTRACE case.  It ought to be straightforward.  But
> *************** child_xfer_memory (CORE_ADDR memaddr, ch
> *** 506,514 ****
>     /* Round ending address up; get number of longwords that makes.  */
>     int count = ((((memaddr + len) - addr) + sizeof (PTRACE_XFER_TYPE) - 1)
>                / sizeof (PTRACE_XFER_TYPE));
>     /* Allocate buffer of that many longwords.  */
> !   PTRACE_XFER_TYPE *buffer =
> !     (PTRACE_XFER_TYPE *) alloca (count * sizeof (PTRACE_XFER_TYPE));
> 
>     if (write)
>       {
> --- 511,530 ----
>     /* Round ending address up; get number of longwords that makes.  */
>     int count = ((((memaddr + len) - addr) + sizeof (PTRACE_XFER_TYPE) - 1)
>                / sizeof (PTRACE_XFER_TYPE));
> +   int alloc = count * sizeof (PTRACE_XFER_TYPE);
> +   PTRACE_XFER_TYPE *buffer;
> +   struct cleanup *old_chain = NULL;
> +
>     /* Allocate buffer of that many longwords.  */
> !   if (len < GDB_MAX_ALLOCA)
> !     {
> !       buffer = (PTRACE_XFER_TYPE *) alloca (alloc);
> !     }
> !   else
> !     {
> !       buffer = (PTRACE_XFER_TYPE *) xmalloc (alloc);
> !       old_chain = make_cleanup (xfree, buffer);
> !     }
> 
>     if (write)
>       {
> *************** child_xfer_memory (CORE_ADDR memaddr, ch
> *** 573,578 ****
> --- 589,596 ----
>               len);
>       }
> 
> +   if (old_chain != NULL)
> +     do_cleanups (old_chain);
>     return len;
>   }
> 
> Index: infttrace.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/infttrace.c,v
> retrieving revision 1.17
> diff -p -r1.17 infttrace.c
> *** infttrace.c 2001/12/19 19:16:50     1.17
> --- infttrace.c 2002/01/03 20:26:08
> *************** child_xfer_memory (CORE_ADDR memaddr, ch
> *** 4922,4929 ****
>     = (((memaddr + len) - addr) + sizeof (TTRACE_XFER_TYPE) - 1)
>     / sizeof (TTRACE_XFER_TYPE);
>     /* Allocate buffer of that many longwords.  */
>     register TTRACE_XFER_TYPE *buffer
> !   = (TTRACE_XFER_TYPE *) alloca (count * sizeof (TTRACE_XFER_TYPE));
> 
>     if (write)
>       {
> --- 4922,4932 ----
>     = (((memaddr + len) - addr) + sizeof (TTRACE_XFER_TYPE) - 1)
>     / sizeof (TTRACE_XFER_TYPE);
>     /* Allocate buffer of that many longwords.  */
> +   /* FIXME (alloca): This code, cloned from infptrace.c, is unsafe
> +      because it uses alloca to allocate a buffer of arbitrary size.
> +      For very large xfers, this could crash GDB's stack.  */
>     register TTRACE_XFER_TYPE *buffer
> !     = (TTRACE_XFER_TYPE *) alloca (count * sizeof (TTRACE_XFER_TYPE));
> 
>     if (write)
>       {
> Index: rs6000-nat.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/rs6000-nat.c,v
> retrieving revision 1.19
> diff -p -r1.19 rs6000-nat.c
> *** rs6000-nat.c        2001/12/20 23:29:24     1.19
> --- rs6000-nat.c        2002/01/03 20:26:08
> *************** child_xfer_memory (CORE_ADDR memaddr, ch
> *** 407,412 ****
> --- 407,415 ----
>       / sizeof (int);
> 
>     /* Allocate word transfer buffer. */
> +   /* FIXME (alloca): This code, cloned from infptrace.c, is unsafe
> +      because it uses alloca to allocate a buffer of arbitrary size.
> +      For very large xfers, this could crash GDB's stack.  */
>     int *buf = (int *) alloca (count * sizeof (int));
> 
>     int arch64 = ARCH64 ();
> Index: symm-nat.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/symm-nat.c,v
> retrieving revision 1.10
> diff -p -r1.10 symm-nat.c
> *** symm-nat.c  2001/07/26 02:23:57     1.10
> --- symm-nat.c  2002/01/03 20:26:08
> *************** child_xfer_memory (CORE_ADDR memaddr, ch
> *** 764,771 ****
>     = (((memaddr + len) - addr) + sizeof (PTRACE_XFER_TYPE) - 1)
>     / sizeof (PTRACE_XFER_TYPE);
>     /* Allocate buffer of that many longwords.  */
>     register PTRACE_XFER_TYPE *buffer
> !   = (PTRACE_XFER_TYPE *) alloca (count * sizeof (PTRACE_XFER_TYPE));
> 
>     if (write)
>       {
> --- 764,774 ----
>     = (((memaddr + len) - addr) + sizeof (PTRACE_XFER_TYPE) - 1)
>     / sizeof (PTRACE_XFER_TYPE);
>     /* Allocate buffer of that many longwords.  */
> +   /* FIXME (alloca): This code, cloned from infptrace.c, is unsafe
> +      because it uses alloca to allocate a buffer of arbitrary size.
> +      For very large xfers, this could crash GDB's stack.  */
>     register PTRACE_XFER_TYPE *buffer
> !     = (PTRACE_XFER_TYPE *) alloca (count * sizeof (PTRACE_XFER_TYPE));
> 
>     if (write)
>       {
> Index: x86-64-linux-nat.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/x86-64-linux-nat.c,v
> retrieving revision 1.1
> diff -p -r1.1 x86-64-linux-nat.c
> *** x86-64-linux-nat.c  2001/09/21 12:19:15     1.1
> --- x86-64-linux-nat.c  2002/01/03 20:26:08
> *************** child_xfer_memory (CORE_ADDR memaddr, ch
> *** 417,422 ****
> --- 416,424 ----
>       = (((memaddr + len) - addr) + sizeof (PTRACE_XFER_TYPE) - 1)
>       / sizeof (PTRACE_XFER_TYPE);
>     /* Allocate buffer of that many longwords.  */
> +   /* FIXME (alloca): This code, cloned from infptrace.c, is unsafe
> +      because it uses alloca to allocate a buffer of arbitrary size.
> +      For very large xfers, this could crash GDB's stack.  */
>     register PTRACE_XFER_TYPE *buffer
>       = (PTRACE_XFER_TYPE *) alloca (count * sizeof (PTRACE_XFER_TYPE));
> 
> Index: testsuite/gdb.base/huge.c
> ===================================================================
> RCS file: huge.c
> diff -N huge.c
> *** /dev/null   Tue May  5 13:32:27 1998
> --- huge.c      Sun Dec 30 16:10:49 2001
> ***************
> *** 0 ****
> --- 1,19 ----
> + /*
> +  * Test GDB's ability to read a very large data object from target memory.
> +  */
> +
> + /*
> +  * A value that will produce a target data object
> +  * large enough to crash GDB.  0x200000 is big enough
> +  * on Linux, other systems may need a larger number.
> +  */
> +
> + #define CRASH_GDB 0x200000
> +
> + static int a[CRASH_GDB], b[CRASH_GDB];
> +
> + main()
> + {
> +   memcpy (a, b, sizeof (a));
> +   return 0;
> + }
> Index: testsuite/gdb.base/huge.exp
> ===================================================================
> RCS file: huge.exp
> diff -N huge.exp
> *** /dev/null   Tue May  5 13:32:27 1998
> --- huge.exp    Sun Dec 30 16:10:49 2001
> ***************
> *** 0 ****
> --- 1,57 ----
> + # Copyright 2001 Free Software Foundation, Inc.
> +
> + # This program is free software; you can redistribute it and/or modify
> + # it under the terms of the GNU General Public License as published by
> + # the Free Software Foundation; either version 2 of the License, or
> + # (at your option) any later version.
> + #
> + # This program is distributed in the hope that it will be useful,
> + # but WITHOUT ANY WARRANTY; without even the implied warranty of
> + # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + # GNU General Public License for more details.
> + #
> + # You should have received a copy of the GNU General Public License
> + # along with this program; if not, write to the Free Software
> + # Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
> +
> + # Please email any bugs, comments, and/or additions to this file to:
> + # bug-gdb@prep.ai.mit.edu
> +
> + # This file was written by Michael Snyder (msnyder@redhat.com)
> +
> + if $tracelevel then {
> +       strace $tracelevel
> + }
> +
> + set prms_id 0
> + set bug_id 0
> +
> + # Define if you want to skip this test
> + # (could be very time-consuming on remote targets with slow connection).
> + #
> + if [target_info exists gdb,skip_huge_test] {
> +     return;
> + }
> +
> + set testfile "huge"
> + set srcfile ${testfile}.c
> + set binfile ${objdir}/${subdir}/${testfile}
> + if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug}] != "" } {
> +      gdb_suppress_entire_file "Testcase compile failed, so all tests in this file will automatically fail."
> + }
> +
> + # Start with a fresh gdb.
> +
> + gdb_exit
> + gdb_start
> + gdb_reinitialize_dir $srcdir/$subdir
> + gdb_load ${binfile}
> +
> + set timeout 30
> +
> + if { ! [ runto main ] } then {
> +     gdb_suppress_entire_file "Run to main failed, so all tests in this file will automatically fail."
> + }
> +
> + gdb_test "print a" ".1 = .0 .repeats \[0123456789\]+ times.." "print a very large data object"
> +


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]