Skip to content

Commit

Permalink
Treat the range of representable values of floating-point types as [-…
Browse files Browse the repository at this point in the history 
…inf, +inf] not as [-max, +max].

Summary:
Prior to r329065, we used [-max, max] as the range of representable
values because LLVM's `fptrunc` did not guarantee defined behavior when
truncating from a larger floating-point type to a smaller one. Now that
has been fixed, we can make clang follow normal IEEE 754 semantics in this
regard and take the larger range [-inf, +inf] as the range of representable
values.

In practice, this affects two parts of the frontend:
 * the constant evaluator no longer treats floating-point evaluations
   that result in +-inf as being undefined (because they no longer leave
   the range of representable values of the type)
 * UBSan no longer treats conversions to floating-point type that are
   outside the [-max, +max] range as being undefined

In passing, also remove the float-divide-by-zero sanitizer from
-fsanitize=undefined, on the basis that while it's undefined per C++
rules (and we disallow it in constant expressions for that reason), it
is defined by Clang / LLVM / IEEE 754.

Reviewers: rnk, BillyONeal

Subscribers: cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D63793

llvm-svn: 365272
  • Loading branch information
@zygoloid
zygoloid committed Jul 6, 2019
1 parent a7145c4 commit 9e52c43
Show file tree
Hide file tree
Showing 8 changed files with 124 additions and 189 deletions.
18 changes: 11 additions & 7 deletions clang/docs/UndefinedBehaviorSanitizer.rst
View file
Expand Up @@ -83,9 +83,13 @@ Available checks are:
type.
- ``-fsanitize=float-cast-overflow``: Conversion to, from, or
between floating-point types which would overflow the
destination.
destination. Because the range of representable values for all
floating-point types supported by Clang is [-inf, +inf], the only
cases detected are conversions from floating point to integer types.
- ``-fsanitize=float-divide-by-zero``: Floating point division by
zero.
zero. This is undefined per the C and C++ standards, but is defined
by Clang (and by ISO/IEC/IEEE 60559 / IEEE 754) as producing either an
infinity or NaN value, so is not included in ``-fsanitize=undefined``.
- ``-fsanitize=function``: Indirect call of a function through a
function pointer of the wrong type (Darwin/Linux, C++ and x86/x86_64
only).
Expand Down Expand Up @@ -163,8 +167,8 @@ Available checks are:

