This is an archive of the discontinued LLVM Phabricator instance.

Treat the range of representable values of floating-point types as [-inf, +inf] not as [-max, +max].
ClosedPublic

Authored by rsmith on Jun 25 2019, 2:45 PM.

Details

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.

Diff Detail

Event Timeline

rsmith created this revision.Jun 25 2019, 2:45 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 25 2019, 2:45 PM
spatel accepted this revision.Jun 28 2019, 4:14 PM

I don't have much experience with the front-end and have no experience with the sanitizers, but the changes match my understanding for this type of cast/conversion, so LGTM.

docs/UndefinedBehaviorSanitizer.rst
181

Independent of this patch but: 1 British behaviour in an otherwise American doc.

This revision is now accepted and ready to land.Jun 28 2019, 4:14 PM
nikic added a subscriber: nikic.Jun 29 2019, 3:26 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 6 2019, 2:06 PM

There are a number of buildbot failures which look related to this, e.g.

I think that removing the float-divide-by-zero sanitizer from -fsanitize=undefined has also caused it to be marked as not supported, so it can't be enabled even explicitly.

MaskRay added a subscriber: MaskRay.Jul 8 2019, 2:50 AM

There are a number of buildbot failures which look related to this, e.g.

I think that removing the float-divide-by-zero sanitizer from -fsanitize=undefined has also caused it to be marked as not supported, so it can't be enabled even explicitly.

Should be fixed by rCRT365307.