NetBSD Problem Report #51121
From martin@duskware.de Sat May 7 16:37:33 2016
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 "Postmaster NetBSD.org" (verified OK))
by mollari.NetBSD.org (Postfix) with ESMTPS id AD7BF7A3DC
for <gnats-bugs@gnats.NetBSD.org>; Sat, 7 May 2016 16:37:33 +0000 (UTC)
Message-Id: <20160507163731.50133A8019@mail.duskware.de>
Date: Sat, 7 May 2016 18:37:31 +0200 (CEST)
From: martin@NetBSD.org
Reply-To: martin@NetBSD.org
To: gnats-bugs@NetBSD.org
Subject: sparc64 can't build perl
X-Send-Pr-Version: 3.95
>Number: 51121
>Category: toolchain
>Synopsis: sparc64 can't build perl
>Confidential: no
>Severity: critical
>Priority: high
>Responsible: martin
>State: closed
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Sat May 07 16:40:00 +0000 2016
>Closed-Date: Fri Aug 26 08:24:29 +0000 2016
>Last-Modified: Fri Aug 26 08:24:29 +0000 2016
>Originator: Martin Husemann
>Release: NetBSD 7.99.29
>Organization:
The NetBSD Foundation, Inc.
>Environment:
System: NetBSD sunny-weather.duskware.de 7.99.29 NetBSD 7.99.29 (MODULAR) #415: Wed May 4 13:42:17 CEST 2016 martin@night-owl.duskware.de:/usr/src/sys/arch/amd64/compile/NIGHT-OWL sparc64
Architecture: sparc64
Machine: sparc64
>Description:
Trying to build perl from pkgsrc fails on first invocation of miniperl, during
exit/dtor handling by jumping through a NULL pointer.
>How-To-Repeat:
s/a
>Fix:
n/a
>Release-Note:
>Audit-Trail:
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: toolchain/51121: sparc64 can't build perl
Date: Tue, 10 May 2016 12:15:08 +0200
The jump to NULL happens here:
50: 0a 68 00 04 bcs %xcc, 60 <.text.exit+0x60>
54: 82 10 20 00 clr %g1
58: 10 68 00 08 b %xcc, 78 <.text.exit+0x78>
5c: 03 00 00 00 sethi %hi(0), %g1
5c: R_SPARC_GOT22 __deregister_frame_info
60: 9f c0 40 00 call %g1
64: ba 07 60 08 add %i5, 8, %i5
(from /usr/lib/crtbegin.o), which at runtime looks like:
0x34bdcc cmp %i5, %i4
0x34bdd0 bcs %xcc, 0x34bde0
0x34bdd4 clr %g1
0x34bdd8 b %xcc, 0x34bdf8
0x34bddc sethi %hi(0xe000), %g1
0x34bde0 call %g1
0x34bde4 add %i5, 8, %i5
and in this case it %xcc has carry set (which means the DTOR list has been
done) and it branches straight to .text.exit+0x60, with %g1 = 0.
gcc output looks like this:
.LLBB5:
.loc 1 125 0
cmp %i5, %i4
blu,pt %xcc, .LL17
mov 0, %g1
.LLBE5:
.loc 1 131 0
ba,pt %xcc, .LL22
sethi %hi(deregister_frame_info), %g1
.LLVL5:
.LL20:
.LL17:
.LLBB6:
.loc 1 126 0
call %g1, 0
add %i5, 8, %i5
so if the branch at the end of the dtor list to .LL17 is taken, %g1 will be
cleared and we necessarily die jumping to NULL.
Martin
From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/51121 CVS commit: src/lib/csu/common
Date: Tue, 10 May 2016 10:23:09 +0000
Module Name: src
Committed By: martin
Date: Tue May 10 10:23:09 UTC 2016
Modified Files:
src/lib/csu/common: Makefile.inc
Log Message:
We need the -O1 hack (for gcc 5.3) for crtbegin.c as well.
Works around PR toolchain/51121.
To generate a diff of this commit:
cvs rdiff -u -r1.30 -r1.31 src/lib/csu/common/Makefile.inc
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: toolchain/51121: sparc64 can't build perl
Date: Wed, 11 May 2016 11:06:03 +0200
Suggested by Andrew Pinski in
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71051
I reworked the crt source slightly and it now works even if compiled wiht -O2
for me.
Suggestions for a better name for the macro hiding the asm() ?
Should we put that somewhere else, in case we need to reuse it elsewhere?
(Linksets?)
Joerg, how does this interact with Clang?
Martin
Index: common/Makefile.inc
===================================================================
RCS file: /cvsroot/src/lib/csu/common/Makefile.inc,v
retrieving revision 1.31
diff -u -p -r1.31 Makefile.inc
--- common/Makefile.inc 10 May 2016 10:23:09 -0000 1.31
+++ common/Makefile.inc 11 May 2016 08:43:43 -0000
@@ -17,14 +17,6 @@ OBJS+= sysident.o
.if ${MKPIC} == "yes"
OBJS+= crtbeginS.o
CFLAGS.crtbegin.c+= -fPIE
-# XXXGCC5 - GCC 5 miscompiles crtbeginS.c on many platforms. on SPARC it
-# XXXGCC5 emits "clr %g1; call %g1", which is effectively jumping to zero.
-. if defined(HAVE_GCC) && ${HAVE_GCC} == 53 && \
- !exists(${ARCHDIR}/crtbegin.S)
-CFLAGS.crt0-common.c+= -O1
-CFLAGS.crtbeginS.c+= -O1
-CFLAGS.crtbegin.c+= -O1
-. endif
.endif
.if ${MACHINE_ARCH} == "alpha"
Index: common/crt0-common.c
===================================================================
RCS file: /cvsroot/src/lib/csu/common/crt0-common.c,v
retrieving revision 1.13
diff -u -p -r1.13 crt0-common.c
--- common/crt0-common.c 31 Jan 2013 22:24:25 -0000 1.13
+++ common/crt0-common.c 11 May 2016 08:43:43 -0000
@@ -89,6 +89,13 @@ do { \
#ifdef HAVE_INITFINI_ARRAY
/*
+ * Make sure the compiler does not know about start/end pointing
+ * to different arrays (so pointer comparison is undefined
+ * behaviour).
+ */
+#define UNALIAS_PTR(PTR) asm("":"+r"(PTR))
+
+/*
* If we are using INIT_ARRAY/FINI_ARRAY and we are linked statically,
* we have to process these instead of relying on RTLD to do it for us.
*
@@ -111,7 +118,12 @@ __weakref_visible const fptr_t fini_arra
static inline void
_preinit(void)
{
- for (const fptr_t *f = preinit_array_start; f < preinit_array_end; f++) {
+ const fptr_t *start = preinit_array_start;
+ const fptr_t *end = preinit_array_end;
+ UNALIAS_PTR(start);
+ UNALIAS_PTR(end);
+
+ for (const fptr_t *f = start; f < end; f++) {
(*f)();
}
}
@@ -119,7 +131,12 @@ _preinit(void)
static inline void
_init(void)
{
- for (const fptr_t *f = init_array_start; f < init_array_end; f++) {
+ const fptr_t *start = init_array_start;
+ const fptr_t *end = init_array_end;
+ UNALIAS_PTR(start);
+ UNALIAS_PTR(end);
+
+ for (const fptr_t *f = start; f < end; f++) {
(*f)();
}
}
@@ -127,7 +144,12 @@ _init(void)
static void
_fini(void)
{
- for (const fptr_t *f = fini_array_start; f < fini_array_end; f++) {
+ const fptr_t *start = fini_array_start;
+ const fptr_t *end = fini_array_end;
+ UNALIAS_PTR(start);
+ UNALIAS_PTR(end);
+
+ for (const fptr_t *f = start; f < end; f++) {
(*f)();
}
}
Index: common/crtbegin.c
===================================================================
RCS file: /cvsroot/src/lib/csu/common/crtbegin.c,v
retrieving revision 1.9
diff -u -p -r1.9 crtbegin.c
--- common/crtbegin.c 6 May 2014 16:02:10 -0000 1.9
+++ common/crtbegin.c 11 May 2016 08:43:43 -0000
@@ -90,7 +90,20 @@ __do_global_ctors_aux(void)
Jv_RegisterClasses(__JCR_LIST__);
#if !defined(HAVE_INITFINI_ARRAY)
- for (const fptr_t *p = __CTOR_LIST_END__; p > __CTOR_LIST__ + 1; ) {
+
+/*
+ * Make sure the compiler does not know about start/end pointing
+ * to different arrays (so pointer comparison is undefined
+ * behaviour).
+ */
+#define UNALIAS_PTR(PTR) asm("":"+r"(PTR))
+
+ const fptr_t *start = __CTOR_LIST__;
+ const fptr_t *end = __CTOR_LIST_END__;
+ UNALIAS_PTR(start);
+ UNALIAS_PTR(end);
+
+ for (const fptr_t *p = end; p > start + 1; ) {
(*(*--p))();
}
#endif
@@ -122,7 +135,12 @@ __do_global_dtors_aux(void)
#endif
#if !defined(HAVE_INITFINI_ARRAY)
- for (const fptr_t *p = __DTOR_LIST__ + 1; p < __DTOR_LIST_END__; ) {
+ const fptr_t *start = __DTOR_LIST__;
+ const fptr_t *end = __DTOR_LIST_END__;
+ UNALIAS_PTR(start);
+ UNALIAS_PTR(end);
+
+ for (const fptr_t *p = start + 1; p < end; ) {
(*(*p++))();
}
#endif
From: "Joerg Sonnenberger" <joerg@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/51121 CVS commit: src/lib/csu/common
Date: Wed, 1 Jun 2016 21:21:55 +0000
Module Name: src
Committed By: joerg
Date: Wed Jun 1 21:21:55 UTC 2016
Modified Files:
src/lib/csu/common: crtbegin.c
Log Message:
PR toolchain/51121:
__CTOR_LIST__ and __CTOR_LIST_END__ are logically the same object, but
due to the start marker, the former has to be declared as array of fixed
size. Newer GCC versions take the liberty of exploiting the UB of
accessing global objects past the end to unconditionally load zero
values in that case. Two fixes are possible:
(1) Pruning via inline assembler as done by GCC's own CRT copy.
(2) Pruning via weak references as done for linker sets.
Since the second part is known and required to work anyway, prefer this
approach. In theory, the labels could be replaced completely, except
that GNU as doesn't provide start/end symbols for sections containing
dots.
To generate a diff of this commit:
cvs rdiff -u -r1.9 -r1.10 src/lib/csu/common/crtbegin.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: wiz@NetBSD.org
State-Changed-When: Thu, 02 Jun 2016 07:55:46 +0000
State-Changed-Why:
Fixed?
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: toolchain/51121 (sparc64 can't build perl)
Date: Thu, 2 Jun 2016 22:34:41 +0200
Can't test right now, -current is completely broken.
Martin
Responsible-Changed-From-To: toolchain-manager->martin
Responsible-Changed-By: martin@NetBSD.org
Responsible-Changed-When: Wed, 15 Jun 2016 07:12:24 +0000
Responsible-Changed-Why:
Take
State-Changed-From-To: feedback->closed
State-Changed-By: martin@NetBSD.org
State-Changed-When: Wed, 15 Jun 2016 07:12:24 +0000
State-Changed-Why:
Assume fixed, will re-test when we switch back to gcc 5.4
State-Changed-From-To: closed->open
State-Changed-By: martin@NetBSD.org
State-Changed-When: Wed, 22 Jun 2016 15:20:48 +0000
State-Changed-Why:
This also causes crashes from sshd and postfix on various architectures,
at least VAX and mips64
From: Martin Husemann <martin@duskware.de>
To: gnats-bugs@NetBSD.org
Cc:
Subject: Re: toolchain/51121: undefined behaviour in CSU causes crashes
Date: Wed, 22 Jun 2016 17:27:24 +0200
We see similar fallout on mips64 and VAX with sshd and also sparc64
when compiled with gcc 5.4 for sshd and postfix. This makes it nearly
impossible to debug locally, neeeds assembler review.
Maybe the patch below can help?
Martin
Index: sys/sys/cdefs.h
===================================================================
RCS file: /cvsroot/src/sys/sys/cdefs.h,v
retrieving revision 1.128
diff -u -p -r1.128 cdefs.h
--- sys/sys/cdefs.h 19 Nov 2015 17:04:01 -0000 1.128
+++ sys/sys/cdefs.h 22 Jun 2016 15:00:06 -0000
@@ -607,6 +607,32 @@ static __inline unsigned long long __zer
#define __zeroull() (0ULL)
#endif
+#ifndef __ASSEMBLER__
+/*
+ * __launder_type(): We use this ugly hack to work around the the compiler
+ * noticing that two types may not alias each other and elide tests in code.
+ * We hit this in the CIRCLEQ macros when comparing 'struct name *' and
+ * 'struct type *' (see CIRCLEQ_HEAD()) and also in the lib/csu code.
+ * Comparing pointers besides == and != for different objects (according
+ * to the non-aliasing rules) is undefined behaviour and some compilers
+ * always evaluate such comparisons as !=.
+ *
+ * This hack is only to be used for comparisons and thus can be fully const.
+ * Do not use for assignment.
+ *
+ * If we ever choose to change the ABI of the CIRCLEQ macros, we could fix
+ * this by changing the head/tail sentinal values, but see the note above
+ * this one.
+ */
+static __inline const void * __launder_type(const void *);
+static __inline const void *
+__launder_type(const void *__x)
+{
+ __asm __volatile("" : "+r" (__x));
+ return __x;
+}
+#endif
+
#define __negative_p(x) (!((x) > 0) && ((x) != 0))
#define __type_min_s(t) ((t)((1ULL << (sizeof(t) * NBBY - 1))))
Index: sys/sys/queue.h
===================================================================
RCS file: /cvsroot/src/sys/sys/queue.h,v
retrieving revision 1.70
diff -u -p -r1.70 queue.h
--- sys/sys/queue.h 2 Nov 2015 15:21:23 -0000 1.70
+++ sys/sys/queue.h 22 Jun 2016 15:00:06 -0000
@@ -663,29 +663,6 @@ struct { \
* is discouraged!
*/
-/*
- * __launder_type(): We use this ugly hack to work around the the compiler
- * noticing that two types may not alias each other and elide tests in code.
- * We hit this in the CIRCLEQ macros when comparing 'struct name *' and
- * 'struct type *' (see CIRCLEQ_HEAD()). Modern compilers (such as GCC
- * 4.8) declare these comparisons as always false, causing the code to
- * not run as designed.
- *
- * This hack is only to be used for comparisons and thus can be fully const.
- * Do not use for assignment.
- *
- * If we ever choose to change the ABI of the CIRCLEQ macros, we could fix
- * this by changing the head/tail sentinal values, but see the note above
- * this one.
- */
-static __inline const void * __launder_type(const void *);
-static __inline const void *
-__launder_type(const void *__x)
-{
- __asm __volatile("" : "+r" (__x));
- return __x;
-}
-
#if defined(QUEUEDEBUG)
#define QUEUEDEBUG_CIRCLEQ_HEAD(head, field) \
if ((head)->cqh_first != CIRCLEQ_ENDC(head) && \
Index: lib/csu/common/crt0-common.c
===================================================================
RCS file: /cvsroot/src/lib/csu/common/crt0-common.c,v
retrieving revision 1.14
diff -u -p -r1.14 crt0-common.c
--- lib/csu/common/crt0-common.c 7 Jun 2016 12:07:35 -0000 1.14
+++ lib/csu/common/crt0-common.c 22 Jun 2016 15:00:06 -0000
@@ -96,16 +96,17 @@ do { \
* to make life easier.
*/
extern const fptr_t __preinit_array_start[] __dso_hidden;
-extern const fptr_t __preinit_array_end[] __dso_hidden __weak;
+extern const fptr_t __preinit_array_end[] __dso_hidden;
extern const fptr_t __init_array_start[] __dso_hidden;
-extern const fptr_t __init_array_end[] __dso_hidden __weak;
+extern const fptr_t __init_array_end[] __dso_hidden;
extern const fptr_t __fini_array_start[] __dso_hidden;
-extern const fptr_t __fini_array_end[] __dso_hidden __weak;
+extern const fptr_t __fini_array_end[] __dso_hidden;
static inline void
_preinit(void)
{
- for (const fptr_t *f = __preinit_array_start; f < __preinit_array_end; f++) {
+ const fptr_t end = __launder_type(__preinit_array_end);
+ for (const fptr_t *f = __preinit_array_start; f < end; f++) {
(*f)();
}
}
@@ -113,7 +114,8 @@ _preinit(void)
static inline void
_init(void)
{
- for (const fptr_t *f = __init_array_start; f < __init_array_end; f++) {
+ const fptr_t end = __launder_type(__init_array_end);
+ for (const fptr_t *f = __init_array_start; f < end; f++) {
(*f)();
}
}
@@ -121,7 +123,8 @@ _init(void)
static void
_fini(void)
{
- for (const fptr_t *f = __fini_array_start; f < __fini_array_end; f++) {
+ const fptr_t end = __launder_type(__fini_array_end);
+ for (const fptr_t *f = __fini_array_start; f < end; f++) {
(*f)();
}
}
State-Changed-From-To: open->closed
State-Changed-By: maya@NetBSD.org
State-Changed-When: Fri, 26 Aug 2016 08:24:29 +0000
State-Changed-Why:
confirmed fixed
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2014
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home