Re: [DWARF] Fix signedness issue in DWARF functions (2)

2021-04-27 Thread Eric Botcazou
> Yes, but even bitsizetype is undistinguishable from other (usually 2 *
> pointer size) precision integral types.

OK, I can propose the attached patch.  The typed_binop_from_tree computation 
works on my Ada testcase in 32-bit mode from within GDB, but not in 64-bit 
mode because GDB chokes with:

That operation is not available on integers of more than 8 bytes.

This is worked around by the div->shift transformation, but then the previous 
change is not exercised any more...  Btw, is the rationale for using unsigned 
types for wide integer modes documented anywhere?  That's not very obvious.


* dwarf2out.c (mem_loc_descriptor) : Fix typo.
(typed_binop_from_tree): New function.
(loc_list_from_tree_1) : For an unsigned type,
turn a divide by a power of 2 into a shift.
: For an unsigned type, use a signed divide if the
size of the mode is lower than DWARF2_ADDR_SIZE; otherwise, do a
typed divide by calling typed_binop_from_tree.

-- 
Eric Botcazoudiff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index 73543190c2d..21ae7fb5ef3 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -16317,11 +16317,13 @@ mem_loc_descriptor (rtx rtl, machine_mode mode,
   if ((!dwarf_strict || dwarf_version >= 5)
 	  && is_a  (mode, _mode))
 	{
-	  if (GET_MODE_SIZE (int_mode) > DWARF2_ADDR_SIZE)
+	  /* We can use a signed divide if the sign bit is not set.  */
+	  if (GET_MODE_SIZE (int_mode) < DWARF2_ADDR_SIZE)
 	{
 	  op = DW_OP_div;
 	  goto do_binop;
 	}
+
 	  mem_loc_result = typed_binop (DW_OP_div, rtl,
 	base_type_for_mode (int_mode, 1),
 	int_mode, mem_mode);
@@ -18413,6 +18415,48 @@ function_to_dwarf_procedure (tree fndecl)
   return dwarf_proc_die;
 }
 
+/* Helper function for loc_list_from_tree.  Perform OP binary op,
+   but after converting arguments to type_die, afterwards convert
+   back to unsigned.  */
+
+static dw_loc_list_ref
+typed_binop_from_tree (enum dwarf_location_atom op, tree loc,
+		   dw_die_ref type_die, scalar_int_mode mode,
+		   struct loc_descr_context *context)
+{
+  dw_loc_list_ref op0, op1;
+  dw_loc_descr_ref cvt, binop;
+
+  if (type_die == NULL)
+return NULL;
+
+  op0 = loc_list_from_tree (TREE_OPERAND (loc, 0), 0, context);
+  op1 = loc_list_from_tree (TREE_OPERAND (loc, 1), 0, context);
+  if (op0 == NULL || op1 == NULL)
+return NULL;
+
+  cvt = new_loc_descr (dwarf_OP (DW_OP_convert), 0, 0);
+  cvt->dw_loc_oprnd1.val_class = dw_val_class_die_ref;
+  cvt->dw_loc_oprnd1.v.val_die_ref.die = type_die;
+  cvt->dw_loc_oprnd1.v.val_die_ref.external = 0;
+  add_loc_descr_to_each (op0, cvt);
+
+  cvt = new_loc_descr (dwarf_OP (DW_OP_convert), 0, 0);
+  cvt->dw_loc_oprnd1.val_class = dw_val_class_die_ref;
+  cvt->dw_loc_oprnd1.v.val_die_ref.die = type_die;
+  cvt->dw_loc_oprnd1.v.val_die_ref.external = 0;
+  add_loc_descr_to_each (op1, cvt);
+
+  add_loc_list (, op1);
+  if (op0 == NULL)
+return NULL;
+
+  binop = new_loc_descr (op, 0, 0);
+  convert_descriptor_to_mode (mode, binop);
+  add_loc_descr_to_each (op0, binop);
+
+  return op0;
+}
 
 /* Generate Dwarf location list representing LOC.
If WANT_ADDRESS is false, expression computing LOC will be computed
@@ -18994,13 +19038,53 @@ loc_list_from_tree_1 (tree loc, int want_address,
   op = DW_OP_or;
   goto do_binop;
 
+case EXACT_DIV_EXPR:
 case FLOOR_DIV_EXPR:
+case TRUNC_DIV_EXPR:
+  /* Turn a divide by a power of 2 into a shift when possible.  */
+  if (TYPE_UNSIGNED (TREE_TYPE (loc))
+	  && tree_fits_uhwi_p (TREE_OPERAND (loc, 1)))
+	{
+	  const int log2 = exact_log2 (tree_to_uhwi (TREE_OPERAND (loc, 1)));
+	  if (log2 > 0)
+	{
+	  list_ret
+		= loc_list_from_tree_1 (TREE_OPERAND (loc, 0), 0, context);
+	  if (list_ret == 0)
+		return 0;
+
+	  add_loc_descr_to_each (list_ret, uint_loc_descriptor (log2));
+	  add_loc_descr_to_each (list_ret,
+ new_loc_descr (DW_OP_shr, 0, 0));
+	  break;
+	}
+	}
+
+  /* fall through */
+
 case CEIL_DIV_EXPR:
 case ROUND_DIV_EXPR:
-case TRUNC_DIV_EXPR:
-case EXACT_DIV_EXPR:
   if (TYPE_UNSIGNED (TREE_TYPE (loc)))
-	return 0;
+	{
+	  enum machine_mode mode = TYPE_MODE (TREE_TYPE (loc));
+	  scalar_int_mode int_mode;
+
+	  if ((dwarf_strict && dwarf_version < 5)
+	  || !is_a  (mode, _mode))
+	return 0;
+
+	  /* We can use a signed divide if the sign bit is not set.  */
+	  if (GET_MODE_SIZE (int_mode) < DWARF2_ADDR_SIZE)
+	{
+	  op = DW_OP_div;
+	  goto do_binop;
+	}
+
+	  list_ret = typed_binop_from_tree (DW_OP_div, loc,
+	base_type_for_mode (int_mode, 1),
+	int_mode, context);
+	  break;
+	}
   op = DW_OP_div;
   goto do_binop;
 


Re: [DWARF] Fix signedness issue in DWARF functions (2)

2021-04-27 Thread Jakub Jelinek via Gcc-patches
On Tue, Apr 27, 2021 at 12:18:44AM +0200, Eric Botcazou wrote:
> > At least sizetype is for GIMPLE compatible with size_t or unsigned long (or
> > whatever unsigned type has the same precision), so this looks wrong to me.
> 
> It's for bitsizetype here, as it's a conversion from bits into bytes, so 
> larger than DWARF2_ADDR_SIZE.

Yes, but even bitsizetype is undistinguishable from other (usually 2 *
pointer size) precision integral types.

> > For non-strict DWARF or DWARF5 and above, I don't see why we can't use
> > typed DWARF ops instead though, see what e.g. mem_loc_descriptor does for
> > UDIV.
> 
> case UDIV:
>   if ((!dwarf_strict || dwarf_version >= 5)
>   && is_a  (mode, _mode))
>   {
> if (GET_MODE_SIZE (int_mode) > DWARF2_ADDR_SIZE)
>   {
> op = DW_OP_div;
> goto do_binop;
>   }
> mem_loc_result = typed_binop (DW_OP_div, rtl,
>   base_type_for_mode (int_mode, 1),
>   int_mode, mem_mode);
>   }
>   break;
> 
> This also looks incorrect if the size is larger than DWARF2_ADDR_SIZE then, 
> isn't there a typo in the code?
> 
> if (GET_MODE_SIZE (int_mode) < DWARF2_ADDR_SIZE)
>   {
> op = DW_OP_div;
> goto do_binop;
>   }

I think you're right.

Jakub



Re: [DWARF] Fix signedness issue in DWARF functions (2)

2021-04-26 Thread Eric Botcazou
> At least sizetype is for GIMPLE compatible with size_t or unsigned long (or
> whatever unsigned type has the same precision), so this looks wrong to me.

It's for bitsizetype here, as it's a conversion from bits into bytes, so 
larger than DWARF2_ADDR_SIZE.

> For non-strict DWARF or DWARF5 and above, I don't see why we can't use
> typed DWARF ops instead though, see what e.g. mem_loc_descriptor does for
> UDIV.

case UDIV:
  if ((!dwarf_strict || dwarf_version >= 5)
&& is_a  (mode, _mode))
{
  if (GET_MODE_SIZE (int_mode) > DWARF2_ADDR_SIZE)
{
  op = DW_OP_div;
  goto do_binop;
}
  mem_loc_result = typed_binop (DW_OP_div, rtl,
base_type_for_mode (int_mode, 1),
int_mode, mem_mode);
}
  break;

This also looks incorrect if the size is larger than DWARF2_ADDR_SIZE then, 
isn't there a typo in the code?

  if (GET_MODE_SIZE (int_mode) < DWARF2_ADDR_SIZE)
{
  op = DW_OP_div;
  goto do_binop;
}

-- 
Eric Botcazou




Re: [DWARF] Fix signedness issue in DWARF functions (2)

2021-04-26 Thread Jakub Jelinek via Gcc-patches
On Mon, Apr 26, 2021 at 06:49:22PM +0200, Eric Botcazou wrote:
> the compiler can synthesize DWARF functions to describe the location and size
> of components in discriminated record types with variant part in Ada, but in
> peculiar cases the compiler drops expressions on the floor, for example in
> the divide case:
> 
>  case ROUND_DIV_EXPR:
>  case TRUNC_DIV_EXPR:
>  case EXACT_DIV_EXPR:
>   if (TYPE_UNSIGNED (TREE_TYPE (loc)))
>   return 0;
>op = DW_OP_div;
>goto do_binop;
> 
> Now sizetype and bitsizetype are unsigned types, which means that any divide
> expression in them is dropped.  But the sign bit is expected to be never set
> for sizetype or bitsizetype quantities so that's unnecessarily cautious.
> 
> Tested on x86-64/Linux, both GCC and GDB, OK for the mainline?

At least sizetype is for GIMPLE compatible with size_t or unsigned long (or
whatever unsigned type has the same precision), so this looks wrong to me.

For non-strict DWARF or DWARF5 and above, I don't see why we can't use
typed DWARF ops instead though, see what e.g. mem_loc_descriptor does for
UDIV.

Jakub



[DWARF] Fix signedness issue in DWARF functions (2)

2021-04-26 Thread Eric Botcazou
Hi,

the compiler can synthesize DWARF functions to describe the location and size
of components in discriminated record types with variant part in Ada, but in
peculiar cases the compiler drops expressions on the floor, for example in
the divide case:

 case ROUND_DIV_EXPR:
 case TRUNC_DIV_EXPR:
 case EXACT_DIV_EXPR:
  if (TYPE_UNSIGNED (TREE_TYPE (loc)))
return 0;
   op = DW_OP_div;
   goto do_binop;

Now sizetype and bitsizetype are unsigned types, which means that any divide
expression in them is dropped.  But the sign bit is expected to be never set
for sizetype or bitsizetype quantities so that's unnecessarily cautious.

Tested on x86-64/Linux, both GCC and GDB, OK for the mainline?


2021-04-26  Eric Botcazou  

* dwarf2out.c (loc_list_from_tree_1) : Do not bail out
for sizetype and bitsizetype.

-- 
Eric Botcazoudiff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index c36fd5a7f6a..c6584aa03c3 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -18994,7 +18999,10 @@ loc_list_from_tree_1 (tree loc, int want_address,
 case ROUND_DIV_EXPR:
 case TRUNC_DIV_EXPR:
 case EXACT_DIV_EXPR:
-  if (TYPE_UNSIGNED (TREE_TYPE (loc)))
+  /* The sign bit is expected to be never set for sizetypes.  */
+  if (TYPE_UNSIGNED (TREE_TYPE (loc))
+	  && TREE_TYPE (loc) != sizetype
+	  && TREE_TYPE (loc) != bitsizetype)
 	return 0;
   op = DW_OP_div;
   goto do_binop;