Repository: biojppm/rapidyaml (language: python)

Issue:
Segfault on successive calls to `Tree::_grow_arena()`
When parsing a large [78MB file](https://drive.google.com/drive/folders/1izYxmnnOT6ZRo75P2LepPTyWqudIL9vK?usp=sharing), a segfault occurs only when the tree has been reserved. Without any reservation, the parsing succeeds. Steps to reproduce:

```cpp

std::string loadFileToString(const std::string& path)
{
  std::ifstream ifs(path.c_str(), std::ios::in | std::ios::binary | std::ios::ate);
  
  std::ifstream::pos_type size = ifs.tellg();
  ifs.seekg(0, std::ios::beg);
  
  std::vector<char> bytes(size);
  
  ifs.read(bytes.data(), size);
  std::cout << "Read bytes finished" << std::endl;
  
  return std::string(bytes.data(), size);
}

int main()
{
  std::string path = "your_path";
  std::string buf = loadFileToString(path);

  ryml::Tree t;
  ryml::NodeRef n = t.rootref();
  // will only trigger error when using reserve
  t.reserve(100000);
  t.reserve_arena(36000000);

  
  // not tested with parse_in_place
  ryml::parse_in_arena("block.yaml", ryml::to_csubstr(buf), n);
}
```
Backtrace in Release Node -> Segfault
``` gdb
Thread 1 "plot_dataset" received signal SIGSEGV, Segmentation fault.
__memmove_avx_unaligned_erms () at ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:538
538	../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S: No such file or directory.
(gdb) bt
#0  __memmove_avx_unaligned_erms () at ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:538
#1  0x00007ffff7df7aca in c4::yml::parse_in_arena(c4::basic_substring<char const>, c4::basic_substring<char const>, c4::yml::NodeRef) [clone .isra.0] ()
   from /home/captain-yoshi/ws/ros/mimic_ws/devel/lib/libmoveit_benchmark_suite_core.so
#2  0x00007ffff7dfb8bc in moveit_benchmark_suite::IO::loadFileToYAML(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, c4::yml::NodeRef&, bool) ()
   from /home/captain-yoshi/ws/ros/mimic_ws/devel/lib/libmoveit_benchmark_suite_core.so
#3  0x00007ffff7e34bd6 in moveit_benchmark_suite::DatasetFilter::loadDataset(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) ()
   from /home/captain-yoshi/ws/ros/mimic_ws/devel/lib/libmoveit_benchmark_suite_core.so
#4  0x00007ffff7e351ef in moveit_benchmark_suite::DatasetFilter::loadDatasets(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) () from /home/captain-yoshi/ws/ros/mimic_ws/devel/lib/libmoveit_benchmark_suite_core.so
#5  0x00007ffff7f4c2b1 in moveit_benchmark_suite::tools::GNUPlotDataset::plot(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) () from /home/captain-yoshi/ws/ros/mimic_ws/devel/lib/libmoveit_benchmark_suite_tools.so
#6  0x000055555555a9ac in main ()
```



Backtrace in Debug Node -> SIGTRAP
``` gdb
Thread 1 "plot_dataset" received signal SIGTRAP, Trace/breakpoint trap.
trap_instruction () at /home/captain-yoshi/ws/ros/mimic_ws/src/moveit_serialization/serialization/ext/ryml/ext/c4core/src/c4/ext/debugbreak/debugbreak.h:48
48	}
(gdb) bt
#0  trap_instruction () at /home/captain-yoshi/ws/ros/mimic_ws/src/moveit_serialization/serialization/ext/ryml/ext/c4core/src/c4/ext/debugbreak/debugbreak.h:48
#1  0x00007ffff7c6b180 in debug_break () at /home/captain-yoshi/ws/ros/mimic_ws/src/moveit_serialization/serialization/ext/ryml/ext/c4core/src/c4/ext/debugbreak/debugbreak.h:143
#2  c4::basic_substring<char>::sub (this=0x7fffffffbfa0, first=0, num=81682685) at /home/captain-yoshi/ws/ros/mimic_ws/src/moveit_serialization/serialization/ext/ryml/ext/c4core/src/c4/substr.hpp:321
#3  0x00007ffff7c64802 in c4::yml::Tree::_request_span (this=0x7fffffffbf78, sz=81682685) at /home/captain-yoshi/ws/ros/mimic_ws/src/moveit_serialization/serialization/ext/ryml/src/c4/yml/tree.hpp:1074
#4  0x00007ffff7cbbf21 in c4::yml::Tree::alloc_arena (this=0x7fffffffbf78, sz=81682685) at /home/captain-yoshi/ws/ros/mimic_ws/src/moveit_serialization/serialization/ext/ryml/src/c4/yml/tree.hpp:1034
#5  0x00007ffff7cbbda1 in c4::yml::Tree::copy_to_arena (this=0x7fffffffbf78, s=...) at /home/captain-yoshi/ws/ros/mimic_ws/src/moveit_serialization/serialization/ext/ryml/src/c4/yml/tree.hpp:1011
#6  0x00007ffff7cbc04a in c4::yml::Parser::parse_in_arena (this=0x7fffffffaf80, filename=..., csrc=..., node=...)
    at /home/captain-yoshi/ws/ros/mimic_ws/src/moveit_serialization/serialization/ext/ryml/src/c4/yml/parse.hpp:220
#7  0x00007ffff7cbc140 in c4::yml::parse_in_arena (filename=..., yaml=..., node=...) at /home/captain-yoshi/ws/ros/mimic_ws/src/moveit_serialization/serialization/ext/ryml/src/c4/yml/parse.hpp:646
#8  0x00007ffff7cb7740 in moveit_benchmark_suite::IO::loadFileToYAML (path="/home/captain-yoshi/.ros/simon/block.yaml", node=..., verbose=true)
    at /home/captain-yoshi/ws/ros/mimic_ws/src/moveit_benchmark_suite/core/src/io.cpp:514
#9  0x00007ffff7d248a6 in moveit_benchmark_suite::DatasetFilter::loadDataset (this=0x7fffffffbf18, filename="/home/captain-yoshi/.ros/simon/block.yaml")
    at /home/captain-yoshi/ws/ros/mimic_ws/src/moveit_benchmark_suite/core/src/dataset_filter.cpp:62
#10 0x00007ffff7d24ee9 in moveit_benchmark_suite::DatasetFilter::loadDatasets (this=0x7fffffffbf18, filenames=std::vector of length 1, capacity 1 = {...})
    at /home/captain-yoshi/ws/ros/mimic_ws/src/moveit_benchmark_suite/core/src/dataset_filter.cpp:93
#11 0x00007ffff7f4c2b1 in moveit_benchmark_suite::tools::GNUPlotDataset::plot(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) () from /home/captain-yoshi/ws/ros/mimic_ws/devel/lib/libmoveit_benchmark_suite_tools.so
#12 0x000055555555a9ac in main ()
(gdb) c
Continuing.

/home/captain-yoshi/ws/ros/mimic_ws/src/moveit_serialization/serialization/ext/ryml/ext/c4core/src/c4/substr.hpp:321: ERROR: check failed: (num >= 0 && num <= len) || (num == npos)
```
Segfault on successive calls to `Tree::_grow_arena()`
When parsing a large [78MB file](https://drive.google.com/drive/folders/1izYxmnnOT6ZRo75P2LepPTyWqudIL9vK?usp=sharing), a segfault occurs only when the tree has been reserved. Without any reservation, the parsing succeeds. Steps to reproduce:

```cpp

std::string loadFileToString(const std::string& path)
{
  std::ifstream ifs(path.c_str(), std::ios::in | std::ios::binary | std::ios::ate);
  
  std::ifstream::pos_type size = ifs.tellg();
  ifs.seekg(0, std::ios::beg);
  
  std::vector<char> bytes(size);
  
  ifs.read(bytes.data(), size);
  std::cout << "Read bytes finished" << std::endl;
  
  return std::string(bytes.data(), size);
}

int main()
{
  std::string path = "your_path";
  std::string buf = loadFileToString(path);

  ryml::Tree t;
  ryml::NodeRef n = t.rootref();
  // will only trigger error when using reserve
  t.reserve(100000);
  t.reserve_arena(36000000);

  
  // not tested with parse_in_place
  ryml::parse_in_arena("block.yaml", ryml::to_csubstr(buf), n);
}
```
Backtrace in Release Node -> Segfault
``` gdb
Thread 1 "plot_dataset" received signal SIGSEGV, Segmentation fault.
__memmove_avx_unaligned_erms () at ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:538
538	../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S: No such file or directory.
(gdb) bt
#0  __memmove_avx_unaligned_erms () at ../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:538
#1  0x00007ffff7df7aca in c4::yml::parse_in_arena(c4::basic_substring<char const>, c4::basic_substring<char const>, c4::yml::NodeRef) [clone .isra.0] ()
   from /home/captain-yoshi/ws/ros/mimic_ws/devel/lib/libmoveit_benchmark_suite_core.so
#2  0x00007ffff7dfb8bc in moveit_benchmark_suite::IO::loadFileToYAML(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, c4::yml::NodeRef&, bool) ()
   from /home/captain-yoshi/ws/ros/mimic_ws/devel/lib/libmoveit_benchmark_suite_core.so
#3  0x00007ffff7e34bd6 in moveit_benchmark_suite::DatasetFilter::loadDataset(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) ()
   from /home/captain-yoshi/ws/ros/mimic_ws/devel/lib/libmoveit_benchmark_suite_core.so
#4  0x00007ffff7e351ef in moveit_benchmark_suite::DatasetFilter::loadDatasets(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) () from /home/captain-yoshi/ws/ros/mimic_ws/devel/lib/libmoveit_benchmark_suite_core.so
#5  0x00007ffff7f4c2b1 in moveit_benchmark_suite::tools::GNUPlotDataset::plot(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) () from /home/captain-yoshi/ws/ros/mimic_ws/devel/lib/libmoveit_benchmark_suite_tools.so
#6  0x000055555555a9ac in main ()
```



Backtrace in Debug Node -> SIGTRAP
``` gdb
Thread 1 "plot_dataset" received signal SIGTRAP, Trace/breakpoint trap.
trap_instruction () at /home/captain-yoshi/ws/ros/mimic_ws/src/moveit_serialization/serialization/ext/ryml/ext/c4core/src/c4/ext/debugbreak/debugbreak.h:48
48	}
(gdb) bt
#0  trap_instruction () at /home/captain-yoshi/ws/ros/mimic_ws/src/moveit_serialization/serialization/ext/ryml/ext/c4core/src/c4/ext/debugbreak/debugbreak.h:48
#1  0x00007ffff7c6b180 in debug_break () at /home/captain-yoshi/ws/ros/mimic_ws/src/moveit_serialization/serialization/ext/ryml/ext/c4core/src/c4/ext/debugbreak/debugbreak.h:143
#2  c4::basic_substring<char>::sub (this=0x7fffffffbfa0, first=0, num=81682685) at /home/captain-yoshi/ws/ros/mimic_ws/src/moveit_serialization/serialization/ext/ryml/ext/c4core/src/c4/substr.hpp:321
#3  0x00007ffff7c64802 in c4::yml::Tree::_request_span (this=0x7fffffffbf78, sz=81682685) at /home/captain-yoshi/ws/ros/mimic_ws/src/moveit_serialization/serialization/ext/ryml/src/c4/yml/tree.hpp:1074
#4  0x00007ffff7cbbf21 in c4::yml::Tree::alloc_arena (this=0x7fffffffbf78, sz=81682685) at /home/captain-yoshi/ws/ros/mimic_ws/src/moveit_serialization/serialization/ext/ryml/src/c4/yml/tree.hpp:1034
#5  0x00007ffff7cbbda1 in c4::yml::Tree::copy_to_arena (this=0x7fffffffbf78, s=...) at /home/captain-yoshi/ws/ros/mimic_ws/src/moveit_serialization/serialization/ext/ryml/src/c4/yml/tree.hpp:1011
#6  0x00007ffff7cbc04a in c4::yml::Parser::parse_in_arena (this=0x7fffffffaf80, filename=..., csrc=..., node=...)
    at /home/captain-yoshi/ws/ros/mimic_ws/src/moveit_serialization/serialization/ext/ryml/src/c4/yml/parse.hpp:220
#7  0x00007ffff7cbc140 in c4::yml::parse_in_arena (filename=..., yaml=..., node=...) at /home/captain-yoshi/ws/ros/mimic_ws/src/moveit_serialization/serialization/ext/ryml/src/c4/yml/parse.hpp:646
#8  0x00007ffff7cb7740 in moveit_benchmark_suite::IO::loadFileToYAML (path="/home/captain-yoshi/.ros/simon/block.yaml", node=..., verbose=true)
    at /home/captain-yoshi/ws/ros/mimic_ws/src/moveit_benchmark_suite/core/src/io.cpp:514
#9  0x00007ffff7d248a6 in moveit_benchmark_suite::DatasetFilter::loadDataset (this=0x7fffffffbf18, filename="/home/captain-yoshi/.ros/simon/block.yaml")
    at /home/captain-yoshi/ws/ros/mimic_ws/src/moveit_benchmark_suite/core/src/dataset_filter.cpp:62
#10 0x00007ffff7d24ee9 in moveit_benchmark_suite::DatasetFilter::loadDatasets (this=0x7fffffffbf18, filenames=std::vector of length 1, capacity 1 = {...})
    at /home/captain-yoshi/ws/ros/mimic_ws/src/moveit_benchmark_suite/core/src/dataset_filter.cpp:93
#11 0x00007ffff7f4c2b1 in moveit_benchmark_suite::tools::GNUPlotDataset::plot(std::vector<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > const&) () from /home/captain-yoshi/ws/ros/mimic_ws/devel/lib/libmoveit_benchmark_suite_tools.so
#12 0x000055555555a9ac in main ()
(gdb) c
Continuing.

/home/captain-yoshi/ws/ros/mimic_ws/src/moveit_serialization/serialization/ext/ryml/ext/c4core/src/c4/substr.hpp:321: ERROR: check failed: (num >= 0 && num <= len) || (num == npos)
```


Discussion / hints:

Fix patch:
diff --git a/changelog/current.md b/changelog/current.md
index 4c3a208cc..dd587c8b0 100644
--- a/changelog/current.md
+++ b/changelog/current.md
@@ -25,6 +25,8 @@
   EXPECT_EQ(tree2["s"].val(), tree["s"].val());
   ```
 - Fix [#280](https://github.com/biojppm/rapidyaml/issues/280) ([PR#281](https://github.com/biojppm/rapidyaml/pull/281)): deserialization of `std::vector<bool>` failed because its `operator[]` returns a `reference` instead of `value_type`.
+- Fix [#288](https://github.com/biojppm/rapidyaml/issues/288) ([PR#290](https://github.com/biojppm/rapidyaml/pull/290)): segfault on successive calls to `Tree::_grow_arena()`, caused by using the arena position instead of its length as starting point for the new arena capacity.
+- Deprecate `Tree::arena_pos()`: use `Tree::arena_size()` instead ([PR#290](https://github.com/biojppm/rapidyaml/pull/290)).
 - YAML test suite events emitter: fix emission of inheriting nodes. The events for `{<<: *anchor, foo: bar}` are now correctly emitted as:
   ```yaml
   =VAL :<<  # previously was =ALI <<
diff --git a/src/c4/yml/tree.cpp b/src/c4/yml/tree.cpp
index 7b7eaa582..51f469d4b 100644
--- a/src/c4/yml/tree.cpp
+++ b/src/c4/yml/tree.cpp
@@ -1835,7 +1835,7 @@ void Tree::resolve_tags()
         return;
     size_t needed_size = _count_resolved_tags_size(this, root_id());
     if(needed_size)
-        reserve_arena(arena_pos() + needed_size);
+        reserve_arena(arena_size() + needed_size);
     _resolve_tags(this, root_id());
 }
 
diff --git a/src/c4/yml/tree.hpp b/src/c4/yml/tree.hpp
index b54a48e4a..43f08cf58 100644
--- a/src/c4/yml/tree.hpp
+++ b/src/c4/yml/tree.hpp
@@ -466,14 +466,10 @@ class RYML_EXPORT Tree
 
     inline bool   empty() const { return m_size == 0; }
 
-    inline size_t size () const { return m_size; }
+    inline size_t size() const { return m_size; }
     inline size_t capacity() const { return m_cap; }
     inline size_t slack() const { RYML_ASSERT(m_cap >= m_size); return m_cap - m_size; }
 
-    inline size_t arena_size() const { return m_arena_pos; }
-    inline size_t arena_capacity() const { return m_arena.len; }
-    inline size_t arena_slack() const { RYML_ASSERT(m_arena.len >= m_arena_pos); return m_arena.len - m_arena_pos; }
-
     Callbacks const& callbacks() const { return m_callbacks; }
     void callbacks(Callbacks const& cb) { m_callbacks = cb; }
 
@@ -935,7 +931,13 @@ class RYML_EXPORT Tree
     /** @{ */
 
     /** get the current size of the tree's internal arena */
-    size_t arena_pos() const { return m_arena_pos; }
+    RYML_DEPRECATED("use arena_size() instead") size_t arena_pos() const { return m_arena_pos; }
+    /** get the current size of the tree's internal arena */
+    inline size_t arena_size() const { return m_arena_pos; }
+    /** get the current capacity of the tree's internal arena */
+    inline size_t arena_capacity() const { return m_arena.len; }
+    /** get the current slack of the tree's internal arena */
+    inline size_t arena_slack() const { RYML_ASSERT(m_arena.len >= m_arena_pos); return m_arena.len - m_arena_pos; }
 
     /** get the current arena */
     substr arena() const { return m_arena.first(m_arena_pos); }
@@ -1047,7 +1049,7 @@ class RYML_EXPORT Tree
 
     substr _grow_arena(size_t more)
     {
-        size_t cap = m_arena_pos + more;
+        size_t cap = m_arena.len + more;
         cap = cap < 2 * m_arena.len ? 2 * m_arena.len : cap;
         cap = cap < 64 ? 64 : cap;
         reserve_arena(cap);


Test patch:
diff --git a/test/test_case.cpp b/test/test_case.cpp
index 90a794091..cacf20e54 100644
--- a/test/test_case.cpp
+++ b/test/test_case.cpp
@@ -771,10 +771,15 @@ size_t test_tree_invariants(NodeRef const& n)
 void test_invariants(Tree const& t)
 {
 
-    EXPECT_LE(t.size(), t.capacity());
+    ASSERT_LE(t.size(), t.capacity());
     EXPECT_EQ(t.size() + t.slack(), t.capacity());
 
-    if(t.empty()) return;
+    ASSERT_LE(t.arena_size(), t.arena_capacity());
+    ASSERT_LE(t.arena_slack(), t.arena_capacity());
+    EXPECT_EQ(t.arena_size() + t.arena_slack(), t.arena_capacity());
+
+    if(t.empty())
+        return;
 
     size_t count = test_tree_invariants(t.rootref());
     EXPECT_EQ(count, t.size());
diff --git a/test/test_tree.cpp b/test/test_tree.cpp
index 82b98b78e..19893f8d3 100644
--- a/test/test_tree.cpp
+++ b/test/test_tree.cpp
@@ -738,6 +738,32 @@ TEST(Tree, reserve)
     test_invariants(t);
 }
 
+// https://github.com/biojppm/rapidyaml/issues/288
+TEST(Tree, reserve_arena_issue288)
+{
+    Tree t;
+    EXPECT_EQ(t.arena_slack(), 0u);
+    EXPECT_EQ(t.arena_capacity(), 0u);
+    EXPECT_EQ(t.arena_size(), 0u);
+    t.reserve_arena(3u);
+    EXPECT_EQ(t.arena_slack(), 3u);
+    EXPECT_GE(t.arena_capacity(), 3u);
+    EXPECT_EQ(t.arena_size(), 0u);
+    // longer than the slack to cause another call to _grow_arena()
+    std::string stars(2 * t.arena_slack(), '*');
+    t.copy_to_arena(to_csubstr(stars));
+    EXPECT_GE(t.arena_capacity(), stars.size());
+    EXPECT_EQ(t.arena_size(), stars.size());
+    EXPECT_EQ(t.arena(), to_csubstr(stars));
+    // again
+    std::string pluses(2 * t.arena_slack(), '+');
+    t.copy_to_arena(to_csubstr(pluses));
+    EXPECT_GE(t.arena_capacity(), stars.size() + pluses.size());
+    EXPECT_EQ(t.arena_size(), stars.size() + pluses.size());
+    EXPECT_EQ(t.arena().first(stars.size()), to_csubstr(stars));
+    EXPECT_EQ(t.arena().last(pluses.size()), to_csubstr(pluses));
+}
+
 TEST(Tree, clear)
 {
     Tree t(16, 64);