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

[teemperor]

Summary

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.

[teemperor]

That's the wrong set of tests...

[teemperor]

• Now uploaded the right tests.

[JDevlieghere]

LGTM

lldb/source/Core/DumpDataExtractor.cpp

238–240

Seems like this could be part of the doxygen comment above?

[shafik]

LGTM besides my minor comment on headers. Thanks for fixing this!

lldb/source/Core/DumpDataExtractor.cpp

241

We are not explicitly including <cmath> for std::isnan we should replace math.h include. We should replace all the C header includes for C++ ones but that feels out of scope for this change.

[shafik]

LGTM besides my minor comment on headers. Thanks for fixing this!

lldb/unittests/Core/DumpDataExtractorTest.cpp

212

We should explicitly include <limits>

lldb/source/Core/DumpDataExtractor.cpp

 
 #include <limits>
 #include <memory>
 #include <string>
 
 #include <assert.h>
 #include <ctype.h>
 #include <inttypes.h>
-#include <math.h>
+#include <cmath>
 
 #include <bitset>
 #include <sstream>
 
 using namespace lldb_private;
 using namespace lldb;
 
 #define NON_PRINTABLE_CHAR '.'
if (TryDumpSpecialEscapedChar(s, c))
     return;
   if (llvm::isPrint(c)) {
     s.PutChar(c);
     return;
   }
   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 differences.
+  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,
     size_t num_per_line, uint64_t base_addr,
     uint32_t item_bit_size,   // If zero, this is not a bitfield value, if
                               // non-zero, the value is a bitfield
     uint32_t item_bit_offset, // If "item_bit_size" is non-zero, this is the
                               // shift amount to apply to a bitfield
case eFormatFloat: {
           float f;
           if (item_byte_size == 2) {
             uint16_t half = DE.GetU16(&offset);
             f = half2float(half);
           } else {
             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",
                     (uint64_t)item_byte_size);
           return offset;
         }
         ss.flush();
         s->Printf("%s", ss.str().c_str());

lldb/unittests/Core/DumpDataExtractorTest.cpp

 //===-- DataDumpExtractorTest.cpp -----------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Core/DumpDataExtractor.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "lldb/Utility/DataExtractor.h"
 #include "lldb/Utility/Endian.h"
 #include "lldb/Utility/StreamString.h"
 #include "gtest/gtest.h"
 #include <complex>
+#include <limits>
 
 using namespace lldb;
 using namespace lldb_private;
 
 static void TestDumpWithAddress(uint64_t base_addr, size_t item_count,
                                 llvm::StringRef expected) {
   std::vector<uint8_t> data{0x11, 0x22};
   StreamString result;

 #else
            "{3.14062}");
 #endif
   // Largest and smallest normal number.
   TestDump(std::vector<uint16_t>{0x0400, 0x7bff},
            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.
   TestDump(
       std::vector<uint64_t>{0x1, 0x1111222233334444, 0xaaaabbbbccccdddd, 0x0},
       lldb::Format::eFormatVectorOfUInt128,
       "{0x11112222333344440000000000000001 "
       "0x0000000000000000aaaabbbbccccdddd}");