On 25/11/16 18:50, Paul Eggert wrote: > Pádraig Brady wrote: >> for UBSAN we should probably build with >> _STRING_ARCH_unaligned defined globally >> to avoid warning for the cases we already handle. > > Yes. Translating this for non-experts: the problem here is a bug in the > bug-finding procedure, not a bug in GNU coreutils or in Gnulib.
Sorry I was a bit terse. coreutils/gnulib should currently be compiled with -D_STRING_ARCH_unaligned=0 -D_STRING_INLINE_unaligned=0 when using UBSAN, to use only alignment portable code. Methods for avoiding false UBSAN warnings automatically are discussed below... > Recent glibc (since 2016-02-18) does not define _STRING_ARCH_unaligned, which > means that this code in gnulib md5.c etc. is no longer exercised on recent > platforms. Oh interesting. I see details in: https://sourceware.org/bugzilla/show_bug.cgi?id=19462 There it suggests that _STRING_ARCH_unaligned is now internal to glibc and _STRING_INLINE_unaligned is the newer stable equivalent. Attached patch to do this for coreutils is attached. > So in some sense the originally-reported bug is already fixed (via an > unexpected glibc change), though this does mean Gnulib md5 etc. is now slower > on > x86-64 etc., which is a performance bug on newer platforms. If we fix the > performance bug I suppose we'll start getting false alarms from UBSAN again. We can explicitly avoid the UBSAN warnings with something like: http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=v8.23-80-g5760532 which might be acceptable given the few places it matters. That's a bit of a big hammer though, defining away all of UBSAN for those routines. Alternatively we might define the non-portable faster path away, if we could detect we where compiling in UBSAN mode. That's easy enough for -fsanitize=address, though it doesn't look like there is currently a way to detect -fsanitize=undefined? http://stackoverflow.com/q/39371798/4421 Another approach would be to support ../configure --with-asan --with-ubsan which would define things appropriately. cheers, Pádraig.
From 952bd8666c7ed887d49b049a7585b56c1e93f323 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1draig=20Brady?= <[email protected]> Date: Fri, 25 Nov 2016 22:10:20 +0000 Subject: [PATCH] shred,sort: ensure faster unaligned access to rand module glibc has changed the public define from _STRING_ARCH_unaligned to _STRING_INLINE_unaligned as per https://sourceware.org/bugzilla/show_bug.cgi?id=19462 * gl/lib/rand-isaac.c: Cater for both defines. * gl/lib/randread.c: Likewise. * src/system.h: Update commented out code. --- gl/lib/rand-isaac.c | 2 +- gl/lib/randread.c | 2 +- src/system.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gl/lib/rand-isaac.c b/gl/lib/rand-isaac.c index 5ad9cae..349bf0e 100644 --- a/gl/lib/rand-isaac.c +++ b/gl/lib/rand-isaac.c @@ -40,7 +40,7 @@ /* If the platform supports unaligned access, then don't have -fsanitize=undefined warn about it. */ #undef ATTRIBUTE_NO_WARN_SANITIZE_UNDEFINED -#if !_STRING_ARCH_unaligned \ +#if !(_STRING_ARCH_unaligned || _STRING_INLINE_unaligned) \ || __GNUC__ < 4 || (__GNUC__ == 4 && __GNUC_MINOR__ < 9) # define ATTRIBUTE_NO_WARN_SANITIZE_UNDEFINED /* empty */ #else diff --git a/gl/lib/randread.c b/gl/lib/randread.c index ff85d56..2d211a0 100644 --- a/gl/lib/randread.c +++ b/gl/lib/randread.c @@ -60,7 +60,7 @@ # define MIN(a, b) ((a) < (b) ? (a) : (b)) #endif -#if _STRING_ARCH_unaligned +#if _STRING_ARCH_unaligned || _STRING_INLINE_unaligned # define ALIGNED_POINTER(ptr, type) true #else # define ALIGNED_POINTER(ptr, type) ((size_t) (ptr) % alignof (type) == 0) diff --git a/src/system.h b/src/system.h index e82dce4..3fa0740 100644 --- a/src/system.h +++ b/src/system.h @@ -515,7 +515,7 @@ is_nul (void const *buf, size_t length) to avoid -fsanitize=undefined warnings. Considering coreutils is mainly concerned with relatively large buffers, we'll just use the defined behavior. */ -#if 0 && _STRING_ARCH_unaligned +#if 0 && (_STRING_ARCH_unaligned || _STRING_INLINE_unaligned) unsigned long word; #else unsigned char word; -- 2.5.5
