From 0fa2eb10e4f0396b0c0eab0c8ecf34c80dd7de9c Mon Sep 17 00:00:00 2001 From: Joao Paulo Magalhaes Date: Fri, 19 Dec 2025 19:29:27 +0000 Subject: [PATCH] fix: Tree::relocate() triggers assertions with zero-length strings ... when they string are placed at the beginning or end of the arena --- .github/workflows-in/benchmarks.ys | 2 +- .github/workflows/benchmarks.yml | 4 +-- changelog/current.md | 1 + src/c4/yml/tree.hpp | 6 ++--- test/test_tree.cpp | 41 ++++++++++++++++++++++++++++++ 5 files changed, 48 insertions(+), 6 deletions(-) diff --git a/.github/workflows-in/benchmarks.ys b/.github/workflows-in/benchmarks.ys index a873ae0a6..7a589a221 100644 --- a/.github/workflows-in/benchmarks.ys +++ b/.github/workflows-in/benchmarks.ys @@ -23,7 +23,7 @@ jobs: - {std: 17, comp: clang18, os: ubuntu-24.04, cmkflags: -DCMAKE_CXX_COMPILER=clang++-18 -DCMAKE_C_COMPILER=clang-18} #- {std: 17, comp: vs2019 , os: windows-2019, cmkflags: -G 'Visual Studio 16 2019' -A x64} - {std: 17, comp: vs2022 , os: windows-2022, cmkflags: -G 'Visual Studio 17 2022' -A x64} - - {std: 17, comp: xcode15, os: macos-13, xcver: 15, cmkflags: -G Xcode -DCMAKE_OSX_ARCHITECTURES=x86_64} + - {std: 17, comp: xcode15, os: macos-15-intel, xcver: 16, cmkflags: -G Xcode -DCMAKE_OSX_ARCHITECTURES=x86_64} env:: -{'BM' 'ON'} + load('share/env.yaml') steps: - :: checkout-action diff --git a/.github/workflows/benchmarks.yml b/.github/workflows/benchmarks.yml index 7a311c6df..ffe75d2e5 100644 --- a/.github/workflows/benchmarks.yml +++ b/.github/workflows/benchmarks.yml @@ -108,8 +108,8 @@ jobs: cmkflags: -G 'Visual Studio 17 2022' -A x64 - std: 17 comp: xcode15 - os: macos-13 - xcver: 15 + os: macos-15-intel + xcver: 16 cmkflags: -G Xcode -DCMAKE_OSX_ARCHITECTURES=x86_64 env: BM: 'ON' diff --git a/changelog/current.md b/changelog/current.md index 29650fd2f..50526d40c 100644 --- a/changelog/current.md +++ b/changelog/current.md @@ -1,5 +1,6 @@ ### Changes +- [PR#565](https://github.com/biojppm/rapidyaml/pull/565) (fixes [#564](https://github.com/biojppm/rapidyaml/issues/564)) - `Tree` arena: allow relocation of zero-length strings when placed at the end (relax assertions triggered in `Tree::_relocated()`) - [PR#561](https://github.com/biojppm/rapidyaml/pull/561) (fixes [#559](https://github.com/biojppm/rapidyaml/issues/559)) - Byte Order Mark: account for BOM when determining block indentation - [PR#563](https://github.com/biojppm/rapidyaml/pull/563) (fixes [#562](https://github.com/biojppm/rapidyaml/issues/562)) - Fix bug in `NodeRef::cend()` - [PR#547](https://github.com/biojppm/rapidyaml/pull/547) - Fix parsing of implicit first documents with empty sequences, caused by a problem in `Tree::set_root_as_stream()`: diff --git a/src/c4/yml/tree.hpp b/src/c4/yml/tree.hpp index 146b38ccf..8637f6e99 100644 --- a/src/c4/yml/tree.hpp +++ b/src/c4/yml/tree.hpp @@ -1009,12 +1009,12 @@ class RYML_EXPORT Tree substr _relocated(csubstr s, substr next_arena) const { - _RYML_CB_ASSERT(m_callbacks, m_arena.is_super(s)); - _RYML_CB_ASSERT(m_callbacks, m_arena.sub(0, m_arena_pos).is_super(s)); + _RYML_CB_ASSERT(m_callbacks, m_arena.is_super(s) || s.len == 0); + _RYML_CB_ASSERT(m_callbacks, m_arena.sub(0, m_arena_pos).is_super(s) || s.len == 0); auto pos = (s.str - m_arena.str); // this is larger than 0 based on the assertions above substr r(next_arena.str + pos, s.len); _RYML_CB_ASSERT(m_callbacks, r.str - next_arena.str == pos); - _RYML_CB_ASSERT(m_callbacks, next_arena.sub(0, m_arena_pos).is_super(r)); + _RYML_CB_ASSERT(m_callbacks, next_arena.sub(0, m_arena_pos).is_super(r) || r.len == 0); return r; } diff --git a/test/test_tree.cpp b/test/test_tree.cpp index c488e4230..a4880633b 100644 --- a/test/test_tree.cpp +++ b/test/test_tree.cpp @@ -10,6 +10,7 @@ #include "./test_lib/callbacks_tester.hpp" #include +#include #if defined(_MSC_VER) # pragma warning(push) @@ -938,6 +939,46 @@ TEST(Tree, reserve_arena_issue288) EXPECT_EQ(t.arena().last(pluses.size()), to_csubstr(pluses)); } +// https://github.com/biojppm/rapidyaml/issues/564 +namespace issue564 { +struct AdjacentSizedBlocks { + std::vector mem; + char* start; + char* next; +}; +using AvailableBlocks = std::unordered_map; +static void free(void* , size_t size, void* user_data) { + (*static_cast(user_data))[size].next -= size; +} +static void* alloc(size_t len, void* /*hint*/, void* user_data) { + auto& blocks = (*static_cast(user_data))[len]; + if (blocks.start == nullptr) { + blocks.mem.resize(10 * len); + blocks.start = blocks.next = blocks.mem.data(); + } + void *ret = blocks.next; + blocks.next += len; + return ret; +} +} // namespace issue564 +TEST(Tree, issue564_relocate_arena_0) +{ + issue564::AvailableBlocks blocks_by_size; + Callbacks cb(&blocks_by_size, issue564::alloc, issue564::free, nullptr); + Tree dest(cb); + Tree source(cb); + parse_in_arena("[]", &dest); + parse_in_arena("[]", &source); + // Shallow copy `source` as a child of `dest` + NodeRef child = dest.rootref().append_child(); + dest.duplicate_contents(&source, source.root_id(), child.id()); + // trigger a relocation in dest + std::string loong(10000, ' '); + ExpectError::check_success([&]{ + dest.to_arena(to_csubstr(loong)); // error + }); +} + TEST(Tree, clear) { Tree t(16, 64);