Branch: refs/heads/master
  Home:   https://github.com/tianocore/edk2
  Commit: 54c7728a04658b09ea1a50b9e35e838fde166003
      
https://github.com/tianocore/edk2/commit/54c7728a04658b09ea1a50b9e35e838fde166003
  Author: Laszlo Ersek <ler...@redhat.com>
  Date:   2018-02-21 (Wed, 21 Feb 2018)

  Changed paths:
    M MdePkg/Library/BaseSafeIntLib/SafeIntLib.c

  Log Message:
  -----------
  MdePkg/BaseSafeIntLib: fix undefined behavior in SafeInt64Sub()

The subtraction in the assignment

  SignedResult = Minuend - Subtrahend;

is performed with unchecked INT64 operands. According to ISO C, if the
mathematical result of signed integer subtraction cannot be represented in
the result type, the behavior is undefined. (Refer to ISO C99 6.5p5.
6.2.5p9 only exempts unsigned integers, and 6.3.1.3p3 does not apply
because it treats the conversion of integers that have been successfully
evaluated first.)

Replace the after-the-fact result checking with checks on the operands,
and only perform the subtraction if it is safe.

Cc: Bret Barkelew <bret.barke...@microsoft.com>
Cc: Liming Gao <liming....@intel.com>
Cc: Michael D Kinney <michael.d.kin...@intel.com>
Cc: Sean Brogan <sean.bro...@microsoft.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <ler...@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
Tested-by: Michael D Kinney <michael.d.kin...@intel.com>


  Commit: 41bfaffd13094d9042110091e6c37adf20c4032c
      
https://github.com/tianocore/edk2/commit/41bfaffd13094d9042110091e6c37adf20c4032c
  Author: Laszlo Ersek <ler...@redhat.com>
  Date:   2018-02-21 (Wed, 21 Feb 2018)

  Changed paths:
    M MdePkg/Library/BaseSafeIntLib/SafeIntLib.c

  Log Message:
  -----------
  MdePkg/BaseSafeIntLib: fix undefined behavior in SafeInt64Add()

The addition in the assignment

  SignedResult = Augend + Addend;

is performed with unchecked INT64 operands. According to ISO C, if the
mathematical result of signed integer addition cannot be represented in
the result type, the behavior is undefined. (Refer to ISO C99 6.5p5.
6.2.5p9 only exempts unsigned integers, and 6.3.1.3p3 does not apply
because it treats the conversion of integers that have been successfully
evaluated first.)

Replace the after-the-fact result checking with checks on the operands,
and only perform the addition if it is safe.

Cc: Bret Barkelew <bret.barke...@microsoft.com>
Cc: Liming Gao <liming....@intel.com>
Cc: Michael D Kinney <michael.d.kin...@intel.com>
Cc: Sean Brogan <sean.bro...@microsoft.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <ler...@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
Tested-by: Michael D Kinney <michael.d.kin...@intel.com>


  Commit: 8c33cc0ec926b17396fcd71686c727b991984d4a
      
https://github.com/tianocore/edk2/commit/8c33cc0ec926b17396fcd71686c727b991984d4a
  Author: Laszlo Ersek <ler...@redhat.com>
  Date:   2018-02-21 (Wed, 21 Feb 2018)

  Changed paths:
    M MdePkg/Library/BaseSafeIntLib/SafeIntLib.c

  Log Message:
  -----------
  MdePkg/BaseSafeIntLib: clean up parentheses in MIN_INT64_MAGNITUDE

The definition of the MIN_INT64_MAGNITUDE macro is correct, but it's
harder to read than necessary: the sub-expression

      (( (UINT64) - (MIN_INT64 + 1) ))

is doubly parenthesized. Reusing one pair of the outer parens, rewrite the
sub-expression (without change in meaning) so that the minus sign cannot
be mistaken for subtraction:

      ( (UINT64)(- (MIN_INT64 + 1)) )

The resultant macro definition matches the following expressions in
SafeInt64Mult():

>     //
>     // Avoid negating the most negative number.
>     //
>     UnsignedMultiplicand = ((UINT64)(- (Multiplicand + 1))) + 1;

and

>     //
>     // Avoid negating the most negative number.
>     //
>     UnsignedMultiplier = ((UINT64)(- (Multiplier + 1))) + 1;

Cc: Bret Barkelew <bret.barke...@microsoft.com>
Cc: Liming Gao <liming....@intel.com>
Cc: Michael D Kinney <michael.d.kin...@intel.com>
Cc: Sean Brogan <sean.bro...@microsoft.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <ler...@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
Tested-by: Michael D Kinney <michael.d.kin...@intel.com>


  Commit: 75505d161133cbeb57185b567d7b05756d255a3f
      
https://github.com/tianocore/edk2/commit/75505d161133cbeb57185b567d7b05756d255a3f
  Author: Laszlo Ersek <ler...@redhat.com>
  Date:   2018-02-21 (Wed, 21 Feb 2018)

  Changed paths:
    M MdePkg/Library/BaseSafeIntLib/SafeIntLib.c

  Log Message:
  -----------
  MdePkg/BaseSafeIntLib: fix undefined behavior in SafeInt64Mult()

If we have to negate UnsignedResult (due to exactly one of Multiplicand
and Multiplier being negative), and UnsignedResult is exactly
MIN_INT64_MAGNITUDE (value 2^63), then the statement
   *Result = - ((INT64)UnsignedResult);

invokes both implementation-defined behavior and undefined behavior.

First, MIN_INT64_MAGNITUDE is not representable as INT64, therefore the
result of the (inner) conversion

  (INT64)MIN_INT64_MAGNITUDE

is implementation-defined, or an implementation-defined signal is raised,
according to ISO C99 6.3.1.3p3.

Second, if we assume that the C language implementation defines the
conversion to INT64 simply as reinterpreting the bit pattern
0x8000_0000_0000_0000 as a signed integer in two's complement
representation, then the conversion immediately produces the negative
value MIN_INT64 (value -(2^63)). In turn, the (outer) negation

  -(MIN_INT64)

invokes undefined behavior, because the mathematical result of the
negation, namely 2^63, cannot be represented in an INT64 object. (Not even
mentioning the fact that the mathematical result would be incorrect.) In
practice, the undefined negation of MIN_INT64 happens to produce an
unchanged, valid-looking result on x86, i.e. (-(MIN_INT64)) == MIN_INT64.

We can summarize this as the undefined -- effectless -- negation canceling
out the botched -- auto-negating -- implementation-defined conversion.
Instead of relying on such behavior, dedicate a branch to this situation:
assign MIN_INT64 directly. The branch can be triggered e.g. by multiplying
(2^62) by (-2).

Cc: Bret Barkelew <bret.barke...@microsoft.com>
Cc: Liming Gao <liming....@intel.com>
Cc: Michael D Kinney <michael.d.kin...@intel.com>
Cc: Sean Brogan <sean.bro...@microsoft.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <ler...@redhat.com>
Reviewed-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
Tested-by: Michael D Kinney <michael.d.kin...@intel.com>


Compare: https://github.com/tianocore/edk2/compare/44e6186eeadf...75505d161133
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
edk2-commits mailing list
edk2-commits@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-commits

Reply via email to