Skip to content

[libc] Fix printf long double inf, bitcast in msan #70067

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 24, 2023

Conversation

michaelrj-google
Copy link
Contributor

These bugs were found with the new printf long double fuzzing. The long
double inf vs nan bug was introduced when we changed to
get_explicit_exponent. The bitcast msan issue hadn't come up previously,
but isn't a real bug, just a poisoning confusion.

These bugs were found with the new printf long double fuzzing. The long
double inf vs nan bug was introduced when we changed to
get_explicit_exponent. The bitcast msan issue hadn't come up previously,
but isn't a real bug, just a poisoning confusion.
@llvmbot llvmbot added the libc label Oct 24, 2023
@llvmbot
Copy link
Member

llvmbot commented Oct 24, 2023

@llvm/pr-subscribers-libc

Author: None (michaelrj-google)

Changes

These bugs were found with the new printf long double fuzzing. The long
double inf vs nan bug was introduced when we changed to
get_explicit_exponent. The bitcast msan issue hadn't come up previously,
but isn't a real bug, just a poisoning confusion.


Full diff: https://github.com/llvm/llvm-project/pull/70067.diff

5 Files Affected:

  • (modified) libc/src/__support/CPP/CMakeLists.txt (+4-1)
  • (modified) libc/src/__support/CPP/bit.h (+2)
  • (modified) libc/src/stdio/printf_core/float_inf_nan_converter.h (+2-2)
  • (modified) libc/test/src/stdio/sprintf_test.cpp (+16-1)
  • (modified) utils/bazel/llvm-project-overlay/libc/BUILD.bazel (+1)
