Re: [PATCH] GDB: fix completion related libstdc++ assert

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jan 24, 2022 at 12:59 PM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@xxxxxxx> wrote:
-----Original Message-----
>       > diff --git a/gdb-10.2.patch b/gdb-10.2.patch
>       > index 1332b6638028..16165839b360 100644
>       > --- a/gdb-10.2.patch
>       > +++ b/gdb-10.2.patch
>       > @@ -1591,3 +1591,32 @@
>       >     max += 2;
>       >     limit = cols / max;
>       >     if (limit != 1 && (limit * max == cols))
>       > +--- 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.

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

--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/crash-utility

[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux