Boundary error when returning a list

[BlackEdder]

I have some code that returns a vector of ResourcePtrs, but when I try to use these, it results in a segsev boundary error. Before I had the code in the C nif and it worked fine. Snippet of code:

  std::vector<fine::ResourcePtr<TSNode>> lst(match.capture_count);
  for (int i = 0; i < match.capture_count; ++i) {
    lst[i] = fine::make_resource<TSNode>(match.captures[i].node);
  }
  // TODO: Do we ever need the match id and pattern_index?
  return fine::Ok(
      std::tuple<uint64_t, uint64_t, std::vector<fine::ResourcePtr<TSNode>>>(
          match.id, match.pattern_index, lst));

The error fish: Job 1, 'mix test' terminated by signal SIGSEGV (Address boundary error)

The full code is available here: https://codeberg.org/edwinvanl/treesitter_elixir/src/commit/cc104432af4b3765d2e4b557bf6105327bb324c4/src/treesitter_nif.cpp#L234
and the original C nif code: https://codeberg.org/edwinvanl/treesitter_elixir/src/commit/30d57f683876a12de303679636c872b19ec6f0a6/src/treesitter_nif.c#L444

[BlackEdder]

It works fine if I wrap the TSNode in a specific wrapper:

class WRNode {
public:
  TSNode data;

  WRNode(TSNode data) : data(data) {}
};
FINE_RESOURCE(WRNode);

although I am not sure why. In both cases the copy constructor of TSNode is used as far as I can tell. Feel free to close though.

[BlackEdder]

EDIT: Simplify and fix the examples

Sorry for the multiple comments, but as another data point. The following gives the same error:

std::variant<fine::Ok<fine::ResourcePtr<WRNode>>, fine::Error<>>
query_cursor_next_capture([[maybe_unused]] ErlNifEnv *env,
                          fine::ResourcePtr<WRQueryCursor> querycursor) {
  TSQueryMatch match;
  uint32_t capture_index;
  if (!ts_query_cursor_next_capture(querycursor->data, &match,
                                    &capture_index)) {
    return fine::Error();
  }

  return fine::Ok(fine::make_resource<WRNode>(match.captures[capture_index].node))
}
FINE_NIF(query_cursor_next_capture, 0);

While if I instantiate it first it works fine. This could be due to fine::Ok handling of rvalues?

std::variant<fine::Ok<fine::ResourcePtr<WRNode>>, fine::Error<>>
query_cursor_next_capture([[maybe_unused]] ErlNifEnv *env,
                          fine::ResourcePtr<WRQueryCursor> querycursor) {
  TSQueryMatch match;
  uint32_t capture_index;
  if (!ts_query_cursor_next_capture(querycursor->data, &match,
                                    &capture_index)) {
    return fine::Error();
  }

  auto node = fine::make_resource<WRNode>(match.captures[capture_index].node);

  return fine::Ok(node);
}
FINE_NIF(query_cursor_next_capture, 0);

[jonatanklosko]

Hey @BlackEdder, this may have to do with Ok implementation, if you have an idea how to fix it, you can modify deps/fine/c_include/fine.hpp and recompile. I tried to run the tests to debug with lldb, but it looks like your repo depends on global tree-sitter and languages installation, so the build fails for me. If you find a minimal reproduction, for example by modifying fine/example, I am happy to take a look :)

[BlackEdder]

Hey, thank you for getting back to me. A bit more digging now suggests that it only happens when I use the captured node to call functions that return strings from the c api. Any tip on what the most memory safe way of doing that is? Especially when I get a char* back. I have tried a couple of variations, but none seem to work. Will try returning it as a binary when I have more time to look into it.

std::string node_string([[maybe_unused]] ErlNifEnv *env,
                        fine::ResourcePtr<WRNode> node) {
  char *buf = ts_node_string(node->data);
  std::string str = buf;
  free(buf);
  return str;
}
FINE_NIF(node_string, 0);

std::string node_type([[maybe_unused]] ErlNifEnv *env,
                      fine::ResourcePtr<WRNode> node) {
  std::string str(ts_node_type(node->data));
  return str;
}
FINE_NIF(node_type, 0);

The below C code did seem to work

static ERL_NIF_TERM node_string(ErlNifEnv *env, int argc,
                                const ERL_NIF_TERM argv[]) {
  WRNode *wrap;
  if (!enif_get_resource(env, argv[0], node_resource_type, (void **)&wrap)) {
    return to_error_string(env, "Unable to unwrap tree");
  }
  char *string = ts_node_string(wrap->data);

  // Free all of the heap-allocated memory.
  ERL_NIF_TERM type = enif_make_string(env, string, ERL_NIF_UTF8);
  free(string);
  return type;
}

static ERL_NIF_TERM node_type(ErlNifEnv *env, int argc,
                              const ERL_NIF_TERM argv[]) {
  WRNode *wrap;
  if (!enif_get_resource(env, argv[0], node_resource_type, (void **)&wrap)) {
    return to_error_string(env, "Unable to unwrap tree");
  }
  const char *string = ts_node_type(wrap->data);

  // Free all of the heap-allocated memory.
  ERL_NIF_TERM type = enif_make_string(env, string, ERL_NIF_UTF8);
  return type;
}

[jonatanklosko]

