symtab.c -- Fix off by one allocation bug

Message ID 31327.1465840699@usendtaylorx2l
State New, archived
Headers

Commit Message

David Taylor June 13, 2016, 5:58 p.m. UTC
  This fixes a problem found by valgrind.  Applying this patch caused no
regressions on GNU/Linux x86-64 and satisfied valgrind.

	* gdb/symtab.c (symbol_set_names): Fix off by one error in
	allocation.
---
 gdb/ChangeLog | 4 ++++
 gdb/symtab.c  | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)
  

Comments

Joel Brobecker June 13, 2016, 6:23 p.m. UTC | #1
> This fixes a problem found by valgrind.  Applying this patch caused no
> regressions on GNU/Linux x86-64 and satisfied valgrind.
> 
> 	* gdb/symtab.c (symbol_set_names): Fix off by one error in
> 	allocation.

Thanks for the patch.

Can you give a little more detail as to why we need that extra byte,
and provide that information in the revision log. This kind of
information is always very precious when doing archelogy.

Thanks!

> ---
>  gdb/ChangeLog | 4 ++++
>  gdb/symtab.c  | 2 +-
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/gdb/ChangeLog b/gdb/ChangeLog
> index 9c09269..aaeeb6e 100644
> --- a/gdb/ChangeLog
> +++ b/gdb/ChangeLog
> @@ -1,3 +1,7 @@
> +2016-06-09  David Taylor  <dtaylor@emc.com>
> +
> +	* symtab.c (symbol_set_names): Fix off by one error in allocation.
> +
>  2016-06-07  Simon Marchi  <simon.marchi@ericsson.com>
>  
>  	* mi/mi-interp.c (mi_record_changed): Add missing braces.
> diff --git a/gdb/symtab.c b/gdb/symtab.c
> index f7a207a..12e1cf5 100644
> --- a/gdb/symtab.c
> +++ b/gdb/symtab.c
> @@ -1010,7 +1010,7 @@ symbol_set_names (struct general_symbol_info *gsymbol,
>  	    = ((struct demangled_name_entry *)
>  	       obstack_alloc (&per_bfd->storage_obstack,
>  			      offsetof (struct demangled_name_entry, demangled)
> -			      + lookup_len + demangled_len + 2));
> +			      + lookup_len + demangled_len + 3));
>  	  mangled_ptr = &((*slot)->demangled[demangled_len + 1]);
>  	  strcpy (mangled_ptr, lookup_name);
>  	  (*slot)->mangled = mangled_ptr;
> -- 
> 1.9.1
  
taylor, david June 16, 2016, 4:25 p.m. UTC | #2
> From: Joel Brobecker [mailto:brobecker@adacore.com]
> Sent: Monday, June 13, 2016 2:23 PM
> To: taylor, david
> Cc: gdb-patches@sourceware.org
> Subject: Re: [PATCH] symtab.c -- Fix off by one allocation bug
> 
> > This fixes a problem found by valgrind.  Applying this patch caused no
> > regressions on GNU/Linux x86-64 and satisfied valgrind.
> >
> > 	* gdb/symtab.c (symbol_set_names): Fix off by one error in
> > 	allocation.
> 
> Thanks for the patch.
> 
> Can you give a little more detail as to why we need that extra byte, and
> provide that information in the revision log. This kind of information is always
> very precious when doing archelogy.
> 
> Thanks!

Sadly, probably not.

This bug was found awhile ago and at the time we were using STAB
debugging format.  We have since switched to DWARF.  I was having
problems finding the source of a problem we were having and threw
valgrind at it.  I ultimately found the other problem via other methods,
but valgrind found this problem.  I remember staring at the code long
enough to convince myself that valgrind was correct.  And after increasing
it, valgrind was happy.

> > ---
> >  gdb/ChangeLog | 4 ++++
> >  gdb/symtab.c  | 2 +-
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 9c09269..aaeeb6e
> > 100644
> > --- a/gdb/ChangeLog
> > +++ b/gdb/ChangeLog
> > @@ -1,3 +1,7 @@
> > +2016-06-09  David Taylor  <dtaylor@emc.com>
> > +
> > +	* symtab.c (symbol_set_names): Fix off by one error in allocation.
> > +
> >  2016-06-07  Simon Marchi  <simon.marchi@ericsson.com>
> >
> >  	* mi/mi-interp.c (mi_record_changed): Add missing braces.
> > diff --git a/gdb/symtab.c b/gdb/symtab.c index f7a207a..12e1cf5 100644
> > --- a/gdb/symtab.c
> > +++ b/gdb/symtab.c
> > @@ -1010,7 +1010,7 @@ symbol_set_names (struct general_symbol_info
> *gsymbol,
> >  	    = ((struct demangled_name_entry *)
> >  	       obstack_alloc (&per_bfd->storage_obstack,
> >  			      offsetof (struct demangled_name_entry,
> demangled)
> > -			      + lookup_len + demangled_len + 2));
> > +			      + lookup_len + demangled_len + 3));
> >  	  mangled_ptr = &((*slot)->demangled[demangled_len + 1]);
> >  	  strcpy (mangled_ptr, lookup_name);
> >  	  (*slot)->mangled = mangled_ptr;
> > --
> > 1.9.1
> 
> --
> Joel
  

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9c09269..aaeeb6e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@ 
+2016-06-09  David Taylor  <dtaylor@emc.com>
+
+	* symtab.c (symbol_set_names): Fix off by one error in allocation.
+
 2016-06-07  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* mi/mi-interp.c (mi_record_changed): Add missing braces.
diff --git a/gdb/symtab.c b/gdb/symtab.c
index f7a207a..12e1cf5 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -1010,7 +1010,7 @@  symbol_set_names (struct general_symbol_info *gsymbol,
 	    = ((struct demangled_name_entry *)
 	       obstack_alloc (&per_bfd->storage_obstack,
 			      offsetof (struct demangled_name_entry, demangled)
-			      + lookup_len + demangled_len + 2));
+			      + lookup_len + demangled_len + 3));
 	  mangled_ptr = &((*slot)->demangled[demangled_len + 1]);
 	  strcpy (mangled_ptr, lookup_name);
 	  (*slot)->mangled = mangled_ptr;