Idiom exclusion is really so important

Posted Nov 13, 2024 10:31 UTC (Wed) by intelfx (subscriber, #130118)
In reply to: Idiom exclusion is really so important by magfr
Parent article: Progress on toolchain security features

> I have a hand time understanding why
>
> x+y<x
>
> is considered easier to read and better than
>
> INT_MAX-y<=x

It is considered better in the eyes of the language lawyers. Which, apparently, is enough to trump any kind of readability concerns.

I'd ask a different question, though. With the amount of compiler extensions already part of "kernel C" (some of which are even in this article), I'm having a hard time understanding reasons for not adding a `__builtin_wraps(expr)` kind of thing that explicitly evaluates `expr` under `-fwrapv` rules and tests that for overflow.

Surely an `if (__builtin_wraps(a + b))` is more readable than both `if (a + b < a)` and `if (INT_MAX - b <= a)`?

to post comments

Idiom exclusion is really so important

Posted Nov 13, 2024 10:36 UTC (Wed) by pm215 (subscriber, #98099) [Link] (5 responses)

The compilers have had that for a while: see __builtin_add_overflow_p in https://gcc.gnu.org/onlinedocs/gcc/Integer-Overflow-Built...

Idiom exclusion is really so important

Posted Nov 13, 2024 10:40 UTC (Wed) by intelfx (subscriber, #130118) [Link] (4 responses)

Nope. Sorry, these builtins are a just a planetary abomination. The identity of the arithmetic operation is completely erased, making it absolutely in-obvious that it's in fact an arithmetic operation rather than a random function call.

It needs to be a `__some_builtin(expr)`, where `expr` is a **regular** arithmetic expression using **regular** + and - symbols.

Idiom exclusion is really so important

Posted Nov 13, 2024 10:50 UTC (Wed) by pm215 (subscriber, #98099) [Link] (2 responses)

I certainly wouldn't use the naked builtin directly, but that's true of most GCC builtins. But I'll absolutely take "looks like a function call and has 'add' as a word rather than '+' as a symbol, but is extremely obviously doing an overflow check" over "has '+' in the expression but is ridiculously opaque about what it is actually doing", especially when the thing it is doing is important for correctness and often for security.

No objections if you want to get nicer facilities added to the compiler, but in the interim I'll use the ones we have. And if I did have to work with a compiler where I had to use "x + y < x" I would wrap it in a function so I could give it a clearer name...

Idiom exclusion is really so important

Posted Nov 13, 2024 11:09 UTC (Wed) by intelfx (subscriber, #130118) [Link] (1 responses)

> But I'll absolutely take "looks like a function call and has 'add' as a word rather than '+' as a symbol, but is extremely obviously doing an overflow check" over "has '+' in the expression but is ridiculously opaque about what it is actually doing"

Who was saying anything about being opaque?

I just proposed a `__builtin_overflows(expr)` thing which makes it equally obvious that it is a test for overflow. **And** that it is an arithmetic expression.

All those builtins disguising arithmetic operations as function calls — no. Just no. The arithmetics are infix for a reason. This is C, not some kind of a Lisp or a reverse Forth.

Idiom exclusion is really so important

Posted Nov 13, 2024 11:20 UTC (Wed) by pm215 (subscriber, #98099) [Link]

You seemed to me to be proposing that until this suggestion of yours is implemented in compilers we should continue to open-code "if (x + y < x)". That open-coding is what I am describing as opaque.

Your idea is clearly nicer than the existing builtins, but we don't have it yet and in the best case won't have it widely available for years, so the question of what is most legible and least bug prone given current compiler facilities still matters, I think.

Idiom exclusion is really so important

Posted Nov 13, 2024 11:01 UTC (Wed) by error27 (subscriber, #8346) [Link]

Yeah. That would be nice. My idea is that it would be scope based. The compiler would trap any integer overflows and goto the label. I've left off \ characters and probably made typos but you get the idea:

#define saturate(math...) (__attribute__(overflow overflowed_label) {
size_t result;
bool overflow = false;
result = (math);
goto done;

overflowed_label:
overflow = true;
done:
overflow ? SIZE_MAX : result;
})

Idiom exclusion is really so important

Posted Nov 13, 2024 10:37 UTC (Wed) by intelfx (subscriber, #130118) [Link]

Ah, apologies, I apparently misread that. My brain thought that you just missed a "not" somewhere in there because I didn't realize someone could be asking _that_ question.

Well, `if (a + b < a)` preserves the identity of participating arithmetic operations. When you see a `if (a + b < a)`, it is immediately obvious that you are dealing with a sum of `a` and `b`, and if you are familiar with wrapping arithmetic, it's also almost immediately parsable as checking for overflow.

On the other hand, `if (INT_MAX - a <= b)` does not preserve neither the identity of the operation, nor the identity of operands. You need significant brain time to mentally carry over the operands to one side, invert the signs and realize that we are talking about `a + b`, not some weird arithmetic expression with unrelated `a`, `b` and a constant.

Personally, it's a no-brainer which one is clearer.