You can also use the following check groups:
- ``-fsanitize=undefined``: All of the checks listed above other than
``unsigned-integer-overflow``, ``implicit-conversion`` and the
``nullability-*`` group of checks.
``float-divide-by-zero``, ``unsigned-integer-overflow``,
``implicit-conversion``, and the ``nullability-*`` group of checks.
- ``-fsanitize=undefined-trap``: Deprecated alias of
``-fsanitize=undefined``.
- ``-fsanitize=implicit-integer-truncation``: Catches lossy integral
Expand All @@ -174,16 +178,16 @@ You can also use the following check groups:
conversions that change the arithmetic value of the integer. Enables
``implicit-signed-integer-truncation`` and ``implicit-integer-sign-change``.
- ``-fsanitize=implicit-conversion``: Checks for suspicious
behaviour of implicit conversions. Enables
behavior of implicit conversions. Enables
``implicit-unsigned-integer-truncation``,
``implicit-signed-integer-truncation`` and
``implicit-signed-integer-truncation``, and
``implicit-integer-sign-change``.
- ``-fsanitize=integer``: Checks for undefined or suspicious integer
behavior (e.g. unsigned integer overflow).
Enables ``signed-integer-overflow``, ``unsigned-integer-overflow``,
``shift``, ``integer-divide-by-zero``,
``implicit-unsigned-integer-truncation``,
``implicit-signed-integer-truncation`` and
``implicit-signed-integer-truncation``, and
``implicit-integer-sign-change``.
- ``-fsanitize=nullability``: Enables ``nullability-arg``,
``nullability-assign``, and ``nullability-return``. While violating
Expand Down
2 changes: 1 addition & 1 deletion clang/include/clang/Basic/Sanitizers.def
View file
Expand Up @@ -130,7 +130,7 @@ SANITIZER("shadow-call-stack", ShadowCallStack)
// ABI or address space layout implications, and only catch undefined behavior.
SANITIZER_GROUP("undefined", Undefined,
Alignment | Bool | Builtin | ArrayBounds | Enum |
FloatCastOverflow | FloatDivideByZero |
FloatCastOverflow |
IntegerDivideByZero | NonnullAttribute | Null | ObjectSize |
PointerOverflow | Return | ReturnsNonnullAttribute | Shift |
SignedIntegerOverflow | Unreachable | VLABound | Function |
Expand Down
22 changes: 13 additions & 9 deletions clang/lib/AST/ExprConstant.cpp
View file
Expand Up @@ -2212,10 +2212,8 @@ static bool HandleFloatToFloatCast(EvalInfo &Info, const Expr *E,
APFloat &Result) {
APFloat Value = Result;
bool ignored;
if (Result.convert(Info.Ctx.getFloatTypeSemantics(DestType),
APFloat::rmNearestTiesToEven, &ignored)
& APFloat::opOverflow)
return HandleOverflow(Info, E, Value, DestType);
Result.convert(Info.Ctx.getFloatTypeSemantics(DestType),
APFloat::rmNearestTiesToEven, &ignored);
return true;
}

Expand All @@ -2236,10 +2234,8 @@ static bool HandleIntToFloatCast(EvalInfo &Info, const Expr *E,
QualType SrcType, const APSInt &Value,
QualType DestType, APFloat &Result) {
Result = APFloat(Info.Ctx.getFloatTypeSemantics(DestType), 1);
if (Result.convertFromAPInt(Value, Value.isSigned(),
APFloat::rmNearestTiesToEven)
& APFloat::opOverflow)
return HandleOverflow(Info, E, Value, DestType);
Result.convertFromAPInt(Value, Value.isSigned(),
APFloat::rmNearestTiesToEven);
return true;
}

Expand Down Expand Up @@ -2457,11 +2453,19 @@ static bool handleFloatFloatBinOp(EvalInfo &Info, const Expr *E,
LHS.subtract(RHS, APFloat::rmNearestTiesToEven);
break;
case BO_Div:
// [expr.mul]p4:
// If the second operand of / or % is zero the behavior is undefined.
if (RHS.isZero())
Info.CCEDiag(E, diag::note_expr_divide_by_zero);
LHS.divide(RHS, APFloat::rmNearestTiesToEven);
break;
}

if (LHS.isInfinity() || LHS.isNaN()) {
// [expr.pre]p4:
// If during the evaluation of an expression, the result is not
// mathematically defined [...], the behavior is undefined.
// FIXME: C++ rules require us to not conform to IEEE 754 here.
if (LHS.isNaN()) {
Info.CCEDiag(E, diag::note_constexpr_float_arithmetic) << LHS.isNaN();
return Info.noteUndefinedBehavior();
}
Expand Down
178 changes: 58 additions & 120 deletions clang/lib/CodeGen/CGExprScalar.cpp
View file
Expand Up @@ -313,7 +313,7 @@ class ScalarExprEmitter
/// boolean (i1) truth value. This is equivalent to "Val != 0".
Value *EmitConversionToBool(Value *Src, QualType DstTy);

/// Emit a check that a conversion to or from a floating-point type does not
/// Emit a check that a conversion from a floating-point type does not
/// overflow.
void EmitFloatConversionCheck(Value *OrigSrc, QualType OrigSrcType,
Value *Src, QualType SrcType, QualType DstType,
Expand Down Expand Up @@ -864,128 +864,63 @@ Value *ScalarExprEmitter::EmitConversionToBool(Value *Src, QualType SrcType) {
void ScalarExprEmitter::EmitFloatConversionCheck(
Value *OrigSrc, QualType OrigSrcType, Value *Src, QualType SrcType,
QualType DstType, llvm::Type *DstTy, SourceLocation Loc) {
assert(SrcType->isFloatingType() && "not a conversion from floating point");
if (!isa<llvm::IntegerType>(DstTy))
return;

CodeGenFunction::SanitizerScope SanScope(&CGF);
using llvm::APFloat;
using llvm::APSInt;

llvm::Type *SrcTy = Src->getType();

llvm::Value *Check = nullptr;
if (llvm::IntegerType *IntTy = dyn_cast<llvm::IntegerType>(SrcTy)) {
// Integer to floating-point. This can fail for unsigned short -> __half
// or unsigned __int128 -> float.
assert(DstType->isFloatingType());
bool SrcIsUnsigned = OrigSrcType->isUnsignedIntegerOrEnumerationType();

APFloat LargestFloat =
APFloat::getLargest(CGF.getContext().getFloatTypeSemantics(DstType));
APSInt LargestInt(IntTy->getBitWidth(), SrcIsUnsigned);

bool IsExact;
if (LargestFloat.convertToInteger(LargestInt, APFloat::rmTowardZero,
&IsExact) != APFloat::opOK)
// The range of representable values of this floating point type includes
// all values of this integer type. Don't need an overflow check.
return;

llvm::Value *Max = llvm::ConstantInt::get(VMContext, LargestInt);
if (SrcIsUnsigned)
Check = Builder.CreateICmpULE(Src, Max);
else {
llvm::Value *Min = llvm::ConstantInt::get(VMContext, -LargestInt);
llvm::Value *GE = Builder.CreateICmpSGE(Src, Min);
llvm::Value *LE = Builder.CreateICmpSLE(Src, Max);
Check = Builder.CreateAnd(GE, LE);
}
} else {
const llvm::fltSemantics &SrcSema =
CGF.getContext().getFloatTypeSemantics(OrigSrcType);
if (isa<llvm::IntegerType>(DstTy)) {
// Floating-point to integer. This has undefined behavior if the source is
// +-Inf, NaN, or doesn't fit into the destination type (after truncation
// to an integer).
unsigned Width = CGF.getContext().getIntWidth(DstType);
bool Unsigned = DstType->isUnsignedIntegerOrEnumerationType();

APSInt Min = APSInt::getMinValue(Width, Unsigned);
APFloat MinSrc(SrcSema, APFloat::uninitialized);
if (MinSrc.convertFromAPInt(Min, !Unsigned, APFloat::rmTowardZero) &
APFloat::opOverflow)
// Don't need an overflow check for lower bound. Just check for
// -Inf/NaN.
MinSrc = APFloat::getInf(SrcSema, true);
else
// Find the largest value which is too small to represent (before
// truncation toward zero).
MinSrc.subtract(APFloat(SrcSema, 1), APFloat::rmTowardNegative);

APSInt Max = APSInt::getMaxValue(Width, Unsigned);
APFloat MaxSrc(SrcSema, APFloat::uninitialized);
if (MaxSrc.convertFromAPInt(Max, !Unsigned, APFloat::rmTowardZero) &
APFloat::opOverflow)
// Don't need an overflow check for upper bound. Just check for
// +Inf/NaN.
MaxSrc = APFloat::getInf(SrcSema, false);
else
// Find the smallest value which is too large to represent (before
// truncation toward zero).
MaxSrc.add(APFloat(SrcSema, 1), APFloat::rmTowardPositive);

// If we're converting from __half, convert the range to float to match
// the type of src.
if (OrigSrcType->isHalfType()) {
const llvm::fltSemantics &Sema =
CGF.getContext().getFloatTypeSemantics(SrcType);
bool IsInexact;
MinSrc.convert(Sema, APFloat::rmTowardZero, &IsInexact);
MaxSrc.convert(Sema, APFloat::rmTowardZero, &IsInexact);
}

llvm::Value *GE =
Builder.CreateFCmpOGT(Src, llvm::ConstantFP::get(VMContext, MinSrc));
llvm::Value *LE =
Builder.CreateFCmpOLT(Src, llvm::ConstantFP::get(VMContext, MaxSrc));
Check = Builder.CreateAnd(GE, LE);
} else {
// FIXME: Maybe split this sanitizer out from float-cast-overflow.
//
// Floating-point to floating-point. This has undefined behavior if the
// source is not in the range of representable values of the destination
// type. The C and C++ standards are spectacularly unclear here. We
// diagnose finite out-of-range conversions, but allow infinities and NaNs
// to convert to the corresponding value in the smaller type.
//
// C11 Annex F gives all such conversions defined behavior for IEC 60559
// conforming implementations. Unfortunately, LLVM's fptrunc instruction
// does not.

// Converting from a lower rank to a higher rank can never have
// undefined behavior, since higher-rank types must have a superset
// of values of lower-rank types.
if (CGF.getContext().getFloatingTypeOrder(OrigSrcType, DstType) != 1)
return;

assert(!OrigSrcType->isHalfType() &&
"should not check conversion from __half, it has the lowest rank");

const llvm::fltSemantics &DstSema =
CGF.getContext().getFloatTypeSemantics(DstType);
APFloat MinBad = APFloat::getLargest(DstSema, false);
APFloat MaxBad = APFloat::getInf(DstSema, false);

bool IsInexact;
MinBad.convert(SrcSema, APFloat::rmTowardZero, &IsInexact);
MaxBad.convert(SrcSema, APFloat::rmTowardZero, &IsInexact);

Value *AbsSrc = CGF.EmitNounwindRuntimeCall(
CGF.CGM.getIntrinsic(llvm::Intrinsic::fabs, Src->getType()), Src);
llvm::Value *GE =
Builder.CreateFCmpOGT(AbsSrc, llvm::ConstantFP::get(VMContext, MinBad));
llvm::Value *LE =
Builder.CreateFCmpOLT(AbsSrc, llvm::ConstantFP::get(VMContext, MaxBad));
Check = Builder.CreateNot(Builder.CreateAnd(GE, LE));
}
}
const llvm::fltSemantics &SrcSema =
CGF.getContext().getFloatTypeSemantics(OrigSrcType);

// Floating-point to integer. This has undefined behavior if the source is
// +-Inf, NaN, or doesn't fit into the destination type (after truncation
// to an integer).
unsigned Width = CGF.getContext().getIntWidth(DstType);
bool Unsigned = DstType->isUnsignedIntegerOrEnumerationType();

APSInt Min = APSInt::getMinValue(Width, Unsigned);
APFloat MinSrc(SrcSema, APFloat::uninitialized);
if (MinSrc.convertFromAPInt(Min, !Unsigned, APFloat::rmTowardZero) &
APFloat::opOverflow)
// Don't need an overflow check for lower bound. Just check for
// -Inf/NaN.
MinSrc = APFloat::getInf(SrcSema, true);
else
// Find the largest value which is too small to represent (before
// truncation toward zero).
MinSrc.subtract(APFloat(SrcSema, 1), APFloat::rmTowardNegative);

APSInt Max = APSInt::getMaxValue(Width, Unsigned);
APFloat MaxSrc(SrcSema, APFloat::uninitialized);
if (MaxSrc.convertFromAPInt(Max, !Unsigned, APFloat::rmTowardZero) &
APFloat::opOverflow)
// Don't need an overflow check for upper bound. Just check for
// +Inf/NaN.
MaxSrc = APFloat::getInf(SrcSema, false);
else
// Find the smallest value which is too large to represent (before
// truncation toward zero).
MaxSrc.add(APFloat(SrcSema, 1), APFloat::rmTowardPositive);

// If we're converting from __half, convert the range to float to match
// the type of src.
if (OrigSrcType->isHalfType()) {
const llvm::fltSemantics &Sema =
CGF.getContext().getFloatTypeSemantics(SrcType);
bool IsInexact;
MinSrc.convert(Sema, APFloat::rmTowardZero, &IsInexact);
MaxSrc.convert(Sema, APFloat::rmTowardZero, &IsInexact);
}

llvm::Value *GE =
Builder.CreateFCmpOGT(Src, llvm::ConstantFP::get(VMContext, MinSrc));
llvm::Value *LE =
Builder.CreateFCmpOLT(Src, llvm::ConstantFP::get(VMContext, MaxSrc));
Check = Builder.CreateAnd(GE, LE);

llvm::Constant *StaticArgs[] = {CGF.EmitCheckSourceLocation(Loc),
CGF.EmitCheckTypeDescriptor(OrigSrcType),
Expand Down Expand Up @@ -1391,9 +1326,12 @@ Value *ScalarExprEmitter::EmitScalarConversion(Value *Src, QualType SrcType,
llvm::Type *ResTy = DstTy;

// An overflowing conversion has undefined behavior if either the source type
// or the destination type is a floating-point type.
// or the destination type is a floating-point type. However, we consider the
// range of representable values for all floating-point types to be
// [-inf,+inf], so no overflow can ever happen when the destination type is a
// floating-point type.
if (CGF.SanOpts.has(SanitizerKind::FloatCastOverflow) &&
(OrigSrcType->isFloatingType() || DstType->isFloatingType()))
OrigSrcType->isFloatingType())
EmitFloatConversionCheck(OrigSrc, OrigSrcType, Src, SrcType, DstType, DstTy,
Loc);

Expand Down
28 changes: 21 additions & 7 deletions clang/test/CXX/expr/expr.const/p2-0x.cpp
View file
Expand Up @@ -136,9 +136,9 @@ namespace UndefinedBehavior {
case (int)10000000000ll: // expected-note {{here}}
case (unsigned int)10000000000ll: // expected-error {{duplicate case value}}
case (int)(unsigned)(long long)4.4e9: // ok
case (int)(float)1e300: // expected-error {{constant expression}} expected-note {{value 1.0E+300 is outside the range of representable values of type 'float'}}
case (int)(float)1e300: // expected-error {{constant expression}} expected-note {{value +Inf is outside the range of representable values of type 'int'}}
case (int)((float)1e37 / 1e30): // ok
case (int)(__fp16)65536: // expected-error {{constant expression}} expected-note {{value 65536 is outside the range of representable values of type '__fp16'}}
case (int)(__fp16)65536: // expected-error {{constant expression}} expected-note {{value +Inf is outside the range of representable values of type 'int'}}
break;
}
}
Expand Down Expand Up @@ -264,14 +264,28 @@ namespace UndefinedBehavior {
static_assert(0u - 1u == 4294967295u, ""); // ok
static_assert(~0u * ~0u == 1u, ""); // ok

template<typename T> constexpr bool isinf(T v) { return v && v / 2 == v; }

// Floating-point overflow and NaN.
constexpr float f1 = 1e38f * 3.4028f; // ok
constexpr float f2 = 1e38f * 3.4029f; // expected-error {{constant expression}} expected-note {{floating point arithmetic produces an infinity}}
constexpr float f2 = 1e38f * 3.4029f; // ok, +inf is in range of representable values
constexpr float f3 = 1e38f / -.2939f; // ok
constexpr float f4 = 1e38f / -.2938f; // expected-error {{constant expression}} expected-note {{floating point arithmetic produces an infinity}}
constexpr float f5 = 2e38f + 2e38f; // expected-error {{constant expression}} expected-note {{floating point arithmetic produces an infinity}}
constexpr float f6 = -2e38f - 2e38f; // expected-error {{constant expression}} expected-note {{floating point arithmetic produces an infinity}}
constexpr float f7 = 0.f / 0.f; // expected-error {{constant expression}} expected-note {{floating point arithmetic produces a NaN}}
constexpr float f4 = 1e38f / -.2938f; // ok, -inf is in range of representable values
constexpr float f5 = 2e38f + 2e38f; // ok, +inf is in range of representable values
constexpr float f6 = -2e38f - 2e38f; // ok, -inf is in range of representable values
constexpr float f7 = 0.f / 0.f; // expected-error {{constant expression}} expected-note {{division by zero}}
constexpr float f8 = 1.f / 0.f; // expected-error {{constant expression}} expected-note {{division by zero}}
constexpr float f9 = 1e308 / 1e-308; // ok, +inf
constexpr float f10 = f2 - f2; // expected-error {{constant expression}} expected-note {{produces a NaN}}
constexpr float f11 = f2 + f4; // expected-error {{constant expression}} expected-note {{produces a NaN}}
constexpr float f12 = f2 / f2; // expected-error {{constant expression}} expected-note {{produces a NaN}}
static_assert(!isinf(f1), "");
static_assert(isinf(f2), "");
static_assert(!isinf(f3), "");
static_assert(isinf(f4), "");
static_assert(isinf(f5), "");
static_assert(isinf(f6), "");
static_assert(isinf(f9), "");
}
}

Expand Down

0 comments on commit 9e52c43

Please sign in to comment.