Fix stack overflow and exponential runtime when comparing nested values by nlohmann · Pull Request #5390 · nlohmann/json

nlohmann · 2026-08-20T19:30:24Z

What & why

Comparing two values compares their containers, which compare their elements, which brings the comparison back once per nesting level. Two values nested deeply enough exhaust the call stack and terminate the process — the same bug as #5387, in the last operation of basic_json that still had it. (The copy constructor is #5389, dump() is #5285, the destructor was #1436.)

While fixing that, a second and worse problem turned up.

An ordered comparison took exponentially long before C++20

std::vector::operator< is a lexicographical comparison: for each element it asks whether the element is less than its counterpart, and then whether the counterpart is less than it. Both questions recurse into everything below that element, so every level doubles the work.

Measured on develop, comparing two equal values nested n levels deep with <:

depth	`develop`
20	5.7 ms
26	237 ms
30	3778 ms
40	≈ 1 hour (extrapolated)

Nothing about such a value is pathological — 30 levels of nesting is ordinary, and reaching this needs no crafted input, just a < b on two parsed documents. C++20 is unaffected: std::lexicographical_compare_three_way asks once, and operator< is derived from <=> there.

The change

A value nested too deeply to descend into is compared on an explicit stack instead, in a single pass that yields less, equal, greater or unordered at once.

Equality and the three-way comparison descend as they always did for the first 128 levels — nothing measurable changes for them — and finish iteratively below that.
An ordered comparison never descends, which is what takes the exponent out of it.

Equality needs no ordering, so it no longer asks for any: the iterative path is templated on whether the values are being ordered, so a key or string type that can only be compared for equality still compiles.

Reproducing the results exactly

Two subtleties had to be reproduced, both found by differential testing rather than by reading:

std::lexicographical_compare steps over a pair it cannot order — a NaN, say — and carries on with the next element, where std::lexicographical_compare_three_way stops at it. The iterative pass does whichever the caller needs.
An object compares its keys with < where its entries are ordered, but with == where they are only checked for equality — not with the object's own comparator, which for nlohmann::ordered_map is std::equal_to and would report every equal key as "less".

Measured

Medians of 7 interleaved runs, clang -O3, C++11:

workload
two equal values nested 30 deep, `<`	3778 ms → 0.002 ms
ordering flat objects	−33.6%
equality, every shape	unchanged
ordering flat arrays of numbers	+27.3%

The last row is the one shape that pays for the single pass, and it buys the row above it.

Public API impact

No breaking changes. No public signature, type, or exception changes; the comparison helpers are private members. Results are unchanged for every pair of values — see below. Nothing is rejected that was accepted before.

Verification

Differential vs. develop: 68,121 comparisons — every pair drawn from a corpus covering NaN, discarded values, mixed number types, binary values, empty containers, both object types and every operator (==, !=, <, <=, >, >=) — identical in C++11, C++17 and C++20, with and without thread_local storage, and with JSON_USE_LEGACY_DISCARDED_VALUE_COMPARISON.
unit-comparison.cpp (4097 assertions) green in all three standards and in legacy mode; full suite green in every configuration; ASan + UBSan clean; warning-clean under the CI clang flag set.
New tests compare values nested 100,000 deep, for arrays and objects, with every operator.

JSON_NO_THREAD_LOCAL earned its keep here: it forces the iterative path for every value, and it is what caught the ordered_map key-comparator bug above — a divergence that would otherwise only appear below depth 128 and be practically untestable.

Note for reviewers

This is stacked on #5389, which introduces the depth counter this reuses. Retarget to develop once that lands.

binary_writer (to_cbor/to_msgpack/to_ubjson/to_bson) is not in this PR because the four formats need four separate iterative writers, and BSON additionally computes each document's length up front through a second recursive walk. Worth its own change.

What still recurses after this PR

what	where	status
copy constructor (and everything built on it)	`json.hpp`	fixed in #5389
`dump()`	`serializer.hpp`	fixed in #5285
`operator==`, `operator<`, `operator<=>`	`json.hpp:3669`	this PR
`to_cbor` / `to_msgpack` / `to_ubjson` / `to_bson`	`binary_writer.hpp`	open
`basic_json::diff`	`json.hpp:5089`	open
`basic_json::merge_patch`	`json.hpp:5231`	open
`json_pointer::flatten`	`json_pointer.hpp:861`	open

diff, merge_patch and flatten were not previously called out anywhere; they recurse once per nesting level on user-controlled data exactly as copying and comparison did. #5387 should not be closed as fully fixed until they are dealt with too.

