Re: [DWARF] Fix signedness issue in DWARF functions (2)
> 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)
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)
> 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)
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)
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;