summaryrefslogtreecommitdiffstats
path: root/MdePkg/Library/BaseSafeIntLib/SafeIntLib.c
diff options
context:
space:
mode:
authorLaszlo Ersek <lersek@redhat.com>2018-02-15 17:09:43 +0100
committerStar Zeng <star.zeng@intel.com>2018-07-06 17:26:16 +0800
commit932366c122baa9e19a0b507261a660fac15e6a59 (patch)
tree699076af676297b1e711c343f913ef271d893ad1 /MdePkg/Library/BaseSafeIntLib/SafeIntLib.c
parent108136f2637ce9376d00ce8f11326ae40297fee0 (diff)
downloadedk2-932366c122baa9e19a0b507261a660fac15e6a59.tar.gz
edk2-932366c122baa9e19a0b507261a660fac15e6a59.tar.bz2
edk2-932366c122baa9e19a0b507261a660fac15e6a59.zip
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.Barkelew@microsoft.com> Cc: Liming Gao <liming.gao@intel.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Sean Brogan <sean.brogan@microsoft.com> Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Tested-by: Michael D Kinney <michael.d.kinney@intel.com> (cherry picked from commit 75505d161133cbeb57185b567d7b05756d255a3f)
Diffstat (limited to 'MdePkg/Library/BaseSafeIntLib/SafeIntLib.c')
-rw-r--r--MdePkg/Library/BaseSafeIntLib/SafeIntLib.c2
1 files changed, 2 insertions, 0 deletions
diff --git a/MdePkg/Library/BaseSafeIntLib/SafeIntLib.c b/MdePkg/Library/BaseSafeIntLib/SafeIntLib.c
index de91ffeca2..c5f13d7e08 100644
--- a/MdePkg/Library/BaseSafeIntLib/SafeIntLib.c
+++ b/MdePkg/Library/BaseSafeIntLib/SafeIntLib.c
@@ -4143,6 +4143,8 @@ SafeInt64Mult (
if (UnsignedResult > MIN_INT64_MAGNITUDE) {
*Result = INT64_ERROR;
Status = RETURN_BUFFER_TOO_SMALL;
+ } else if (UnsignedResult == MIN_INT64_MAGNITUDE) {
+ *Result = MIN_INT64;
} else {
*Result = - ((INT64)UnsignedResult);
}