Re: undefined-behavior obstack.c:139

[Bruno Haible]

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

ubsan.diff
Description: Text Data