diff mbox series

debug: Fix __int128 handling in dwarf2out [PR99562]

Message ID 20210313081321.GE231854@tucnak
State New
Headers show
Series debug: Fix __int128 handling in dwarf2out [PR99562] | expand

Commit Message

Jakub Jelinek March 13, 2021, 8:13 a.m. UTC
Hi!

The PR66728 changes broke __int128 handling.
It emits wide_int numbers in their minimum unsigned precision
rather than in their full precision.
The problem is then that e.g. the DW_OP_implicit_value path:
          int_mode = as_a <scalar_int_mode> (mode);
          loc_result = new_loc_descr (DW_OP_implicit_value,
                                      GET_MODE_SIZE (int_mode), 0);
          loc_result->dw_loc_oprnd2.val_class = dw_val_class_wide_int;
          loc_result->dw_loc_oprnd2.v.val_wide = ggc_alloc<wide_int> ();
          *loc_result->dw_loc_oprnd2.v.val_wide = rtx_mode_t (rtl, int_mode);
emits invalid DWARF.  In particular this patch fixes there multiple
occurences of:
 	.byte	0x9e	# DW_OP_implicit_value
 	.uleb128 0x10
 	.quad	0xffffffffffffffff
+	.quad	0
 	.quad	.LVL46	# Location list begin address (*.LLST40)
 	.quad	.LFE14	# Location list end address (*.LLST40)
where we said the value has 16 byte size but then only emitted 8 byte value.
My understanding is that most of the places that use val_wide expect
the precision they chose (the one of the mode they want etc.), the only
exception is the add_const_value_attribute case where it deals with
VOIDmode CONST_WIDE_INTs, for that I agree when we don't have a mode
we need to fallback to minimum precision (not sure if maximum of
min_precision UNSIGNED and SIGNED wouldn't be better, then consumers
would know if it is signed or unsigned by looking at the MSB),
but that code already computes the precision, just decided to
create the wide_int with much larger precision (e.g. 512 bit
on x86_64).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2021-03-13  Jakub Jelinek  <jakub@redhat.com>

	PR debug/99562
	PR debug/66728
	* dwarf2out.c (get_full_len): Use get_precision rather than
	min_precision.
	(add_const_value_attribute): Make sure add_AT_wide argument has
	precision prec rather than some very wide one.


	Jakub

Comments

Jason Merrill March 17, 2021, 8:55 p.m. UTC | #1
On 3/13/21 3:13 AM, Jakub Jelinek wrote:
> Hi!
> 
> The PR66728 changes broke __int128 handling.
> It emits wide_int numbers in their minimum unsigned precision
> rather than in their full precision.
> The problem is then that e.g. the DW_OP_implicit_value path:
>            int_mode = as_a <scalar_int_mode> (mode);
>            loc_result = new_loc_descr (DW_OP_implicit_value,
>                                        GET_MODE_SIZE (int_mode), 0);
>            loc_result->dw_loc_oprnd2.val_class = dw_val_class_wide_int;
>            loc_result->dw_loc_oprnd2.v.val_wide = ggc_alloc<wide_int> ();
>            *loc_result->dw_loc_oprnd2.v.val_wide = rtx_mode_t (rtl, int_mode);
> emits invalid DWARF.  In particular this patch fixes there multiple
> occurences of:
>   	.byte	0x9e	# DW_OP_implicit_value
>   	.uleb128 0x10
>   	.quad	0xffffffffffffffff
> +	.quad	0
>   	.quad	.LVL46	# Location list begin address (*.LLST40)
>   	.quad	.LFE14	# Location list end address (*.LLST40)
> where we said the value has 16 byte size but then only emitted 8 byte value.
> My understanding is that most of the places that use val_wide expect
> the precision they chose (the one of the mode they want etc.), the only
> exception is the add_const_value_attribute case where it deals with
> VOIDmode CONST_WIDE_INTs, for that I agree when we don't have a mode
> we need to fallback to minimum precision (not sure if maximum of
> min_precision UNSIGNED and SIGNED wouldn't be better, then consumers
> would know if it is signed or unsigned by looking at the MSB),
> but that code already computes the precision, just decided to
> create the wide_int with much larger precision (e.g. 512 bit
> on x86_64).
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

