Paul Eggert wrote:
> > On the contrary, I will try to find and eliminate such alarms.
>
> Please don't complicate and/or slow down shared Gnulib code just to
> pacify this particular false alarm from Clang. The obstack fix was fine,
> because it made obstack clearer and no slower. But we shouldn't have to
> insert unnecessary "is the size zero?" checks merely to pacify the false
> alarms.
Building a gnulib testdir with these settings:
CC="clang
-fsanitize=address,undefined,signed-integer-overflow,shift,integer-divide-by-zero"
produced no relevant findings (other than 'argp' things).
But building coreutils with these settings gives two findings:
lib/linebuffer.c:65:22: runtime error: applying zero offset to null pointer
lib/mpsort.c:155:34: runtime error: applying zero offset to null pointer
In both cases, the ability to add NULL + 0 unifies code paths for
empty arrays and non-empty arrays. Such unified code paths are desirable:
they make the code look simpler.
Marc Nieper-Wißkirchen wrote:
> I would suggest writing macros that encapsulate code like "NULL + 0" and
> replace the code with macro invocations. On legacy systems, the macro
> would expand into the legacy code; on modern systems where checks like "is
> this zero size?" should be eliminated by the compiler, the macro would
> expand into the correct ISO C code.
That's an interesting idea. It applies to both lib/linebuffer.c and
lib/mpsort.c.
Paul, is the attached patch OK with you? It allows coreutils to build and
"make check" with clang's ASAN+UBSAN, without taking out some categories
of UBSAN.
The downside is an additional macro. Which I could move to a separate
include file.
The macro does not produce slower code:
- clang optimizes the conditional expression nicely.
- With gcc, which does not yet optimize the conditional expression well,
we continue to use the simple but not ISO C compliant code.
It is quite possible that newer GCCs will, in the future, trigger the same
runtime error: applying zero offset to null pointer
as clang does. For this reason, I would report a GCC bug, asking them to
optimize
((n) ? (array) + (n) : (array))
into identical code as
((array) + (n))
Bruno
diff --git a/lib/linebuffer.c b/lib/linebuffer.c
index 4124bbcb37..9c8e12ab32 100644
--- a/lib/linebuffer.c
+++ b/lib/linebuffer.c
@@ -31,6 +31,19 @@
# include "unlocked-io.h"
#endif
+/* END_OF_ARRAY(array,n) returns a pointer past the last element of the array
+ ARRAY that has N elements.
+ For clang, it uses a conditional expression that avoids adding 0 to a null
+ pointer, which is undefined according to ISO C 23 § 6.5.6.(9) and which
+ triggers an error in clang's undefined-behaviour sanitizer. When no
+ sanitizer is in effect, clang optimizes this conditional expression to
+ exactly the same code. */
+#if defined __clang__
+# define END_OF_ARRAY(array,n) ((n) ? (array) + (n) : (array))
+#else
+# define END_OF_ARRAY(array,n) ((array) + (n))
+#endif
+
/* Initialize linebuffer LINEBUFFER for use. */
void
@@ -62,7 +75,7 @@ readlinebuffer_delim (struct linebuffer *linebuffer, FILE *stream,
int c;
char *buffer = linebuffer->buffer;
char *p = linebuffer->buffer;
- char *end = buffer + linebuffer->size; /* Sentinel. */
+ char *end = END_OF_ARRAY (buffer, linebuffer->size); /* Sentinel. */
if (feof (stream))
return NULL;
diff --git a/lib/mpsort.c b/lib/mpsort.c
index 32cc5e1f82..2741d7199d 100644
--- a/lib/mpsort.c
+++ b/lib/mpsort.c
@@ -23,6 +23,19 @@
#include <string.h>
+/* END_OF_ARRAY(array,n) returns a pointer past the last element of the array
+ ARRAY that has N elements.
+ For clang, it uses a conditional expression that avoids adding 0 to a null
+ pointer, which is undefined according to ISO C 23 § 6.5.6.(9) and which
+ triggers an error in clang's undefined-behaviour sanitizer. When no
+ sanitizer is in effect, clang optimizes this conditional expression to
+ exactly the same code. */
+#if defined __clang__
+# define END_OF_ARRAY(array,n) ((n) ? (array) + (n) : (array))
+#else
+# define END_OF_ARRAY(array,n) ((array) + (n))
+#endif
+
/* The type of qsort-style comparison functions. */
typedef int (*comparison_function) (void const *, void const *);
@@ -152,5 +165,5 @@ mpsort_with_tmp (void const **restrict base, size_t n,
void
mpsort (void const **base, size_t n, comparison_function cmp)
{
- mpsort_with_tmp (base, n, base + n, cmp);
+ mpsort_with_tmp (base, n, END_OF_ARRAY (base, n), cmp);
}
