makefiles: Add -fwrapv to CFLAGS

[maribu]

Contribution description

This commit makes overflow of signed integers to behave as expected by at 90%
of the C developers, even though overflow of signed integers are strictly
undefined behavior.

Note: Please do not add code relying on a specific behavior for the overflow of
signed integers, even though -fwrpav will make that code work. This is
intended to mitigate the risk of bugs in overflow checks being exploited,
not to encourage adding new bugs.

See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=30475 for details and see
http://c-faq.com/misc/intovf.html on how to implement overflow checks properly.

Testing procedure

I guess if Murdock still compiles everything, this should be sufficient

Issues/PRs references

This issue was discussed in #10740, but that PR is not much related otherwise

[kaspar030]

How about enabling all the warnings there are (-Wstrict-overflow)?

[maribu]

@kaspar030: I agree that addressing the problem (fixing the code that relies on undefined behavior) instead of putting duct tape around the problem (making code relying on undefined behavior work as intended instead of fixing the bug) would be much better. And making the compiler warn about those bugs would be the perfect tool for that.

The problem is when I use this buggy code:

#include <stdio.h>
#include <limits.h>

int main(void)
{
    int a = INT_MAX;
    if (a + 1 < a) { /* <-- Undefined behavior here */
        puts("a + 1 < a!");
    }

    printf("a + 1 = %d, a = %d\n", a + 1, a);
    return 0;
}

The "expected" (expected != correct here) output would be:

a + 1 < a!
a + 1 = -2147483648, a = 2147483647

But when I compile and run I get with GCC 8.2.0:

$ gcc -Wstrict-overflow -Wall -Wextra -std=c99 -pedantic -o test test.c && ./test
a + 1 = -2147483648, a = 2147483647

clang does compile and even produces the expected output in this case, but does not give a warning either. (But beware: clang does also optimize checks out, e.g. the assert in http://ptrace.fefe.de/int.c)

So -Wstrict-overflow does not detect this issue :-(

[fjmolinas]

How about enabling all the warnings there are (-Wstrict-overflow)?

Should we do both then?

[fjmolinas]

@kaspar030 are you fine with the change now?

[kaspar030]

ACK.

[kaspar030]

please squash!

[fjmolinas]

All failures seem to be msp430 related, I tested that it works in #12457. I would for now simply add a:

ifneq (,$(filter arch_msp430,$(FEATURES_USED)))
...
endif

Around both CFLAGS, maybe add a comment referencing #12457

[kaspar030]

All failures seem to be msp430 related, I tested that it works in #12457.

I'll rebase and revert that commit in #12457 once this is merged.

[fjmolinas]

@maribu why did you close? If I remember there was a CI issue last time?

[maribu]

Sorry, I thought this was already addressed elsewhere.

[fjmolinas]

All green, ACK!

[fjmolinas]

@maribu seems like it needs a rebase for all the checks.

[fjmolinas]

There is a test failing, but it's on native and related to timing. I would think the cause is server load https://ci.riot-os.org/RIOT-OS/RIOT/10748/01382dc4ad1d5f7448755a01daf28404dfdb1406/output/compile/tests/posix_semaphore/native:gnu.txt, I can't reproduce locally.

[maribu]

There is a test failing, but it's on native and related to timing. I would think the cause is server load https://ci.riot-os.org/RIOT-OS/RIOT/10748/01382dc4ad1d5f7448755a01daf28404dfdb1406/output/compile/tests/posix_semaphore/native:gnu.txt, I can't reproduce locally.

The same worker also showed another similar issue in another PR: #15760 (comment) - which couldn't be reproduced either by CI or locally.

[maribu]

Thanks. Good to have this in :-)