diff mbox

[PATCHv2,4/5] gdb: Introduce new language field la_is_string_type_p

Message ID dc4d48cfd02d2d37d6bbaf7374118dcbca56ec2e.1555455013.git.andrew.burgess@embecosm.com
State New
Headers show

Commit Message

Andrew Burgess April 16, 2019, 11:06 p.m. UTC
This commit is preparation work for the next commit, and by itself
makes no user visible change to GDB.  I've split this work into a
separate commit in order to make code review easier.

This commit adds a new field 'la_is_string_type_p' to the language
struct, this predicate will return true if a type is a string type for
the given language.

Some languages already have a "is this a string" predicate that I was
able to reuse, while for other languages I've had to add a new
predicate.  In this case I took inspiration from the value printing
code for that language - what different conditions would result in
printing something as a string.

A default "is this a string" method has also been added that looks for
TYPE_CODE_STRING, this is the fallback I've used for a couple of
languages.

In this commit I add the new field and initialise it for each
language, however at this stage the new field is never used.

gdb/ChangeLog:

	* ada-lang.c (ada_language_defn): Initialise new field.
	* c-lang.c (c_is_string_type_p): New function.
	(c_language_defn): Initialise new field.
	(cplus_language_defn): Initialise new field.
	(asm_language_defn): Initialise new field.
	(minimal_language_defn): Initialise new field.
	* c-lang.h (c_is_string_type_p): Declare new function.
	* d-lang.c (d_language_defn): Initialise new field.
	* f-lang.c (f_is_string_type_p): New function.
	(f_language_defn): Initialise new field.
	* go-lang.c (go_is_string_type_p): New function.
	(go_language_defn): Initialise new field.
	* language.c (default_is_string_type_p): New function.
	(unknown_language_defn): Initialise new field.
	(auto_language_defn): Initialise new field.
	* language.h (struct language_defn) <la_is_string_type_p>: New
	member variable.
	(default_is_string_type_p): Declare new function.
	* m2-lang.c (m2_language_defn): Initialise new field.
	* objc-lang.c (objc_language_defn): Initialise new field.
	* opencl-lang.c (opencl_language_defn): Initialise new field.
	* p-lang.c (pascal_is_string_type_p): New function.
	(pascal_language_defn): Initialise new field.
	* rust-lang.c (rust_is_string_type_p): New function.
	(rust_language_defn): Initialise new field.
---
 gdb/ChangeLog     | 28 ++++++++++++++++++++++++++++
 gdb/ada-lang.c    |  1 +
 gdb/c-lang.c      | 35 +++++++++++++++++++++++++++++++++++
 gdb/c-lang.h      |  5 +++++
 gdb/d-lang.c      |  1 +
 gdb/f-lang.c      | 11 +++++++++++
 gdb/go-lang.c     | 11 +++++++++++
 gdb/language.c    | 16 ++++++++++++++++
 gdb/language.h    |  7 +++++++
 gdb/m2-lang.c     | 22 ++++++++++++++++++++++
 gdb/objc-lang.c   |  1 +
 gdb/opencl-lang.c |  1 +
 gdb/p-lang.c      | 11 +++++++++++
 gdb/rust-lang.c   | 21 +++++++++++++++++++++
 14 files changed, 171 insertions(+)

Comments

Pedro Alves April 18, 2019, 5:07 p.m. UTC | #1
On 4/17/19 12:06 AM, Andrew Burgess wrote:
> This commit is preparation work for the next commit, and by itself
> makes no user visible change to GDB.  I've split this work into a
> separate commit in order to make code review easier.
> 
> This commit adds a new field 'la_is_string_type_p' to the language
> struct, this predicate will return true if a type is a string type for
> the given language.
> 
> Some languages already have a "is this a string" predicate that I was
> able to reuse, while for other languages I've had to add a new
> predicate.  In this case I took inspiration from the value printing
> code for that language - what different conditions would result in
> printing something as a string.
> 
> A default "is this a string" method has also been added that looks for
> TYPE_CODE_STRING, this is the fallback I've used for a couple of
> languages.
> 
> In this commit I add the new field and initialise it for each
> language, however at this stage the new field is never used.
> 

Thanks.  This nicely addresses one of my previous review comments.

