Skip to content

Fix long double inf test under Valgrind. Fixes #5175#5176

Merged
nlohmann merged 17 commits into
nlohmann:developfrom
RUSLoker:patch-1
May 17, 2026
Merged

Fix long double inf test under Valgrind. Fixes #5175#5176
nlohmann merged 17 commits into
nlohmann:developfrom
RUSLoker:patch-1

Conversation

@RUSLoker

@RUSLoker RUSLoker commented May 16, 2026

Copy link
Copy Markdown
Contributor

What

ci_test_valgrind started failing after #3929 added long-double serialization coverage. The two failing assertions are:

CHECK(long_double_json( std::numeric_limits<long double>::infinity()).dump() == "null");
CHECK(long_double_json(-std::numeric_limits<long double>::infinity()).dump() == "null");

doctest reports:

values: CHECK( 1.18973149535723176509e+4932 == null )

This PR keeps the assertions but guards them with a runtime probe, so the section continues to validate the production behavior on every real platform while not failing under Valgrind:

volatile long double inf_probe = std::numeric_limits<long double>::infinity();
if (!std::isfinite(inf_probe))
{
    CHECK(long_double_json( std::numeric_limits<long double>::infinity()).dump() == "null");
    CHECK(long_double_json(-std::numeric_limits<long double>::infinity()).dump() == "null");
}

Only tests/src/unit-serialization.cpp is touched (+15/−2). No library code is modified.

Why

The library itself is correct — dump_float() already calls if (!std::isfinite(x)) { write "null"; return; }. The failure is in Valgrind, not in the JSON code.

