Thank you for the comment, Kazu.
On Mon, Jan 24, 2022 at 8:42 AM HAGIO KAZUHITO(萩尾 一仁) <[email protected]>
wrote:
> Hi Lianbo,
>
> thanks for writing the patch!
>
> -----Original Message-----
> > Currently, crash may generate core dump and print the following
> > error when running the commands "l panic" or "p" TAB completion.
>
> I have still not reproduce this with upstream sources, it looks like a
> combination of build flags is needed, but not sure about the minimum set.
> And I think that, more exactly, the gdb list command or tab-completion of
> symbols cause it, so please modify like this:
> ---
> Currently crash built with some specific flags (-D_GLIBCXX_ASSERTIONS and
> etc.) may abort and print the following error when running the gdb list
> command or tab-completion of symbols.
>
>
Looks good.
> For example:
> ---
>
> >
> > crash> l panic
> > /usr/include/c++/11/string_view:234: ...
> > Aborted (core dumped)
> >
> > crash> p "TAB completion"
> > crash> p /usr/include/c++/11/string_view:234: ...
> > Aborted (core dumped)
> >
> > When the name string is null(the length of name is zero), there
> > are multiple places where array access is out of bounds in the
> > gdb/ada-lang.c(see ada_fold_name() and ada_lookup_name_info()).
>
> Please add this here:
> ---
> The patch backports these gdb patches:
> 6a780b676637 ("Fix completion related libstdc++ assert when using
> -D_GLIBCXX_DEBUG")
> 2ccee230f830 ("Fix off-by-one error in ada_fold_name")
> ---
>
Also good to me.
> >
> > Signed-off-by: Lianbo Jiang <[email protected]>
> > Signed-off-by: Kazuhito Hagio <[email protected]>
> > ---
> > gdb-10.2.patch | 29 +++++++++++++++++++++++++++++
> > 1 file changed, 29 insertions(+)
> >
> > 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.
We should backport gdb patches as they are if possible, for future
> backports.
>
Agree with you, but sometimes, if crash has to backport more additional
patches for a simple fix, it doesn't seem worth it, just like this case.
Thanks.
Lianbo
> + }
> > +
> > +@@ -13596,7 +13596,7 @@ ada_lookup_name_info::ada_lookup_name_info
> (const lookup_name_info &lookup_name)
> > + {
> > + gdb::string_view user_name = lookup_name.name ();
> > +
> > +- if (user_name[0] == '<')
> > ++ if (user_name.size () > 0 && user_name[0] == '<')
> > + {
> > + if (user_name.back () == '>')
> > + m_encoded_name
> > --
> > 2.20.1
>
>
--
Crash-utility mailing list
[email protected]
https://listman.redhat.com/mailman/listinfo/crash-utility
