fix: ubsan signed overflow violations

[q66]

In these instances, the uint8_t inputs multiplied with the literal, which is a signed long, will make a signed long. For certain inputs, this will overflow, despite the return type being unsigned. Fix by making it always unsigned in the first place.

I found this because a user in my distribution (which builds all prod packages with a subset of ubsan by default) reported crashes newly introduced in newest nodejs, which apparently brings ada as a dependency. While the branch used by nodejs appears to be different, just putting this here anyway.

[anonrig]

Thank you for your contribution. I'm leaving this unmerged for @lemire to review as well.

[anonrig]

Can you also update ada/idna repository with the relevant changes as well?

[q66]

yes, i can do that once we've finished this one

[anonrig]

@q66 Can you provide a minimum reproduction so we can add it to our testing CI and not regress in the future?

[q66]

you can reproduce this with something like:

#include <cstdio>

int main() {
    auto broadcast = [](unsigned char v) -> unsigned long long {
        return 0x101010101010101 * v;
    };
    printf("%llu\n", broadcast(0xFF));
}

compiled with -fsanitize=undefined:

$ clang++ test.cc -o test -fsanitize=undefined
$ ./test
test.cc:5:34: runtime error: signed integer overflow: 72340172838076673 * 255 cannot be represented in type 'long'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior test.cc:5:34 in 
18446744073709551615

It will generally fail for any value that does not fit within basic 7-bit ASCII (i.e. 128 and up).

I'm not sure what the best strategy for integration into test suite would be.

[q66]

(note that the functions that take a uint64_t should be unaffected, as no promotion takes place; with uint8_t the multiplication with a signed literal will yield a signed value again, but that is not the case if you already have a uint64_t operand on one side of the expression)

[lemire]

See #348