- 
                Notifications
    You must be signed in to change notification settings 
- Fork 21
Description
Chapter
Expressions
Guideline Title
Integer shift shall only be performed through checked_ APIs
Category
Required
Status
Draft
Release Begin
1.7.0
Release End
latest
FLS Paragraph ID
fls_sru4wi5jomoe
Decidability
Decidable
Scope
Module
Tags
numerics, reduce-human-error, maintainability, portability, surprising-behavior, subset
Amplification
In particular, the user should only perform left shifts via the checked_shl function and right shifts via the checked_shr function. Both of these functions exist in core.
This rule applies to the following primitive types:
- i8
- i16
- i32
- i64
- i128
- u8
- u16
- u32
- u64
- u128
- usize
- isize
Exception(s)
If, for a specific instance of a shifting operation, the user can prove that the RHS operand will always be inside the range 0..=N-1, where N is the number of bits of the LHS operand, then this Guideline does not apply to that instance.
Rationale
This is a Subset rule, directly inspired by INT34-C. Do not shift an expression by a negative number of bits or by greater than or equal to the number of bits that exist in the operand.
In Rust these out-of-range shifts don't give rise to Undefined Behavior; however, they are still problematic in Safety Critical contexts for two reasons.
- 
Reason 1: inconsistent behavior The behavior of shift operations depends on the compilation mode. Say for example, that we have a number xof typeuN, and we perform the operationx << MThen, it will behave like this: Compilation Mode 0 <= M < NM < 0N <= MDebug Shifts normally Panics Panics Release Shifts normally Shifts by M mod NShifts by M mod NNote: the behavior is exactly the same for the >>operator.Panicking in Debugis an issue by itself, however, a perhaps larger issue there is that its behavior is different from that ofRelease. Such inconsistencies aren't acceptable in Safety Critical scenarios.Therefore, a consistently-behaved operation should be required for performing shifts. 
- 
Reason 2: programmer intent There is no scenario in which it makes sense to perform a shift of negative length, or of more than N - 1bits. The operation itself becomes meaningless.Therefore, an API that restricts the length of the shift to the range [0, N - 1]should be used instead of the<<and>>operators.
- 
The Solution The ideal solution for this exists in core:checked_shlandchecked_shr.<T>::checked_shl(M)returns a value of typeOption<T>, in the following way:- If M < 0, the output isNone
- If 0 <= M < NforTofNbits, then the output isSome(T)
- If N <= M, the output isNone
 This API has consistent behavior across DebugandRelease, and makes the programmer intent explicit, which effectively solves this issue.
- If 
Non-Compliant Example - Prose
As seen in the example below:
- 
A Debugbuild panics,
- 
Whereas a Releasebuild prints the values:61 << -1 = 2147483648 61 << 4 = 976 61 << 40 = 15616
This shows Reason 1 prominently.
Reason 2 is not seen in the code, because it is a reason of programmer intent: shifts by less than 0 or by more than N - 1 (N being the bit-length of the value being shifted) are both meaningless.
Non-Compliant Example - Code
fn bad_shl(bits: u32, shift: i32) -> u32 {
    bits << shift
}
    
let bits : u32 = 61;
let shifts = vec![-1, 4, 40];
    
for sh in shifts {
    println!("{bits} << {sh} = {}", bad_shl(bits, sh));
}Compliant Example - Prose
As seen in the example below:
- Both DebugandReleasegive the same exact output, which addresses Reason 1.
- Shifting by negative values is impossible due to the fact that checked_shlonly accepts unsigned integers as shift lengths.
- Shifting by more than N - 1(Nbeing the bit-length of the value being shifted) returns aNonevalue:61 << 4 = Some(976) 61 << 40 = None
The last 2 observations show how this addresses Reason 2.
Compliant Example - Code
fn good_shl(bits: u32, shift: u32) -> Option<u32> {
    bits.checked_shl(shift)
}
    
let bits : u32 = 61;
// let shifts = vec![-1, 4, 40];
//                    ^--- Would not typecheck, as checked_shl
//                         only accepts positive shift amounts
let shifts = vec![4, 40];
    
for sh in shifts {
    println!("{bits} << {sh} = {:?}", good_shl(bits, sh));
}Metadata
Metadata
Assignees
Labels
Type
Projects
Status