I can't speak to whether the implementation for the different languages
are correct.  E.g., I'm curious from where you extracted the M2 and
the Rust bits.  Didn't seem like it was a refactoring job?

Formatting comments below.

> +/* Return true if TYPE is a string.  */
> +static bool

Empty line after comment.

> +static bool
> +m2_is_string_type_p (struct type *type)
> +{
> +  type = check_typedef (type);
> +  if (TYPE_CODE (type) == TYPE_CODE_ARRAY
> +      && TYPE_LENGTH (type) > 0
> +      && TYPE_LENGTH (TYPE_TARGET_TYPE (type)) > 0)
> +    {
> +      struct type *elttype = check_typedef (TYPE_TARGET_TYPE (type));
> +
> +      if (TYPE_LENGTH (elttype) == 1 &&

&& goes on the next line.

> +	  (TYPE_CODE (elttype) == TYPE_CODE_INT
> +	   || TYPE_CODE (elttype) == TYPE_CODE_CHAR))
> +	return true;
> +    }
> +
> +  return false;
> +}
> +

Thanks,
Pedro Alves
Tom Tromey April 19, 2019, 2:45 p.m. UTC | #2
>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> Some languages already have a "is this a string" predicate that I was
Andrew> able to reuse, while for other languages I've had to add a new
Andrew> predicate.  In this case I took inspiration from the value printing
Andrew> code for that language - what different conditions would result in
Andrew> printing something as a string.

This looks essentially fine, but I had some questions.

