On 06/10/2025 23:13, Collin Funk wrote:
Pádraig Brady <[email protected]> writes:
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.
I guess there is benefit to not having to enable sanitizers which slow
things down too much for normal use.
Can we write 'assume (false)' instead of 'assume (0)', though?
Yes, that's better.
$ git ls-files | grep '\.[ch]' | xargs grep -F 'affirm (false);' | wc -l
9
$ git ls-files | grep '\.[ch]' | xargs grep -F 'affirm (0);'
src/digest.c: affirm (0);
src/digest.c: affirm (0);
Not sure it deserves a syntax check, but it wouldn't be too difficult.
I didn't add a syntax check as I expect we'll be revisiting this soon.
I've pushed this patch set now.
thanks for the review,
Padraig
