Undefined signed integer overflow in seed calculation

[jeffkdev]

The seed calculation uses signed integer arithmetic to transform the seed input which causes undefined integer overflow:

    for (i = 0; i < 256; i++)
        source[i] = (int16_t) i;
    seed = seed * 6364136223846793005LL + 1442695040888963407LL;
    seed = seed * 6364136223846793005LL + 1442695040888963407LL;
    seed = seed * 6364136223846793005LL + 1442695040888963407LL;
    for (i = 255; i >= 0; i--) {
        seed = seed * 6364136223846793005LL + 1442695040888963407LL;

This can be fixed by making it unsigned:

    uint64_t seedU = seed;
    for (i = 0; i < 256; i++)
        source[i] = (int16_t) i;
    seedU = seedU * 6364136223846793005ULL + 1442695040888963407LL;
    seedU = seedU * 6364136223846793005ULL + 1442695040888963407LL;
    seedU = seedU * 6364136223846793005ULL + 1442695040888963407LL;
    for (i = 255; i >= 0; i--) {
        seedU = seedU * 6364136223846793005ULL + 1442695040888963407ULL;
        r = (int)((seedU + 31) % (i + 1));

Since the existing code causes undefined behavior is this may or may not give the same result as after the fix. So maybe this change is undesirable to previous users who expect the same (yet, undefined) result as before.

Only ran into this issue since I was using this code with Zig, which treats signed integer overflow as a runtime exception while debugging. Thanks for porting this to C by the way, I used Kurt Spencer's Java implementation in my last project. I can make a PR if you would like.

[smcameron]

Thanks, a PR would be good.

Now that I think about it, doesn't gcc have some flags for finding at least some undefined behaviors?... -fsanitize=undefined. Maybe I should enable that for the tests.

[smcameron]

Hmm, my compiler is too old for fsanitize=undefined, at least on my laptop, so presumably on lots of other people's computers too, so it cannot be on by default.

[jeffkdev]

Great, made a PR. I didn't have the libpng link setup for the test in my environment, so may be worth running on your end to make sure it passes before merging. But otherwise did make sure it compiles with make, and works on my Zig project.