Skip to content

Commit

Permalink
[libc] Fix printf long double inf, bitcast in msan (#70067)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
michaelrj-google committed Oct 24, 2023
1 parent 567a660 commit b4e5529
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 4 deletions.
5 changes: 4 additions & 1 deletion libc/src/__support/CPP/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 2 additions & 0 deletions libc/src/__support/CPP/bit.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions libc/src/stdio/printf_core/float_inf_nan_converter.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
17 changes: 16 additions & 1 deletion libc/test/src/stdio/sprintf_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions utils/bazel/llvm-project-overlay/libc/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ libc_support_library(
":__support_cpp_type_traits",
":__support_macros_attributes",
":__support_macros_config",
":__support_macros_sanitizer",
":libc_root",
],
)
Expand Down

0 comments on commit b4e5529

Please sign in to comment.