diff --git a/libc/src/__support/CPP/CMakeLists.txt b/libc/src/__support/CPP/CMakeLists.txt
index a2d3bd7df9e9069..6853bfa3b304a34 100644
--- a/libc/src/__support/CPP/CMakeLists.txt
+++ b/libc/src/__support/CPP/CMakeLists.txt
@@ -15,7 +15,10 @@ add_header_library(
   HDRS
     bit.h
   DEPENDS
-    libc.src.__support.macros.properties.compiler
+    .type_traits
+    libc.src.__support.macros.attributes
+    libc.src.__support.macros.config
+    libc.src.__support.macros.sanitizer
 )
 
 add_header_library(
diff --git a/libc/src/__support/CPP/bit.h b/libc/src/__support/CPP/bit.h
index 456266626ae9b20..f6f3131c1ccfd81 100644
--- a/libc/src/__support/CPP/bit.h
+++ b/libc/src/__support/CPP/bit.h
@@ -12,6 +12,7 @@
 #include "src/__support/CPP/type_traits.h"
 #include "src/__support/macros/attributes.h"
 #include "src/__support/macros/config.h" // LIBC_HAS_BUILTIN
+#include "src/__support/macros/sanitizer.h"
 
 namespace LIBC_NAMESPACE::cpp {
 
@@ -31,6 +32,7 @@ LIBC_INLINE constexpr To bit_cast(const From &from) {
   static_assert(cpp::is_trivially_copyable<To>::value &&
                     cpp::is_trivially_copyable<From>::value,
                 "Cannot bit-cast instances of non-trivially copyable classes.");
+  MSAN_UNPOISON(&from, sizeof(From));
 #if defined(LLVM_LIBC_HAS_BUILTIN_BIT_CAST)
   return __builtin_bit_cast(To, from);
 #else
diff --git a/libc/src/stdio/printf_core/float_inf_nan_converter.h b/libc/src/stdio/printf_core/float_inf_nan_converter.h
index 3c24c393b354834..27a034cdcab1b72 100644
--- a/libc/src/stdio/printf_core/float_inf_nan_converter.h
+++ b/libc/src/stdio/printf_core/float_inf_nan_converter.h
@@ -34,13 +34,13 @@ LIBC_INLINE int convert_inf_nan(Writer *writer, const FormatSection &to_conv) {
     fputil::FPBits<long double>::UIntType float_raw = to_conv.conv_val_raw;
     fputil::FPBits<long double> float_bits(float_raw);
     is_negative = float_bits.get_sign();
-    mantissa = float_bits.get_explicit_mantissa();
+    mantissa = float_bits.get_mantissa();
   } else {
     fputil::FPBits<double>::UIntType float_raw =
         static_cast<fputil::FPBits<double>::UIntType>(to_conv.conv_val_raw);
     fputil::FPBits<double> float_bits(float_raw);
     is_negative = float_bits.get_sign();
-    mantissa = float_bits.get_explicit_mantissa();
+    mantissa = float_bits.get_mantissa();
   }
 
   char sign_char = 0;
diff --git a/libc/test/src/stdio/sprintf_test.cpp b/libc/test/src/stdio/sprintf_test.cpp
index f3d5dd698cbea44..5aa346c414b1fe1 100644
--- a/libc/test/src/stdio/sprintf_test.cpp
+++ b/libc/test/src/stdio/sprintf_test.cpp
@@ -572,7 +572,6 @@ TEST_F(LlvmLibcSPrintfTest, FloatHexExpConv) {
   ForceRoundingMode r(RoundingMode::Nearest);
   double inf = LIBC_NAMESPACE::fputil::FPBits<double>::inf().get_val();
   double nan = LIBC_NAMESPACE::fputil::FPBits<double>::build_nan(1);
-
   written = LIBC_NAMESPACE::sprintf(buff, "%a", 1.0);
   ASSERT_STREQ_LEN(written, buff, "0x1p+0");
 
@@ -937,6 +936,10 @@ TEST_F(LlvmLibcSPrintfTest, FloatDecimalConv) {
   ForceRoundingMode r(RoundingMode::Nearest);
   double inf = LIBC_NAMESPACE::fputil::FPBits<double>::inf().get_val();
   double nan = LIBC_NAMESPACE::fputil::FPBits<double>::build_nan(1);
+  long double ld_inf =
+      LIBC_NAMESPACE::fputil::FPBits<long double>::inf().get_val();
+  long double ld_nan =
+      LIBC_NAMESPACE::fputil::FPBits<long double>::build_nan(1);
 
   char big_buff[10000]; // Used for long doubles and other extremely wide
                         // numbers.
@@ -996,6 +999,18 @@ TEST_F(LlvmLibcSPrintfTest, FloatDecimalConv) {
   written = LIBC_NAMESPACE::sprintf(buff, "%F", -nan);
   ASSERT_STREQ_LEN(written, buff, "-NAN");
 
+  written = LIBC_NAMESPACE::sprintf(buff, "%Lf", ld_inf);
+  ASSERT_STREQ_LEN(written, buff, "inf");
+
+  written = LIBC_NAMESPACE::sprintf(buff, "%LF", -ld_inf);
+  ASSERT_STREQ_LEN(written, buff, "-INF");
+
+  written = LIBC_NAMESPACE::sprintf(buff, "%Lf", ld_nan);
+  ASSERT_STREQ_LEN(written, buff, "nan");
+
+  written = LIBC_NAMESPACE::sprintf(buff, "%LF", -ld_nan);
+  ASSERT_STREQ_LEN(written, buff, "-NAN");
+
   // Length Modifier Tests.
 
   // TODO(michaelrj): Add tests for LONG_DOUBLE_IS_DOUBLE and 128 bit long
diff --git a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
index fdf60cd0df89c28..3ae68193dccd2b2 100644
--- a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
@@ -170,6 +170,7 @@ libc_support_library(
         ":__support_cpp_type_traits",
         ":__support_macros_attributes",
         ":__support_macros_config",
+        ":__support_macros_sanitizer",
         ":libc_root",
     ],
 )

@michaelrj-google michaelrj-google merged commit b4e5529 into llvm:main Oct 24, 2023
@michaelrj-google michaelrj-google deleted the libcPrintfLDNaN branch November 1, 2023 22:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants