Re: [PATCH] Fix strict aliasing violations in a few string functions
Paul Eggert <[email protected]> Sun, 26 Jul 2026 13:55:44 -0700
| Newsgroups | gmane.comp.lib.gnulib.bugs |
|---|---|
| Organization | UCLA Computer Science Department |
| Message-ID | <[email protected]> |
On 2026-07-26 05:37, Bruno Haible via Gnulib discussion list wrote: > Let's hear Paul's opinion. I also don't see downsides to Lasse's patch. It looks like a good patch and necessary for some perhaps-theoretical optimizations. So I installed it. Thanks, Lasse. The idea about adding __attribute__((no_sanitize_address)) to rawmemchr and strchrnul is iffier, as it could cause these functions to not report internal errors that they should report. Instead, these functions can simply avoid the optimized-but-in-theory-invalid behavior if address sanitization is in effect. Although that slows things down, sanitization is already slower so it's not that big a deal. I installed the attached to do that.
0001-Pacify-fsanitize-address-in-rawmemchr-strchrnul.patch
(text/x-patch, 8.4 KB)
From f6b3efe5018211f371305e693c8d786d9489a769 Mon Sep 17 00:00:00 2001 From: Paul Eggert <[email protected]> Date: Sun, 26 Jul 2026 13:46:46 -0700 Subject: [PATCH] Pacify -fsanitize=address in rawmemchr, strchrnul Problem reported by Lasse Collin in: https://lists.gnu.org/r/bug-gnulib/2026-07/msg00174.html * lib/memchr.c (__memchr), lib/memchr2.c (memchr2): * lib/rawmemchr.c (rawmemchr), lib/strchrnul.c (strchrnul): Omit the longword optimization if __SANITIZE_ADDRESS__. --- ChangeLog | 7 +++++++ lib/memchr.c | 18 +++++++++++++++--- lib/memchr2.c | 26 +++++++++++++++++++------- lib/rawmemchr.c | 24 ++++++++++++++++-------- lib/strchrnul.c | 27 +++++++++++++++++++-------- 5 files changed, 76 insertions(+), 26 deletions(-) diff --git a/ChangeLog b/ChangeLog index e74699ea91..109821b783 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,12 @@ 2026-07-26 Paul Eggert <[email protected]> + Pacify -fsanitize=address in rawmemchr, strchrnul + Problem reported by Lasse Collin in: + https://lists.gnu.org/r/bug-gnulib/2026-07/msg00174.html + * lib/memchr.c (__memchr), lib/memchr2.c (memchr2): + * lib/rawmemchr.c (rawmemchr), lib/strchrnul.c (strchrnul): + Omit the longword optimization if __SANITIZE_ADDRESS__. + Fix strict aliasing violations in memchr etc Problem and fix reported by Lasse Collin in: https://lists.gnu.org/r/bug-gnulib/2026-07/msg00172.html diff --git a/lib/memchr.c b/lib/memchr.c index 0e3c2c8a35..b023f478bf 100644 --- a/lib/memchr.c +++ b/lib/memchr.c @@ -58,6 +58,16 @@ void * __memchr (void const *s, int c_in, size_t n) { + unsigned reg_char c = c_in; + + /* Examine the memory an aligned longword at a time, when possible. + Doing so can go past the end of a lesser-aligned object, + which is harmless on most platforms but violates the rules of C, + so suppress this optimization on platforms where it is known to be + dangerous, namely, those using address sanitization. */ + +#if ! (defined __SANITIZE_ADDRESS__ || defined __CHERI_PURE_CAPABILITY__) + /* On 32-bit hardware, choosing longword to be a 32-bit unsigned long instead of a 64-bit uintmax_t tends to give better performance. On 64-bit hardware, unsigned long is generally 64 @@ -65,8 +75,6 @@ __memchr (void const *s, int c_in, size_t n) performance. */ typedef unsigned long int longword _GL_ATTRIBUTE_MAY_ALIAS; - unsigned reg_char c = (unsigned char) c_in; - const longword *longword_ptr; /* Handle the first few bytes by reading one byte at a time. @@ -144,10 +152,14 @@ __memchr (void const *s, int c_in, size_t n) longword_ptr++; n -= sizeof (longword); } + + s = longword_ptr; } +#endif + { - const unsigned char *char_ptr = (const unsigned char *) longword_ptr; + const unsigned char *char_ptr = s; /* At this point, we know that either n < sizeof (longword), or one of the sizeof (longword) bytes starting at char_ptr is == c. On little-endian diff --git a/lib/memchr2.c b/lib/memchr2.c index 610f9165a0..ff2bd9c0ad 100644 --- a/lib/memchr2.c +++ b/lib/memchr2.c @@ -35,6 +35,17 @@ void * memchr2 (void const *s, int c1_in, int c2_in, size_t n) { + unsigned char c1 = c1_in; + unsigned char c2 = c2_in; + + /* Examine the memory an aligned longword at a time, when possible. + Doing so can go past the end of a lesser-aligned object, + which is harmless on most platforms but violates the rules of C, + so suppress this optimization on platforms where it is known to be + dangerous, namely, those using address sanitization. */ + +#if ! (defined __SANITIZE_ADDRESS__ || defined __CHERI_PURE_CAPABILITY__) + /* On 32-bit hardware, choosing longword to be a 32-bit unsigned long instead of a 64-bit uintmax_t tends to give better performance. On 64-bit hardware, unsigned long is generally 64 @@ -42,9 +53,6 @@ memchr2 (void const *s, int c1_in, int c2_in, size_t n) performance. */ typedef unsigned long int longword _GL_ATTRIBUTE_MAY_ALIAS; - unsigned char c1 = (unsigned char) c1_in; - unsigned char c2 = (unsigned char) c2_in; - if (c1 == c2) return (void *) memchr (s, c1, n); @@ -137,10 +145,6 @@ memchr2 (void const *s, int c1_in, int c2_in, size_t n) longword_ptr++; n -= sizeof (longword); } - } - - { - const unsigned char *char_ptr = (const unsigned char *) longword_ptr; /* At this point, we know that either n < sizeof (longword), or one of the sizeof (longword) bytes starting at char_ptr is == c1 or == c2. On @@ -149,6 +153,14 @@ memchr2 (void const *s, int c1_in, int c2_in, size_t n) from the last loop iteration. But this does not work on big-endian machines. Choose code that works in both cases. */ + s = longword_ptr; + } + +#endif + + { + unsigned char const *char_ptr = s; + for (; n > 0; --n, ++char_ptr) { if (*char_ptr == c1 || *char_ptr == c2) diff --git a/lib/rawmemchr.c b/lib/rawmemchr.c index 4eaada4774..e3cb63bd21 100644 --- a/lib/rawmemchr.c +++ b/lib/rawmemchr.c @@ -41,13 +41,21 @@ rawmemchr (const void *s, int c_in) - __builtin_cheri_offset_get (s))); # else + unsigned char c = c_in; + + /* Examine the memory an aligned longword at a time, when possible. + Doing so can go past the end of a lesser-aligned object, + which is harmless on most platforms but violates the rules of C, + so suppress this optimization on platforms where it is known to be + dangerous, namely, those using address sanitization. */ + +# ifndef __SANITIZE_ADDRESS__ + /* You can change this typedef to experiment with performance. */ typedef uintptr_t longword _GL_ATTRIBUTE_MAY_ALIAS; /* Verify that the longword type lacks padding bits. */ static_assert (UINTPTR_WIDTH == UCHAR_WIDTH * sizeof (uintptr_t)); - unsigned char c = c_in; - { const unsigned char *char_ptr; @@ -123,12 +131,6 @@ rawmemchr (const void *s, int c_in) longword_ptr++; } - s = longword_ptr; - } - - { - const unsigned char *char_ptr = s; - /* At this point, we know that one of the sizeof (longword) bytes starting at char_ptr is == c. If we knew endianness, we could determine the first such byte without any further memory @@ -136,6 +138,12 @@ rawmemchr (const void *s, int c_in) iteration. However, the following simple and portable code does not attempt this potential optimization. */ + s = longword_ptr; + } +# endif + + { + const unsigned char *char_ptr = s; while (*char_ptr != c) char_ptr++; return (void *) char_ptr; diff --git a/lib/strchrnul.c b/lib/strchrnul.c index 2e813bf580..74dd7e2aaf 100644 --- a/lib/strchrnul.c +++ b/lib/strchrnul.c @@ -23,6 +23,18 @@ char * strchrnul (const char *s, int c_in) { + unsigned char c = c_in; + if (!c) + return rawmemchr (s, 0); + + /* Examine the memory an aligned longword at a time, when possible. + Doing so can go past the end of a lesser-aligned object, + which is harmless on most platforms but violates the rules of C, + so suppress this optimization on platforms where it is known to be + dangerous, namely, those using address sanitization. */ + +#if ! (defined __SANITIZE_ADDRESS__ || defined __CHERI_PURE_CAPABILITY__) + /* On 32-bit hardware, choosing longword to be a 32-bit unsigned long instead of a 64-bit uintmax_t tends to give better performance. On 64-bit hardware, unsigned long is generally 64 @@ -30,10 +42,6 @@ strchrnul (const char *s, int c_in) performance. */ typedef unsigned long int longword _GL_ATTRIBUTE_MAY_ALIAS; - unsigned char c = (unsigned char) c_in; - if (!c) - return rawmemchr (s, 0); - const longword *longword_ptr; { @@ -121,10 +129,6 @@ strchrnul (const char *s, int c_in) break; longword_ptr++; } - } - - { - const unsigned char *char_ptr = (const unsigned char *) longword_ptr; /* At this point, we know that one of the sizeof (longword) bytes starting at char_ptr is == 0 or == c. On little-endian machines, @@ -133,6 +137,13 @@ strchrnul (const char *s, int c_in) iteration. But this does not work on big-endian machines. Choose code that works in both cases. */ + s = (char const *) longword_ptr; + } + +#endif + + { + unsigned char const *char_ptr = (unsigned char const *) s; while (*char_ptr && (*char_ptr != c)) char_ptr++; return (char *) char_ptr; -- 2.53.0