What tools can diagnose C++ portability issues due to plain char signedness?

user_1424877 0
Viewed 75

We recently discovered a line of code that was doing the equivalent of

bool should_escape_control_char(char ch) {
    return (ch < 0x20);  // control chars are 0x00 through 0x1F
}

This works if plain char is unsigned; but if plain char is signed, then this filter accidentally catches negative chars as well. (The ultimate effect was that a naïve JSON encoder was encoding "é" as "\u00c3\u00a9" because to the encoder, it looked like a pair of negative chars which were then individually encoded.)

IMO, the original sin here is that we're comparing a plain char expression against an integer, in such a way that the result depends on the signedness of char. I wish the compiler had told us:

fantasy-warning: this comparison's result may depend on the signedness of plain char
    return (ch < 0x20);  // control chars are 0x00 through 0x1F
            ^~~~~~~~~
fantasy-note: cast the operand to silence this diagnostic
    return (ch < 0x20);  // control chars are 0x00 through 0x1F
            ~~
            (signed char)(ch)

I was surprised to discover that Clang gives no option to warn in this situation; and I don't see any warning option in GCC either.

  • Am I just not looking in the right place?
  • What tools / linters / static-analyzers exist that do warn in this situation?
tool-rec static-analysis char c c++
2 Answers

Your code is not portable even if you changed it to

bool should_escape_control_char(unsigned char ch)

as you're still making assumptions about the character encoding on your platform. Use

int std::iscntrl( int ch );

instead, or the C equivalent depending on the language you're using.

Reference https://en.cppreference.com/w/cpp/string/byte/iscntrl

(The C version is reachable from that site).

user_23808300

Static analysers that I use didn't diagnose the original example. Writing unit tests and compiling them with both unsigned and signed char could help catch bugs like this in automated testing stage.

When using unsigned numbers, it is safer to compare them with explicitly unsigned operands instead of letting signed operand be implicitly converted. So, given that char was assumed to be unsigned:

bool should_escape_control_char(char ch) {
    return ch < 0x20u;  // control chars are 0x00 through 0x1F
//                  ^
}

In this case, if the assumed signedness of char is wrong, (at least some?) compilers would warn about this when the char is signed and warnings are enabled:

warning: comparison of integer expressions of different signedness: 'char' and 'unsigned int' [-Wsign-compare]

Instead of relying magic numbers, it would be better to use std::iscntrl from the standard library:

bool
is_control_c0(unsigned char ch) {
    return std::iscntrl(ch
        // provide locale if not using currently active
    );
}

Note that a function accepting a single narrow char - i.e. a code unit - cannot match all control code points in UTF-8 because C1 control codes are encoded as two code units.

user_20793030
Related