Valgrind has had incomplete 80-bit x87 long-double support since 2009 (Valgrind bug #197915, status ASSIGNED — Valgrind tracks its bugs on bugs.kde.org). On every long-double operation Valgrind round-trips through 64-bit double, and +inf/-inf come back out looking like LDBL_MAX. Reproduced locally with g++ 13.3 / valgrind 3.22.0 on Ubuntu 24.04 — the bit pattern of std::numeric_limits<long double>::infinity() is preserved in memory, but std::isfinite() returns true, std::isinf() returns false, and snprintf("%Lg", …) prints the LDBL_MAX decimal instead of "inf". That's exactly the symptom in the issue.

Why a runtime probe (and why volatile)

  • The guard asks the only question that actually matters here — "does this platform's std::isfinite() recognize long-double infinity?" — so it adapts to any toolchain that exhibits the same defect, with no dependency on Valgrind-specific headers or build flags. The test file stays standard C++.
  • volatile is load-bearing. Without it, std::isfinite of a constexpr infinity is folded to false at compile time and the guard becomes dead code. volatile forces a runtime read through the same FP path dump_float() exercises, so the probe truly reflects platform behavior.
  • The NaN assertion still runs everywhere — Valgrind handles long-double NaN correctly; only +/-inf is mishandled.
  • A TODO next to the guard links to the upstream Valgrind bug so the workaround can be removed once Valgrind ships proper 80-bit support and the project bumps its minimum Valgrind version.

Verification

Tested against the matrix of compilers/standards the CI exercises (Ubuntu 24.04 / GCC 13.3 / Clang 18.1 / Valgrind 3.22.0):

Configuration Normal Under Valgrind
GCC, C++11/14/17/20/23 31/31 ✓ 29/29 ✓ (2 inf assertions probe-skipped, NaN still run)
GCC, project's strict warning set including -Wvolatile, -Werror clean
Clang 18 / C++20 31/31 ✓ unchanged from before this PR (pre-existing round-trip issue, unrelated; ci_test_valgrind uses GCC)
ctest -L valgrind -R test-serialization_cpp11_valgrind (CI target) Passed

Full test-serialization_cpp11 under Valgrind: 7/7 cases pass, 105/105 assertions pass, no Memcheck errors.


Read the Contribution Guidelines for detailed information.

When you use long double as a floating point type with the current version of this file and try to dump json it prints trash instead of actual number. This if-else fixes the problem. On using long double you just need to add an 'L' modifier before 'g' in format string.

Signed-off-by: Kirill Lokotkov <klokotkov@ya.ru>
Signed-off-by: Kirill Lokotkov <klokotkov@ya.ru>
Signed-off-by: Kirill Lokotkov <klokotkov@ya.ru>
Signed-off-by: rusloker <klokotkov@ya.ru>
Signed-off-by: rusloker <klokotkov@ya.ru>
# Conflicts:
#	include/nlohmann/detail/output/serializer.hpp
#	single_include/nlohmann/json.hpp
Signed-off-by: rusloker <klokotkov@ya.ru>
…type safety

Signed-off-by: rusloker <klokotkov@ya.ru>
Signed-off-by: rusloker <klokotkov@ya.ru>
Signed-off-by: rusloker <klokotkov@ya.ru>
Signed-off-by: rusloker <klokotkov@ya.ru>

@nlohmann nlohmann left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@nlohmann nlohmann left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are 2 Clang-Tidy warnings to be fixed:

/__w/json/json/tests/src/unit-serialization.cpp:353:9: error: missing username/bug in TODO [google-readability-todo,-warnings-as-errors]
  353 |         // TODO: remove this guard once Valgrind's 80-bit long double support
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |         // TODO(unknown): remove this guard once Valgrind's 80-bit long double support
/__w/json/json/tests/src/unit-serialization.cpp:357:9: error: variable 'inf_probe' of type 'volatile long double' can be declared 'const' [misc-const-correctness,-warnings-as-errors]
  357 |         volatile long double inf_probe = std::numeric_limits<long double>::infinity();
      |         ^
      |                              const 
431 warnings generated.

Signed-off-by: rusloker <klokotkov@ya.ru>
@RUSLoker RUSLoker requested a review from nlohmann May 16, 2026 17:47

@nlohmann nlohmann left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@nlohmann nlohmann added this to the Release 3.12.1 milestone May 17, 2026
@nlohmann nlohmann merged commit 4e5fa3b into nlohmann:develop May 17, 2026
143 checks passed
@nlohmann

Copy link
Copy Markdown
Owner

Thanks for the quick fix!

GhostVaibhav pushed a commit to GhostVaibhav/json that referenced this pull request May 19, 2026
…ann#5176)

* Fix for printing long doubles bug in dump_float

When you use long double as a floating point type with the current version of this file and try to dump json it prints trash instead of actual number. This if-else fixes the problem. On using long double you just need to add an 'L' modifier before 'g' in format string.

Signed-off-by: Kirill Lokotkov <klokotkov@ya.ru>

* C++11 compatibility

Signed-off-by: Kirill Lokotkov <klokotkov@ya.ru>

* Shorter solution

Signed-off-by: Kirill Lokotkov <klokotkov@ya.ru>

* Applied amalgamate

Signed-off-by: rusloker <klokotkov@ya.ru>

* Add unit tests for `dump()` with `long double` in custom `basic_json`

Signed-off-by: rusloker <klokotkov@ya.ru>

* Fix UB in `snprintf_float` by using `%.*Lg` for `long double`

Signed-off-by: rusloker <klokotkov@ya.ru>

* Use `std::array` for `values` in serialization unit tests to improve type safety

Signed-off-by: rusloker <klokotkov@ya.ru>

* Fix brace initialization for `std::array` in serialization unit tests

Signed-off-by: rusloker <klokotkov@ya.ru>

* Remove comments in `snprintf_float` regarding `%Lg` usage

Signed-off-by: rusloker <klokotkov@ya.ru>

* Skip `long double` infinity dump assertions under Valgrind

Signed-off-by: rusloker <klokotkov@ya.ru>

* Clarify Valgrind bug-tracker reference in `long double` test

Signed-off-by: rusloker <klokotkov@ya.ru>

* Satisfy clang-tidy in `long double` infinity probe

Signed-off-by: rusloker <klokotkov@ya.ru>

---------

Signed-off-by: Kirill Lokotkov <klokotkov@ya.ru>
Signed-off-by: rusloker <klokotkov@ya.ru>
Signed-off-by: Vaibhav Sharma <48472541+GhostVaibhav@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Test "dump for basic_json with long double number_float_t" fails when run with Valgrind

2 participants