Skip to content

Commit

Permalink
[LLDB] Fix operators <= and >= returning a wrong result when comparin…
Browse files Browse the repository at this point in the history
…g to a floating point NaN (llvm#108060)

Implement operators `<=` and `>=` to explicitly check the comparison
results to be `cmpLessThan` or `cmpEqual` instead of negating the result
of `operators<`.

Fixes llvm#85947
  • Loading branch information
kuilpd authored Sep 18, 2024
1 parent 13b4d1b commit 9690b30
Show file tree
Hide file tree
Showing 5 changed files with 168 additions and 32 deletions.
10 changes: 6 additions & 4 deletions lldb/include/lldb/Utility/Scalar.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -220,9 +221,9 @@ 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<(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>=(const Scalar &lhs, const Scalar &rhs);
Expand All @@ -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);
Expand All @@ -251,9 +253,9 @@ 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<(const Scalar &lhs, const 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);
Expand Down
49 changes: 21 additions & 28 deletions lldb/source/Utility/Scalar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -852,57 +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;
bool lldb_private::operator!=(const Scalar &lhs, const Scalar &rhs) {
return compare(lhs, rhs) != APFloat::cmpEqual;
}

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::cmpLessThan;
}

bool lldb_private::operator<=(const Scalar &lhs, const Scalar &rhs) {
return !(rhs < lhs);
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>=(const Scalar &lhs, const Scalar &rhs) {
return !(lhs < rhs);
APFloat::cmpResult Res = compare(lhs, rhs);
return Res == APFloat::cmpGreaterThan || Res == APFloat::cmpEqual;
}

bool Scalar::ClearBit(uint32_t bit) {
Expand Down
3 changes: 3 additions & 0 deletions lldb/test/API/lang/cpp/fpnan/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
CXX_SOURCES := main.cpp

include Makefile.rules
130 changes: 130 additions & 0 deletions lldb/test/API/lang/cpp/fpnan/TestFPNaN.py
Original file line number Diff line number Diff line change
@@ -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"],
)
8 changes: 8 additions & 0 deletions lldb/test/API/lang/cpp/fpnan/main.cpp
Original file line number Diff line number Diff line change
@@ -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.
}

0 comments on commit 9690b30

Please sign in to comment.