Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Segfault on successive calls to Tree::_grow_arena() #288

Closed
captain-yoshi opened this issue Jul 31, 2022 · 2 comments · Fixed by #290
Closed

Segfault on successive calls to Tree::_grow_arena() #288

captain-yoshi opened this issue Jul 31, 2022 · 2 comments · Fixed by #290

Comments

@captain-yoshi
Copy link
Contributor

captain-yoshi commented Jul 31, 2022

When parsing a large 78MB file, a segfault occurs only when the tree has been reserved. Without any reservation, the parsing succeeds. Steps to reproduce:

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

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

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)
@captain-yoshi captain-yoshi changed the title 78MB file segfault when using tree.reserved() Parsing 78MB file segfaults when using tree.reserved() Jul 31, 2022
@captain-yoshi captain-yoshi changed the title Parsing 78MB file segfaults when using tree.reserved() Segfault when parsing a 78 MB file using tree.reserved() Jul 31, 2022
@captain-yoshi captain-yoshi changed the title Segfault when parsing a 78 MB file using tree.reserved() Segfault when parsing 78MB file using tree.reserved() Jul 31, 2022
@biojppm
Copy link
Owner

biojppm commented Aug 1, 2022

Thanks for reporting. The problem seems to be in Tree::_grow_arena(); I'll set up a PR with a repro and fix.

A couple of suggestions:

  • FYI whenever you suspect ryml of misbehaving, compiling in Debug will enable all the C4_ASSERT() calls, and generally these will point you in the direction of the problem. [EDIT: nevermind, you posted above the trap from the failed assertion]
  • You can spare a copy of the file by allocating directly in the tree's arena, followed by a call to parse_in_place():
ryml::substr loadFileToString(const std::string& path, ryml::Tree &tree)
{
  std::ifstream ifs(path.c_str(), std::ios::in | std::ios::binary | std::ios::ate);
  
  std::ifstream::pos_type size = ifs.tellg();
  ryml::substr bytes = tree.alloc_arena(size);
  
  ifs.seekg(0, std::ios::beg);
  ifs.read(bytes.data(), bytes.size());
  
  std::cout << "Read bytes finished" << std::endl;
  return bytes;
}
// ... elsewhere:
ryml::parse_in_place("block.yaml", bytes, n);

(actually, you are doing two extra copies because of the intermediate std::vector). I realize this is just for the bug report, but it may be helpful to point it out.

@biojppm
Copy link
Owner

biojppm commented Aug 1, 2022

FWIW, this seems to be the MVE:

    Tree t;
    t.reserve_arena(3);
    // longer than the slack to cause another call to _grow_arena()
    size_t slack = t.arena_slack();
    std::string large(2 * slack, '*');
    t.copy_to_arena(to_csubstr(large)); // triggers an assert in Debug, segfault in release

biojppm added a commit that referenced this issue Aug 1, 2022
@biojppm biojppm changed the title Segfault when parsing 78MB file using tree.reserved() Segfault on successive calls to Tree::_grow_arena() Aug 1, 2022
biojppm added a commit that referenced this issue Aug 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants