[PATCH] Use rust_demangle to fix a crash

[PATCH] Use rust_demangle to fix a crash

[Tom Tromey]

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.)  */
 
 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

Re: [PATCH] Use rust_demangle to fix a crash

[Andrew Burgess]

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

Re: [PATCH] Use rust_demangle to fix a crash

[Tom Tromey]

>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:

>> +   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.)  */

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

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

Yep, I removed it.

Andrew> Looks good with that change.

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

Thanks, I'm going to check it in.

Tom