The changes are described in detail, both the what and why.
If applicable, an existing issue is referenced. (Stack overflow in copy constructor and dump() on deeply nested json (destructor was fixed in #1436) #5387)
The Code coverage remained at 100%. A test case for every new line of code.
If applicable, the documentation is updated. (the JSON_NO_THREAD_LOCAL page and the macro overview now cover comparison as well as copying, including what the macro costs a comparison)
The source code is amalgamated by running make amalgamate.

Written by Claude Code.

gregmarr · 2026-08-21T04:36:07Z

        return depth;
    }
+
+    /// @brief how many levels the comparison going on in this thread has descended into


Why do you need a separate variable for this, rather than renaming copy_depth to nesting_depth?

gregmarr · 2026-08-21T04:38:10Z

+
+    Does nothing without thread_local storage, where no descent is made at all.
+    */
+    class compare_depth_guard


Why is this needed when you have nesting_depth_guard?

gregmarr · 2026-08-21T04:41:04Z

+    /// is left without the call stack
+    static constexpr std::size_t compare_depth_limit()
+    {
+        return 128;


Needed? It's the same as the other depth limit. Is this so that it can be different in the future?

gregmarr · 2026-08-21T04:41:42Z

+    template<bool Ordered>
+    static compare_result compare_leaves(const_reference lhs, const_reference rhs) noexcept
+    {
+        if (lhs == rhs)


What happens here if one is an array and one is an object?

compare_leaves is only reached when the pair is not both arrays or both objects, so lhs == rhs hits operator=='s type-mismatch path and returns without descending; order_leaves then falls back to comparing the value_t values.

gregmarr · 2026-08-21T04:49:33Z

~~Haven't looked at all of this in detail yet.~~
Done.

Comparing two values compared their containers, which compare their elements, which brought the comparison back once per nesting level. Two values nested deeply enough exhausted the call stack and terminated the process with a segmentation fault - the same bug as #5387, in the last operation that still had it. Worse, an ordered comparison took exponentially long in the nesting depth before C++20. std::vector's operator< is a lexicographical comparison, which asks whether an element is less than its counterpart and then whether the counterpart is less than it - two full comparisons of everything below that element, at every level. Comparing two equal values nested 30 levels deep, which is nothing unusual, took 3.8 seconds; 40 levels would have taken an hour, and nothing about the value has to be pathological to get there. C++20 is unaffected: std::lexicographical_compare_three_way asks once. Compare a value that is nested too deeply to descend into on an explicit stack instead, in a single pass that yields less, equal, greater or unordered at once. Equality and the three-way comparison descend as they always did for the first 128 levels, which nothing measurable costs them; an ordered comparison no longer descends at all, which is what takes the exponent out of it. Objects and arrays that are not nested deeply are otherwise compared exactly as before. The results are unchanged for every pair of values: 68121 comparisons of a corpus that covers NaN, discarded values, mixed number types, binary values, empty containers and both object types are identical to develop, in C++11, C++17 and C++20, with and without thread_local storage and legacy discarded comparison. Reproducing that meant reproducing two subtleties: a lexicographic comparison steps over a pair it cannot order, where a three-way comparison stops at it, and an object compares its keys with < where its entries are ordered but with == where they are only checked for equality - not with the object's own comparator, which for nlohmann::ordered_map tells equality. Equality needs no ordering, so it no longer asks for any: a key or string type that can only be compared for equality still works. Measured (medians of 7 interleaved runs, clang -O3, C++11): comparing two equal values nested 30 levels deep 3778 ms -> 0.002 ms; ordering flat objects -33.6%; ordering flat arrays of numbers +27.3%, the one shape that pays for the single pass; equality unchanged throughout. Signed-off-by: Niels Lohmann <mail@nlohmann.me>

Comparing two values now bounds its descent with a thread_local counter just as copying does, so the JSON_NO_THREAD_LOCAL page, the macro overview and the ci_test_no_thread_local target cover both rather than copying alone. Also record what switching the macro on costs a comparison: on the benchmark documents, comparing two equal values takes 10% to 90% longer. Signed-off-by: Niels Lohmann <mail@nlohmann.me>

MSVC reports the test of a constant as C4127 ("conditional expression is constant"), which the Windows builds treat as an error: may_descend is false for operator<, so the operand short-circuits the whole condition. Passing it to compare_descent_exhausted() puts the test where the value is an ordinary parameter, and leaves the call sites with no condition of their own. Signed-off-by: Niels Lohmann <mail@nlohmann.me>

The macro page describes what the library defines JSON_NO_THREAD_LOCAL for by itself in terms of copying alone; comparing falls back the same way. Signed-off-by: Niels Lohmann <mail@nlohmann.me>

clang-tidy reports the mixed * and + as readability-math-missing- parentheses, as it does for the identical line in the copy test. Signed-off-by: Niels Lohmann <mail@nlohmann.me>

Comparing kept a thread_local count, a limit and a guard of its own beside the ones copying already had, all three the same thing under a different name. They are gone; the shared count, limit and guard do the work. The guard grows a second constructor here, because the comparison operators are written as a macro and a macro cannot use the preprocessor: it cannot look the count up behind an #ifdef the way copy_structured does, so the guard looks it up for it. nesting_depth_exhausted() arrives for the same reason - whether an operator descends at all is a constant at every call site, and testing it there is what MSVC reports as C4127. Also say in compare_leaves what happens to a pair that is an array on one side and an object on the other, since the answer is not obvious from the code: an operator only descends into two values of the same type, so such a pair is told apart by its types alone - unequal, and ordered the way the types are - exactly as it is above the bound. And record what the explicit stack costs: the comparison operators are noexcept and the container comparison this replaces allocated nothing, so running out of memory here ends the process instead of throwing. It takes a value nested past the bound and an exhausted heap to reach, and the same comparison used to exhaust the call stack, but it is a new way to fail. Signed-off-by: Niels Lohmann <mail@nlohmann.me>

gregmarr · 2026-08-21T15:25:08Z

+                {                                                                                        \
+                    return (deep_result);                                                                \
+                }                                                                                        \
+                const nesting_depth_guard guard;                                                         \


Can you do it like this, or is this where you hit C4127?

case value_t::array: { const nesting_depth_guard guard; if (may_descend && guard.okay()) { return (deep_result); } return (*lhs.m_data.m_value.array) op (*rhs.m_data.m_value.array); }

gregmarr · 2026-08-21T21:54:08Z

+            return compare_result::greater;
+        }
+
+        return compare_result::equal;


Could this take advantage of three way compare when available?

github-actions Bot added L tests labels Aug 20, 2026

nlohmann marked this pull request as draft August 20, 2026 19:46

nlohmann force-pushed the claude/issue-5387-comparison-binary branch from e8e9f5f to b1bb90b Compare August 20, 2026 19:55

nlohmann mentioned this pull request Aug 20, 2026

Fix stack overflow when copying a deeply nested value (#5387) #5389

Draft

5 tasks

nlohmann force-pushed the claude/issue-5387-comparison-binary branch from b1bb90b to 2797ae1 Compare August 20, 2026 20:58

github-actions Bot added documentation CMake labels Aug 20, 2026

nlohmann force-pushed the claude/issue-5387-comparison-binary branch 7 times, most recently from ae3aeb5 to 4c590dd Compare August 21, 2026 01:20

gregmarr reviewed Aug 21, 2026

View reviewed changes

nlohmann force-pushed the claude/issue-5387-comparison-binary branch from 0d4138c to 835a5e6 Compare August 21, 2026 06:56

This was referenced Aug 21, 2026

Stack overflow in binary format writers (CBOR/MessagePack/UBJSON/BSON) due to unbounded recursion #5392

Open

Stack overflow in diff(), merge_patch(), and flatten() due to unbounded recursion #5393

Open

nlohmann added 6 commits August 21, 2026 12:25

Note the comparison fallback in the no-thread-local documentation

616b395

The macro page describes what the library defines JSON_NO_THREAD_LOCAL for by itself in terms of copying alone; comparing falls back the same way. Signed-off-by: Niels Lohmann <mail@nlohmann.me>

Parenthesise the reserve() computation in the comparison test

5fe4cd2

clang-tidy reports the mixed * and + as readability-math-missing- parentheses, as it does for the identical line in the copy test. Signed-off-by: Niels Lohmann <mail@nlohmann.me>

nlohmann force-pushed the claude/issue-5387-comparison-binary branch from 835a5e6 to 596e33c Compare August 21, 2026 10:31

gregmarr reviewed Aug 21, 2026

View reviewed changes

Navigation Menu

Uh oh!

Fix stack overflow and exponential runtime when comparing nested values - #5390

Fix stack overflow and exponential runtime when comparing nested values#5390
nlohmann wants to merge 6 commits into
claude/issue-5387-duplicate-check-bd7853from
claude/issue-5387-comparison-binary

nlohmann commented Aug 20, 2026 •

edited

Loading

Uh oh!

gregmarr Aug 21, 2026

Uh oh!

nlohmann Aug 21, 2026

Uh oh!

gregmarr Aug 21, 2026

Uh oh!

nlohmann Aug 21, 2026

Uh oh!

gregmarr Aug 21, 2026

Uh oh!

nlohmann Aug 21, 2026

Uh oh!

gregmarr Aug 21, 2026

Uh oh!

nlohmann Aug 21, 2026

Uh oh!

gregmarr Aug 21, 2026

Uh oh!

gregmarr commented Aug 21, 2026 •

edited

Loading

Uh oh!

gregmarr Aug 21, 2026

Uh oh!

gregmarr Aug 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

nlohmann commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What & why

An ordered comparison took exponentially long before C++20

The change

Reproducing the results exactly

Measured

Public API impact

Verification

Note for reviewers

What still recurses after this PR

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gregmarr commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nlohmann commented Aug 20, 2026 •

edited

Loading

gregmarr commented Aug 21, 2026 •

edited

Loading