Skip to content

Commit 9690b30

Browse files
kuilpdkuilpd
authored
[LLDB] Fix operators <= and >= returning a wrong result when comparing to a floating point NaN (#108060)
Implement operators `<=` and `>=` to explicitly check the comparison results to be `cmpLessThan` or `cmpEqual` instead of negating the result of `operators<`. Fixes #85947
1 parent 13b4d1b commit 9690b30

File tree

5 files changed

+168
-32
lines changed

5 files changed

+168
-32
lines changed

‎lldb/include/lldb/Utility/Scalar.h‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,7 @@ class Scalar {
210210
static PromotionKey GetFloatPromoKey(const llvm::fltSemantics &semantics);
211211

212212
private:
213+
friend llvm::APFloat::cmpResult compare(Scalar lhs, Scalar rhs);
213214
friend const Scalar operator+(const Scalar &lhs, const Scalar &rhs);
214215
friend const Scalar operator-(Scalar lhs, Scalar rhs);
215216
friend const Scalar operator/(Scalar lhs, Scalar rhs);
@@ -220,9 +221,9 @@ class Scalar {
220221
friend const Scalar operator^(Scalar lhs, Scalar rhs);
221222
friend const Scalar operator<<(const Scalar &lhs, const Scalar &rhs);
222223
friend const Scalar operator>>(const Scalar &lhs, const Scalar &rhs);
223-
friend bool operator==(Scalar lhs, Scalar rhs);
224+
friend bool operator==(const Scalar &lhs, const Scalar &rhs);
224225
friend bool operator!=(const Scalar &lhs, const Scalar &rhs);
225-
friend bool operator<(Scalar lhs, Scalar rhs);
226+
friend bool operator<(const Scalar &lhs, const Scalar &rhs);
226227
friend bool operator<=(const Scalar &lhs, const Scalar &rhs);
227228
friend bool operator>(const Scalar &lhs, const Scalar &rhs);
228229
friend bool operator>=(const Scalar &lhs, const Scalar &rhs);
@@ -241,6 +242,7 @@ class Scalar {
241242
// Item 19 of "Effective C++ Second Edition" by Scott Meyers
242243
// Differentiate among members functions, non-member functions, and
243244
// friend functions
245+
llvm::APFloat::cmpResult compare(Scalar lhs, Scalar rhs);
244246
const Scalar operator+(const Scalar &lhs, const Scalar &rhs);
245247
const Scalar operator-(Scalar lhs, Scalar rhs);
246248
const Scalar operator/(Scalar lhs, Scalar rhs);
@@ -251,9 +253,9 @@ const Scalar operator%(Scalar lhs, Scalar rhs);
251253
const Scalar operator^(Scalar lhs, Scalar rhs);
252254
const Scalar operator<<(const Scalar &lhs, const Scalar &rhs);
253255
const Scalar operator>>(const Scalar &lhs, const Scalar &rhs);
254-
bool operator==(Scalar lhs, Scalar rhs);
256+
bool operator==(const Scalar &lhs, const Scalar &rhs);
255257
bool operator!=(const Scalar &lhs, const Scalar &rhs);
256-
bool operator<(Scalar lhs, Scalar rhs);
258+
bool operator<(const Scalar &lhs, const Scalar &rhs);
257259
bool operator<=(const Scalar &lhs, const Scalar &rhs);
258260
bool operator>(const Scalar &lhs, const Scalar &rhs);
259261
bool operator>=(const Scalar &lhs, const Scalar &rhs);

‎lldb/source/Utility/Scalar.cpp‎

Lines changed: 21 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -852,57 +852,50 @@ llvm::APFloat Scalar::CreateAPFloatFromAPFloat(lldb::BasicType basic_type) {
852852
}
853853
}
854854

855-
bool lldb_private::operator==(Scalar lhs, Scalar rhs) {
855+
APFloat::cmpResult lldb_private::compare(Scalar lhs, Scalar rhs) {
856856
// If either entry is void then we can just compare the types
857857
if (lhs.m_type == Scalar::e_void || rhs.m_type == Scalar::e_void)
858-
return lhs.m_type == rhs.m_type;
858+
return lhs.m_type == rhs.m_type ? APFloat::cmpEqual : APFloat::cmpUnordered;
859859

860-
llvm::APFloat::cmpResult result;
861860
switch (Scalar::PromoteToMaxType(lhs, rhs)) {
862861
case Scalar::e_void:
863862
break;
864863
case Scalar::e_int:
865-
return lhs.m_integer == rhs.m_integer;
864+
if (lhs.m_integer < rhs.m_integer)
865+
return APFloat::cmpLessThan;
866+
if (lhs.m_integer > rhs.m_integer)
867+
return APFloat::cmpGreaterThan;
868+
return APFloat::cmpEqual;
866869
case Scalar::e_float:
867-
result = lhs.m_float.compare(rhs.m_float);
868-
if (result == llvm::APFloat::cmpEqual)
869-
return true;
870+
return lhs.m_float.compare(rhs.m_float);
870871
}
871-
return false;
872+
return APFloat::cmpUnordered;
872873
}
873874

874-
bool lldb_private::operator!=(const Scalar &lhs, const Scalar &rhs) {
875-
return !(lhs == rhs);
875+
bool lldb_private::operator==(const Scalar &lhs, const Scalar &rhs) {
876+
return compare(lhs, rhs) == APFloat::cmpEqual;
876877
}
877878

878-
bool lldb_private::operator<(Scalar lhs, Scalar rhs) {
879-
if (lhs.m_type == Scalar::e_void || rhs.m_type == Scalar::e_void)
880-
return false;
879+
bool lldb_private::operator!=(const Scalar &lhs, const Scalar &rhs) {
880+
return compare(lhs, rhs) != APFloat::cmpEqual;
881+
}
881882

882-
llvm::APFloat::cmpResult result;
883-
switch (Scalar::PromoteToMaxType(lhs, rhs)) {
884-
case Scalar::e_void:
885-
break;
886-
case Scalar::e_int:
887-
return lhs.m_integer < rhs.m_integer;
888-
case Scalar::e_float:
889-
result = lhs.m_float.compare(rhs.m_float);
890-
if (result == llvm::APFloat::cmpLessThan)
891-
return true;
892-
}
893-
return false;
883+
bool lldb_private::operator<(const Scalar &lhs, const Scalar &rhs) {
884+
return compare(lhs, rhs) == APFloat::cmpLessThan;
894885
}
895886

896887
bool lldb_private::operator<=(const Scalar &lhs, const Scalar &rhs) {
897-
return !(rhs < lhs);
888+
APFloat::cmpResult Res = compare(lhs, rhs);
889+
return Res == APFloat::cmpLessThan || Res == APFloat::cmpEqual;
898890
}
899891

900892
bool lldb_private::operator>(const Scalar &lhs, const Scalar &rhs) {
901-
return rhs < lhs;
893+
return compare(lhs, rhs) == APFloat::cmpGreaterThan;
902894
}
903895

904896
bool lldb_private::operator>=(const Scalar &lhs, const Scalar &rhs) {
905-
return !(lhs < rhs);
897+
APFloat::cmpResult Res = compare(lhs, rhs);
898+
return Res == APFloat::cmpGreaterThan || Res == APFloat::cmpEqual;
906899
}
907900

908901
bool Scalar::ClearBit(uint32_t bit) {

‎lldb/test/API/lang/cpp/fpnan/Makefile‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
CXX_SOURCES := main.cpp
2+
3+
include Makefile.rules

‎lldb/test/API/lang/cpp/fpnan/TestFPNaN.py‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
"""
2+
Test floating point expressions with zero, NaN, dernormalized and infinite
3+
numbers.
4+
"""
5+
6+
import lldb
7+
from lldbsuite.test.lldbtest import *
8+
from lldbsuite.test import lldbutil
9+
10+
11+
class FPNaNTestCase(TestBase):
12+
def setUp(self):
13+
# Call super's setUp().
14+
TestBase.setUp(self)
15+
# Find the line number to break inside main().
16+
self.line = line_number("main.cpp", "// Set break point at this line.")
17+
18+
def test(self):
19+
self.build()
20+
exe = self.getBuildArtifact("a.out")
21+
self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
22+
23+
# Break inside the main.
24+
lldbutil.run_break_set_by_file_and_line(
25+
self, "main.cpp", self.line, num_expected_locations=1
26+
)
27+
28+
self.runCmd("run", RUN_SUCCEEDED)
29+
# Zero and denorm
30+
self.expect(
31+
"expr +0.0",
32+
VARIABLES_DISPLAYED_CORRECTLY,
33+
substrs=["double", "0"],
34+
)
35+
self.expect(
36+
"expr -0.0",
37+
VARIABLES_DISPLAYED_CORRECTLY,
38+
substrs=["double", "0"],
39+
)
40+
self.expect(
41+
"expr 0.0 / 0",
42+
VARIABLES_DISPLAYED_CORRECTLY,
43+
substrs=["double", "NaN"],
44+
)
45+
self.expect(
46+
"expr 0 / 0.0",
47+
VARIABLES_DISPLAYED_CORRECTLY,
48+
substrs=["double", "NaN"],
49+
)
50+
self.expect(
51+
"expr 1 / +0.0",
52+
VARIABLES_DISPLAYED_CORRECTLY,
53+
substrs=["double", "+Inf"],
54+
)
55+
self.expect(
56+
"expr 1 / -0.0",
57+
VARIABLES_DISPLAYED_CORRECTLY,
58+
substrs=["double", "-Inf"],
59+
)
60+
self.expect(
61+
"expr +0.0 / +0.0 != +0.0 / +0.0",
62+
VARIABLES_DISPLAYED_CORRECTLY,
63+
substrs=["bool", "true"],
64+
)
65+
self.expect(
66+
"expr -1.f * 0",
67+
VARIABLES_DISPLAYED_CORRECTLY,
68+
substrs=["float", "-0"],
69+
)
70+
self.expect(
71+
"expr 0x0.123p-1",
72+
VARIABLES_DISPLAYED_CORRECTLY,
73+
substrs=["double", "0.0355224609375"],
74+
)
75+
# NaN
76+
self.expect(
77+
"expr fnan < fnan",
78+
VARIABLES_DISPLAYED_CORRECTLY,
79+
substrs=["bool", "false"],
80+
)
81+
self.expect(
82+
"expr fnan <= fnan",
83+
VARIABLES_DISPLAYED_CORRECTLY,
84+
substrs=["bool", "false"],
85+
)
86+
self.expect(
87+
"expr fnan > fnan",
88+
VARIABLES_DISPLAYED_CORRECTLY,
89+
substrs=["bool", "false"],
90+
)
91+
self.expect(
92+
"expr fnan >= fnan",
93+
VARIABLES_DISPLAYED_CORRECTLY,
94+
substrs=["bool", "false"],
95+
)
96+
self.expect(
97+
"expr fnan == fnan",
98+
VARIABLES_DISPLAYED_CORRECTLY,
99+
substrs=["bool", "false"],
100+
)
101+
self.expect(
102+
"expr fnan != fnan",
103+
VARIABLES_DISPLAYED_CORRECTLY,
104+
substrs=["bool", "true"],
105+
)
106+
self.expect(
107+
"expr 1.0 <= fnan",
108+
VARIABLES_DISPLAYED_CORRECTLY,
109+
substrs=["bool", "false"],
110+
)
111+
self.expect(
112+
"expr 1.0f < fnan",
113+
VARIABLES_DISPLAYED_CORRECTLY,
114+
substrs=["bool", "false"],
115+
)
116+
self.expect(
117+
"expr 1.0f != fnan",
118+
VARIABLES_DISPLAYED_CORRECTLY,
119+
substrs=["bool", "true"],
120+
)
121+
self.expect(
122+
"expr (unsigned int) fdenorm",
123+
VARIABLES_DISPLAYED_CORRECTLY,
124+
substrs=["int", "0"],
125+
)
126+
self.expect(
127+
"expr (unsigned int) (1.0f + fdenorm)",
128+
VARIABLES_DISPLAYED_CORRECTLY,
129+
substrs=["int", "1"],
130+
)

‎lldb/test/API/lang/cpp/fpnan/main.cpp‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
#include <limits>
2+
3+
int main() {
4+
float fnan = std::numeric_limits<float>::quiet_NaN();
5+
float fdenorm = std::numeric_limits<float>::denorm_min();
6+
7+
// Set break point at this line.
8+
}

0 commit comments

Comments
 (0)