Skip to content

Commit 72825fd

Browse files
authored
[libc++][math] Fix undue overflowing of std::hypot(x,y,z) (#100820)
This is in relation to mr #93350. It was merged to main, but reverted because of failing sanitizer builds on PowerPC. The fix includes replacing the hard-coded threshold constants (e.g. `__overflow_threshold`) for different floating-point sizes by a general computation using `std::ldexp`. Thus, it should now work for all architectures. This has the drawback of not being `constexpr` anymore as `std::ldexp` is not implemented as `constexpr` (even though the standard mandates it for C++23). Closes #92782
1 parent 7e8a902 commit 72825fd

File tree

11 files changed

+178
-65
lines changed

11 files changed

+178
-65
lines changed

libcxx/include/__math/hypot.h

+61
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,25 @@
99
#ifndef _LIBCPP___MATH_HYPOT_H
1010
#define _LIBCPP___MATH_HYPOT_H
1111

12+
#include <__algorithm/max.h>
1213
#include <__config>
14+
#include <__math/abs.h>
15+
#include <__math/exponential_functions.h>
16+
#include <__math/roots.h>
1317
#include <__type_traits/enable_if.h>
1418
#include <__type_traits/is_arithmetic.h>
1519
#include <__type_traits/is_same.h>
1620
#include <__type_traits/promote.h>
21+
#include <__utility/pair.h>
22+
#include <limits>
1723

1824
#if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
1925
# pragma GCC system_header
2026
#endif
2127

28+
_LIBCPP_PUSH_MACROS
29+
#include <__undef_macros>
30+
2231
_LIBCPP_BEGIN_NAMESPACE_STD
2332

2433
namespace __math {
@@ -41,8 +50,60 @@ inline _LIBCPP_HIDE_FROM_ABI typename __promote<_A1, _A2>::type hypot(_A1 __x, _
4150
return __math::hypot((__result_type)__x, (__result_type)__y);
4251
}
4352

53+
#if _LIBCPP_STD_VER >= 17
54+
// Computes the three-dimensional hypotenuse: `std::hypot(x,y,z)`.
55+
// The naive implementation might over-/underflow which is why this implementation is more involved:
56+
// If the square of an argument might run into issues, we scale the arguments appropriately.
57+
// See https://github.com/llvm/llvm-project/issues/92782 for a detailed discussion and summary.
58+
template <class _Real>
59+
_LIBCPP_HIDE_FROM_ABI _Real __hypot(_Real __x, _Real __y, _Real __z) {
60+
// Factors needed to determine if over-/underflow might happen
61+
constexpr int __exp = std::numeric_limits<_Real>::max_exponent / 2;
62+
const _Real __overflow_threshold = __math::ldexp(_Real(1), __exp);
63+
const _Real __overflow_scale = __math::ldexp(_Real(1), -(__exp + 20));
64+
65+
// Scale arguments depending on their size
66+
const _Real __max_abs = std::max(__math::fabs(__x), std::max(__math::fabs(__y), __math::fabs(__z)));
67+
_Real __scale;
68+
if (__max_abs > __overflow_threshold) { // x*x + y*y + z*z might overflow
69+
__scale = __overflow_scale;
70+
} else if (__max_abs < 1 / __overflow_threshold) { // x*x + y*y + z*z might underflow
71+
__scale = 1 / __overflow_scale;
72+
} else {
73+
__scale = 1;
74+
}
75+
__x *= __scale;
76+
__y *= __scale;
77+
__z *= __scale;
78+
79+
// Compute hypot of scaled arguments and undo scaling
80+
return __math::sqrt(__x * __x + __y * __y + __z * __z) / __scale;
81+
}
82+
83+
inline _LIBCPP_HIDE_FROM_ABI float hypot(float __x, float __y, float __z) { return __math::__hypot(__x, __y, __z); }
84+
85+
inline _LIBCPP_HIDE_FROM_ABI double hypot(double __x, double __y, double __z) { return __math::__hypot(__x, __y, __z); }
86+
87+
inline _LIBCPP_HIDE_FROM_ABI long double hypot(long double __x, long double __y, long double __z) {
88+
return __math::__hypot(__x, __y, __z);
89+
}
90+
91+
template <class _A1,
92+
class _A2,
93+
class _A3,
94+
std::enable_if_t< is_arithmetic_v<_A1> && is_arithmetic_v<_A2> && is_arithmetic_v<_A3>, int> = 0 >
95+
_LIBCPP_HIDE_FROM_ABI typename __promote<_A1, _A2, _A3>::type hypot(_A1 __x, _A2 __y, _A3 __z) _NOEXCEPT {
96+
using __result_type = typename __promote<_A1, _A2, _A3>::type;
97+
static_assert(!(
98+
std::is_same_v<_A1, __result_type> && std::is_same_v<_A2, __result_type> && std::is_same_v<_A3, __result_type>));
99+
return __math::__hypot(
100+
static_cast<__result_type>(__x), static_cast<__result_type>(__y), static_cast<__result_type>(__z));
101+
}
102+
#endif
103+
44104
} // namespace __math
45105

46106
_LIBCPP_END_NAMESPACE_STD
107+
_LIBCPP_POP_MACROS
47108

48109
#endif // _LIBCPP___MATH_HYPOT_H

libcxx/include/cmath

+1-24
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,7 @@ constexpr long double lerp(long double a, long double b, long double t) noexcept
313313
*/
314314

315315
#include <__config>
316+
#include <__math/hypot.h>
316317
#include <__type_traits/enable_if.h>
317318
#include <__type_traits/is_arithmetic.h>
318319
#include <__type_traits/is_constant_evaluated.h>
@@ -553,30 +554,6 @@ using ::scalbnl _LIBCPP_USING_IF_EXISTS;
553554
using ::tgammal _LIBCPP_USING_IF_EXISTS;
554555
using ::truncl _LIBCPP_USING_IF_EXISTS;
555556

556-
#if _LIBCPP_STD_VER >= 17
557-
inline _LIBCPP_HIDE_FROM_ABI float hypot(float __x, float __y, float __z) {
558-
return sqrt(__x * __x + __y * __y + __z * __z);
559-
}
560-
inline _LIBCPP_HIDE_FROM_ABI double hypot(double __x, double __y, double __z) {
561-
return sqrt(__x * __x + __y * __y + __z * __z);
562-
}
563-
inline _LIBCPP_HIDE_FROM_ABI long double hypot(long double __x, long double __y, long double __z) {
564-
return sqrt(__x * __x + __y * __y + __z * __z);
565-
}
566-
567-
template <class _A1, class _A2, class _A3>
568-
inline _LIBCPP_HIDE_FROM_ABI
569-
typename enable_if_t< is_arithmetic<_A1>::value && is_arithmetic<_A2>::value && is_arithmetic<_A3>::value,
570-
__promote<_A1, _A2, _A3> >::type
571-
hypot(_A1 __lcpp_x, _A2 __lcpp_y, _A3 __lcpp_z) _NOEXCEPT {
572-
typedef typename __promote<_A1, _A2, _A3>::type __result_type;
573-
static_assert(
574-
!(is_same<_A1, __result_type>::value && is_same<_A2, __result_type>::value && is_same<_A3, __result_type>::value),
575-
"");
576-
return std::hypot((__result_type)__lcpp_x, (__result_type)__lcpp_y, (__result_type)__lcpp_z);
577-
}
578-
#endif
579-
580557
template <class _A1, __enable_if_t<is_floating_point<_A1>::value, int> = 0>
581558
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR bool __constexpr_isnan(_A1 __lcpp_x) _NOEXCEPT {
582559
#if __has_builtin(__builtin_isnan)

libcxx/test/libcxx/transitive_includes/cxx03.csv

+3
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,9 @@ chrono type_traits
131131
chrono vector
132132
chrono version
133133
cinttypes cstdint
134+
cmath cstddef
135+
cmath cstdint
136+
cmath initializer_list
134137
cmath limits
135138
cmath type_traits
136139
cmath version

libcxx/test/libcxx/transitive_includes/cxx11.csv

+3
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,9 @@ chrono type_traits
131131
chrono vector
132132
chrono version
133133
cinttypes cstdint
134+
cmath cstddef
135+
cmath cstdint
136+
cmath initializer_list
134137
cmath limits
135138
cmath type_traits
136139
cmath version

libcxx/test/libcxx/transitive_includes/cxx14.csv

+3
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,9 @@ chrono type_traits
132132
chrono vector
133133
chrono version
134134
cinttypes cstdint
135+
cmath cstddef
136+
cmath cstdint
137+
cmath initializer_list
135138
cmath limits
136139
cmath type_traits
137140
cmath version

libcxx/test/libcxx/transitive_includes/cxx17.csv

+3
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,9 @@ chrono type_traits
132132
chrono vector
133133
chrono version
134134
cinttypes cstdint
135+
cmath cstddef
136+
cmath cstdint
137+
cmath initializer_list
135138
cmath limits
136139
cmath type_traits
137140
cmath version

libcxx/test/libcxx/transitive_includes/cxx20.csv

+3
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,9 @@ chrono type_traits
137137
chrono vector
138138
chrono version
139139
cinttypes cstdint
140+
cmath cstddef
141+
cmath cstdint
142+
cmath initializer_list
140143
cmath limits
141144
cmath type_traits
142145
cmath version

libcxx/test/libcxx/transitive_includes/cxx23.csv

+3
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,9 @@ chrono string_view
8484
chrono vector
8585
chrono version
8686
cinttypes cstdint
87+
cmath cstddef
88+
cmath cstdint
89+
cmath initializer_list
8790
cmath limits
8891
cmath version
8992
codecvt cctype

libcxx/test/libcxx/transitive_includes/cxx26.csv

+3
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,9 @@ chrono string_view
8484
chrono vector
8585
chrono version
8686
cinttypes cstdint
87+
cmath cstddef
88+
cmath cstdint
89+
cmath initializer_list
8790
cmath limits
8891
cmath version
8992
codecvt cctype

libcxx/test/std/numerics/c.math/cmath.pass.cpp

+75-16
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,17 @@
1212

1313
// <cmath>
1414

15+
#include <array>
1516
#include <cmath>
1617
#include <limits>
1718
#include <type_traits>
1819
#include <cassert>
1920

21+
#include "fp_compare.h"
2022
#include "test_macros.h"
2123
#include "hexfloat.h"
2224
#include "truncate_fp.h"
25+
#include "type_algorithms.h"
2326

2427
// convertible to int/float/double/etc
2528
template <class T, int N=0>
@@ -1113,6 +1116,56 @@ void test_fmin()
11131116
assert(std::fmin(1,0) == 0);
11141117
}
11151118

1119+
#if TEST_STD_VER >= 17
1120+
struct TestHypot3 {
1121+
template <class Real>
1122+
void operator()() const {
1123+
const auto check = [](Real elem, Real abs_tol) {
1124+
assert(std::isfinite(std::hypot(elem, Real(0), Real(0))));
1125+
assert(fptest_close(std::hypot(elem, Real(0), Real(0)), elem, abs_tol));
1126+
assert(std::isfinite(std::hypot(elem, elem, Real(0))));
1127+
assert(fptest_close(std::hypot(elem, elem, Real(0)), std::sqrt(Real(2)) * elem, abs_tol));
1128+
assert(std::isfinite(std::hypot(elem, elem, elem)));
1129+
assert(fptest_close(std::hypot(elem, elem, elem), std::sqrt(Real(3)) * elem, abs_tol));
1130+
};
1131+
1132+
{ // check for overflow
1133+
const auto [elem, abs_tol] = []() -> std::array<Real, 2> {
1134+
if constexpr (std::is_same_v<Real, float>)
1135+
return {1e20f, 1e16f};
1136+
else if constexpr (std::is_same_v<Real, double>)
1137+
return {1e300, 1e287};
1138+
else { // long double
1139+
# if __DBL_MAX_EXP__ == __LDBL_MAX_EXP__
1140+
return {1e300l, 1e287l}; // 64-bit
1141+
# else
1142+
return {1e4000l, 1e3985l}; // 80- or 128-bit
1143+
# endif
1144+
}
1145+
}();
1146+
check(elem, abs_tol);
1147+
}
1148+
1149+
{ // check for underflow
1150+
const auto [elem, abs_tol] = []() -> std::array<Real, 2> {
1151+
if constexpr (std::is_same_v<Real, float>)
1152+
return {1e-20f, 1e-24f};
1153+
else if constexpr (std::is_same_v<Real, double>)
1154+
return {1e-287, 1e-300};
1155+
else { // long double
1156+
# if __DBL_MAX_EXP__ == __LDBL_MAX_EXP__
1157+
return {1e-287l, 1e-300l}; // 64-bit
1158+
# else
1159+
return {1e-3985l, 1e-4000l}; // 80- or 128-bit
1160+
# endif
1161+
}
1162+
}();
1163+
check(elem, abs_tol);
1164+
}
1165+
}
1166+
};
1167+
#endif
1168+
11161169
void test_hypot()
11171170
{
11181171
static_assert((std::is_same<decltype(std::hypot((float)0, (float)0)), float>::value), "");
@@ -1135,25 +1188,31 @@ void test_hypot()
11351188
static_assert((std::is_same<decltype(hypot(Ambiguous(), Ambiguous())), Ambiguous>::value), "");
11361189
assert(std::hypot(3,4) == 5);
11371190

1138-
#if TEST_STD_VER > 14
1139-
static_assert((std::is_same<decltype(std::hypot((float)0, (float)0, (float)0)), float>::value), "");
1140-
static_assert((std::is_same<decltype(std::hypot((float)0, (bool)0, (float)0)), double>::value), "");
1141-
static_assert((std::is_same<decltype(std::hypot((float)0, (unsigned short)0, (double)0)), double>::value), "");
1142-
static_assert((std::is_same<decltype(std::hypot((float)0, (int)0, (long double)0)), long double>::value), "");
1143-
static_assert((std::is_same<decltype(std::hypot((float)0, (double)0, (long)0)), double>::value), "");
1144-
static_assert((std::is_same<decltype(std::hypot((float)0, (long double)0, (unsigned long)0)), long double>::value), "");
1145-
static_assert((std::is_same<decltype(std::hypot((float)0, (int)0, (long long)0)), double>::value), "");
1146-
static_assert((std::is_same<decltype(std::hypot((float)0, (int)0, (unsigned long long)0)), double>::value), "");
1147-
static_assert((std::is_same<decltype(std::hypot((float)0, (double)0, (double)0)), double>::value), "");
1148-
static_assert((std::is_same<decltype(std::hypot((float)0, (long double)0, (long double)0)), long double>::value), "");
1149-
static_assert((std::is_same<decltype(std::hypot((float)0, (float)0, (double)0)), double>::value), "");
1150-
static_assert((std::is_same<decltype(std::hypot((float)0, (float)0, (long double)0)), long double>::value), "");
1151-
static_assert((std::is_same<decltype(std::hypot((float)0, (double)0, (long double)0)), long double>::value), "");
1152-
static_assert((std::is_same<decltype(std::hypot((int)0, (int)0, (int)0)), double>::value), "");
1153-
static_assert((std::is_same<decltype(hypot(Ambiguous(), Ambiguous(), Ambiguous())), Ambiguous>::value), "");
1191+
#if TEST_STD_VER >= 17
1192+
// clang-format off
1193+
static_assert((std::is_same_v<decltype(std::hypot((float)0, (float)0, (float)0)), float>));
1194+
static_assert((std::is_same_v<decltype(std::hypot((float)0, (bool)0, (float)0)), double>));
1195+
static_assert((std::is_same_v<decltype(std::hypot((float)0, (unsigned short)0, (double)0)), double>));
1196+
static_assert((std::is_same_v<decltype(std::hypot((float)0, (int)0, (long double)0)), long double>));
1197+
static_assert((std::is_same_v<decltype(std::hypot((float)0, (double)0, (long)0)), double>));
1198+
static_assert((std::is_same_v<decltype(std::hypot((float)0, (long double)0, (unsigned long)0)), long double>));
1199+
static_assert((std::is_same_v<decltype(std::hypot((float)0, (int)0, (long long)0)), double>));
1200+
static_assert((std::is_same_v<decltype(std::hypot((float)0, (int)0, (unsigned long long)0)), double>));
1201+
static_assert((std::is_same_v<decltype(std::hypot((float)0, (double)0, (double)0)), double>));
1202+
static_assert((std::is_same_v<decltype(std::hypot((float)0, (long double)0, (long double)0)), long double>));
1203+
static_assert((std::is_same_v<decltype(std::hypot((float)0, (float)0, (double)0)), double>));
1204+
static_assert((std::is_same_v<decltype(std::hypot((float)0, (float)0, (long double)0)), long double>));
1205+
static_assert((std::is_same_v<decltype(std::hypot((float)0, (double)0, (long double)0)), long double>));
1206+
static_assert((std::is_same_v<decltype(std::hypot((int)0, (int)0, (int)0)), double>));
1207+
static_assert((std::is_same_v<decltype(hypot(Ambiguous(), Ambiguous(), Ambiguous())), Ambiguous>));
1208+
// clang-format on
11541209

11551210
assert(std::hypot(2,3,6) == 7);
11561211
assert(std::hypot(1,4,8) == 9);
1212+
1213+
// Check for undue over-/underflows of intermediate results.
1214+
// See discussion at https://github.com/llvm/llvm-project/issues/92782.
1215+
types::for_each(types::floating_point_types(), TestHypot3());
11571216
#endif
11581217
}
11591218

libcxx/test/support/fp_compare.h

+20-25
Original file line numberDiff line numberDiff line change
@@ -9,39 +9,34 @@
99
#ifndef SUPPORT_FP_COMPARE_H
1010
#define SUPPORT_FP_COMPARE_H
1111

12-
#include <cmath> // for std::abs
13-
#include <algorithm> // for std::max
12+
#include <cmath> // for std::abs
13+
#include <algorithm> // for std::max
1414
#include <cassert>
15+
#include <__config>
1516

1617
// See https://www.boost.org/doc/libs/1_70_0/libs/test/doc/html/boost_test/testing_tools/extended_comparison/floating_point/floating_points_comparison_theory.html
1718

18-
template<typename T>
19-
bool fptest_close(T val, T expected, T eps)
20-
{
21-
constexpr T zero = T(0);
22-
assert(eps >= zero);
19+
template <typename T>
20+
bool fptest_close(T val, T expected, T eps) {
21+
_LIBCPP_CONSTEXPR T zero = T(0);
22+
assert(eps >= zero);
2323

24-
// Handle the zero cases
25-
if (eps == zero) return val == expected;
26-
if (val == zero) return std::abs(expected) <= eps;
27-
if (expected == zero) return std::abs(val) <= eps;
24+
// Handle the zero cases
25+
if (eps == zero)
26+
return val == expected;
27+
if (val == zero)
28+
return std::abs(expected) <= eps;
29+
if (expected == zero)
30+
return std::abs(val) <= eps;
2831

29-
return std::abs(val - expected) < eps
30-
&& std::abs(val - expected)/std::abs(val) < eps;
32+
return std::abs(val - expected) < eps && std::abs(val - expected) / std::abs(val) < eps;
3133
}
3234

33-
template<typename T>
34-
bool fptest_close_pct(T val, T expected, T percent)
35-
{
36-
constexpr T zero = T(0);
37-
assert(percent >= zero);
38-
39-
// Handle the zero cases
40-
if (percent == zero) return val == expected;
41-
T eps = (percent / T(100)) * std::max(std::abs(val), std::abs(expected));
42-
43-
return fptest_close(val, expected, eps);
35+
template <typename T>
36+
bool fptest_close_pct(T val, T expected, T percent) {
37+
assert(percent >= T(0));
38+
T eps = (percent / T(100)) * std::max(std::abs(val), std::abs(expected));
39+
return fptest_close(val, expected, eps);
4440
}
4541

46-
4742
#endif // SUPPORT_FP_COMPARE_H

0 commit comments

Comments
 (0)