Re: [PATCH] Use rust_demangle to fix a crash

[Andrew Burgess <aburgess@redhat.com>]

Tom Tromey <tom@tromey.com> writes:

> PR rust/30211 points out a crash caused by a particular completion.
> This turns out to happen because a Rust minsym winds up in a
> C++-specific path in strncmp_iw_with_mode, which ultimately causes the
> completer to pass invalid arguments to string::append.
>
> This patch fixes the bug by reordering the language constants so that
> Rust comes before C++, and then using rust_demangle.  This ensures
> that minsyms are correctly marked as "Rust", avoiding this code and
> thus the crash.
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=20367
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=30211
> ---
>  gdb/defs.h                         | 8 ++++----
>  gdb/rust-lang.h                    | 4 ++--
>  gdb/testsuite/gdb.rust/methods.exp | 3 +++
>  3 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/gdb/defs.h b/gdb/defs.h
> index 90748c586f4..a83171e92b9 100644
> --- a/gdb/defs.h
> +++ b/gdb/defs.h
> @@ -204,9 +204,9 @@ extern void quit_serial_event_clear (void);
>     these languages, so some symbols could be successfully demangled by
>     several languages.  For that reason, the constants here are sorted
>     in the order we'll attempt demangling them.  For example: Rust uses
> -   C++ mangling, so must come after C++; Ada must come last (see
> -   ada_sniff_from_mangled_name).  (Keep this order in sync with the
> -   'languages' array in language.c.)  */
> +   a C++-compatible mangling, so must come before C++; Ada must come
> +   last (see ada_sniff_from_mangled_name).  (Keep this order in sync
> +   with the 'languages' array in language.c.)  */

Could you drop the last bit: "(Keep this order in sync with ...".

This is no longer true, the languages array automatically fills itself
as the languages are initialised, and always places the entries in the
correct order.

Looks good with that change.

Reviewed-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew

>  
>  enum language
>    {
> @@ -214,6 +214,7 @@ enum language
>      language_auto,		/* Placeholder for automatic setting */
>      language_c,			/* C */
>      language_objc,		/* Objective-C */
> +    language_rust,		/* Rust */
>      language_cplus,		/* C++ */
>      language_d,			/* D */
>      language_go,		/* Go */
> @@ -222,7 +223,6 @@ enum language
>      language_asm,		/* Assembly language */
>      language_pascal,		/* Pascal */
>      language_opencl,		/* OpenCL */
> -    language_rust,		/* Rust */
>      language_minimal,		/* All other languages, minimal support only */
>      language_ada,		/* Ada */
>      nr_languages
> diff --git a/gdb/rust-lang.h b/gdb/rust-lang.h
> index 37a22e7a404..efe721c5707 100644
> --- a/gdb/rust-lang.h
> +++ b/gdb/rust-lang.h
> @@ -93,7 +93,7 @@ class rust_language : public language_defn
>         (const char *mangled, gdb::unique_xmalloc_ptr<char> *demangled)
>         const override
>    {
> -    *demangled = gdb_demangle (mangled, DMGL_PARAMS | DMGL_ANSI);
> +    demangled->reset (rust_demangle (mangled, 0));
>      return *demangled != NULL;
>    }
>  
> @@ -102,7 +102,7 @@ class rust_language : public language_defn
>    gdb::unique_xmalloc_ptr<char> demangle_symbol (const char *mangled,
>  						 int options) const override
>    {
> -    return gdb_demangle (mangled, options);
> +    return gdb::unique_xmalloc_ptr<char> (rust_demangle (mangled, options));
>    }
>  
>    /* See language.h.  */
> diff --git a/gdb/testsuite/gdb.rust/methods.exp b/gdb/testsuite/gdb.rust/methods.exp
> index 8374ab162ec..72be6a11ab0 100644
> --- a/gdb/testsuite/gdb.rust/methods.exp
> +++ b/gdb/testsuite/gdb.rust/methods.exp
> @@ -59,3 +59,6 @@ gdb_test "print *self" " = 23"
>  gdb_test "info functions HasMethods::new" \
>      "fn methods::HasMethods::new\\(\\) -> methods::HasMethods;"
>  
> +# Regression test for PR rust/20367 and PR rust/30211.  This used to
> +# crash.
> +gdb_test_no_output "complete break what"
> -- 
> 2.39.1