Andrew> +bool
Andrew> +c_is_string_type_p (struct type *type)
Andrew> +{
Andrew> +  type = check_typedef (type);
Andrew> +  while (TYPE_CODE (type) == TYPE_CODE_REF)
Andrew> +    {
Andrew> +      type = TYPE_TARGET_TYPE (type);
Andrew> +      type = check_typedef (type);
Andrew> +    }
Andrew> +
Andrew> +  switch (TYPE_CODE (type))
Andrew> +    {
Andrew> +    case TYPE_CODE_ARRAY:
Andrew> +      {
Andrew> +	/* See if target type looks like a string.  */
Andrew> +	struct type *array_target_type = TYPE_TARGET_TYPE (type);
Andrew> +	return (TYPE_LENGTH (type) > 0
Andrew> +		&& TYPE_LENGTH (array_target_type) > 0
Andrew> +		&& c_textual_element_type (array_target_type, 0));
Andrew> +      }
Andrew> +    case TYPE_CODE_STRING:
Andrew> +      return true;

It seems to me that a "char *" should be considered a string in C;
and probably a "wchar_t *" as well.  Maybe see c-lang.c:classify_type.

Andrew> +/* Return true if TYPE is a string type.  */
Andrew> +static bool
Andrew> +rust_is_string_type_p (struct type *type)
Andrew> +{
Andrew> +  LONGEST low_bound, high_bound;
Andrew> +
Andrew> +  type = check_typedef (type);
Andrew> +  return ((TYPE_CODE (type) == TYPE_CODE_STRING)
Andrew> +	  || (TYPE_CODE (type) == TYPE_CODE_PTR
Andrew> +	      && (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_ARRAY
Andrew> +		  && rust_u8_type_p (TYPE_TARGET_TYPE (TYPE_TARGET_TYPE (type)))
Andrew> +		  && get_array_bounds (TYPE_TARGET_TYPE (type), &low_bound,
Andrew> +				       &high_bound)))
Andrew> +	  || ((TYPE_CODE (type) == TYPE_CODE_UNION
Andrew> +	       || (TYPE_CODE (type) == TYPE_CODE_STRUCT
Andrew> +		   && !rust_enum_p (type)))
Andrew> +	      && rust_slice_type_p (type)
Andrew> +	      && strcmp (TYPE_NAME (type), "&str") == 0));

I didn't understand the reason for TYPE_CODE_UNION here.

Also, I think an array or slice of 'char' should probably be considered
a string in Rust.  See rust_chartype_p.

thanks,
Tom
Andrew Burgess April 19, 2019, 9 p.m. UTC | #3
* Pedro Alves <palves@redhat.com> [2019-04-18 18:07:55 +0100]:

> On 4/17/19 12:06 AM, Andrew Burgess wrote:
> > This commit is preparation work for the next commit, and by itself
> > makes no user visible change to GDB.  I've split this work into a
> > separate commit in order to make code review easier.
> > 
> > This commit adds a new field 'la_is_string_type_p' to the language
> > struct, this predicate will return true if a type is a string type for
> > the given language.
> > 
> > Some languages already have a "is this a string" predicate that I was
> > able to reuse, while for other languages I've had to add a new
> > predicate.  In this case I took inspiration from the value printing
> > code for that language - what different conditions would result in
> > printing something as a string.
> > 
> > A default "is this a string" method has also been added that looks for
> > TYPE_CODE_STRING, this is the fallback I've used for a couple of
> > languages.
> > 
> > In this commit I add the new field and initialise it for each
> > language, however at this stage the new field is never used.
> > 
> 
> Thanks.  This nicely addresses one of my previous review comments.
> 
> I can't speak to whether the implementation for the different languages
> are correct.  E.g., I'm curious from where you extracted the M2 and
> the Rust bits.  Didn't seem like it was a refactoring job?

It was mostly me looking for calls to LA_PRINT_STRING in each
languages value printing code then combining all of the conditions
required to reach these calls.

Could we refactor to make use of these new is_string functions within
the value printing code?  Sure, but I don't think it would be pretty.
Most languages base their value printing around a switch statement,
and also have additional format related conditions mixed in with the
logic.

Ideally, at least for this commit I'd prefer to no mess with the value
printing code more than I need too...

Thanks,
Andrew




> 
> Formatting comments below.
> 
> > +/* Return true if TYPE is a string.  */
> > +static bool
> 
> Empty line after comment.
> 
> > +static bool
> > +m2_is_string_type_p (struct type *type)
> > +{
> > +  type = check_typedef (type);
> > +  if (TYPE_CODE (type) == TYPE_CODE_ARRAY
> > +      && TYPE_LENGTH (type) > 0
> > +      && TYPE_LENGTH (TYPE_TARGET_TYPE (type)) > 0)
> > +    {
> > +      struct type *elttype = check_typedef (TYPE_TARGET_TYPE (type));
> > +
> > +      if (TYPE_LENGTH (elttype) == 1 &&
> 
> && goes on the next line.
> 
> > +	  (TYPE_CODE (elttype) == TYPE_CODE_INT
> > +	   || TYPE_CODE (elttype) == TYPE_CODE_CHAR))
> > +	return true;
> > +    }
> > +
> > +  return false;
> > +}
> > +
> 
> Thanks,
> Pedro Alves
Andrew Burgess April 19, 2019, 10:22 p.m. UTC | #4
* Tom Tromey <tom@tromey.com> [2019-04-19 08:45:03 -0600]:

> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> 
> Andrew> Some languages already have a "is this a string" predicate that I was
> Andrew> able to reuse, while for other languages I've had to add a new
> Andrew> predicate.  In this case I took inspiration from the value printing
> Andrew> code for that language - what different conditions would result in
> Andrew> printing something as a string.
> 
> This looks essentially fine, but I had some questions.
> 
> Andrew> +bool
> Andrew> +c_is_string_type_p (struct type *type)
> Andrew> +{
> Andrew> +  type = check_typedef (type);
> Andrew> +  while (TYPE_CODE (type) == TYPE_CODE_REF)
> Andrew> +    {
> Andrew> +      type = TYPE_TARGET_TYPE (type);
> Andrew> +      type = check_typedef (type);
> Andrew> +    }
> Andrew> +
> Andrew> +  switch (TYPE_CODE (type))
> Andrew> +    {
> Andrew> +    case TYPE_CODE_ARRAY:
> Andrew> +      {
> Andrew> +	/* See if target type looks like a string.  */
> Andrew> +	struct type *array_target_type = TYPE_TARGET_TYPE (type);
> Andrew> +	return (TYPE_LENGTH (type) > 0
> Andrew> +		&& TYPE_LENGTH (array_target_type) > 0
> Andrew> +		&& c_textual_element_type (array_target_type, 0));
> Andrew> +      }
> Andrew> +    case TYPE_CODE_STRING:
> Andrew> +      return true;
> 
> It seems to me that a "char *" should be considered a string in C;
> and probably a "wchar_t *" as well.  Maybe see c-lang.c:classify_type.

Thanks, I'll take a look at these cases.

> 
> Andrew> +/* Return true if TYPE is a string type.  */
> Andrew> +static bool
> Andrew> +rust_is_string_type_p (struct type *type)
> Andrew> +{
> Andrew> +  LONGEST low_bound, high_bound;
> Andrew> +
> Andrew> +  type = check_typedef (type);
> Andrew> +  return ((TYPE_CODE (type) == TYPE_CODE_STRING)
> Andrew> +	  || (TYPE_CODE (type) == TYPE_CODE_PTR
> Andrew> +	      && (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_ARRAY
> Andrew> +		  && rust_u8_type_p (TYPE_TARGET_TYPE (TYPE_TARGET_TYPE (type)))
> Andrew> +		  && get_array_bounds (TYPE_TARGET_TYPE (type), &low_bound,
> Andrew> +				       &high_bound)))
> Andrew> +	  || ((TYPE_CODE (type) == TYPE_CODE_UNION
> Andrew> +	       || (TYPE_CODE (type) == TYPE_CODE_STRUCT
> Andrew> +		   && !rust_enum_p (type)))
> Andrew> +	      && rust_slice_type_p (type)
> Andrew> +	      && strcmp (TYPE_NAME (type), "&str") == 0));
> 
> I didn't understand the reason for TYPE_CODE_UNION here.

Most of the _is_string functions were created by looking through the
language's value printing code looking for places where the language
identifies something as a string, then taking all of the conditions
that led to that point and placing them in the _is_string function.

For rust, rust_val_print_str is called from val_print_struct, which is
called for TYPE_CODE_STRUCT and TYPE_CODE_UNION, hence the above...

...but, after prompting, I took a closer look, and rust_slice_type_p
is only true for TYPE_CODE_STRUCT, which makes the union check
redundant - so its gone!

> 
> Also, I think an array or slice of 'char' should probably be considered
> a string in Rust.  See rust_chartype_p.

That sounds sensible, but .... I don't believe these things that you
describe are currently printed as strings.  Here's what I tried
(excuse my poor rust skills, maybe I'm not trying what you're
suggesting):

    $ cat str.rs
    #![allow(dead_code)]
    #![allow(unused_variables)]
    #![allow(unused_assignments)]
    
    fn main () {
       let str1: &str = "str1";
       let str2: String = "str2".to_string();
       let str3: [char; 4] = [ 's', 't', 'r', '3' ];
       let str4: &[char] = &str3;
    
       println!("hello world");	// Break here.
    }
    $ rustc -g -o str str.rs 
    $ gdb str
    <... snip ...>
    (gdb) b 11
    Breakpoint 1 at 0x3d3b: file str.rs, line 11.
    (gdb) r
    <... snip ...>
    Breakpoint 1, str::main () at str.rs:11
    11	   println!("hello world");	// Break here.
    (gdb) p str1
    $1 = "str1"
    (gdb) p str2
    $2 = alloc::string::String {vec: alloc::vec::Vec<u8> {buf: alloc::raw_vec::RawVec<u8, alloc::alloc::Global> {ptr: core::ptr::Unique<u8> {pointer: core::nonzero::NonZero<*const u8> (0x555555784a40 "str2\000"), _marker: core::marker::PhantomData<u8>}, cap: 4, a: alloc::alloc::Global}, len: 4}}
    (gdb) p str3
    $3 = [115 's', 116 't', 114 'r', 51 '3']
    (gdb) p str4
    $4 = &[char] {data_ptr: 0x7fffffffcd90 "s\000\000\000t\000\000\000r\000\000\000\063\000\000\000\xffffcd90\x7fff\004\000\000\000", length: 4}

So I think that currently only the '&str' (String slice?) is printed
as a string.

Like I said, my rust-foo is weak, so if you have an example of a
different type that is currently printed as a string then I'm happy to
fold this info a rust test and make any fixes for rust's _is_string
function as needed.

You'll have noticed (maybe?) that the original patch didn't include a
rust test at all.  This was because some of rusts value printing seems
a little broken right now.  For example, printing an array slice
(&str) variable works fine, but place this inside a struct and it no
longer works, for example:

  $ cat nested.rs
  #![allow(dead_code)]
  #![allow(unused_variables)]
  #![allow(unused_assignments)]
  
  pub struct Bad {
      pub field1: i32,
      pub field2: &'static str,
  }
  
  fn main () {
     let s = Bad { field1: 1, field2: "hello" };
     println!("hello world");	// Break here.
  }
  $ rustc -g -o nested nested.rs 
  $ gdb nested
  <... snip ...>
  (gdb) b 12
  Breakpoint 1 at 0x3d00: file nested.rs, line 12.
  (gdb) r
  <... snip ...>
  Breakpoint 1, nested::main () at nested.rs:12
  12	   println!("hello world");	// Break here.
  (gdb) p s
  $1 = nested::Bad {field1: 1, field2: <error reading variable>}
  (gdb) 

However, I fixed that with this patch:

    diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
    index 2fada465d65..597986a4b34 100644
    --- a/gdb/rust-lang.c
    +++ b/gdb/rust-lang.c
    @@ -378,6 +378,8 @@ val_print_struct (struct type *type, int embedded_offset,

       if (rust_slice_type_p (type) && strcmp (TYPE_NAME (type), "&str") == 0)
         {
    +      if (embedded_offset != 0)
    +	val = value_at_lazy (type, value_address (val) + embedded_offset);
           rust_val_print_str (stream, val, options);
           return;
         }

So, I think the summary is, I'm happy to fix rust_is_string_p to cover
any cases that currently print as a string, but I think that if
something _doesn't_ currently print as a string then rust_is_string_p
shouldn't identify it as a string - if it did then we'd end up
printing a structure at a depth when it should have been replaced with
ellipsis.

Thanks,
Andrew
Tom Tromey April 24, 2019, 7:32 p.m. UTC | #5
>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> ...but, after prompting, I took a closer look, and rust_slice_type_p
Andrew> is only true for TYPE_CODE_STRUCT, which makes the union check
Andrew> redundant - so its gone!

Thanks.

>> Also, I think an array or slice of 'char' should probably be considered
>> a string in Rust.  See rust_chartype_p.

Andrew> That sounds sensible, but .... I don't believe these things that you
Andrew> describe are currently printed as strings.

Thanks for looking into that.  Maybe it's just an oversight in the Rust
code, or maybe it's just uncommon to do this kind of thing.  Not your
problem :)

Andrew> You'll have noticed (maybe?) that the original patch didn't include a
Andrew> rust test at all.  This was because some of rusts value printing seems
Andrew> a little broken right now.  For example, printing an array slice
Andrew> (&str) variable works fine, but place this inside a struct and it no
Andrew> longer works, for example:

Thanks for noticing that.  I can deal with it, unless you want to.

Andrew> So, I think the summary is, I'm happy to fix rust_is_string_p to cover
Andrew> any cases that currently print as a string, but I think that if
Andrew> something _doesn't_ currently print as a string then rust_is_string_p
Andrew> shouldn't identify it as a string - if it did then we'd end up
Andrew> printing a structure at a depth when it should have been replaced with
Andrew> ellipsis.

Agreed.

Tom
diff mbox

Patch

diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index fddb0d7278c..6184067c805 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -14381,6 +14381,7 @@  extern const struct language_defn ada_language_defn = {
   &ada_varobj_ops,
   NULL,
   NULL,
+  ada_is_string_type,
   "(...)"			/* la_struct_too_deep_ellipsis */
 };
 
diff --git a/gdb/c-lang.c b/gdb/c-lang.c
index 3be5ef57252..4d5284e2e80 100644
--- a/gdb/c-lang.c
+++ b/gdb/c-lang.c
@@ -715,6 +715,37 @@  c_watch_location_expression (struct type *type, CORE_ADDR addr)
     (xstrprintf ("* (%s *) %s", name.c_str (), core_addr_to_string (addr)));
 }
 
+/* See c-lang.h.  */
+
+bool
+c_is_string_type_p (struct type *type)
+{
+  type = check_typedef (type);
+  while (TYPE_CODE (type) == TYPE_CODE_REF)
+    {
+      type = TYPE_TARGET_TYPE (type);
+      type = check_typedef (type);
+    }
+
+  switch (TYPE_CODE (type))
+    {
+    case TYPE_CODE_ARRAY:
+      {
+	/* See if target type looks like a string.  */
+	struct type *array_target_type = TYPE_TARGET_TYPE (type);
+	return (TYPE_LENGTH (type) > 0
+		&& TYPE_LENGTH (array_target_type) > 0
+		&& c_textual_element_type (array_target_type, 0));
+      }
+    case TYPE_CODE_STRING:
+      return true;
+    default:
+      break;
+    }
+
+  return false;
+}
+
 
 /* Table mapping opcodes into strings for printing operators
    and precedences of the operators.  */
@@ -874,6 +905,7 @@  extern const struct language_defn c_language_defn =
   &c_varobj_ops,
   c_get_compile_context,
   c_compute_program,
+  c_is_string_type_p,
   "{...}"			/* la_struct_too_deep_ellipsis */
 };
 
@@ -1019,6 +1051,7 @@  extern const struct language_defn cplus_language_defn =
   &cplus_varobj_ops,
   cplus_get_compile_context,
   cplus_compute_program,
+  c_is_string_type_p,
   "{...}"			/* la_struct_too_deep_ellipsis */
 };
 
@@ -1073,6 +1106,7 @@  extern const struct language_defn asm_language_defn =
   &default_varobj_ops,
   NULL,
   NULL,
+  c_is_string_type_p,
   "{...}"			/* la_struct_too_deep_ellipsis */
 };
 
@@ -1127,5 +1161,6 @@  extern const struct language_defn minimal_language_defn =
   &default_varobj_ops,
   NULL,
   NULL,
+  c_is_string_type_p,
   "{...}"			/* la_struct_too_deep_ellipsis */
 };
diff --git a/gdb/c-lang.h b/gdb/c-lang.h
index e7b6d5ef737..70a95eadbf8 100644
--- a/gdb/c-lang.h
+++ b/gdb/c-lang.h
@@ -148,6 +148,11 @@  extern int cp_is_vtbl_ptr_type (struct type *);
 
 extern int cp_is_vtbl_member (struct type *);
 
+/* Return true if TYPE is a string type.  Unlike DEFAULT_IS_STRING_TYPE_P
+   this will detect arrays of characters not just TYPE_CODE_STRING.  */
+
+extern bool c_is_string_type_p (struct type *type);
+
 /* These are in c-valprint.c.  */
 
 extern int c_textual_element_type (struct type *, char);
diff --git a/gdb/d-lang.c b/gdb/d-lang.c
index 751c521a75d..0f8f916c9b1 100644
--- a/gdb/d-lang.c
+++ b/gdb/d-lang.c
@@ -251,6 +251,7 @@  extern const struct language_defn d_language_defn =
   &default_varobj_ops,
   NULL,
   NULL,
+  c_is_string_type_p,
   "{...}"			/* la_struct_too_deep_ellipsis */
 };
 
