[Lldb-commits] [lldb] [LLDB] Fix operators <= and >= returning a wrong result when comparing to a floating point NaN (PR #108060)
Ilia Kuklin via lldb-commits
lldb-commits at lists.llvm.org
Wed Sep 11 14:58:13 PDT 2024
https://github.com/kuilpd updated https://github.com/llvm/llvm-project/pull/108060
>From 1e83cdfeb7e3a6fba4b190e95dd794723031370d Mon Sep 17 00:00:00 2001
From: Ilia Kuklin <ikuklin at accesssoftek.com>
Date: Tue, 3 Sep 2024 03:08:48 +0500
Subject: [PATCH 1/2] Implement operators <= and >= and add a test
---
lldb/include/lldb/Utility/Scalar.h | 8 +-
lldb/source/Utility/Scalar.cpp | 38 ++++++-
lldb/test/API/lang/cpp/fpnan/Makefile | 3 +
lldb/test/API/lang/cpp/fpnan/TestFPNaN.py | 130 ++++++++++++++++++++++
lldb/test/API/lang/cpp/fpnan/main.cpp | 8 ++
5 files changed, 179 insertions(+), 8 deletions(-)
create mode 100644 lldb/test/API/lang/cpp/fpnan/Makefile
create mode 100644 lldb/test/API/lang/cpp/fpnan/TestFPNaN.py
create mode 100644 lldb/test/API/lang/cpp/fpnan/main.cpp
diff --git a/lldb/include/lldb/Utility/Scalar.h b/lldb/include/lldb/Utility/Scalar.h
index 0d8eba3c9726d5..a8fe7fee3a2b9f 100644
--- a/lldb/include/lldb/Utility/Scalar.h
+++ b/lldb/include/lldb/Utility/Scalar.h
@@ -223,9 +223,9 @@ class Scalar {
friend bool operator==(Scalar lhs, Scalar rhs);
friend bool operator!=(const Scalar &lhs, const Scalar &rhs);
friend bool operator<(Scalar lhs, Scalar rhs);
- friend bool operator<=(const Scalar &lhs, const Scalar &rhs);
+ friend bool operator<=(Scalar lhs, Scalar rhs);
friend bool operator>(const Scalar &lhs, const Scalar &rhs);
- friend bool operator>=(const Scalar &lhs, const Scalar &rhs);
+ friend bool operator>=(Scalar lhs, Scalar rhs);
};
// Split out the operators into a format where the compiler will be able to
@@ -254,9 +254,9 @@ const Scalar operator>>(const Scalar &lhs, const Scalar &rhs);
bool operator==(Scalar lhs, Scalar rhs);
bool operator!=(const Scalar &lhs, const Scalar &rhs);
bool operator<(Scalar lhs, Scalar rhs);
-bool operator<=(const Scalar &lhs, const Scalar &rhs);
+bool operator<=(Scalar lhs, Scalar rhs);
bool operator>(const Scalar &lhs, const Scalar &rhs);
-bool operator>=(const Scalar &lhs, const Scalar &rhs);
+bool operator>=(Scalar lhs, Scalar rhs);
llvm::raw_ostream &operator<<(llvm::raw_ostream &os, const Scalar &scalar);
diff --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp
index 329f5b6e4b9a5b..bdbd79d375a4f7 100644
--- a/lldb/source/Utility/Scalar.cpp
+++ b/lldb/source/Utility/Scalar.cpp
@@ -893,16 +893,46 @@ bool lldb_private::operator<(Scalar lhs, Scalar rhs) {
return false;
}
-bool lldb_private::operator<=(const Scalar &lhs, const Scalar &rhs) {
- return !(rhs < lhs);
+bool lldb_private::operator<=(Scalar lhs, Scalar rhs) {
+ if (lhs.m_type == Scalar::e_void || rhs.m_type == Scalar::e_void)
+ return false;
+
+ llvm::APFloat::cmpResult result;
+ switch (Scalar::PromoteToMaxType(lhs, rhs)) {
+ case Scalar::e_void:
+ break;
+ case Scalar::e_int:
+ return lhs.m_integer <= rhs.m_integer;
+ case Scalar::e_float:
+ result = lhs.m_float.compare(rhs.m_float);
+ if (result == llvm::APFloat::cmpLessThan ||
+ result == llvm::APFloat::cmpEqual)
+ return true;
+ }
+ return false;
}
bool lldb_private::operator>(const Scalar &lhs, const Scalar &rhs) {
return rhs < lhs;
}
-bool lldb_private::operator>=(const Scalar &lhs, const Scalar &rhs) {
- return !(lhs < rhs);
+bool lldb_private::operator>=(Scalar lhs, Scalar rhs) {
+ if (lhs.m_type == Scalar::e_void || rhs.m_type == Scalar::e_void)
+ return false;
+
+ llvm::APFloat::cmpResult result;
+ switch (Scalar::PromoteToMaxType(lhs, rhs)) {
+ case Scalar::e_void:
+ break;
+ case Scalar::e_int:
+ return lhs.m_integer >= rhs.m_integer;
+ case Scalar::e_float:
+ result = lhs.m_float.compare(rhs.m_float);
+ if (result == llvm::APFloat::cmpGreaterThan ||
+ result == llvm::APFloat::cmpEqual)
+ return true;
+ }
+ return false;
}
bool Scalar::ClearBit(uint32_t bit) {
diff --git a/lldb/test/API/lang/cpp/fpnan/Makefile b/lldb/test/API/lang/cpp/fpnan/Makefile
new file mode 100644
index 00000000000000..99998b20bcb050
--- /dev/null
+++ b/lldb/test/API/lang/cpp/fpnan/Makefile
@@ -0,0 +1,3 @@
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/lang/cpp/fpnan/TestFPNaN.py b/lldb/test/API/lang/cpp/fpnan/TestFPNaN.py
new file mode 100644
index 00000000000000..6093ef91ac1f03
--- /dev/null
+++ b/lldb/test/API/lang/cpp/fpnan/TestFPNaN.py
@@ -0,0 +1,130 @@
+"""
+Test floating point expressions with zero, NaN, dernormalized and infinite
+numbers.
+"""
+
+import lldb
+from lldbsuite.test.lldbtest import *
+from lldbsuite.test import lldbutil
+
+
+class FPNaNTestCase(TestBase):
+ def setUp(self):
+ # Call super's setUp().
+ TestBase.setUp(self)
+ # Find the line number to break inside main().
+ self.line = line_number("main.cpp", "// Set break point at this line.")
+
+ def test(self):
+ self.build()
+ exe = self.getBuildArtifact("a.out")
+ self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
+
+ # Break inside the main.
+ lldbutil.run_break_set_by_file_and_line(
+ self, "main.cpp", self.line, num_expected_locations=1
+ )
+
+ self.runCmd("run", RUN_SUCCEEDED)
+ # Zero and denorm
+ self.expect(
+ "expr +0.0",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=["double", "0"],
+ )
+ self.expect(
+ "expr -0.0",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=["double", "0"],
+ )
+ self.expect(
+ "expr 0.0 / 0",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=["double", "NaN"],
+ )
+ self.expect(
+ "expr 0 / 0.0",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=["double", "NaN"],
+ )
+ self.expect(
+ "expr 1 / +0.0",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=["double", "+Inf"],
+ )
+ self.expect(
+ "expr 1 / -0.0",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=["double", "-Inf"],
+ )
+ self.expect(
+ "expr +0.0 / +0.0 != +0.0 / +0.0",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=["bool", "true"],
+ )
+ self.expect(
+ "expr -1.f * 0",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=["float", "-0"],
+ )
+ self.expect(
+ "expr 0x0.123p-1",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=["double", "0.0355224609375"],
+ )
+ # NaN
+ self.expect(
+ "expr fnan < fnan",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=["bool", "false"],
+ )
+ self.expect(
+ "expr fnan <= fnan",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=["bool", "false"],
+ )
+ self.expect(
+ "expr fnan > fnan",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=["bool", "false"],
+ )
+ self.expect(
+ "expr fnan >= fnan",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=["bool", "false"],
+ )
+ self.expect(
+ "expr fnan == fnan",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=["bool", "false"],
+ )
+ self.expect(
+ "expr fnan != fnan",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=["bool", "true"],
+ )
+ self.expect(
+ "expr 1.0 <= fnan",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=["bool", "false"],
+ )
+ self.expect(
+ "expr 1.0f < fnan",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=["bool", "false"],
+ )
+ self.expect(
+ "expr 1.0f != fnan",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=["bool", "true"],
+ )
+ self.expect(
+ "expr (unsigned int) fdenorm",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=["int", "0"],
+ )
+ self.expect(
+ "expr (unsigned int) (1.0f + fdenorm)",
+ VARIABLES_DISPLAYED_CORRECTLY,
+ substrs=["int", "1"],
+ )
diff --git a/lldb/test/API/lang/cpp/fpnan/main.cpp b/lldb/test/API/lang/cpp/fpnan/main.cpp
new file mode 100644
index 00000000000000..8bcfebfaea8e1e
--- /dev/null
+++ b/lldb/test/API/lang/cpp/fpnan/main.cpp
@@ -0,0 +1,8 @@
+#include <limits>
+
+int main() {
+ float fnan = std::numeric_limits<float>::quiet_NaN();
+ float fdenorm = std::numeric_limits<float>::denorm_min();
+
+ // Set break point at this line.
+}
>From 65063082cb416d36bb8764eece27ec54b944a97d Mon Sep 17 00:00:00 2001
From: Ilia Kuklin <ikuklin at accesssoftek.com>
Date: Thu, 12 Sep 2024 02:43:51 +0500
Subject: [PATCH 2/2] Add a compare function and use it in all operators
---
lldb/include/lldb/Utility/Scalar.h | 18 ++++---
lldb/source/Utility/Scalar.cpp | 83 +++++++++---------------------
2 files changed, 33 insertions(+), 68 deletions(-)
diff --git a/lldb/include/lldb/Utility/Scalar.h b/lldb/include/lldb/Utility/Scalar.h
index a8fe7fee3a2b9f..b4b9c7e1895825 100644
--- a/lldb/include/lldb/Utility/Scalar.h
+++ b/lldb/include/lldb/Utility/Scalar.h
@@ -210,6 +210,7 @@ class Scalar {
static PromotionKey GetFloatPromoKey(const llvm::fltSemantics &semantics);
private:
+ friend llvm::APFloat::cmpResult compare(Scalar lhs, Scalar rhs);
friend const Scalar operator+(const Scalar &lhs, const Scalar &rhs);
friend const Scalar operator-(Scalar lhs, Scalar rhs);
friend const Scalar operator/(Scalar lhs, Scalar rhs);
@@ -220,12 +221,12 @@ class Scalar {
friend const Scalar operator^(Scalar lhs, Scalar rhs);
friend const Scalar operator<<(const Scalar &lhs, const Scalar &rhs);
friend const Scalar operator>>(const Scalar &lhs, const Scalar &rhs);
- friend bool operator==(Scalar lhs, Scalar rhs);
+ friend bool operator==(const Scalar &lhs, const Scalar &rhs);
friend bool operator!=(const Scalar &lhs, const Scalar &rhs);
- friend bool operator<(Scalar lhs, Scalar rhs);
- friend bool operator<=(Scalar lhs, Scalar rhs);
+ friend bool operator<(const Scalar &lhs, const Scalar &rhs);
+ friend bool operator<=(const Scalar &lhs, const Scalar &rhs);
friend bool operator>(const Scalar &lhs, const Scalar &rhs);
- friend bool operator>=(Scalar lhs, Scalar rhs);
+ friend bool operator>=(const Scalar &lhs, const Scalar &rhs);
};
// Split out the operators into a format where the compiler will be able to
@@ -241,6 +242,7 @@ class Scalar {
// Item 19 of "Effective C++ Second Edition" by Scott Meyers
// Differentiate among members functions, non-member functions, and
// friend functions
+llvm::APFloat::cmpResult compare(Scalar lhs, Scalar rhs);
const Scalar operator+(const Scalar &lhs, const Scalar &rhs);
const Scalar operator-(Scalar lhs, Scalar rhs);
const Scalar operator/(Scalar lhs, Scalar rhs);
@@ -251,12 +253,12 @@ const Scalar operator%(Scalar lhs, Scalar rhs);
const Scalar operator^(Scalar lhs, Scalar rhs);
const Scalar operator<<(const Scalar &lhs, const Scalar &rhs);
const Scalar operator>>(const Scalar &lhs, const Scalar &rhs);
-bool operator==(Scalar lhs, Scalar rhs);
+bool operator==(const Scalar &lhs, const Scalar &rhs);
bool operator!=(const Scalar &lhs, const Scalar &rhs);
-bool operator<(Scalar lhs, Scalar rhs);
-bool operator<=(Scalar lhs, Scalar rhs);
+bool operator<(const Scalar &lhs, const Scalar &rhs);
+bool operator<=(const Scalar &lhs, const Scalar &rhs);
bool operator>(const Scalar &lhs, const Scalar &rhs);
-bool operator>=(Scalar lhs, Scalar rhs);
+bool operator>=(const Scalar &lhs, const Scalar &rhs);
llvm::raw_ostream &operator<<(llvm::raw_ostream &os, const Scalar &scalar);
diff --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp
index bdbd79d375a4f7..f07a9f3bed00c7 100644
--- a/lldb/source/Utility/Scalar.cpp
+++ b/lldb/source/Utility/Scalar.cpp
@@ -852,87 +852,50 @@ llvm::APFloat Scalar::CreateAPFloatFromAPFloat(lldb::BasicType basic_type) {
}
}
-bool lldb_private::operator==(Scalar lhs, Scalar rhs) {
+APFloat::cmpResult lldb_private::compare(Scalar lhs, Scalar rhs) {
// If either entry is void then we can just compare the types
if (lhs.m_type == Scalar::e_void || rhs.m_type == Scalar::e_void)
- return lhs.m_type == rhs.m_type;
+ return lhs.m_type == rhs.m_type ? APFloat::cmpEqual : APFloat::cmpUnordered;
- llvm::APFloat::cmpResult result;
switch (Scalar::PromoteToMaxType(lhs, rhs)) {
case Scalar::e_void:
break;
case Scalar::e_int:
- return lhs.m_integer == rhs.m_integer;
+ if (lhs.m_integer < rhs.m_integer)
+ return APFloat::cmpLessThan;
+ if (lhs.m_integer > rhs.m_integer)
+ return APFloat::cmpGreaterThan;
+ return APFloat::cmpEqual;
case Scalar::e_float:
- result = lhs.m_float.compare(rhs.m_float);
- if (result == llvm::APFloat::cmpEqual)
- return true;
+ return lhs.m_float.compare(rhs.m_float);
}
- return false;
+ return APFloat::cmpUnordered;
}
-bool lldb_private::operator!=(const Scalar &lhs, const Scalar &rhs) {
- return !(lhs == rhs);
+bool lldb_private::operator==(const Scalar &lhs, const Scalar &rhs) {
+ return compare(lhs, rhs) == APFloat::cmpEqual;
}
-bool lldb_private::operator<(Scalar lhs, Scalar rhs) {
- if (lhs.m_type == Scalar::e_void || rhs.m_type == Scalar::e_void)
- return false;
-
- llvm::APFloat::cmpResult result;
- switch (Scalar::PromoteToMaxType(lhs, rhs)) {
- case Scalar::e_void:
- break;
- case Scalar::e_int:
- return lhs.m_integer < rhs.m_integer;
- case Scalar::e_float:
- result = lhs.m_float.compare(rhs.m_float);
- if (result == llvm::APFloat::cmpLessThan)
- return true;
- }
- return false;
+bool lldb_private::operator!=(const Scalar &lhs, const Scalar &rhs) {
+ return compare(lhs, rhs) != APFloat::cmpEqual;
}
-bool lldb_private::operator<=(Scalar lhs, Scalar rhs) {
- if (lhs.m_type == Scalar::e_void || rhs.m_type == Scalar::e_void)
- return false;
+bool lldb_private::operator<(const Scalar &lhs, const Scalar &rhs) {
+ return compare(lhs, rhs) == APFloat::cmpLessThan;
+}
- llvm::APFloat::cmpResult result;
- switch (Scalar::PromoteToMaxType(lhs, rhs)) {
- case Scalar::e_void:
- break;
- case Scalar::e_int:
- return lhs.m_integer <= rhs.m_integer;
- case Scalar::e_float:
- result = lhs.m_float.compare(rhs.m_float);
- if (result == llvm::APFloat::cmpLessThan ||
- result == llvm::APFloat::cmpEqual)
- return true;
- }
- return false;
+bool lldb_private::operator<=(const Scalar &lhs, const Scalar &rhs) {
+ APFloat::cmpResult Res = compare(lhs, rhs);
+ return Res == APFloat::cmpLessThan || Res == APFloat::cmpEqual;
}
bool lldb_private::operator>(const Scalar &lhs, const Scalar &rhs) {
- return rhs < lhs;
+ return compare(lhs, rhs) == APFloat::cmpGreaterThan;
}
-bool lldb_private::operator>=(Scalar lhs, Scalar rhs) {
- if (lhs.m_type == Scalar::e_void || rhs.m_type == Scalar::e_void)
- return false;
-
- llvm::APFloat::cmpResult result;
- switch (Scalar::PromoteToMaxType(lhs, rhs)) {
- case Scalar::e_void:
- break;
- case Scalar::e_int:
- return lhs.m_integer >= rhs.m_integer;
- case Scalar::e_float:
- result = lhs.m_float.compare(rhs.m_float);
- if (result == llvm::APFloat::cmpGreaterThan ||
- result == llvm::APFloat::cmpEqual)
- return true;
- }
- return false;
+bool lldb_private::operator>=(const Scalar &lhs, const Scalar &rhs) {
+ APFloat::cmpResult Res = compare(lhs, rhs);
+ return Res == APFloat::cmpGreaterThan || Res == APFloat::cmpEqual;
}
bool Scalar::ClearBit(uint32_t bit) {
More information about the lldb-commits
mailing list