The string copy looks good to me. The question is what causes the segfault, is it node->data, ts_node_string, or std::string str = buf. You can investigate this with lldb, the easiest way to do this is:

1. At the top of your test_helper.exs add:

   IO.gets("PID: #{:os.getpid()} Press enter...")

2. In a separate terminal run lldb, once it pops up type attach PRINTED_PID (+ Enter), then type continue (+ Enter).

3. Go back to the first terminal and hit Enter.

4. Once the tests segfault, the process should be paused. If you go to lldb it should show a segfault. You can then type bt + Enter to expand more stacktrace. Hopefully this tells you where the segfault happens.

[BlackEdder]

Thanks that was very useful. Seems the error happens when accessing the wrapped node.

(lldb) bt
* thread #9, name = 'erts_sched_5', stop reason = signal SIGSEGV: address not mapped to object (fault address=0xffffffff000000f6)
  * frame #0: 0x000070d6ca404b62 libtree-sitter.so.0.25`ts_node_string + 82
    frame #1: 0x000070d6ca6bf760 briefly-576460749323258548-oIg5yae0XSjUepjMKgGM.so`node_string[abi:cxx11](env=0x000070d6cbb8ac30, node=ResourcePtr<WRNode> @ 0x000070d6cbb8aaa8) at treesitter_nif.cpp:117:29
    frame #2: 0x000070d6ca6ca57e briefly-576460749323258548-oIg5yae0XSjUepjMKgGM.so`unsigned long fine::__private__::nif_impl<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, fine::ResourcePtr<WRNode>, 0ul>(env=0x000070d6cbb8ac30, argv=0x000070d6cbb8ad00, fun=(briefly-576460749323258548-oIg5yae0XSjUepjMKgGM.so`node_string[abi:cxx11](enif_environment_t*, fine::ResourcePtr<WRNode>) at treesitter_nif.cpp:113:57), (null)=std::index_sequence @ 0x000070d6cbb8ab60) at fine.hpp:1081:10
    frame #3: 0x000070d6ca6c5b6a briefly-576460749323258548-oIg5yae0XSjUepjMKgGM.so`unsigned long fine::nif<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char>>, fine::ResourcePtr<WRNode>>(env=0x000070d6cbb8ac30, argc=1, argv=0x000070d6cbb8ad00, fun=(briefly-576460749323258548-oIg5yae0XSjUepjMKgGM.so`node_string[abi:cxx11](enif_environment_t*, fine::ResourcePtr<WRNode>) at treesitter_nif.cpp:113:57)) at fine.hpp:1108:31
    frame #4: 0x000070d6ca6bf86c briefly-576460749323258548-oIg5yae0XSjUepjMKgGM.so`node_string_nif(env=0x000070d6cbb8ac30, argc=1, argv=0x000070d6cbb8ad00) at treesitter_nif.cpp:122:1
    frame #5: 0x00005a8b72e4814d beam.smp`beam_jit_call_nif(process*, void const*, unsigned long*, unsigned long (*)(enif_environment_t*, int, unsigned long*), erl_module_nif*) + 189

I tried a couple of different function and the stacktrace always refers to fine.hpp:1081, but that might just be because it is always when accessing the wrapped node.

[jonatanklosko]

The top entry is libtree-sitter.so.0.25`ts_node_string + 82, which means the segfault happens within ts_node_string. The wrap->data we pass is a TSNode struct, which consists of pointers, so it seems that either those pointers are not set correctly or have already been deallocated. The tree-sitter code tries to access data at those pointers, and that results in a segfault.

And it is still passing temporary vs local variable to fine::Ok that makes the difference?

[edwinvanl]

Yeah, that makes sense. It is just strange that it works fine in the C version, and that it seems to work fine for most functions. Does resourcePtr work like a shared_ptr and count the number of references to the data?

I did notice this null, which looks significant?

(null)=std::index_sequence @ 0x000070d6cbb8ab60) at fine.hpp:1081:10

EDIT: Sorry this is BlackEdder, just using the wrong account :)

[jonatanklosko]

To make sure we are on the same page, changing from temporary variable to before fine::Ok avoids the error in node_string?

I did notice this null, which looks significant?

That's expected. fine.hpp:1081 points to a nif wrapping function that Fine generates, std::index_sequence is a compile time construct and at runtime it is an empty object (therefore null).

[BlackEdder]

Ah sorry, no, I thought that fixed it, but it that was because I also changed the test at the same time :(. Currently ts_node_type seems to sometimes result in a segmentation fault, irregardless of how I "capture" the TSNode.

[jonatanklosko]

You say "sometimes", so it's not deterministic and sometimes it works?

[BlackEdder]

It is not completely deterministic, but almost always. Using lldb the crash either happens when calling node_type, or the next time the node is used/accessed. In about 5% of the cases, it does not crash, but seems to return the wrong output. If I remove all calls to node_type it seems to work as expected.

[jonatanklosko]

In about 5% of the cases, it does not crash, but seems to return the wrong output.

Makes sense. It means the code accesses pointer with invalid value (e.g. into deallocated memory), and it crashes or returns garbage, depending on whether the target memory is accessible for the process (it may be accessible, if process has something else allocated at that memory).

I don't have more ideas. To investigate further, I would need a reproduction that I can compile.