Skip to content

Commit 57b6ba6

Browse files
Add guideline for issue #174 (#181)
Co-authored-by: felix91gr <felix91gr@users.noreply.github.com> Note: tables were written by hand due to limitations in our automation Co-authored-by: felix91gr <11747623+felix91gr@users.noreply.github.com>
1 parent 01afd49 commit 57b6ba6

File tree

1 file changed

+136
-1
lines changed

1 file changed

+136
-1
lines changed

src/coding-guidelines/expressions.rst

Lines changed: 136 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,7 @@ Expressions
509509
``x << M``
510510

511511
Then, it will behave like this:
512-
512+
513513
+------------------+-----------------+-----------------------+-----------------------+
514514
| Compilation Mode | ``0 <= M < N`` | ``M < 0`` | ``N <= M`` |
515515
+==================+=================+=======================+=======================+
@@ -614,3 +614,138 @@ Expressions
614614
for sh in shifts {
615615
println!("{bits} << {sh} = {:?}", good_shl(bits, sh));
616616
}
617+
618+
619+
.. 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
620+
:id: gui_LvmzGKdsAgI5
621+
:category: mandatory
622+
:status: draft
623+
:release: 1.0.0-latest
624+
:fls: fls_sru4wi5jomoe
625+
:decidability: undecidable
626+
:scope: module
627+
:tags: numerics, surprising-behavior, defect
628+
629+
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``.
630+
631+
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``.
632+
633+
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:
634+
635+
636+
* ``i8``
637+
* ``i16``
638+
* ``i32``
639+
* ``i64``
640+
* ``i128``
641+
* ``isize``
642+
* ``u8``
643+
* ``u16``
644+
* ``u32``
645+
* ``u64``
646+
* ``u128``
647+
* ``usize``
648+
649+
.. rationale::
650+
:id: rat_tVkDl6gOqz25
651+
:status: draft
652+
653+
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 <https://wiki.sei.cmu.edu/confluence/x/ItcxBQ>`_.
654+
655+
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.
656+
657+
658+
*
659+
**Reason 1: inconsistent behavior**
660+
661+
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
662+
663+
``x << M``
664+
665+
Then, it will behave like this:
666+
667+
+------------------+-----------------+-----------------------+-----------------------+
668+
| Compilation Mode | ``0 <= M < N`` | ``M < 0`` | ``N <= M`` |
669+
+==================+=================+=======================+=======================+
670+
| Debug | Shifts normally | Panics | Panics |
671+
+------------------+-----------------+-----------------------+-----------------------+
672+
| Release | Shifts normally | Shifts by ``M mod N`` | Shifts by ``M mod N`` |
673+
+------------------+-----------------+-----------------------+-----------------------+
674+
675+
676+
..
677+
678+
Note: the behavior is exactly the same for the ``>>`` operator.
679+
680+
681+
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.
682+
683+
*
684+
**Reason 2: programmer intent**
685+
686+
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.
687+
688+
For both of these reasons, the programmer must ensure the RHS operator stays in the range ``0..=N-1``.
689+
690+
.. non_compliant_example::
691+
:id: non_compl_ex_aTtUjdIuDdbv
692+
:status: draft
693+
694+
As seen in the example below:
695+
696+
697+
* A ``Debug`` build **panics**\ ,
698+
*
699+
Whereas a ``Release`` build prints the values:
700+
701+
.. code-block::
702+
703+
61 << -1 = 2147483648
704+
61 << 4 = 976
705+
61 << 40 = 15616
706+
707+
This shows **Reason 1** prominently.
708+
709+
**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.
710+
711+
.. code-block:: rust
712+
713+
let bits : u32 = 61;
714+
let shifts = vec![-1, 4, 40];
715+
716+
for sh in shifts {
717+
println!("{bits} << {sh} = {}", bits << sh);
718+
}
719+
720+
.. compliant_example::
721+
:id: compl_ex_Ux1WqHbGKV73
722+
:status: draft
723+
724+
As seen in the example below:
725+
726+
727+
* Both ``Debug`` and ``Release`` give the same exact output, which addresses **Reason 1**.
728+
* Out-of-range shifts are caught and avoided before they happen.
729+
*
730+
The output shows what's happening:
731+
732+
.. code-block::
733+
734+
Performing 61 << -1 would be meaningless and crash-prone; we avoided it!
735+
61 << 4 = 976
736+
Performing 61 << 40 would be meaningless and crash-prone; we avoided it!
737+
738+
The output shows how this addresses **Reason 2**.
739+
740+
.. code-block:: rust
741+
742+
let bits : u32 = 61;
743+
let shifts = vec![-1, 4, 40];
744+
745+
for sh in shifts {
746+
if 0 <= sh && sh < 32 {
747+
println!("{bits} << {sh} = {}", bits << sh);
748+
} else {
749+
println!("Performing {bits} << {sh} would be meaningless and crash-prone; we avoided it!");
750+
}
751+
}

0 commit comments

Comments
 (0)