Thank you for the comment, Kazu.
On Mon, Jan 24, 2022 at 8:42 AM HAGIO KAZUHITO(萩尾 一仁) <k-hagio-ab@xxxxxxx> 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 <lijiang@xxxxxxxxxx>
> Signed-off-by: Kazuhito Hagio <k-hagio-ab@xxxxxxx>
> ---
> 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 Crash-utility@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/crash-utility
