[ping] Re: Fix PR debug/66728

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

Hi Richard,

seems this still hasn't gone upstream ...  Any news?

Thanks,
Ulrich

> This is yet another bug caused by rtx having modeless scalar integer
> constants.  We need to use context to find the actual mode of a
> CONST_INT or CONST_WIDE_INT.
> 
> Getting a mode is especially awkward here.  Decls have two modes
> associated with them: TYPE_MODE (TREE_TYPE (decl)) and DECL_MODE (decl).
> Promotion via things like promote_decl_mode can lead to the rtl having
> a mode that is different from both of them.  Sometimes structure decls
> have BLKmode but are assigned an integer-mode rtl (e.g. when passing
> 3-byte structures by value to functions).
> 
> I think in this case we're just going to have to assume that none of
> these fancy mode changes happen for something that's big enough to
> need a CONST_WIDE_INT.
> 
> loc_descriptor refuses to use CONST_INT for BLKmode decls (which aren't
> actually integers at the source level).  That seems like the right
> behaviour, so this patch does that for add_const_value_attribute too.
> It asserts that the mode is otherwise sensible for both CONST_INT
> and CONST_WIDE_INT.  Asserting for CONST_INT isn't strictly necessary
> but means that the assumption will get much more coverage than asserting
> only for CONST_WIDE_INT does.
> 
> Tested on x86_64-linux-gnu and aarch64-linux-gnu.  Also tested against
> the gdb testsuite.  OK to install?
> 
> Thanks,
> Richard
> 
> gcc/
> 	PR debug/66728
> 	* dwarf2out.c (loc_descriptor): Remove redundant GET_MODE of
> 	CONST_WIDE_INTs.  Handle BLKmode for CONST_WIDE_INT too.
> 	(add_const_value_attribute): Add a mode parameter.
> 	Check it for CONST_INT and CONST_WIDE_INT.  Use it to build
> 	wide_int values.
> 	(add_location_or_const_value_attribute): Update call.
> 	(tree_add_const_value_attribute): Likewise.
> 
> gcc/testsuite/
> 	PR debug/66728
> 	* gcc.dg/debug/dwarf2/pr66728.c: New test.
> 
> diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
> index d9d3063..383d705 100644
> --- a/gcc/dwarf2out.c
> +++ b/gcc/dwarf2out.c
> @@ -3259,7 +3259,7 @@ static HOST_WIDE_INT field_byte_offset (const_tree);
>  static void add_AT_location_description	(dw_die_ref, enum dwarf_attribute,
>  					 dw_loc_list_ref);
>  static void add_data_member_location_attribute (dw_die_ref, tree);
> -static bool add_const_value_attribute (dw_die_ref, rtx);
> +static bool add_const_value_attribute (dw_die_ref, machine_mode, rtx);
>  static void insert_int (HOST_WIDE_INT, unsigned, unsigned char *);
>  static void insert_wide_int (const wide_int &, unsigned char *, int);
>  static void insert_float (const_rtx, unsigned char *);
> @@ -13795,10 +13795,9 @@ loc_descriptor (rtx rtl, machine_mode mode,
>        break;
> =20
>      case CONST_WIDE_INT:
> -      if (mode =3D=3D VOIDmode)
> -	mode =3D GET_MODE (rtl);
> -
> -      if (mode !=3D VOIDmode && (dwarf_version >=3D 4 || !dwarf_strict))
> +      if (mode !=3D VOIDmode
> +	  && mode !=3D BLKmode
> +	  && (dwarf_version >=3D 4 || !dwarf_strict))
>  	{
>  	  loc_result =3D new_loc_descr (DW_OP_implicit_value,
>  				      GET_MODE_SIZE (mode), 0);
> @@ -15576,25 +15575,33 @@ insert_float (const_rtx rtl, unsigned char *array)
>     constants do not necessarily get memory "homes".  */
> =20
>  static bool
> -add_const_value_attribute (dw_die_ref die, rtx rtl)
> +add_const_value_attribute (dw_die_ref die, machine_mode mode, rtx rtl)
>  {
>    switch (GET_CODE (rtl))
>      {
>      case CONST_INT:
> -      {
> -	HOST_WIDE_INT val =3D INTVAL (rtl);
> +      if (mode !=3D BLKmode)
> +	{
> +	  gcc_checking_assert (SCALAR_INT_MODE_P (mode));
> +	  HOST_WIDE_INT val =3D INTVAL (rtl);
> =20
> -	if (val < 0)
> -	  add_AT_int (die, DW_AT_const_value, val);
> -	else
> -	  add_AT_unsigned (die, DW_AT_const_value, (unsigned HOST_WIDE_INT) val);
> -      }
> -      return true;
> +	  if (val < 0)
> +	    add_AT_int (die, DW_AT_const_value, val);
> +	  else
> +	    add_AT_unsigned (die, DW_AT_const_value,
> +			     (unsigned HOST_WIDE_INT) val);
> +	  return true;
> +	}
> +      return false;
> =20
>      case CONST_WIDE_INT:
> -      add_AT_wide (die, DW_AT_const_value,
> -		   std::make_pair (rtl, GET_MODE (rtl)));
> -      return true;
> +      if (mode !=3D BLKmode)
> +	{
> +	  gcc_checking_assert (SCALAR_INT_MODE_P (mode));
> +	  add_AT_wide (die, DW_AT_const_value, std::make_pair (rtl, mode));
> +	  return true;
> +	}
> +      return false;
> =20
>      case CONST_DOUBLE:
>        /* Note that a CONST_DOUBLE rtx could represent either an integer or=
>  a
> @@ -15671,7 +15678,7 @@ add_const_value_attribute (dw_die_ref die, rtx rtl)
> =20
>      case CONST:
>        if (CONSTANT_P (XEXP (rtl, 0)))
> -	return add_const_value_attribute (die, XEXP (rtl, 0));
> +	return add_const_value_attribute (die, mode, XEXP (rtl, 0));
>        /* FALLTHROUGH */
>      case SYMBOL_REF:
>        if (!const_ok_for_output (rtl))
> @@ -16171,7 +16178,7 @@ add_location_or_const_value_attribute (dw_die_ref d=
> ie, tree decl, bool cache_p,
> =20
>    rtl =3D rtl_for_decl_location (decl);
>    if (rtl && (CONSTANT_P (rtl) || GET_CODE (rtl) =3D=3D CONST_STRING)
> -      && add_const_value_attribute (die, rtl))
> +      && add_const_value_attribute (die, DECL_MODE (decl), rtl))
>      return true;
> =20
>    /* See if we have single element location list that is equivalent to
> @@ -16192,7 +16199,7 @@ add_location_or_const_value_attribute (dw_die_ref d=
> ie, tree decl, bool cache_p,
>        if (GET_CODE (rtl) =3D=3D EXPR_LIST)
>  	rtl =3D XEXP (rtl, 0);
>        if ((CONSTANT_P (rtl) || GET_CODE (rtl) =3D=3D CONST_STRING)
> -	  && add_const_value_attribute (die, rtl))
> +	  && add_const_value_attribute (die, DECL_MODE (decl), rtl))
>  	 return true;
>      }
>    /* If this decl is from BLOCK_NONLOCALIZED_VARS, we might need its
> @@ -16395,7 +16402,7 @@ tree_add_const_value_attribute (dw_die_ref die, tre=
> e t)
> =20
>    rtl =3D rtl_for_decl_init (init, type);
>    if (rtl)
> -    return add_const_value_attribute (die, rtl);
> +    return add_const_value_attribute (die, TYPE_MODE (type), rtl);
>    /* If the host and target are sane, try harder.  */
>    else if (CHAR_BIT =3D=3D 8 && BITS_PER_UNIT =3D=3D 8
>  	   && initializer_constant_valid_p (init, type))
> diff --git a/gcc/testsuite/gcc.dg/debug/dwarf2/pr66728.c b/gcc/testsuite/gc=
> c.dg/debug/dwarf2/pr66728.c
> new file mode 100644
> index 0000000..ba41e97
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/debug/dwarf2/pr66728.c
> @@ -0,0 +1,14 @@
> +/* { dg-do compile { target { x86_64-*-* && lp64 } } } */
> +/* { dg-options "-O -gdwarf -dA" } */
> +
> +__uint128_t
> +test (void)
> +{
> +  static const __uint128_t foo =3D ((((__uint128_t) 0x22334455) << 96)
> +				  | 0x99aabb);
> +
> +  return foo;
> +}
> +
> +/* { dg-final { scan-assembler {\.quad\t0x99aabb\t# DW_AT_const_value} } }=
>  */
> +/* { dg-final { scan-assembler {\.quad\t0x2233445500000000\t} } } */
> 


-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com