[Lldb-commits] [lldb] ae58cf5 - [lldb] Fix that LLDB doesn't print NaN's sign on Darwin

[Raphael Isemann via lldb-commits]

Author: Raphael Isemann
Date: 2021-05-25T13:33:28+02:00
New Revision: ae58cf5f45a9c159afbf86e93c0c257a22c4ee02

URL: https://github.com/llvm/llvm-project/commit/ae58cf5f45a9c159afbf86e93c0c257a22c4ee02
DIFF: https://github.com/llvm/llvm-project/commit/ae58cf5f45a9c159afbf86e93c0c257a22c4ee02.diff

LOG: [lldb] Fix that LLDB doesn't print NaN's sign on Darwin

It seems std::ostringstream ignores NaN signs on Darwin while it prints them on
Linux. This causes that LLDB behaves differently on those platforms which is
both confusing for users and it also means we have to deal with that in our
tests.

This patch manually implements the NaN/Inf printing (which are apparently
implementation defined) to make LLDB print the same thing on all platforms. The
only output difference in practice seems to be that we now print negative NaNs
as `-nan`, but this potentially also changes the output on other systems I
haven't tested this on.

Reviewed By: JDevlieghere

Differential Revision: https://reviews.llvm.org/D102845

Added: 
    

Modified: 
    lldb/source/Core/DumpDataExtractor.cpp
    lldb/unittests/Core/DumpDataExtractorTest.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/source/Core/DumpDataExtractor.cpp b/lldb/source/Core/DumpDataExtractor.cpp
index 34c9353c9feaa..63e2cf2f2cc1e 100644
--- a/lldb/source/Core/DumpDataExtractor.cpp
+++ b/lldb/source/Core/DumpDataExtractor.cpp
@@ -37,7 +37,7 @@
 #include <assert.h>
 #include <ctype.h>
 #include <inttypes.h>
-#include <math.h>
+#include <cmath>
 
 #include <bitset>
 #include <sstream>
@@ -230,6 +230,29 @@ static void DumpCharacter(Stream &s, const char c) {
   s.Printf("\\x%2.2x", c);
 }
 
+/// Dump a floating point type.
+template <typename FloatT>
+void DumpFloatingPoint(std::ostringstream &ss, FloatT f) {
+  static_assert(std::is_floating_point<FloatT>::value,
+                "Only floating point types can be dumped.");
+  // NaN and Inf are potentially implementation defined and on Darwin it
+  // seems NaNs are printed without their sign. Manually implement dumping them
+  // here to avoid having to deal with platform 
diff erences.
+  if (std::isnan(f)) {
+    if (std::signbit(f))
+      ss << '-';
+    ss << "nan";
+    return;
+  }
+  if (std::isinf(f)) {
+    if (std::signbit(f))
+      ss << '-';
+    ss << "inf";
+    return;
+  }
+  ss << f;
+}
+
 lldb::offset_t lldb_private::DumpDataExtractor(
     const DataExtractor &DE, Stream *s, offset_t start_offset,
     lldb::Format item_format, size_t item_byte_size, size_t item_count,
@@ -570,14 +593,14 @@ lldb::offset_t lldb_private::DumpDataExtractor(
             f = DE.GetFloat(&offset);
           }
           ss.precision(std::numeric_limits<float>::digits10);
-          ss << f;
+          DumpFloatingPoint(ss, f);
         } else if (item_byte_size == sizeof(double)) {
           ss.precision(std::numeric_limits<double>::digits10);
-          ss << DE.GetDouble(&offset);
+          DumpFloatingPoint(ss, DE.GetDouble(&offset));
         } else if (item_byte_size == sizeof(long double) ||
                    item_byte_size == 10) {
           ss.precision(std::numeric_limits<long double>::digits10);
-          ss << DE.GetLongDouble(&offset);
+          DumpFloatingPoint(ss, DE.GetLongDouble(&offset));
         } else {
           s->Printf("error: unsupported byte size (%" PRIu64
                     ") for float format",

diff  --git a/lldb/unittests/Core/DumpDataExtractorTest.cpp b/lldb/unittests/Core/DumpDataExtractorTest.cpp
index cfd24ff41491b..f76014aa938c0 100644
--- a/lldb/unittests/Core/DumpDataExtractorTest.cpp
+++ b/lldb/unittests/Core/DumpDataExtractorTest.cpp
@@ -13,6 +13,7 @@
 #include "lldb/Utility/StreamString.h"
 #include "gtest/gtest.h"
 #include <complex>
+#include <limits>
 
 using namespace lldb;
 using namespace lldb_private;
@@ -197,13 +198,34 @@ TEST(DumpDataExtractorTest, Formats) {
            lldb::Format::eFormatVectorOfFloat16, "{6.10352e-05 65504}");
   TestDump(std::vector<uint16_t>{0xabcd, 0x1234},
            lldb::Format::eFormatVectorOfFloat16, "{-0.0609436 0.000757217}");
+
+  // quiet/signaling NaNs.
+  TestDump(std::vector<uint16_t>{0xffff, 0xffc0, 0x7fff, 0x7fc0},
+           lldb::Format::eFormatVectorOfFloat16, "{-nan -nan nan nan}");
+  // +/-Inf.
+  TestDump(std::vector<uint16_t>{0xfc00, 0x7c00},
+           lldb::Format::eFormatVectorOfFloat16, "{-inf inf}");
+
   TestDump(std::vector<float>{std::numeric_limits<float>::min(),
                               std::numeric_limits<float>::max()},
            lldb::Format::eFormatVectorOfFloat32, "{1.17549e-38 3.40282e+38}");
+  TestDump(std::vector<float>{std::numeric_limits<float>::quiet_NaN(),
+                              std::numeric_limits<float>::signaling_NaN(),
+                              -std::numeric_limits<float>::quiet_NaN(),
+                              -std::numeric_limits<float>::signaling_NaN()},
+           lldb::Format::eFormatVectorOfFloat32, "{nan nan -nan -nan}");
   TestDump(std::vector<double>{std::numeric_limits<double>::min(),
                                std::numeric_limits<double>::max()},
            lldb::Format::eFormatVectorOfFloat64,
            "{2.2250738585072e-308 1.79769313486232e+308}");
+  TestDump(
+      std::vector<double>{
+          std::numeric_limits<double>::quiet_NaN(),
+          std::numeric_limits<double>::signaling_NaN(),
+          -std::numeric_limits<double>::quiet_NaN(),
+          -std::numeric_limits<double>::signaling_NaN(),
+      },
+      lldb::Format::eFormatVectorOfFloat64, "{nan nan -nan -nan}");
 
   // Not sure we can rely on having uint128_t everywhere so emulate with
   // uint64_t.