diff --git a/src/coding-guidelines/expressions.rst b/src/coding-guidelines/expressions.rst index 6851d74..b3b80bb 100644 --- a/src/coding-guidelines/expressions.rst +++ b/src/coding-guidelines/expressions.rst @@ -399,3 +399,144 @@ Expressions /* ... */ } + +.. guideline:: 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 + :id: gui_8BiWvapv0lUa + :category: mandatory + :status: draft + :release: 1.0.0-latest + :fls: fls_sru4wi5jomoe + :decidability: undecidable + :scope: module + :tags: numerics, surprising-behavior, defect + + In particular, the user should limit the Right Hand Side (RHS) parameter used for left shifts and right shifts (i.e. the ``<<`` and ``>>`` binary operators) to only the range ``0..=N-1``\ , where ``N`` is the number of bits of the Left Hand Side (LHS) parameter. For example, in ``a << b``\ , if ``a`` is of type ``u32``\ , then ``b`` **must belong to** the range ``0..=31``. + + This rule applies to all types which implement the ``core::ops::Shl`` and / or ``core::ops::Shr`` traits, for Rust Version greater than or equal to ``1.6.0``. + + For versions prior to ``1.6.0``\ , this rule applies to all types for which the ``<<`` and ``>>`` operators are valid. That is, it applies to the following primitive types: + + + * ``i8`` + * ``i16`` + * ``i32`` + * ``i64`` + * ``i128`` + * ``isize`` + * ``u8`` + * ``u16`` + * ``u32`` + * ``u64`` + * ``u128`` + * ``usize`` + + .. rationale:: + :id: rat_aMguLhw0ORnD + :status: draft + + This is a Defect Avoidance 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 ``x`` of type ``uN``\ , and we perform the operation + + ``x << M`` + + Then, it will behave like this: + + .. list-table:: + :header-rows: 1 + + * - **Compilation Mode** + - **\ ``0 <= M < N``\ ** + - **\ ``M < 0``\ ** + - **\ ``N <= M``\ ** + * - Debug + - Shifts normally + - Panics + - Panics + * - Release + - Shifts normally + - Shifts by ``M mod N`` + - Shifts by ``M mod N`` + + + .. + + Note: the behavior is exactly the same for the ``>>`` operator. + + + Panicking in ``Debug`` is an issue by itself, however, a perhaps larger issue there is that its behavior is different from that of ``Release``. Such inconsistencies aren't acceptable in Safety Critical scenarios. + + 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 - 1`` bits. The operation itself becomes meaningless. + + For both of these reasons, the programmer must ensure the RHS operator stays in the range ``0..=N-1``. + + .. non_compliant_example:: + :id: non_compl_ex_4YFDofvjh9eV + :status: draft + + As seen in the example below: + + + * A ``Debug`` build **panics**\ , + * + Whereas a ``Release`` build prints the values: + + .. code-block:: + + 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. + + .. code-block:: rust + + let bits : u32 = 61; + let shifts = vec![-1, 4, 40]; + + for sh in shifts { + println!("{bits} << {sh} = {}", bits << sh); + } + + .. compliant_example:: + :id: compl_ex_3BkrRwwK5zhX + :status: draft + + As seen in the example below: + + + * Both ``Debug`` and ``Release`` give the same exact output, which addresses **Reason 1**. + * Out-of-range shifts are caught and avoided before they happen. + * + The output shows what's happening: + + .. code-block:: + + Performing 61 << -1 would be meaningless and crash-prone; we avoided it! + 61 << 4 = 976 + Performing 61 << 40 would be meaningless and crash-prone; we avoided it! + + The output shows how this addresses **Reason 2**. + + .. code-block:: rust + + let bits : u32 = 61; + let shifts = vec![-1, 4, 40]; + + for sh in shifts { + if 0 <= sh && sh < 32 { + println!("{bits} << {sh} = {}", bits << sh); + } else { + println!("Performing {bits} << {sh} would be meaningless and crash-prone; we avoided it!"); + } + }