diff --git a/gdb/f-lang.c b/gdb/f-lang.c
index 77eb50a0761..b03f1f58b41 100644
--- a/gdb/f-lang.c
+++ b/gdb/f-lang.c
@@ -308,6 +308,16 @@  evaluate_subexp_f (struct type *expect_type, struct expression *exp,
   return nullptr;
 }
 
+/* Return true if TYPE is a string.  */
+static bool
+f_is_string_type_p (struct type *type)
+{
+  type = check_typedef (type);
+  return (TYPE_CODE (type) == TYPE_CODE_STRING
+	  || (TYPE_CODE (type) == TYPE_CODE_ARRAY
+	      && TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_CHAR));
+}
+
 static const char *f_extensions[] =
 {
   ".f", ".F", ".for", ".FOR", ".ftn", ".FTN", ".fpp", ".FPP",
@@ -378,6 +388,7 @@  extern const struct language_defn f_language_defn =
   &default_varobj_ops,
   NULL,
   NULL,
+  f_is_string_type_p,
   "(...)"			/* la_struct_too_deep_ellipsis */
 };
 
diff --git a/gdb/go-lang.c b/gdb/go-lang.c
index b8617cb162d..6473468d4d9 100644
--- a/gdb/go-lang.c
+++ b/gdb/go-lang.c
@@ -130,6 +130,16 @@  go_classify_struct_type (struct type *type)
   return GO_TYPE_NONE;
 }
 
