Skip to content

Commit 2b6b8cb

Browse files
ekatzekatz
committed
[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
1 parent 1351672 commit 2b6b8cb

File tree

4 files changed

+18
-15
lines changed

4 files changed

+18
-15
lines changed

‎llvm/include/llvm/ADT/APFloat.h

+3
Original file line numberDiff line numberDiff line change
@@ -851,6 +851,9 @@ class APFloat : public APFloatBase {
851851
APFloat(const fltSemantics &Semantics) : U(Semantics) {}
852852
APFloat(const fltSemantics &Semantics, StringRef S);
853853
APFloat(const fltSemantics &Semantics, integerPart I) : U(Semantics, I) {}
854+
template <typename T, typename = typename std::enable_if<
855+
std::is_floating_point<T>::value>::type>
856+
APFloat(const fltSemantics &Semantics, T V) = delete;
854857
// TODO: Remove this constructor. This isn't faster than the first one.
855858
APFloat(const fltSemantics &Semantics, uninitializedTag)
856859
: U(Semantics, uninitialized) {}

‎llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -3386,7 +3386,7 @@ Instruction *InstCombiner::visitCallInst(CallInst &CI) {
33863386

33873387
if (const ConstantFP *C = dyn_cast<ConstantFP>(Src)) {
33883388
const APFloat &ArgVal = C->getValueAPF();
3389-
APFloat Val(ArgVal.getSemantics(), 1.0);
3389+
APFloat Val(ArgVal.getSemantics(), 1);
33903390
APFloat::opStatus Status = Val.divide(ArgVal,
33913391
APFloat::rmNearestTiesToEven);
33923392
// Only do this if it was exact and therefore not dependent on the

‎llvm/lib/Transforms/Utils/SimplifyLibCalls.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -1735,7 +1735,7 @@ Value *LibCallSimplifier::optimizePow(CallInst *Pow, IRBuilder<> &B) {
17351735
// TODO: This whole transformation should be backend specific (e.g. some
17361736
// backends might prefer libcalls or the limit for the exponent might
17371737
// be different) and it should also consider optimizing for size.
1738-
APFloat LimF(ExpoF->getSemantics(), 33.0),
1738+
APFloat LimF(ExpoF->getSemantics(), 33),
17391739
ExpoA(abs(*ExpoF));
17401740
if (ExpoA.compare(LimF) == APFloat::cmpLessThan) {
17411741
// This transformation applies to integer or integer+0.5 exponents only.

‎llvm/unittests/ADT/APFloatTest.cpp

+13-13
Original file line numberDiff line numberDiff line change
@@ -520,9 +520,9 @@ TEST(APFloatTest, FMA) {
520520

521521
// Test x87 extended precision case from http://llvm.org/PR20728.
522522
{
523-
APFloat M1(APFloat::x87DoubleExtended(), 1.0);
524-
APFloat M2(APFloat::x87DoubleExtended(), 1.0);
525-
APFloat A(APFloat::x87DoubleExtended(), 3.0);
523+
APFloat M1(APFloat::x87DoubleExtended(), 1);
524+
APFloat M2(APFloat::x87DoubleExtended(), 1);
525+
APFloat A(APFloat::x87DoubleExtended(), 3);
526526

527527
bool losesInfo = false;
528528
M1.fusedMultiplyAdd(M1, A, APFloat::rmNearestTiesToEven);
@@ -600,9 +600,9 @@ TEST(APFloatTest, Denormal) {
600600
{
601601
const char *MinNormalStr = "1.17549435082228750797e-38";
602602
EXPECT_FALSE(APFloat(APFloat::IEEEsingle(), MinNormalStr).isDenormal());
603-
EXPECT_FALSE(APFloat(APFloat::IEEEsingle(), 0.0).isDenormal());
603+
EXPECT_FALSE(APFloat(APFloat::IEEEsingle(), 0).isDenormal());
604604

605-
APFloat Val2(APFloat::IEEEsingle(), 2.0e0);
605+
APFloat Val2(APFloat::IEEEsingle(), 2);
606606
APFloat T(APFloat::IEEEsingle(), MinNormalStr);
607607
T.divide(Val2, rdmd);
608608
EXPECT_TRUE(T.isDenormal());
@@ -612,9 +612,9 @@ TEST(APFloatTest, Denormal) {
612612
{
613613
const char *MinNormalStr = "2.22507385850720138309e-308";
614614
EXPECT_FALSE(APFloat(APFloat::IEEEdouble(), MinNormalStr).isDenormal());
615-
EXPECT_FALSE(APFloat(APFloat::IEEEdouble(), 0.0).isDenormal());
615+
EXPECT_FALSE(APFloat(APFloat::IEEEdouble(), 0).isDenormal());
616616

617-
APFloat Val2(APFloat::IEEEdouble(), 2.0e0);
617+
APFloat Val2(APFloat::IEEEdouble(), 2);
618618
APFloat T(APFloat::IEEEdouble(), MinNormalStr);
619619
T.divide(Val2, rdmd);
620620
EXPECT_TRUE(T.isDenormal());
@@ -624,9 +624,9 @@ TEST(APFloatTest, Denormal) {
624624
{
625625
const char *MinNormalStr = "3.36210314311209350626e-4932";
626626
EXPECT_FALSE(APFloat(APFloat::x87DoubleExtended(), MinNormalStr).isDenormal());
627-
EXPECT_FALSE(APFloat(APFloat::x87DoubleExtended(), 0.0).isDenormal());
627+
EXPECT_FALSE(APFloat(APFloat::x87DoubleExtended(), 0).isDenormal());
628628

629-
APFloat Val2(APFloat::x87DoubleExtended(), 2.0e0);
629+
APFloat Val2(APFloat::x87DoubleExtended(), 2);
630630
APFloat T(APFloat::x87DoubleExtended(), MinNormalStr);
631631
T.divide(Val2, rdmd);
632632
EXPECT_TRUE(T.isDenormal());
@@ -636,9 +636,9 @@ TEST(APFloatTest, Denormal) {
636636
{
637637
const char *MinNormalStr = "3.36210314311209350626267781732175260e-4932";
638638
EXPECT_FALSE(APFloat(APFloat::IEEEquad(), MinNormalStr).isDenormal());
639-
EXPECT_FALSE(APFloat(APFloat::IEEEquad(), 0.0).isDenormal());
639+
EXPECT_FALSE(APFloat(APFloat::IEEEquad(), 0).isDenormal());
640640

641-
APFloat Val2(APFloat::IEEEquad(), 2.0e0);
641+
APFloat Val2(APFloat::IEEEquad(), 2);
642642
APFloat T(APFloat::IEEEquad(), MinNormalStr);
643643
T.divide(Val2, rdmd);
644644
EXPECT_TRUE(T.isDenormal());
@@ -1153,8 +1153,8 @@ TEST(APFloatTest, makeNaN) {
11531153
#ifdef GTEST_HAS_DEATH_TEST
11541154
#ifndef NDEBUG
11551155
TEST(APFloatTest, SemanticsDeath) {
1156-
EXPECT_DEATH(APFloat(APFloat::IEEEsingle(), 0.0f).convertToDouble(), "Float semantics are not IEEEdouble");
1157-
EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), 0.0 ).convertToFloat(), "Float semantics are not IEEEsingle");
1156+
EXPECT_DEATH(APFloat(APFloat::IEEEsingle(), 0).convertToDouble(), "Float semantics are not IEEEdouble");
1157+
EXPECT_DEATH(APFloat(APFloat::IEEEdouble(), 0).convertToFloat(), "Float semantics are not IEEEsingle");
11581158
}
11591159

11601160
TEST(APFloatTest, StringDecimalDeath) {

0 commit comments

Comments
 (0)