NetBSD Problem Report #58352

From www@netbsd.org  Tue Jun 18 13:05:04 2024
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)
	 key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256
	 client-signature RSA-PSS (2048 bits) client-digest SHA256)
	(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
	by mollari.NetBSD.org (Postfix) with ESMTPS id 5F1C81A9238
	for <gnats-bugs@gnats.NetBSD.org>; Tue, 18 Jun 2024 13:05:04 +0000 (UTC)
Message-Id: <20240618130503.4A2A71A923A@mollari.NetBSD.org>
Date: Tue, 18 Jun 2024 13:05:03 +0000 (UTC)
From: campbell+netbsd@mumble.net
Reply-To: campbell+netbsd@mumble.net
To: gnats-bugs@NetBSD.org
Subject: ld: /usr/lib/libubsan.so: undefined reference to `__sync_val_compare_and_swap_8'
X-Send-Pr-Version: www-1.0

>Number:         58352
>Category:       port-sparc
>Synopsis:       ld: /usr/lib/libubsan.so: undefined reference to `__sync_val_compare_and_swap_8'
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    port-sparc-maintainer
>State:          feedback
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Jun 18 13:10:00 +0000 2024
>Closed-Date:    
>Last-Modified:  Tue Jul 02 02:53:35 +0000 2024
>Originator:     Taylor R Campbell
>Release:        current
>Organization:
The NetBSD __sync_Foundation_8
>Environment:
NetBSD 10.99.10 (GENERIC) #0: Sun Jun 16 08:35:47 UTC 2024 root@babylon5.netbsd.org:/tmp/build/2024.06.16.00.12.33-sparc/obj/sys/arch/sparc/compile/GENERIC
>Description:
Since the gcc12 update, ubsan tests are failing with undefined references to __sync_val_compare_and_swap_8 -- presumably they previously didn't try to use 64-bit atomics and now they do, and sparc doesn't have 64-bit atomics (may affect other ports without 64-bit atomics too):

ld: /usr/lib/libubsan.so: undefined reference to `__sync_val_compare_and_swap_8'
Fail: regexp signed integer overflow not in stderr
execvp(./test) failed: No such file or directory

https://releng.netbsd.org/b5reports/sparc/2024/2024.06.16.00.12.33/test.html#usr.bin_c++_t_ubsan_int_add_overflow_int_add_overflow
>How-To-Repeat:
cd /usr/tests/usr.bin/c++
atf-run t_ubsan_int_add_overflow
>Fix:
Yes, please!

>Release-Note:

>Audit-Trail:
From: Rin Okuyama <rokuyama.rk@gmail.com>
To: gnats-bugs@netbsd.org, port-sparc-maintainer@netbsd.org,
 gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, campbell+netbsd@mumble.net
Cc: matthew green <mrg@NetBSD.org>
Subject: Re: port-sparc/58352: ld: /usr/lib/libubsan.so: undefined reference
 to `__sync_val_compare_and_swap_8'
Date: Tue, 18 Jun 2024 23:15:29 +0900

 CC: mrg@

 Here's my patch to hack this around:

 https://gist.github.com/rokuyama/24d0ed7e537fffc07e2975c3880817f1

 Yes, this makes things difficult to catch up with upstream...

 Thanks,
 rin

From: matthew green <mrg@eterna23.net>
To: gnats-bugs@netbsd.org
Cc: port-sparc-maintainer@netbsd.org, gnats-admin@netbsd.org,
    netbsd-bugs@netbsd.org
Subject: re: port-sparc/58352: ld: /usr/lib/libubsan.so: undefined reference to `__sync_val_compare_and_swap_8'
Date: Wed, 19 Jun 2024 02:52:06 +1000

 campbell+netbsd@mumble.net writes:
 > >Number:         58352
 > >Category:       port-sparc
 > >Synopsis:       ld: /usr/lib/libubsan.so: undefined reference to `__syn=
 c_val_compare_and_swap_8'

 hmm, i thought i'd avoided this already.

 it's one of the notes in README.gcc12.


 .mrg.

From: Rin Okuyama <rokuyama.rk@gmail.com>
To: gnats-bugs@netbsd.org, Taylor R Campbell <riastradh@NetBSD.org>,
 matthew green <mrg@NetBSD.org>
Cc: port-sparc-maintainer@netbsd.org, gnats-admin@netbsd.org,
 netbsd-bugs@netbsd.org
Subject: Re: port-sparc/58352: ld: /usr/lib/libubsan.so: undefined reference
 to `__sync_val_compare_and_swap_8'
Date: Wed, 19 Jun 2024 23:44:09 +0900

 On 2024/06/19 1:52, matthew green wrote:
 > campbell+netbsd@mumble.net writes:
 >>> Number:         58352
 >>> Category:       port-sparc
 >>> Synopsis:       ld: /usr/lib/libubsan.so: undefined reference to `__sync_val_compare_and_swap_8'
 > 
 > hmm, i thought i'd avoided this already.
 > 
 > it's one of the notes in README.gcc12.

 It seems that `-mcpu=v8` is not sufficient for sparc:

 (1) 64-bit atomic ops requires V8PLUS:

 https://github.com/NetBSD/src/blob/trunk/external/gpl3/gcc/dist/gcc/config/sparc/sync.md#L23

 (2) V8PLUS requires both VIS and V9:

 https://github.com/NetBSD/src/blob/trunk/external/gpl3/gcc/dist/gcc/config/sparc/sparc.cc#L1938-L1948

 (3) Earliest CPU model satisfy (2) seems `ultrasparc3`, IIUC:
 (I'm not familiar to sparc although...)

 https://github.com/NetBSD/src/blob/trunk/external/gpl3/gcc/dist/gcc/config/sparc/sparc.cc#L1766-L1767

 Actually, lib*san.so built with -mcpu=v8 contain undefined reference to
 __sync_val_compare_and_swap8, but they do not with -mcpu=ultrasparc3.

 Thanks,
 rin
 ----
 diff --git a/external/gpl3/gcc/lib/Makefile.sanitizer 
 b/external/gpl3/gcc/lib/Makefile.sanitizer
 index e574f902c22..e20693c3241 100644
 --- a/external/gpl3/gcc/lib/Makefile.sanitizer
 +++ b/external/gpl3/gcc/lib/Makefile.sanitizer
 @@ -128,7 +128,7 @@ COPTS.ubsan_init.cc += -O1
   # - 32-bit SPARC needs v8 to supply eg __sync_add_and_fetch_4
   # - i386 needs i586 for __sync_val_compare_and_swap_8.
   .if ${MACHINE_ARCH} == "sparc"
 -COPTS+= -mcpu=v8
 +COPTS+= -mcpu=ultrasparc3
   .endif
   .if ${MACHINE_ARCH} == "i386"
   COPTS+= -march=i586

From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: port-sparc/58352: ld: /usr/lib/libubsan.so: undefined reference
 to `__sync_val_compare_and_swap_8'
Date: Wed, 19 Jun 2024 16:51:37 +0200

 On Wed, Jun 19, 2024 at 02:45:02PM +0000, Rin Okuyama wrote:
 >  (3) Earliest CPU model satisfy (2) seems `ultrasparc3`, IIUC:
 >  (I'm not familiar to sparc although...)

 All ultrasparc CPUs support VIS and can do 64bit atomic ops.

 The ultrasparc3 introduced "VIS 2" with additional instructions.

 However, for 32bit sparc CPUs we can not assume support (and it is pretty
 lame of ubsan to use it). Can we patch the ubsan code to only require 64bit
 atomics #ifdef _LP64 ?

 Martin

From: Rin Okuyama <rokuyama.rk@gmail.com>
To: gnats-bugs@netbsd.org, port-sparc-maintainer@netbsd.org,
 gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, campbell+netbsd@mumble.net
Cc: 
Subject: Re: port-sparc/58352: ld: /usr/lib/libubsan.so: undefined reference
 to `__sync_val_compare_and_swap_8'
Date: Thu, 20 Jun 2024 00:12:04 +0900

 On 2024/06/19 23:55, Martin Husemann wrote:
 > The following reply was made to PR port-sparc/58352; it has been noted by GNATS.
 > 
 > From: Martin Husemann <martin@duskware.de>
 > To: gnats-bugs@netbsd.org
 > Cc:
 > Subject: Re: port-sparc/58352: ld: /usr/lib/libubsan.so: undefined reference
 >   to `__sync_val_compare_and_swap_8'
 > Date: Wed, 19 Jun 2024 16:51:37 +0200
 > 
 >   On Wed, Jun 19, 2024 at 02:45:02PM +0000, Rin Okuyama wrote:
 >   >  (3) Earliest CPU model satisfy (2) seems `ultrasparc3`, IIUC:
 >   >  (I'm not familiar to sparc although...)
 >   
 >   All ultrasparc CPUs support VIS and can do 64bit atomic ops.
 >   
 >   The ultrasparc3 introduced "VIS 2" with additional instructions.

 Hmm, I tried `-mcpu=ultrasparc`, but generated library has reference to
 __sync_val_compare_and_swap_8. I've not figured out why, at the moment.

 >   However, for 32bit sparc CPUs we can not assume support (and it is pretty
 >   lame of ubsan to use it). Can we patch the ubsan code to only require 64bit
 >   atomics #ifdef _LP64 ?

 Yeah, this is just what I tried. It works at least on i386, but
 diff is *really* large:

 https://gist.github.com/rokuyama/24d0ed7e537fffc07e2975c3880817f1

 I wonder whether we can ship atomic64 runtime routines with
 lib*san as, e.g., hidden symbols...

 Thanks,
 rin

From: matthew green <mrg@eterna23.net>
To: Rin Okuyama <rokuyama.rk@gmail.com>
Cc: gnats-bugs@netbsd.org, port-sparc-maintainer@netbsd.org,
    gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
    campbell+netbsd@mumble.net
Subject: re: port-sparc/58352: ld: /usr/lib/libubsan.so: undefined reference to `__sync_val_compare_and_swap_8'
Date: Thu, 20 Jun 2024 03:04:52 +1000

 > Yeah, this is just what I tried. It works at least on i386, but
 > diff is *really* large:
 >
 > https://gist.github.com/rokuyama/24d0ed7e537fffc07e2975c3880817f1

 i don't think this is that large.

 the only thing i'd really prefer would be to move the define
 for SANITIZER_NO_64BIT_ATOMIC_OPS from the Makefile into a common
 header such that you can define SANITIZER_STATE_T like you have.

 in the common header (not sure which one off hand), inside a
 SANITIZER_NETBSD section, check for our __HAVE_64BIT_ATOMIC_OPS
 or whatever it is, and define SANITIZER_NO_64BIT_ATOMIC_OPS to
 suit...


 i think this is do-able.  it's slightly more diffs than the
 current ones in gcc/dist/libsanitizer, but most of it is one
 bigger chunk, and all of them are fairly simple conversion
 (u64 -> SANITIZER_STATE_T), so keeping it in the future would
 not be too hard.  (it only adds about 4% to the total in all
 of gcc/dist.)

 thanks.


 .mrg.

From: Martin Husemann <martin@duskware.de>
To: matthew green <mrg@eterna23.net>
Cc: Rin Okuyama <rokuyama.rk@gmail.com>, gnats-bugs@netbsd.org,
	port-sparc-maintainer@netbsd.org, gnats-admin@netbsd.org,
	netbsd-bugs@netbsd.org, campbell+netbsd@mumble.net
Subject: Re: port-sparc/58352: ld: /usr/lib/libubsan.so: undefined reference
 to `__sync_val_compare_and_swap_8'
Date: Wed, 19 Jun 2024 21:29:10 +0200

 On Thu, Jun 20, 2024 at 03:04:52AM +1000, matthew green wrote:
 > i think this is do-able.  it's slightly more diffs than the
 > current ones in gcc/dist/libsanitizer, but most of it is one
 > bigger chunk, and all of them are fairly simple conversion
 > (u64 -> SANITIZER_STATE_T), so keeping it in the future would
 > not be too hard.  (it only adds about 4% to the total in all
 > of gcc/dist.)

 And then (for bonus points) try to push the u64 -> SANITIZER_STATE_T change
 upstream :-)

 Martin

From: Rin Okuyama <rokuyama.rk@gmail.com>
To: matthew green <mrg@NetBSD.org>, Martin Husemann <martin@duskware.de>
Cc: gnats-bugs@netbsd.org, port-sparc-maintainer@netbsd.org,
 gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, campbell+netbsd@mumble.net
Subject: Re: port-sparc/58352: ld: /usr/lib/libubsan.so: undefined reference
 to `__sync_val_compare_and_swap_8'
Date: Thu, 27 Jun 2024 17:55:19 +0900

 On 2024/06/20 2:04, matthew green wrote:
 >> Yeah, this is just what I tried. It works at least on i386, but
 >> diff is *really* large:
 >>
 >> https://gist.github.com/rokuyama/24d0ed7e537fffc07e2975c3880817f1
 > 
 > i don't think this is that large.
 > 
 > the only thing i'd really prefer would be to move the define
 > for SANITIZER_NO_64BIT_ATOMIC_OPS from the Makefile into a common
 > header such that you can define SANITIZER_STATE_T like you have.
 > 
 > in the common header (not sure which one off hand), inside a
 > SANITIZER_NETBSD section, check for our __HAVE_64BIT_ATOMIC_OPS
 > or whatever it is, and define SANITIZER_NO_64BIT_ATOMIC_OPS to
 > suit...

 I got it! I will rework the patch.

 I've confirmed that the original patch can be cleanly
 applied to recent -current. ubsan works just fine for i386,
 powerpc, and vax (with WIP patches). asan also works for i386.

 > i think this is do-able.  it's slightly more diffs than the
 > current ones in gcc/dist/libsanitizer, but most of it is one
 > bigger chunk, and all of them are fairly simple conversion
 > (u64 -> SANITIZER_STATE_T), so keeping it in the future would
 > not be too hard.  (it only adds about 4% to the total in all
 > of gcc/dist.)

 Oops > 4%. Let me thank you again for your continuous efforts to
 make GCC up to date!

 On 2024/06/20 4:29, Martin Husemann wrote:
  > And then (for bonus points) try to push the u64 -> SANITIZER_STATE_T 
 change
  > upstream :-)

 Yeah, I hope :)

 Thanks,
 rin

