[Crash-utility] [PATCH] GDB: fix completion related libstdc++ assert

[HAGIO KAZUHITO(萩尾 一仁)]

-----Original Message-----
> 	>       > +--- gdb-10.2/gdb/ada-lang.c.orig
> 	>       > ++++ gdb-10.2/gdb/ada-lang.c
> 	>       > +@@ -997,7 +997,7 @@ ada_fold_name (gdb::string_view name)
> 	>       > +   int len = name.size ();
> 	>       > +   GROW_VECT (fold_buffer, fold_buffer_size, len + 1);
> 	>       > +
> 	>       > +-  if (name[0] == '\'')
> 	>       > ++  if (name.size () > 0 && name[0] == '\'')
> 	>       > +     {
> 	>       > +       strncpy (fold_buffer, name.data () + 1, len - 2);
> 	>       > +       fold_buffer[len - 2] = '\000';
> 	>       > +@@ -1006,7 +1006,7 @@ ada_fold_name (gdb::string_view name)
> 	>       > +     {
> 	>       > +       int i;
> 	>       > +
> 	>       > +-      for (i = 0; i <= len; i += 1)
> 	>       > ++      for (i = 0; i < len; i++)
> 	>       > +         fold_buffer[i] = tolower (name[i]);
> 	>
> 	>       According to 2ccee230f830 ("Fix off-by-one error in ada_fold_name"),
> 	>       please add this:
> 	>
> 	>       +      fold_buffer[i] = '\0';
> 	>
> 	>
> 	>
> 	> No,  the above change will definitely cause a core dump because it assigns the value '\0' to a
> null pointer
> 	> when the name string is null.
> 
> 	Hmm, I'm not familiar with gdb source, could you explain a little more?
> 	The following is the function with your patch.
> 
> 	static char *
> 	ada_fold_name (gdb::string_view name)
> 	{
> 	  static char *fold_buffer = NULL;
> 	  static size_t fold_buffer_size = 0;
> 
> 	  int len = name.size ();
> 	  GROW_VECT (fold_buffer, fold_buffer_size, len + 1);
> 
> 	  if (name.size () > 0 && name[0] == '\'')
> 	    {
> 	      strncpy (fold_buffer, name.data () + 1, len - 2);
> 	      fold_buffer[len - 2] = '\000';
> 	    }
> 	  else
> 	    {
> 	      int i;
> 
> 	      for (i = 0; i < len; i++)
> 	        fold_buffer[i] = tolower (name[i]);
> 	    }
> 
> 	  return fold_buffer;
> 	}
> 
> 	The GROW_VECT() looks to alloc 1 byte to fold_buffer if name.size() is zero.
> 
> 
> 
> Theoretically yes, but you could notice that the definition of 'fold_buffer_size', actually it is a static
> variable, the ada_fold_name() may be called many times, but variable 'fold_buffer_size' is only initialized
> to zero once, it means that the value of 'fold_buffer_size' may not be zero(>1) in the subsequent calls.
> For this case, the GROW_VECT() will never allocate new memory.

Thanks, but I noticed that the fold_buffer is also a static variable,
it looks like once it's allocated, never be freed.  So it always has
1 byte at least?

Thanks,
Kazu

> 
> See the definition of GROW_VECT():
> 
> #define GROW_VECT(v, s, m)                                    \
>    if ((s) < (m)) (v) = (char *) grow_vect (v, &(s), m, sizeof *(v));
> 
> /* Assuming VECT points to an array of *SIZE objects of size
>    ELEMENT_SIZE, grow it to contain at least MIN_SIZE objects,
>    updating *SIZE as necessary and returning the (new) array.  */
> 
> static void *
> grow_vect (void *vect, size_t *size, size_t min_size, int element_size)
> {
>   if (*size < min_size)
>     {
>       *size *= 2;
>       if (*size < min_size)
>         *size = min_size;
>       vect = xrealloc (vect, *size * element_size);
>     }
>   return vect;
> }
> 
> 
> Hope this helps.
> 
> Thanks
> Lianbo
> 
> 
> 	Then len is zero, and nothing is done in the for loop, and fold_buffer[i]
> 	(== fold_buffer[0]) can be set '\0', I thought.
> 
> 	>       +      fold_buffer[i] = '\0';
> 
> 	And as far as I've tried, no abort occured.
> 
> 	Thanks,
> 	Kazu
> 
>