I'm not that familiar with the wide_int code, but sounds reasonable to 
me.  OK if nobody objects this week.

> 2021-03-13  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR debug/99562
> 	PR debug/66728
> 	* dwarf2out.c (get_full_len): Use get_precision rather than
> 	min_precision.
> 	(add_const_value_attribute): Make sure add_AT_wide argument has
> 	precision prec rather than some very wide one.
> 
> --- gcc/dwarf2out.c.jj	2021-03-11 14:01:43.385194205 +0100
> +++ gcc/dwarf2out.c	2021-03-12 17:34:49.365207265 +0100
> @@ -385,13 +385,12 @@ dump_struct_debug (tree type, enum debug
>   #endif
>   
>   /* Get the number of HOST_WIDE_INTs needed to represent the precision
> -   of the number.  Some constants have a large uniform precision, so
> -   we get the precision needed for the actual value of the number.  */
> +   of the number.  */
>   
>   static unsigned int
>   get_full_len (const wide_int &op)
>   {
> -  int prec = wi::min_precision (op, UNSIGNED);
> +  int prec = wi::get_precision (op);
>     return ((prec + HOST_BITS_PER_WIDE_INT - 1)
>   	  / HOST_BITS_PER_WIDE_INT);
>   }
> @@ -19732,8 +19731,9 @@ add_const_value_attribute (dw_die_ref di
>         {
>   	wide_int w1 = rtx_mode_t (rtl, MAX_MODE_INT);
>   	unsigned int prec = MIN (wi::min_precision (w1, UNSIGNED),
> -				 (unsigned int)CONST_WIDE_INT_NUNITS (rtl) * HOST_BITS_PER_WIDE_INT);
> -	wide_int w = wi::zext (w1, prec);
> +				 (unsigned int) CONST_WIDE_INT_NUNITS (rtl)
> +				 * HOST_BITS_PER_WIDE_INT);
> +	wide_int w = wide_int::from (w1, prec, UNSIGNED);
>   	add_AT_wide (die, DW_AT_const_value, w);
>         }
>         return true;
> 
> 	Jakub
>
diff mbox series

Patch

--- gcc/dwarf2out.c.jj	2021-03-11 14:01:43.385194205 +0100
+++ gcc/dwarf2out.c	2021-03-12 17:34:49.365207265 +0100
@@ -385,13 +385,12 @@  dump_struct_debug (tree type, enum debug
 #endif
 
 /* Get the number of HOST_WIDE_INTs needed to represent the precision
-   of the number.  Some constants have a large uniform precision, so
-   we get the precision needed for the actual value of the number.  */
+   of the number.  */
 
 static unsigned int
 get_full_len (const wide_int &op)
 {
-  int prec = wi::min_precision (op, UNSIGNED);
+  int prec = wi::get_precision (op);
   return ((prec + HOST_BITS_PER_WIDE_INT - 1)
 	  / HOST_BITS_PER_WIDE_INT);
 }
@@ -19732,8 +19731,9 @@  add_const_value_attribute (dw_die_ref di
       {
 	wide_int w1 = rtx_mode_t (rtl, MAX_MODE_INT);
 	unsigned int prec = MIN (wi::min_precision (w1, UNSIGNED),
-				 (unsigned int)CONST_WIDE_INT_NUNITS (rtl) * HOST_BITS_PER_WIDE_INT);
-	wide_int w = wi::zext (w1, prec);
+				 (unsigned int) CONST_WIDE_INT_NUNITS (rtl)
+				 * HOST_BITS_PER_WIDE_INT);
+	wide_int w = wide_int::from (w1, prec, UNSIGNED);
 	add_AT_wide (die, DW_AT_const_value, w);
       }
       return true;