hsutter / cppfront

A personal experimental C++ Syntax 2 -> Syntax 1 compiler
Other
5.52k stars 246 forks source link

[SUGGESTION] Defined behavior for shifting with oversized or negative values #755

Open SRNissen opened 1 year ago

SRNissen commented 1 year ago

Problem - undefined behavior in shifting

https://en.cppreference.com/w/cpp/language/operator_arithmetic

In any case, if the value of the right operand is negative or is greater or equal to the number of bits in the promoted left operand, the behavior is undefined.

Proposed solution: Define all shifts

Define all logical and arithmetic shifts.

Solves real vulnerabilities

Those were the issues found on the first page of of NVD when searching for "shift."

https://nvd.nist.gov/vuln/search/results?form_type=Basic&results_type=overview&query=shift&search_type=all&isCpeNameSearch=false

Notice that they are, each of them, from year 202X. This is not an old irrelevant problem.

Replaces real guidance

MISRA 12.7 and 12.8

Paraphrased:

There are probably more but I do not have access to a lot of guidance texts.

Considered alternatives:

Rely on compilers

If the underlying compilers get good at warning on this, that might be a decent solution to the issue.

However: Compilers have had years to implement warnings on potentially UB shifts and they don.t

https://godbolt.org/z/83461WW7P

In this scenario it is obvious that the shift is done with an unsanitized input value, and yet:

Lift warning/error to to CppFront

Rather than relying on the underlying compilers, Cppfront could refuse to compile and/or warn on shifts with unclamped values. This would make the performance loss from clamping explicit in the code at the call site

I suspect the implementation time spent creating a warning on unclamped values is probably higher than inserting an unconditioanl call to std::clamp before every shift.

Implementation

For a simple solution, this can be done by inserting an unconditional std::clamp before each shift.

Alternatively, a slightly more complex behavior could be implemented where e.g. (1 >> -2) is equivalent to (1 << 2).

(I don't personally know the use case for that but you could convince me that it exists.)

More detailed look at the simple clamp scenario

Performance impact

In otherwise-defined cases, doing an unconditional std::clamp is slower than shifting with the raw value

With the attribute [[assume]], situations where the performance loss is unacceptable can be reverted to just-as-performant by telling the compiler to assume no clamping is required to make the shift defined.

JohelEGP commented 1 year ago

Related discussion: #739.