[PATCH]: Fix PR58542, Arguments of __atomic_* functions are converted in unsigned mode

Uros Bizjak ubizjak@gmail.com
Wed Oct 9 20:38:00 GMT 2013


On Wed, Oct 9, 2013 at 6:23 PM, Richard Henderson <rth@redhat.com> wrote:

> This doesn't seem right at all.
>
> The bug is that I gets set to UINT64_MAX, right?  Where's the
> incorrect conversion from int to __int128_t?  Surely you can

Please see Comment #5 of PR58542:

--cut here--

The problem actually starts in expand_atomic_compare_and_swap, in:

(gdb) list
7339          create_convert_operand_to (&ops[3], expected, mode, true);
7340          create_convert_operand_to (&ops[4], desired, mode, true);
7341          create_integer_operand (&ops[5], is_weak);
7342          create_integer_operand (&ops[6], succ_model);
7343          create_integer_operand (&ops[7], fail_model);
7344          expand_insn (icode, 8, ops);

ops[4] is converted in unsigned mode, so from "desired" operand:

(gdb) p debug_rtx (desired)
(const_int -1 [0xffffffffffffffff])

we got:

(gdb) p ops[4]
$45 = {type = EXPAND_CONVERT_TO, unsigned_p = 1, unused = 0, mode =
TImode, value = 0x7fffeffc21e0}
(gdb) p debug_rtx (ops[4].value)
(const_double -1 [0xffffffffffffffff] 0 [0] 0 [0] 0 [0])

So, it is actually expansion of atomic_compare_and_swap, which doesn't
account for signedness of "desired" operand.

Manually changing the argument from "true" to "false" for ops[4]
generates correct code.

--cut here--

> produce a reduced test case that doesn't involve all of <atomic>
> to show that.

I did try, but without necessary c++ expertise, I was not able to
create equivalent c testcase.

Uros.



More information about the Gcc-patches mailing list