Skip to content

Commit 90a32a9

Browse files
felix91grgithub-actions[bot]
authored andcommitted
Add guideline for issue #174
Co-authored-by: felix91gr <[email protected]>
1 parent 9bb143a commit 90a32a9

File tree

1 file changed

+130
-0
lines changed

1 file changed

+130
-0
lines changed

src/coding-guidelines/expressions.rst

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -463,3 +463,133 @@ Expressions
463463
/* ... */
464464
}
465465
466+
467+
.. 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
468+
:id: gui_LvmzGKdsAgI5
469+
:category: mandatory
470+
:status: draft
471+
:release: 1.0.0-latest
472+
:fls: fls_sru4wi5jomoe
473+
:decidability: undecidable
474+
:scope: module
475+
:tags: numerics, surprising-behavior, defect
476+
477+
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``.
478+
479+
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``.
480+
481+
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:
482+
483+
484+
* ``i8``
485+
* ``i16``
486+
* ``i32``
487+
* ``i64``
488+
* ``i128``
489+
* ``isize``
490+
* ``u8``
491+
* ``u16``
492+
* ``u32``
493+
* ``u64``
494+
* ``u128``
495+
* ``usize``
496+
497+
.. rationale::
498+
:id: rat_tVkDl6gOqz25
499+
:status: draft
500+
501+
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>`_.
502+
503+
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.
504+
505+
506+
*
507+
**Reason 1: inconsistent behavior**
508+
509+
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
510+
511+
``x << M``
512+
513+
Then, it will behave like this:
514+
515+
| **Compilation Mode** | **\ ``0 <= M < N``\ ** | **\ ``M < 0``\ ** | **\ ``N <= M``\ ** |
516+
|:--------------------:|:----------------:|:---------------------:|:-------------------:|
517+
| Debug | Shifts normally | Panics | Panics |
518+
| Release | Shifts normally | Shifts by ``M mod N`` | Shifts by ``M mod N`` |
519+
520+
..
521+
522+
Note: the behavior is exactly the same for the ``>>`` operator.
523+
524+
525+
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.
526+
527+
*
528+
**Reason 2: programmer intent**
529+
530+
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.
531+
532+
For both of these reasons, the programmer must ensure the RHS operator stays in the range ``0..=N-1``.
533+
534+
.. non_compliant_example::
535+
:id: non_compl_ex_aTtUjdIuDdbv
536+
:status: draft
537+
538+
As seen in the example below:
539+
540+
541+
* A ``Debug`` build **panics**\ ,
542+
*
543+
Whereas a ``Release`` build prints the values:
544+
545+
.. code-block::
546+
547+
61 << -1 = 2147483648
548+
61 << 4 = 976
549+
61 << 40 = 15616
550+
551+
This shows **Reason 1** prominently.
552+
553+
**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.
554+
555+
.. code-block:: rust
556+
557+
let bits : u32 = 61;
558+
let shifts = vec![-1, 4, 40];
559+
560+
for sh in shifts {
561+
println!("{bits} << {sh} = {}", bits << sh);
562+
}
563+
564+
.. compliant_example::
565+
:id: compl_ex_Ux1WqHbGKV73
566+
:status: draft
567+
568+
As seen in the example below:
569+
570+
571+
* Both ``Debug`` and ``Release`` give the same exact output, which addresses **Reason 1**.
572+
* Out-of-range shifts are caught and avoided before they happen.
573+
*
574+
The output shows what's happening:
575+
576+
.. code-block::
577+
578+
Performing 61 << -1 would be meaningless and crash-prone; we avoided it!
579+
61 << 4 = 976
580+
Performing 61 << 40 would be meaningless and crash-prone; we avoided it!
581+
582+
The output shows how this addresses **Reason 2**.
583+
584+
.. code-block:: rust
585+
586+
let bits : u32 = 61;
587+
let shifts = vec![-1, 4, 40];
588+
589+
for sh in shifts {
590+
if 0 <= sh && sh < 32 {
591+
println!("{bits} << {sh} = {}", bits << sh);
592+
} else {
593+
println!("Performing {bits} << {sh} would be meaningless and crash-prone; we avoided it!");
594+
}
595+
}

0 commit comments

Comments
 (0)