From: Rin Okuyama <rokuyama.rk@gmail.com>
To: matthew green <mrg@NetBSD.org>, Martin Husemann <martin@duskware.de>
Cc: gnats-bugs@netbsd.org, port-sparc-maintainer@netbsd.org,
 gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, campbell+netbsd@mumble.net
Subject: Re: port-sparc/58352: ld: /usr/lib/libubsan.so: undefined reference
 to `__sync_val_compare_and_swap_8'
Date: Sun, 30 Jun 2024 15:28:31 +0900

 After some investigation, I've found that sanitizer already
 has its own 64-bit atomic ops for platforms without them.

 With the attached patch:
 - libasan works on i386
 - libubsan works on i386, powerpc, and vax (with WIP patches)

 Yes, this adds some performance penalty for these platforms,
 however, it is better than having extra diff to upstream, IMO.

 Comments?

 Thanks,
 rin
 ----
 diff --git a/external/gpl3/gcc/dist/libsanitizer/sanitizer_common/sanitizer_atomic_clang.h b/external/gpl3/gcc/dist/libsanitizer/sanitizer_common/sanitizer_atomic_clang.h
 index ccf18f0786d..79f6ad71096 100644
 --- a/external/gpl3/gcc/dist/libsanitizer/sanitizer_common/sanitizer_atomic_clang.h
 +++ b/external/gpl3/gcc/dist/libsanitizer/sanitizer_common/sanitizer_atomic_clang.h
 @@ -94,11 +94,17 @@ inline bool atomic_compare_exchange_weak(volatile T *a,
   }  // namespace __sanitizer

   // This include provides explicit template instantiations for atomic_uint64_t
 -// on MIPS32, which does not directly support 8 byte atomics. It has to
 +// on platforms, which do not directly support 8 byte atomics. It has to
   // proceed the template definitions above.
   #if defined(_MIPS_SIM) && defined(_ABIO32) && _MIPS_SIM == _ABIO32
   #  include "sanitizer_atomic_clang_mips.h"
   #endif
 +#if SANITIZER_NETBSD
 +#  include <machine/types.h>
 +#  ifndef __HAVE_ATOMIC64_OPS
 +#    include "sanitizer_atomic_clang_mips.h"
 +#  endif
 +#endif

   #undef ATOMIC_ORDER

 diff --git a/external/gpl3/gcc/lib/Makefile.sanitizer b/external/gpl3/gcc/lib/Makefile.sanitizer
 index e574f902c22..00e9f149405 100644
 --- a/external/gpl3/gcc/lib/Makefile.sanitizer
 +++ b/external/gpl3/gcc/lib/Makefile.sanitizer
 @@ -123,13 +123,3 @@ COPTS.sanitizer_symbolizer_report.cc += -O1
   COPTS.ubsan_diag.cc += -O1
   COPTS.ubsan_init.cc += -O1
   .endif
 -
 -# XXX GCC 12 sanitizers has higher minimal requirements upon some CPUs.
 -# - 32-bit SPARC needs v8 to supply eg __sync_add_and_fetch_4
 -# - i386 needs i586 for __sync_val_compare_and_swap_8.
 -.if ${MACHINE_ARCH} == "sparc"
 -COPTS+= -mcpu=v8
 -.endif
 -.if ${MACHINE_ARCH} == "i386"
 -COPTS+= -march=i586
 -.endif

From: matthew green <mrg@eterna23.net>
To: Rin Okuyama <rokuyama.rk@gmail.com>
Cc: gnats-bugs@netbsd.org, port-sparc-maintainer@netbsd.org,
    gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,
    campbell+netbsd@mumble.net, Martin Husemann <martin@duskware.de>
Subject: re: port-sparc/58352: ld: /usr/lib/libubsan.so: undefined reference to `__sync_val_compare_and_swap_8'
Date: Sun, 30 Jun 2024 17:24:02 +1000

 Rin Okuyama writes:
 > After some investigation, I've found that sanitizer already
 > has its own 64-bit atomic ops for platforms without them.
 >
 > With the attached patch:
 > - libasan works on i386
 > - libubsan works on i386, powerpc, and vax (with WIP patches)
 >
 > Yes, this adds some performance penalty for these platforms,
 > however, it is better than having extra diff to upstream, IMO.

 nice!  this seems more maintainable, if slower, and i suspect
 32-bit platform users doing this are less common anyway.

 amused it is (a) in "sanitizer_atomic_clang.h", which is also
 used by GCC, and (b) includes "sanitizer_atomic_clang_mips.h",
 which has nothing mips- or clang- specific in it.

 thanks.


 .mrg.

