Skip to content

Commit 0fa2eb1

Browse files
committed
fix: Tree::relocate() triggers assertions with zero-length strings
... when they string are placed at the beginning or end of the arena
1 parent 0852909 commit 0fa2eb1

File tree

5 files changed

+48
-6
lines changed

5 files changed

+48
-6
lines changed

.github/workflows-in/benchmarks.ys

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ jobs:
2323
- {std: 17, comp: clang18, os: ubuntu-24.04, cmkflags: -DCMAKE_CXX_COMPILER=clang++-18 -DCMAKE_C_COMPILER=clang-18}
2424
#- {std: 17, comp: vs2019 , os: windows-2019, cmkflags: -G 'Visual Studio 16 2019' -A x64}
2525
- {std: 17, comp: vs2022 , os: windows-2022, cmkflags: -G 'Visual Studio 17 2022' -A x64}
26-
- {std: 17, comp: xcode15, os: macos-13, xcver: 15, cmkflags: -G Xcode -DCMAKE_OSX_ARCHITECTURES=x86_64}
26+
- {std: 17, comp: xcode15, os: macos-15-intel, xcver: 16, cmkflags: -G Xcode -DCMAKE_OSX_ARCHITECTURES=x86_64}
2727
env:: -{'BM' 'ON'} + load('share/env.yaml')
2828
steps:
2929
- :: checkout-action

.github/workflows/benchmarks.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,8 +108,8 @@ jobs:
108108
cmkflags: -G 'Visual Studio 17 2022' -A x64
109109
- std: 17
110110
comp: xcode15
111-
os: macos-13
112-
xcver: 15
111+
os: macos-15-intel
112+
xcver: 16
113113
cmkflags: -G Xcode -DCMAKE_OSX_ARCHITECTURES=x86_64
114114
env:
115115
BM: 'ON'

changelog/current.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
### Changes
22

3+
- [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()`)
34
- [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
45
- [PR#563](https://github.com/biojppm/rapidyaml/pull/563) (fixes [#562](https://github.com/biojppm/rapidyaml/issues/562)) - Fix bug in `NodeRef::cend()`
56
- [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()`:

src/c4/yml/tree.hpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1009,12 +1009,12 @@ class RYML_EXPORT Tree
10091009

10101010
substr _relocated(csubstr s, substr next_arena) const
10111011
{
1012-
_RYML_CB_ASSERT(m_callbacks, m_arena.is_super(s));
1013-
_RYML_CB_ASSERT(m_callbacks, m_arena.sub(0, m_arena_pos).is_super(s));
1012+
_RYML_CB_ASSERT(m_callbacks, m_arena.is_super(s) || s.len == 0);
1013+
_RYML_CB_ASSERT(m_callbacks, m_arena.sub(0, m_arena_pos).is_super(s) || s.len == 0);
10141014
auto pos = (s.str - m_arena.str); // this is larger than 0 based on the assertions above
10151015
substr r(next_arena.str + pos, s.len);
10161016
_RYML_CB_ASSERT(m_callbacks, r.str - next_arena.str == pos);
1017-
_RYML_CB_ASSERT(m_callbacks, next_arena.sub(0, m_arena_pos).is_super(r));
1017+
_RYML_CB_ASSERT(m_callbacks, next_arena.sub(0, m_arena_pos).is_super(r) || r.len == 0);
10181018
return r;
10191019
}
10201020

test/test_tree.cpp

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include "./test_lib/callbacks_tester.hpp"
1111

1212
#include <gtest/gtest.h>
13+
#include <unordered_map>
1314

1415
#if defined(_MSC_VER)
1516
# pragma warning(push)
@@ -938,6 +939,46 @@ TEST(Tree, reserve_arena_issue288)
938939
EXPECT_EQ(t.arena().last(pluses.size()), to_csubstr(pluses));
939940
}
940941

942+
// https://github.com/biojppm/rapidyaml/issues/564
943+
namespace issue564 {
944+
struct AdjacentSizedBlocks {
945+
std::vector<char> mem;
946+
char* start;
947+
char* next;
948+
};
949+
using AvailableBlocks = std::unordered_map<size_t, AdjacentSizedBlocks>;
950+
static void free(void* , size_t size, void* user_data) {
951+
(*static_cast<AvailableBlocks *>(user_data))[size].next -= size;
952+
}
953+
static void* alloc(size_t len, void* /*hint*/, void* user_data) {
954+
auto& blocks = (*static_cast<AvailableBlocks*>(user_data))[len];
955+
if (blocks.start == nullptr) {
956+
blocks.mem.resize(10 * len);
957+
blocks.start = blocks.next = blocks.mem.data();
958+
}
959+
void *ret = blocks.next;
960+
blocks.next += len;
961+
return ret;
962+
}
963+
} // namespace issue564
964+
TEST(Tree, issue564_relocate_arena_0)
965+
{
966+
issue564::AvailableBlocks blocks_by_size;
967+
Callbacks cb(&blocks_by_size, issue564::alloc, issue564::free, nullptr);
968+
Tree dest(cb);
969+
Tree source(cb);
970+
parse_in_arena("[]", &dest);
971+
parse_in_arena("[]", &source);
972+
// Shallow copy `source` as a child of `dest`
973+
NodeRef child = dest.rootref().append_child();
974+
dest.duplicate_contents(&source, source.root_id(), child.id());
975+
// trigger a relocation in dest
976+
std::string loong(10000, ' ');
977+
ExpectError::check_success([&]{
978+
dest.to_arena(to_csubstr(loong)); // error
979+
});
980+
}
981+
941982
TEST(Tree, clear)
942983
{
943984
Tree t(16, 64);

0 commit comments

Comments
 (0)