[09/55] Use common_val_print in c-valprint.c

Message ID 20191208182958.10181-10-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey Dec. 8, 2019, 6:29 p.m. UTC
  This changes c_value_print to call common_val_print.  This is more
complicated than the usual sort of common_val_print change, due to the
handling of RTTI.

gdb/ChangeLog
2019-12-08  Tom Tromey  <tom@tromey.com>

	* c-valprint.c (c_value_print): Use common_val_print.

Change-Id: Ifae442de4e508a90a2ccdc4f31792bfe47f79e6f
---
 gdb/ChangeLog    |  4 ++++
 gdb/c-valprint.c | 53 ++++++++++++++++--------------------------------
 2 files changed, 22 insertions(+), 35 deletions(-)
  

Comments

Simon Marchi Jan. 15, 2020, 5:01 a.m. UTC | #1
On 2019-12-08 1:29 p.m., Tom Tromey wrote:
> @@ -593,12 +589,12 @@ c_value_print (struct value *val, struct ui_file *stream,
>           type is indicated by the quoted string anyway.
>           (Don't use c_textual_element_type here; quoted strings
>           are always exactly (char *), (wchar_t *), or the like.  */
> -      if (TYPE_CODE (val_type) == TYPE_CODE_PTR
> -	  && TYPE_NAME (val_type) == NULL
> -	  && TYPE_NAME (TYPE_TARGET_TYPE (val_type)) != NULL
> -	  && (strcmp (TYPE_NAME (TYPE_TARGET_TYPE (val_type)),
> +      if (TYPE_CODE (type) == TYPE_CODE_PTR
> +	  && TYPE_NAME (type) == NULL
> +	  && TYPE_NAME (TYPE_TARGET_TYPE (type)) != NULL
> +	  && (strcmp (TYPE_NAME (TYPE_TARGET_TYPE (type)),
>  		      "char") == 0
> -	      || textual_name (TYPE_NAME (TYPE_TARGET_TYPE (val_type)))))
> +	      || textual_name (TYPE_NAME (TYPE_TARGET_TYPE (type)))))
>  	{
>  	  /* Print nothing.  */
>  	}

This introduces a behavior change.  It's not dramatic, but I don't think it was
intended, otherwise you would have mentioned it.

Considering this program:

---
typedef char *charptr;

charptr hello = "hello";

int main()
{
  return 0;
}
---

Before:

(gdb) p hello
$1 = (charptr) 0x400574 "hello"

After:

(gdb) p hello
$1 = 0x400574 "hello"

To restore the original behavior, the checks above should use the
value's original type (like they do today).

> @@ -667,36 +659,27 @@ c_value_print (struct value *val, struct ui_file *stream,
>  	  /* We have RTTI information, so use it.  */
>  	  val = value_full_object (val, real_type, 
>  				   full, top, using_enc);
> +	  /* In a destructor we might see a real type that is a
> +	     superclass of the object's type.  In this case it is
> +	     better to leave the object as-is.  */
> +	  if (!(full
> +		&& (TYPE_LENGTH (real_type)
> +		    < TYPE_LENGTH (value_enclosing_type (val)))))
> +	    val = value_cast (real_type, val);
>  	  fprintf_filtered (stream, "(%s%s) ",
>  			    TYPE_NAME (real_type),
>  			    full ? "" : _(" [incomplete object]"));
>  	  /* Print out object: enclosing type is same as real_type if
>  	     full.  */

This comment was most likely attached to the val_print call, it's a
bit odd to leave it there.

Simon
  
Tom Tromey Jan. 24, 2020, 12:54 a.m. UTC | #2
>>>>> "Simon" == Simon Marchi <simark@simark.ca> writes:

>> +      if (TYPE_CODE (type) == TYPE_CODE_PTR
>> +	  && TYPE_NAME (type) == NULL
>> +	  && TYPE_NAME (TYPE_TARGET_TYPE (type)) != NULL
>> +	  && (strcmp (TYPE_NAME (TYPE_TARGET_TYPE (type)),
>> "char") == 0
>> -	      || textual_name (TYPE_NAME (TYPE_TARGET_TYPE (val_type)))))
>> +	      || textual_name (TYPE_NAME (TYPE_TARGET_TYPE (type)))))
>> {
>> /* Print nothing.  */
>> }

Simon> This introduces a behavior change.  It's not dramatic, but I don't think it was
Simon> intended, otherwise you would have mentioned it.

Wow, nice catch.  And thank you.

I've added a test case for this to the patch, and I've fixed it.

Tom
  

Patch

diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c
index 607fb80c58f..d20a411252f 100644
--- a/gdb/c-valprint.c
+++ b/gdb/c-valprint.c
@@ -567,7 +567,7 @@  void
 c_value_print (struct value *val, struct ui_file *stream, 
 	       const struct value_print_options *options)
 {
-  struct type *type, *real_type, *val_type;
+  struct type *type, *real_type;
   int full, using_enc;
   LONGEST top;
   struct value_print_options opts = *options;
@@ -581,11 +581,7 @@  c_value_print (struct value *val, struct ui_file *stream,
      C++: if it is a member pointer, we will take care
      of that when we print it.  */
 
-  /* Preserve the original type before stripping typedefs.  We prefer
-     to pass down the original type when possible, but for local
-     checks it is better to look past the typedefs.  */
-  val_type = value_type (val);
-  type = check_typedef (val_type);
+  type = check_typedef (value_type (val));
 
   if (TYPE_CODE (type) == TYPE_CODE_PTR || TYPE_IS_REFERENCE (type))
     {
@@ -593,12 +589,12 @@  c_value_print (struct value *val, struct ui_file *stream,
          type is indicated by the quoted string anyway.
          (Don't use c_textual_element_type here; quoted strings
          are always exactly (char *), (wchar_t *), or the like.  */
-      if (TYPE_CODE (val_type) == TYPE_CODE_PTR
-	  && TYPE_NAME (val_type) == NULL
-	  && TYPE_NAME (TYPE_TARGET_TYPE (val_type)) != NULL
-	  && (strcmp (TYPE_NAME (TYPE_TARGET_TYPE (val_type)),
+      if (TYPE_CODE (type) == TYPE_CODE_PTR
+	  && TYPE_NAME (type) == NULL
+	  && TYPE_NAME (TYPE_TARGET_TYPE (type)) != NULL
+	  && (strcmp (TYPE_NAME (TYPE_TARGET_TYPE (type)),
 		      "char") == 0
-	      || textual_name (TYPE_NAME (TYPE_TARGET_TYPE (val_type)))))
+	      || textual_name (TYPE_NAME (TYPE_TARGET_TYPE (type)))))
 	{
 	  /* Print nothing.  */
 	}
@@ -624,7 +620,6 @@  c_value_print (struct value *val, struct ui_file *stream,
 	      if (real_type)
 		{
 		  /* RTTI entry found.  */
-		  type = real_type;
 
 		  /* Need to adjust pointer value.  */
 		  val = value_from_pointer (real_type,
@@ -637,14 +632,11 @@  c_value_print (struct value *val, struct ui_file *stream,
 	    }
 
 	  if (is_ref)
-	    {
-	      val = value_ref (value_ind (val), refcode);
-	      type = value_type (val);
-	    }
+	    val = value_ref (value_ind (val), refcode);
 
+	  type = value_type (val);
 	  type_print (type, "", stream, -1);
 	  fprintf_filtered (stream, ") ");
-	  val_type = type;
 	}
       else
 	{
@@ -667,36 +659,27 @@  c_value_print (struct value *val, struct ui_file *stream,
 	  /* We have RTTI information, so use it.  */
 	  val = value_full_object (val, real_type, 
 				   full, top, using_enc);
+	  /* In a destructor we might see a real type that is a
+	     superclass of the object's type.  In this case it is
+	     better to leave the object as-is.  */
+	  if (!(full
+		&& (TYPE_LENGTH (real_type)
+		    < TYPE_LENGTH (value_enclosing_type (val)))))
+	    val = value_cast (real_type, val);
 	  fprintf_filtered (stream, "(%s%s) ",
 			    TYPE_NAME (real_type),
 			    full ? "" : _(" [incomplete object]"));
 	  /* Print out object: enclosing type is same as real_type if
 	     full.  */
-	  val_print (value_enclosing_type (val),
-		     0,
-		     value_address (val), stream, 0,
-		     val, &opts, current_language);
-	  return;
-          /* Note: When we look up RTTI entries, we don't get any
-             information on const or volatile attributes.  */
 	}
       else if (type != check_typedef (value_enclosing_type (val)))
 	{
 	  /* No RTTI information, so let's do our best.  */
 	  fprintf_filtered (stream, "(%s ?) ",
 			    TYPE_NAME (value_enclosing_type (val)));
-	  val_print (value_enclosing_type (val),
-		     0,
-		     value_address (val), stream, 0,
-		     val, &opts, current_language);
-	  return;
+	  val = value_cast (value_enclosing_type (val), val);
 	}
-      /* Otherwise, we end up at the return outside this "if".  */
     }
 
-  val_print (val_type,
-	     value_embedded_offset (val),
-	     value_address (val),
-	     stream, 0,
-	     val, &opts, current_language);
+  common_val_print (val, stream, 0, &opts, current_language);
 }