+/* Return true if TYPE is a string.  */
+
+static bool
+go_is_string_type_p (struct type *type)
+{
+  type = check_typedef (type);
+  return (TYPE_CODE (type) == TYPE_CODE_STRUCT
+	  && go_classify_struct_type (type) == GO_TYPE_STRING);
+}
+
 /* Subroutine of unpack_mangled_go_symbol to simplify it.
    Given "[foo.]bar.baz", store "bar" in *PACKAGEP and "baz" in *OBJECTP.
    We stomp on the last '.' to nul-terminate "bar".
@@ -612,6 +622,7 @@  extern const struct language_defn go_language_defn =
   &default_varobj_ops,
   NULL,
   NULL,
+  go_is_string_type_p,
   "{...}"			/* la_struct_too_deep_ellipsis */
 };
 
diff --git a/gdb/language.c b/gdb/language.c
index da8dd1bf7ae..9d0eb03b420 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -723,6 +723,20 @@  default_symbol_name_matcher (const char *symbol_search_name,
 
 /* See language.h.  */
 
+bool
+default_is_string_type_p (struct type *type)
+{
+  type = check_typedef (type);
+  while (TYPE_CODE (type) == TYPE_CODE_REF)
+    {
+      type = TYPE_TARGET_TYPE (type);
+      type = check_typedef (type);
+    }
+  return (TYPE_CODE (type)  == TYPE_CODE_STRING);
+}
+
+/* See language.h.  */
+
 symbol_name_matcher_ftype *
 get_symbol_name_matcher (const language_defn *lang,
 			 const lookup_name_info &lookup_name)
@@ -877,6 +891,7 @@  const struct language_defn unknown_language_defn =
   &default_varobj_ops,
   NULL,
   NULL,
+  default_is_string_type_p,
   "{...}"			/* la_struct_too_deep_ellipsis */
 };
 
