Re: toolchain/51121: undefined behaviour in CSU causes crashes

[Martin Husemann <martin%duskware.de@localhost>]

The following reply was made to PR toolchain/51121; it has been noted by GNATS.

From: Martin Husemann <martin%duskware.de@localhost>
To: gnats-bugs%NetBSD.org@localhost
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)();
  	}
  }