NetBSD Problem Report #56839

From www@netbsd.org  Mon May 16 15:13:18 2022
Return-Path: <www@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
	(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits))
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id CA1881A921F
	for <gnats-bugs@gnats.NetBSD.org>; Mon, 16 May 2022 15:13:18 +0000 (UTC)
Message-Id: <20220516151317.5F00B1A923A@mollari.NetBSD.org>
Date: Mon, 16 May 2022 15:13:17 +0000 (UTC)
From: rokuyama.rk@gmail.com
Reply-To: rokuyama.rk@gmail.com
To: gnats-bugs@NetBSD.org
Subject: GCC emits wrong codes for compare_and_swap_1 bultins on armv5 (el & eb)
X-Send-Pr-Version: www-1.0

>Number:         56839
>Category:       port-arm
>Synopsis:       GCC emits wrong codes for compare_and_swap_1 bultins on armv5 (el & eb)
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    port-arm-maintainer
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon May 16 15:15:00 +0000 2022
>Last-Modified:  Tue Aug 15 04:05:01 +0000 2023
>Originator:     Rin Okuyama
>Release:        9.99.96
>Organization:
Department of Physics, Meiji University
>Environment:
NetBSD hdlg 9.99.96 NetBSD 9.99.96 (HDL_G) #5: Sun May 15 19:24:35 JST 2022  rin@latipes:/build/src/sys/arch/evbarm/compile/HDL_G evbarm
>Description:
Tests for __sync_{bool,val}_compare_and_swap_1() fail on armv5:

https://www.netbsd.org/~martin/evbarm-atf/402_atf.html#lib_libc_atomic_t___sync_compare_and_swap___sync_bool_compare_and_swap_1

These failures are observed both for little- and big-endian.

This is because GCC emits wrong codes for these built-in functions.
For some cases, GCC uses "mvn" insn to generate 2nd ("expected") and
3rd ("new") arguments for these builtins.

For example, it emits "mvn reg,#0x77" for 0x88, which results in
0xffffff88. This is wrong as these arguments are passed via registers;
inappropriate 0xff's in upper 3 bytes for "expected" argument make
comparison with uint8_t value of "*ptr" unconditionally fail.

This kind of wrong immediate generations are not observed for other
functions that take uint8_t arguments. Therefore, something is wrong
for treatments peculiar to these builtins.

Note that Linux version of __sync_bool_compare_and_swap_N() takes
signed integers as the 2nd and 3rd arguments on arm, whereas these are
unsigned for us. This may be related to this wrong "sign-extension".

The above-mentioned scenario is confirmed by a minimal stripped-down
version of the test for __sync_bool_compare_and_swap_1() (and
_atomic_cas_8(), which is used internally for this builtin):

https://gist.github.com/rokuyama/f469cbf96774db1f4658f0bb932c7a23

It fails for __sync_bool_compare_and_swap_1(), and succeeds for
_atomic_cas_8():

----
$ cc -g -O2 -c arm_builtin_compare_and_swap_1.c
$ cc arm_builtin_compare_and_swap_1.o
$ ./a.out
__sync_bool_compare_and_swap_1: val expects 0xf0 but 0x88
----

(Here, "-O2" is only for readability of objdump (see below).
The same failure occurs even with "-O0".)

objdump for this test case reads:

https://gist.github.com/rokuyama/eaf499d1e57767dbbe3e7bf30b2e0fce

Here, GCC generates 0x88 by "mvn" and "mov" for arguments for
__sync_bool_compare_and_swap_1() and _atomic_cas_8(), respectively.
>How-To-Repeat:
cd /usr/tests/lib/libc/atomic && atf-run t___sync_compare_and_swap

The stripped-down version of this test is provided above.
>Fix:
N/A

>Audit-Trail:
From: Nick Hudson <nick.hudson@gmx.co.uk>
To: gnats-bugs@netbsd.org, port-arm-maintainer@netbsd.org,
 gnats-admin@netbsd.org, netbsd-bugs@netbsd.org
Cc: 
Subject: Re: port-arm/56839: GCC emits wrong codes for compare_and_swap_1
 bultins on armv5 (el & eb)
Date: Wed, 18 May 2022 12:48:54 +0100

 I think this fixes things. It's a bit ugly...

 https://www.netbsd.org/~skrll/pr56839.diff

 Nick

From: Rin Okuyama <rokuyama.rk@gmail.com>
To: Nick Hudson <nick.hudson@gmx.co.uk>, gnats-bugs@netbsd.org,
 port-arm-maintainer@netbsd.org, gnats-admin@netbsd.org,
 netbsd-bugs@netbsd.org
Cc: 
Subject: Re: port-arm/56839: GCC emits wrong codes for compare_and_swap_1
 bultins on armv5 (el & eb)
Date: Fri, 20 May 2022 10:32:32 +0900

 Hi,

 On 2022/05/18 20:48, Nick Hudson wrote:
 > I think this fixes things. It's a bit ugly...
 > 
 > https://www.netbsd.org/~skrll/pr56839.diff

 This does not work:

 (1) GCC expands __sync_bool_compare_and_swap_N() into
      __sync_val_compare_and_swap_N() as optimization.

 (2) The patch does not cover __sync_val_compare_and_swap_N()
      ( == _atomic_cas_8N() ).

 On the other hand, this patch for _atomic_cas_8N() fixes the problem:

 https://gist.github.com/rokuyama/3d791f2ceb2757dc389a95910162c9cb

 However, IMO, this is not the real fix:

 (a) I've found that MI part, not arm-dependent part, of GCC carries
      out sign-extension; something is wrong in
      expand_builtin_compare_and_swap() @ dist/gcc/gcc/builtins.c,
      but I've never fully understand what goes wrong yet...
      (Strangely enough, GCC itself recognizes that these builtins
      takes unsigned integers...)

 (b) Therefore, archs other than arm that do not have 1- and 2-byte
      atomic cas builtins (== missing equivalent instructions) should
      be affected.

 (c) It should be better to fix GCC rather than adding workaround to
      libc routines.

 Thanks,
 rin

From: Rin Okuyama <rokuyama.rk@gmail.com>
To: "gnats-bugs@NetBSD.org" <gnats-bugs@netbsd.org>, netbsd-bugs@netbsd.org
Cc: 
Subject: Re: port-arm/56839 (GCC emits wrong codes for compare_and_swap_1
 bultins on armv5 (el & eb))
Date: Tue, 15 Aug 2023 13:02:24 +0900

 This still fails in the same manner even for GCC 12.3.0 and 10.5.0.

Home	PR Database Search