From: "Rin Okuyama" <rin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/58352 CVS commit: src/external/gpl3/gcc
Date: Tue, 2 Jul 2024 02:36:22 +0000

 Module Name:	src
 Committed By:	rin
 Date:		Tue Jul  2 02:36:22 UTC 2024

 Modified Files:
 	src/external/gpl3/gcc/dist/libsanitizer/sanitizer_common:
 	    sanitizer_atomic_clang.h
 	src/external/gpl3/gcc/lib: Makefile.sanitizer

 Log Message:
 gcc: lib*san: Enable built-in 64-bit atomic ops for !__HAVE_ATOMIC64_OPS

 as already done for some mips platforms by upstream.

 Now, libubsan (as well as libasan if platform itself is supported)
 work on some ILP32 platforms.

 Fix PR port-sparc/58352.

 This may be slower than another possible fix where all 64-bit atomic
 variables are replaced by 32-bit ones. But, it should still be better
 than having relatively large diffs to upstream; sanitizers are not
 enabled for normal use, anyway.

 XXX
 Note that filename confusion in upstream codes:
 - "sanitizer_atomic_clang.h" is used also for GCC.
 - "sanitizer_atomic_clang_mips.h" has no mips specific codes.


 To generate a diff of this commit:
 cvs rdiff -u -r1.4 -r1.5 \
     src/external/gpl3/gcc/dist/libsanitizer/sanitizer_common/sanitizer_atomic_clang.h
 cvs rdiff -u -r1.18 -r1.19 src/external/gpl3/gcc/lib/Makefile.sanitizer

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

