diff options
| author | Ehud Katz <ehudkatz@gmail.com> | 2019-12-04 11:57:38 +0200 |
|---|---|---|
| committer | Ehud Katz <ehudkatz@gmail.com> | 2019-12-04 12:02:04 +0200 |
| commit | 2b6b8cb10c870d64f7cc29d21fc27cef3c7e0056 (patch) | |
| tree | 1b70dcf6b8d367eb44137170e94107a0b6d9f910 | |
| parent | 1351672eedbf9716def4fc789d9046c481c7cd25 (diff) | |
| download | bcm5719-llvm-2b6b8cb10c870d64f7cc29d21fc27cef3c7e0056.tar.gz bcm5719-llvm-2b6b8cb10c870d64f7cc29d21fc27cef3c7e0056.zip | |
[APFloat] Prevent construction of APFloat with Semantics and FP value
Constructor invocations such as `APFloat(APFloat::IEEEdouble(), 0.0)`
may seem like they accept a FP (floating point) value, but the overload
they reach is actually the `integerPart` one, not a `float` or `double`
overload (which only exists when `fltSemantics` isn't passed).
This may lead to possible loss of data, by the conversion from `float`
or `double` to `integerPart`.
To prevent future mistakes, a new constructor overload, which accepts
any FP value and marked with `delete`, to prevent its usage.
Fixes PR34095.
Differential Revision: https://reviews.llvm.org/D70425
| -rw-r--r-- | llvm/include/llvm/ADT/APFloat.h | 3 | ||||
| -rw-r--r-- | llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp | 2 | ||||
| -rw-r--r-- | llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp | 2 | ||||
| -rw-r--r-- | llvm/unittests/ADT/APFloatTest.cpp | 26 |
4 files changed, 18 insertions, 15 deletions
diff --git a/llvm/include/llvm/ADT/APFloat.h b/llvm/include/llvm/ADT/APFloat.h index 70fc19e82b3..afeed67e3f9 100644 --- a/llvm/include/llvm/ADT/APFloat.h +++ b/llvm/include/llvm/ADT/APFloat.h @@ -851,6 +851,9 @@ public: APFloat(const fltSemantics &Semantics) : U(Semantics) {} APFloat(const fltSemantics &Semantics, StringRef S); APFloat(const fltSemantics &Semantics, integerPart I) : U(Semantics, I) {} + template <typename T, typename = typename std::enable_if< + std::is_floating_point<T>::value>::type> + APFloat(const fltSemantics &Semantics, T V) = delete; // TODO: Remove this constructor. This isn't faster than the first one. APFloat(const fltSemantics &Semantics, uninitializedTag) : U(Semantics, uninitialized) {} diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp index 157885e0310..92e9a8814f8 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp @@ -3386,7 +3386,7 @@ Instruction *InstCombiner::visitCallInst(CallInst &CI) { if (const ConstantFP *C = dyn_cast<ConstantFP>(Src)) { const APFloat &ArgVal = C->getValueAPF(); - APFloat Val(ArgVal.getSemantics(), 1.0); + APFloat Val(ArgVal.getSemantics(), 1); APFloat::opStatus Status = Val.divide(ArgVal, APFloat::rmNearestTiesToEven); // Only do this if it was exact and therefore not dependent on the diff --git a/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp b/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp index ef2af01f8af..44513b1f682 100644 --- a/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp +++ b/llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp @@ -1735,7 +1735,7 @@ Value *LibCallSimplifier::optimizePow(CallInst *Pow, IRBuilder<> &B) { // TODO: This whole transformation should be backend specific (e.g. some // backends might prefer libcalls or the limit for the exponent might // be different) and it should also consider optimizing for size. - APFloat LimF(ExpoF->getSemantics(), 33.0), + APFloat LimF(ExpoF->getSemantics(), 33), ExpoA(abs(*ExpoF)); if (ExpoA.compare(LimF) == APFloat::cmpLessThan) { // This transformation applies to integer or integer+0.5 exponents only. diff --git a/llvm/unittests/ADT/APFloatTest.cpp b/llvm/unittests/ADT/APFloatTest.cpp index 656945b325e..518b207e23e 100644 --- a/llvm/unittests/ADT/APFloatTest.cpp +++ b/llvm/unittests/ADT/APFloatTest.cpp @@ -520,9 +520,9 @@ TEST(APFloatTest, FMA) { // Test x87 extended precision case from http://llvm.org/PR20728. { - APFloat M1(APFloat::x87DoubleExtended(), 1.0); - APFloat M2(APFloat::x87DoubleExtended(), 1.0); - APFloat A(APFloat::x87DoubleExtended(), 3.0); + APFloat M1(APFloat::x87DoubleExtended(), 1); + APFloat M2(APFloat::x87DoubleExtended(), 1); + APFloat A(APFloat::x87DoubleExtended(), 3); bool losesInfo = false; M1.fusedMultiplyAdd(M1, A, APFloat::rmNearestTiesToEven); @@ -600,9 +600,9 @@ TEST(APFloatTest, Denormal) { { const char *MinNormalStr = "1.17549435082228750797e-38"; EXPECT_FALSE(APFloat(APFloat::IEEEsingle(), MinNormalStr).isDenormal()); - EXPECT_FALSE(APFloat(APFloat::IEEEsingle(), 0.0).isDenormal()); + EXPECT_FALSE(APFloat(APFloat::IEEEsingle(), 0).isDenormal()); - APFloat Val2(APFloat::IEEEsingle(), 2.0e0); + APFloat Val2(APFloat::IEEEsingle(), 2); APFloat T(APFloat::IEEEsingle(), MinNormalStr); T.divide(Val2, rdmd); EXPECT_TRUE(T.isDenormal()); @@ -612,9 +612,9 @@ TEST(APFloatTest, Denormal) { { const char *MinNormalStr = "2.22507385850720138309e-308"; EXPECT_FALSE(APFloat(APFloat::IEEEdouble(), MinNormalStr).isDenormal()); - EXPECT_FALSE(APFloat(APFloat::IEEEdouble(), 0.0).isDenormal()); + EXPECT_FALSE(APFloat(APFloat::IEEEdouble(), 0).isDenormal()); - APFloat Val2(APFloat::IEEEdouble(), 2.0e0); + APFloat Val2(APFloat::IEEEdouble(), 2); APFloat T(APFloat::IEEEdouble(), MinNormalStr); T.divide(Val2, rdmd); EXPECT_TRUE(T.isDenormal()); @@ -624,9 +624,9 @@ TEST(APFloatTest, Denormal) { { const char *MinNormalStr = "3.36210314311209350626e-4932"; EXPECT_FALSE(APFloat(APFloat::x87DoubleExtended(), MinNormalStr).isDenormal()); - EXPECT_FALSE(APFloat(APFloat::x87DoubleExtended(), 0.0).isDenormal()); + EXPECT_FALSE(APFloat(APFloat::x87DoubleExtended(), 0).isDenormal()); - APFloat Val2(APFloat::x87DoubleExtended(), 2.0e0); + APFloat Val2(APFloat::x87DoubleExtended(), 2); APFloat T(APFloat::x87DoubleExtended(), MinNormalStr); T.divide(Val2, rdmd); EXPECT_TRUE(T.isDenormal()); @@ -636,9 +636,9 @@ TEST(APFloatTest, Denormal) { { const char *MinNormalStr = "3.36210314311209350626267781732175260e-4932"; EXPECT_FALSE(APFloat(APFloat::IEEEquad(), MinNormalStr).isDenormal()); - EXPECT_FALSE(APFloat(APFloat::IEEEquad(), 0.0).isDenormal()); + EXPECT_FALSE(APFloat(APFloat::IEEEquad(), 0).isDenormal()); - APFloat Val2(APFloat::IEEEquad(), 2.0e0); + APFloat Val2(APFloat::IEEEquad(), 2); APFloat T(APFloat::IEEEquad(), MinNormalStr); T.divide(Val2, rdmd); EXPECT_TRUE(T.isDenormal()); @@ -1153,8 +1153,8 @@ TEST(APFloatTest, makeNaN) { #ifdef GTEST_HAS_DEATH_TEST #ifndef NDEBUG TEST(APFloatTest, SemanticsDeath) { - EXPECT_DEATH(APFloat(APFloat::IEEEsingle(), 0.0f).convertToDouble(), "Float semantics are not IEEEdouble"); - EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), 0.0 ).convertToFloat(), "Float semantics are not IEEEsingle"); + EXPECT_DEATH(APFloat(APFloat::IEEEsingle(), 0).convertToDouble(), "Float semantics are not IEEEdouble"); + EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), 0).convertToFloat(), "Float semantics are not IEEEsingle"); } TEST(APFloatTest, StringDecimalDeath) { |

