On 06/10/2025 20:51, Collin Funk wrote:
Pádraig Brady <[email protected]> writes:

* src/digest.c (sha2_sum_stream): Change from unreachable()
to affirm() so that we have defined behavior unless
we configure with --disable-assert.
[...]
  #include "assure.h"
  #include "system.h"
  #include "argmatch.h"
  #include "c-ctype.h"
@@ -300,7 +301,7 @@ sha2_sum_stream (FILE *stream, void *resstream, uintmax_t 
*length)
      case SHA512_DIGEST_SIZE:
        return sha512_stream (stream, resstream);
      default:
-      unreachable ();
+      affirm (0);
      }

Paul and I discussed this previously and both said we prefer
unreachable () [1].

You may be unaware like I was that you can make unreachable () kill the
program if reached:

     $ cat main.c
     #include <stddef.h>
     int
     main (void)
     {
       unreachable ();
     }
     $ gcc -std=gnu23 -fsanitize=undefined main.c
     $ ./a.out
     main.c:5:3: runtime error: execution reached an unreachable program point

I was going to change all the affirm (false) occurrences to
unreachable (), but decided it was best not to before the release.

Collin

[1] https://lists.gnu.org/archive/html/coreutils/2025-09/msg00112.html

Hmm.

Note the (undefined) behavior without this patch was
gcc caused sha2_sum_stream() to just return 0,
while clang jumped to somewhat arbitrary code!
I.e. using unreachable() is only appropriate if the code
definitely can not be reached, and not appropriate for
where you want to assert assumptions.
Otherwise you just get very undefined behavior.

Note -funreachable-traps had no impact on the above behavior,
I'm presuming as the functions were simple and inlined etc.
maybe even due to the unreachable().

BTW I used affirm() instead so that --disable-assert is effective
and can be used to help optimization if desired in release builds,
while the normal / dev builds will have assert protection in place.

As for -fsanitize=undefined, that's OK for devs,
but you don't get that protection for folks doing
a standard build for platform testing for example.

Testing -fsanitize=undefined with the bug just now...

We've most warnings disabled with clang, so I do get a good error:
$ echo 'SHA2-128 (/dev/null) = 38b060a751ac96384cd9327eb1b1e36a' | src/cksum 
--check
src/digest.c:303:7: runtime error: execution reached an unreachable program 
point
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/digest.c:303:7

However when I enable -fsanitize=undefined with gcc 15.2.1
I get a slew of -Werror=maybe-uninitialized.
So not very plug and play yet even for devs.

cheers,
Padraig.

Reply via email to