@@ -928,6 +943,7 @@  const struct language_defn auto_language_defn =
   &default_varobj_ops,
   NULL,
   NULL,
+  default_is_string_type_p,
   "{...}"			/* la_struct_too_deep_ellipsis */
 };
 
diff --git a/gdb/language.h b/gdb/language.h
index 261f5a33cc7..e7446efa07f 100644
--- a/gdb/language.h
+++ b/gdb/language.h
@@ -450,6 +450,9 @@  struct language_defn
 				       const struct block *expr_block,
 				       CORE_ADDR expr_pc);
 
+    /* Return true if TYPE is a string type.  */
+    bool (*la_is_string_type_p) (struct type *type);
+
     /* This string is used by the 'set print max-depth' setting.  When GDB
        replaces a struct or union (during value printing) that is "too
        deep" this string is displayed instead.  */
@@ -575,6 +578,10 @@  extern enum language set_language (enum language);
 
 extern int pointer_type (struct type *);
 
+/* Return true if TYPE is a string type, otherwise return false.  This
+   default implementation only detects TYPE_CODE_STRING.  */
+extern bool default_is_string_type_p (struct type *type);
+
 /* Error messages */
 
 extern void range_error (const char *, ...) ATTRIBUTE_PRINTF (1, 2);
diff --git a/gdb/m2-lang.c b/gdb/m2-lang.c
index 9f2a97d54fd..1f7113bd71b 100644
--- a/gdb/m2-lang.c
+++ b/gdb/m2-lang.c
@@ -174,6 +174,27 @@  m2_printstr (struct ui_file *stream, struct type *type, const gdb_byte *string,
     fputs_filtered ("...", stream);
 }
 
