[PATCH] symtab.c -- Fix off by one allocation bug

[PATCH] symtab.c -- Fix off by one allocation bug

[David Taylor]

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(-)

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

Re: [PATCH] symtab.c -- Fix off by one allocation bug

[Joel Brobecker]

> 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

-- 
Joel

RE: [PATCH] symtab.c -- Fix off by one allocation bug

[taylor, david]

> 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