NetBSD Problem Report #55239
From martin@duskware.de Wed May 6 10:21:52 2020
Return-Path: <martin@duskware.de>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-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 6AC441A9213
for <gnats-bugs@gnats.NetBSD.org>; Wed, 6 May 2020 10:21:52 +0000 (UTC)
From: martin@NetBSD.org
Reply-To: martin@NetBSD.org
To: gnats-bugs@NetBSD.org
Subject: _atomic_cas_8_mp broken on evbarm (armv5)
X-Send-Pr-Version: 3.95
>Number: 55239
>Category: port-arm
>Synopsis: _atomic_cas_8_mp broken on evbarm (armv5)
>Confidential: no
>Severity: serious
>Priority: high
>Responsible: skrll
>State: closed
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Wed May 06 10:25:00 +0000 2020
>Closed-Date: Tue May 17 05:29:36 +0000 2022
>Last-Modified: Tue May 17 05:29:36 +0000 2022
>Originator: Martin Husemann
>Release: NetBSD 9.99.60
>Organization:
The NetBSD Foundation, Inc.
>Environment:
System: NetBSD unpluged.duskware.de 9.99.60 NetBSD 9.99.60 (UNPLUGED) #319: Tue May 5 19:56:53 CEST 2020 martin@seven-days-to-the-wolves.aprisoft.de:/work/src/sys/arch/evbarm/compile/UNPLUGED evbarm
Architecture: earm
Machine: evbarm
>Description:
_atomic_cas_8_mp is miscompiled on evbarm. This make two test cases
fail:
cd /usr/tests/lib/libc/atomic && atf-run | atf-report
[..]
Failed test cases:
t___sync_compare_and_swap:__sync_bool_compare_and_swap_1,
t___sync_compare_and_swap:__sync_val_compare_and_swap_1
Summary for 15 test programs:
95 passed test cases.
2 failed test cases.
0 expected failed test cases.
0 skipped test cases.
Excerpts from a debug session with the test case:
(gdb)
0xbbdb68d8 in _atomic_cas_8_mp (ptr=0xbfffe777 "\210", old=136 '\210',
new=240 '\360')
at /work/src/lib/libc/../../common/lib/libc/atomic/atomic_init_testset.c:234
234 in /work/src/lib/libc/../../common/lib/libc/atomic/atomic_init_testset.c
(gdb)
0xbbdb68dc 234 in /work/src/lib/libc/../../common/lib/libc/atomic/atomic_init_testset.c
(gdb) n
235 in /work/src/lib/libc/../../common/lib/libc/atomic/atomic_init_testset.c
(gdb)
236 in /work/src/lib/libc/../../common/lib/libc/atomic/atomic_init_testset.c
(gdb) p ptr
$18 = (volatile uint8_t *) 0xbfffe777 "\210"
(gdb) p *ptr
$19 = 136 '\210'
(gdb) n
237 in /work/src/lib/libc/../../common/lib/libc/atomic/atomic_init_testset.c
(gdb)
240 in /work/src/lib/libc/../../common/lib/libc/atomic/atomic_init_testset.c
(gdb) p old
$20 = 136 '\210'
(gdb) p ret
$21 = 136 '\210'
note that old == ret so the if (__predict_true(ret == old)) should have
been take and in line 238 the *ptr value been modified. Alas:
242 in /work/src/lib/libc/../../common/lib/libc/atomic/atomic_init_testset.c
(gdb)
failed: /work/src/tests/lib/libc/atomic/t___sync_compare_and_swap.c:133: successful case: val expects 0xf0 but 0x88
[Inferior 1 (process 29651) exited with code 01]
Disassembly:
0xbbdb68d0 <_atomic_cas_8_mp>: push {r4, r5, lr}
0xbbdb68d4 <_atomic_cas_8_mp+4>: mov lr, #1
0xbbdb68d8 <_atomic_cas_8_mp+8>:
ldr r3, [pc, #76] ; 0xbbdb692c <_atomic_cas_8_mp+92>
0xbbdb68dc <_atomic_cas_8_mp+12>: lsr r12, r0, #3
0xbbdb68e0 <_atomic_cas_8_mp+16>: add r3, pc, r3
0xbbdb68e4 <_atomic_cas_8_mp+20>: and r12, r12, #127 ; 0x7f
0xbbdb68e8 <_atomic_cas_8_mp+24>: add r3, r3, r12, lsl #2
0xbbdb68ec <_atomic_cas_8_mp+28>: swp r4, lr, [r3]
0xbbdb68f0 <_atomic_cas_8_mp+32>:
b 0xbbdb68fc <_atomic_cas_8_mp+44>
0xbbdb68f4 <_atomic_cas_8_mp+36>: cmp r5, #0
0xbbdb68f8 <_atomic_cas_8_mp+40>:
bne 0xbbdb68ec <_atomic_cas_8_mp+28>
0xbbdb68fc <_atomic_cas_8_mp+44>: cmp r4, #0
0xbbdb6900 <_atomic_cas_8_mp+48>:
bne 0xbbdb68ec <_atomic_cas_8_mp+28>
0xbbdb6904 <_atomic_cas_8_mp+52>: ldrb r3, [r0]
0xbbdb6908 <_atomic_cas_8_mp+56>: and r3, r3, #255 ; 0xff
0xbbdb690c <_atomic_cas_8_mp+60>: cmp r3, r1
0xbbdb6910 <_atomic_cas_8_mp+64>: mov r1, #0
0xbbdb6914 <_atomic_cas_8_mp+68>: strbeq r2, [r0]
0xbbdb6918 <_atomic_cas_8_mp+72>:
ldr r2, [pc, #16] ; 0xbbdb6930 <_atomic_cas_8_mp+96>
0xbbdb691c <_atomic_cas_8_mp+76>: mov r0, r3
0xbbdb6920 <_atomic_cas_8_mp+80>: add r2, pc, r2
0xbbdb6924 <_atomic_cas_8_mp+84>: str r1, [r2, r12, lsl #2]
0xbbdb6928 <_atomic_cas_8_mp+88>: pop {r4, r5, pc}
0xbbdb692c <_atomic_cas_8_mp+92>: andeq r4, sp, r0, lsl r2
0xbbdb6930 <_atomic_cas_8_mp+96>: ldrdeq r4, [sp], -r0
>How-To-Repeat:
see above
>Fix:
n/a
>Release-Note:
>Audit-Trail:
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: port-arm/55239: _atomic_cas_8_mp broken on evbarm (armv5)
Date: Wed, 6 May 2020 13:11:13 +0200
Wondering why it uses the MP version on a single CPU machine I found a
bug in the init section, patch to fix it below.
I'm not committing it right now to make it easier to fix the real issue
in this PR first.
Martin
Index: atomic_init_testset.c
===================================================================
RCS file: /cvsroot/src/common/lib/libc/atomic/atomic_init_testset.c,v
retrieving revision 1.16
diff -u -p -r1.16 atomic_init_testset.c
--- atomic_init_testset.c 18 Feb 2019 11:22:56 -0000 1.16
+++ atomic_init_testset.c 6 May 2020 11:08:34 -0000
@@ -296,30 +296,28 @@ __libc_atomic_init(void)
return;
if (ncpu > 1)
return;
+
if (rasctl(RAS_ADDR(_atomic_cas), RAS_SIZE(_atomic_cas),
RAS_INSTALL) == 0) {
_atomic_cas_fn = _atomic_cas_up;
- return;
}
+
#ifdef __HAVE_ATOMIC_CAS_64_UP
if (rasctl(RAS_ADDR(_atomic_cas_64), RAS_SIZE(_atomic_cas_64),
RAS_INSTALL) == 0) {
_atomic_cas_64_fn = _atomic_cas_64_up;
- return;
}
#endif
if (rasctl(RAS_ADDR(_atomic_cas_16), RAS_SIZE(_atomic_cas_16),
RAS_INSTALL) == 0) {
_atomic_cas_16_fn = _atomic_cas_16_up;
- return;
}
if (rasctl(RAS_ADDR(_atomic_cas_8), RAS_SIZE(_atomic_cas_8),
RAS_INSTALL) == 0) {
_atomic_cas_8_fn = _atomic_cas_8_up;
- return;
}
}
Responsible-Changed-From-To: port-arm-maintainer->skrll
Responsible-Changed-By: skrll@NetBSD.org
Responsible-Changed-When: Fri, 08 May 2020 06:10:38 +0000
Responsible-Changed-Why:
Take
From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/55239 CVS commit: src/common/lib/libc/atomic
Date: Fri, 15 May 2020 15:20:40 +0000
Module Name: src
Committed By: martin
Date: Fri May 15 15:20:40 UTC 2020
Modified Files:
src/common/lib/libc/atomic: atomic_init_testset.c
Log Message:
PR 55239: initialize all RAS sections for non-MP configurations
To generate a diff of this commit:
cvs rdiff -u -r1.16 -r1.17 src/common/lib/libc/atomic/atomic_init_testset.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: open->feedback
State-Changed-By: skrll@NetBSD.org
State-Changed-When: Sun, 15 May 2022 20:17:39 +0000
State-Changed-Why:
Can this be closed.
The code looks OK to me. If someone ever tries to do MP on armv5
then maybe it can be revisited
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@netbsd.org
Cc: skrll@netbsd.org, netbsd-bugs@netbsd.org, gnats-admin@netbsd.org,
martin@NetBSD.org
Subject: Re: port-arm/55239 (_atomic_cas_8_mp broken on evbarm (armv5))
Date: Mon, 16 May 2022 11:04:49 +0200
On Sun, May 15, 2022 at 08:17:40PM +0000, skrll@NetBSD.org wrote:
> Can this be closed.
> The code looks OK to me. If someone ever tries to do MP on armv5
> then maybe it can be revisited
I would be happy with antything that makes the tests pass (they still
fail, on non-MP armv5).
Martin
From: Rin Okuyama <rokuyama.rk@gmail.com>
To: Martin Husemann <martin@duskware.de>, gnats-bugs@netbsd.org
Cc: skrll@netbsd.org, netbsd-bugs@netbsd.org, gnats-admin@netbsd.org,
martin@NetBSD.org
Subject: Re: port-arm/55239 (_atomic_cas_8_mp broken on evbarm (armv5))
Date: Mon, 16 May 2022 18:10:30 +0900
On 2022/05/16 18:04, Martin Husemann wrote:
> On Sun, May 15, 2022 at 08:17:40PM +0000, skrll@NetBSD.org wrote:
>> Can this be closed.
>> The code looks OK to me. If someone ever tries to do MP on armv5
>> then maybe it can be revisited
>
> I would be happy with antything that makes the tests pass (they still
> fail, on non-MP armv5).
You mean __sync_{bool,val}_compare_and_swap_1?
https://www.netbsd.org/~martin/evbarm-atf/402_atf.html#lib_libc_atomic_t___sync_compare_and_swap___sync_bool_compare_and_swap_1
This smells like a compiler bug. I was just going to examine this :)
I will send details later...
Thanks,
rin
From: Martin Husemann <martin@duskware.de>
To: Rin Okuyama <rokuyama.rk@gmail.com>
Cc: gnats-bugs@netbsd.org, skrll@netbsd.org, netbsd-bugs@netbsd.org,
gnats-admin@netbsd.org, martin@NetBSD.org
Subject: Re: port-arm/55239 (_atomic_cas_8_mp broken on evbarm (armv5))
Date: Mon, 16 May 2022 11:34:32 +0200
On Mon, May 16, 2022 at 06:10:30PM +0900, Rin Okuyama wrote:
> You mean __sync_{bool,val}_compare_and_swap_1?
Yep - failure in visual pattern matching.
So just close this PR.
Martin
From: Rin Okuyama <rokuyama.rk@gmail.com>
To: Martin Husemann <martin@duskware.de>
Cc: gnats-bugs@netbsd.org, skrll@netbsd.org, netbsd-bugs@netbsd.org,
gnats-admin@netbsd.org, martin@NetBSD.org
Subject: Re: port-arm/55239 (_atomic_cas_8_mp broken on evbarm (armv5))
Date: Tue, 17 May 2022 00:17:45 +0900
On 2022/05/16 18:34, Martin Husemann wrote:
> On Mon, May 16, 2022 at 06:10:30PM +0900, Rin Okuyama wrote:
>> You mean __sync_{bool,val}_compare_and_swap_1?
>
> Yep - failure in visual pattern matching.
> So just close this PR.
I've reported __sync_{bool,val}_compare_and_swap_1() problems as
port-arm/56839.
Thanks,
rin
State-Changed-From-To: feedback->closed
State-Changed-By: skrll@NetBSD.org
State-Changed-When: Tue, 17 May 2022 05:29:36 +0000
State-Changed-Why:
OK'ed by martin
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.46 2020/01/03 16:35:01 leot Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2020
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home