+/* Return true if TYPE is a string.  */
+
+static bool
+m2_is_string_type_p (struct type *type)
+{
+  type = check_typedef (type);
+  if (TYPE_CODE (type) == TYPE_CODE_ARRAY
+      && TYPE_LENGTH (type) > 0
+      && TYPE_LENGTH (TYPE_TARGET_TYPE (type)) > 0)
+    {
+      struct type *elttype = check_typedef (TYPE_TARGET_TYPE (type));
+
+      if (TYPE_LENGTH (elttype) == 1 &&
+	  (TYPE_CODE (elttype) == TYPE_CODE_INT
+	   || TYPE_CODE (elttype) == TYPE_CODE_CHAR))
+	return true;
+    }
+
+  return false;
+}
+
 static struct value *
 evaluate_subexp_modula2 (struct type *expect_type, struct expression *exp,
 			 int *pos, enum noside noside)
@@ -399,6 +420,7 @@  extern const struct language_defn m2_language_defn =
   &default_varobj_ops,
   NULL,
   NULL,
+  m2_is_string_type_p,
   "{...}"			/* la_struct_too_deep_ellipsis */
 };
 
diff --git a/gdb/objc-lang.c b/gdb/objc-lang.c
index af92e55a437..b25a98106c1 100644
--- a/gdb/objc-lang.c
+++ b/gdb/objc-lang.c
@@ -409,6 +409,7 @@  extern const struct language_defn objc_language_defn = {
   &default_varobj_ops,
   NULL,
   NULL,
+  c_is_string_type_p,
   "{...}"			/* la_struct_too_deep_ellipsis */
 };
 