From: Rin Okuyama <rokuyama.rk@gmail.com>
To: matthew green <mrg@NetBSD.org>
Cc: gnats-bugs@netbsd.org, port-sparc-maintainer@netbsd.org,
 gnats-admin@netbsd.org, netbsd-bugs@netbsd.org, campbell+netbsd@mumble.net,
 Martin Husemann <martin@duskware.de>
Subject: Re: port-sparc/58352: ld: /usr/lib/libubsan.so: undefined reference
 to `__sync_val_compare_and_swap_8'
Date: Tue, 2 Jul 2024 11:41:42 +0900

 On 2024/06/30 16:24, matthew green wrote:
 > Rin Okuyama writes:
 >> After some investigation, I've found that sanitizer already
 >> has its own 64-bit atomic ops for platforms without them.
 >>
 >> With the attached patch:
 >> - libasan works on i386
 >> - libubsan works on i386, powerpc, and vax (with WIP patches)
 >>
 >> Yes, this adds some performance penalty for these platforms,
 >> however, it is better than having extra diff to upstream, IMO.
 > 
 > nice!  this seems more maintainable, if slower, and i suspect
 > 32-bit platform users doing this are less common anyway.

 Committed. Thanks for comment!

 > amused it is (a) in "sanitizer_atomic_clang.h", which is also
 > used by GCC, and (b) includes "sanitizer_atomic_clang_mips.h",
 > which has nothing mips- or clang- specific in it.

 Shrug ;) I've added XXX comment into the commit.

 Thanks,
 rin

State-Changed-From-To: open->feedback
State-Changed-By: rin@NetBSD.org
State-Changed-When: Tue, 02 Jul 2024 02:53:35 +0000
State-Changed-Why:
Can you please try on sparc?


>Unformatted:

Home	PR Database Search