KSPP / linux

Linux kernel source tree (Kernel Self Protection Project)
https://kernsec.org/wiki/index.php/Kernel_Self_Protection_Project
Other
81 stars 5 forks source link

Evaluate what it would take to build the kernel with -Wconversion #175

Open kees opened 2 years ago

kees commented 2 years ago

There is so much implicit type conversion with int, I suspect -Wcoversion is far from reach.

kees commented 2 years ago

Evaluate -Wshorten-64-to-32 and -Wnarrowing

keith-packard commented 2 years ago

Because all C expressions are evaluated with types no smaller than int (or unsigned), there is always implicit conversion from any smaller type. For instance:

#include <stdio.h>

int
main(void)
{
    unsigned short  a = 12;

    a = (a << 8) | (a >> 8);
    return 0;
}

Naïvely, this byte swapping operation should not involve conversion between unsigned short and int, and yet in C, (a << 8) and (a >> 8) both require converting a to an int before shifting, so the assignment is from 'int' to 'unsigned short'. GCC and Clang both emit warnings in this case, when -Wconversion is enabled. In the GCC man page, -Wconversion says:

Warnings about conversion from arithmetic on a small type back tothat type are only given with -Warith-conversion.

That led me to think that there was an intent of making the above code not elicit a warning. And, in fact, if you carefully limit your code to expressions containing only a single operation, you can write the above as:

#include <stdio.h>

int
main(void)
{
    unsigned short a, b, c;

    a = 12;
    b = a << 8;
    c = a >> 8;
    a = b | c;
    return a;
}

This generates no warnings from gcc, but does still generate a warning from clang (on the statement with <<).

It gets a bit more confusing though; assignment operators are a mixed bag with arithmetic silent and logic generating warnings:

#include <stdio.h>

int
main(void)
{
    unsigned short a, b;

    a = 12;
    b = 7;

    a |= b & 1;
    a += b & 1;
    return a;
}

This generates a warning for a |= b & 1 but not for a += b & 1. Given that a += b & 1 could overflow, while a |= b & 1 could not, this is troublesome as we might actually want a warning about potential arithmetic overflow (although, there are so many places where that could happen...).

I've taken a smaller project and added -Werror=conversion to the compiler flags. This embedded operating system is about 180kloc and I had to add casts in about 1000 places. This caught about a dozen bugs still lurking after a recent change from 16 to 32 bit time values (don't ask). I think a lot of the casts which do bit manipulation on small types would not have been needed if GCC allowed more complex expressions involving small types to pass through the -Wconversion net.