diff --git a/gdb/opencl-lang.c b/gdb/opencl-lang.c
index c95bf05a98b..93d8e2f5dd3 100644
--- a/gdb/opencl-lang.c
+++ b/gdb/opencl-lang.c
@@ -1087,6 +1087,7 @@  extern const struct language_defn opencl_language_defn =
   &default_varobj_ops,
   NULL,
   NULL,
+  c_is_string_type_p,
   "{...}"			/* la_struct_too_deep_ellipsis */
 };
 
diff --git a/gdb/p-lang.c b/gdb/p-lang.c
index 0b85f70dfd2..9b9f19b69cf 100644
--- a/gdb/p-lang.c
+++ b/gdb/p-lang.c
@@ -150,6 +150,16 @@  is_pascal_string_type (struct type *type,int *length_pos,
   return 0;
 }
 
+/* This is a wrapper around IS_PASCAL_STRING_TYPE that returns true if TYPE
+   is a string.  */
+
+static bool
+pascal_is_string_type_p (struct type *type)
+{
+  return is_pascal_string_type (type, nullptr, nullptr, nullptr,
+				nullptr, nullptr) > 0;
+}
+
 static void pascal_one_char (int, struct ui_file *, int *);
 
 /* Print the character C on STREAM as part of the contents of a literal
@@ -460,5 +470,6 @@  extern const struct language_defn pascal_language_defn =
   &default_varobj_ops,
   NULL,
   NULL,
+  pascal_is_string_type_p,
   "{...}"			/* la_struct_too_deep_ellipsis */
 };
diff --git a/gdb/rust-lang.c b/gdb/rust-lang.c
index 8faafd49cdd..d878aec6adc 100644
--- a/gdb/rust-lang.c
+++ b/gdb/rust-lang.c
@@ -226,6 +226,26 @@  rust_chartype_p (struct type *type)
 	  && TYPE_UNSIGNED (type));
 }
 
+/* Return true if TYPE is a string type.  */
+static bool
+rust_is_string_type_p (struct type *type)
+{
+  LONGEST low_bound, high_bound;
+
+  type = check_typedef (type);
+  return ((TYPE_CODE (type) == TYPE_CODE_STRING)
+	  || (TYPE_CODE (type) == TYPE_CODE_PTR
+	      && (TYPE_CODE (TYPE_TARGET_TYPE (type)) == TYPE_CODE_ARRAY
+		  && rust_u8_type_p (TYPE_TARGET_TYPE (TYPE_TARGET_TYPE (type)))
+		  && get_array_bounds (TYPE_TARGET_TYPE (type), &low_bound,
+				       &high_bound)))
+	  || ((TYPE_CODE (type) == TYPE_CODE_UNION
+	       || (TYPE_CODE (type) == TYPE_CODE_STRUCT
+		   && !rust_enum_p (type)))
+	      && rust_slice_type_p (type)
+	      && strcmp (TYPE_NAME (type), "&str") == 0));
+}
+
 /* If VALUE represents a trait object pointer, return the underlying
    pointer with the correct (i.e., runtime) type.  Otherwise, return
    NULL.  */
@@ -2142,5 +2162,6 @@  extern const struct language_defn rust_language_defn =
   &default_varobj_ops,
   NULL,
   NULL,
+  rust_is_string_type_p,
   "{...}"			/* la_struct_too_deep_ellipsis */
 };