segfault when using a nested flatbuffer

[AndrewJDR]

First off -- this is probably a foolish error on my part.

But here goes. Given:

#include <stdio.h>
#include "qmsg.h"

#undef ns
#define ns(x) FLATBUFFERS_WRAP_NAMESPACE(QQQ, x)

int main()
{
  flatcc_builder_t builder;
  flatcc_builder_init(&builder);

  ns(Msg_start_as_root(&builder));
    ns(Msg_payload_start_as_root(&builder));
      ns(Payload_someId_add(&builder, 456));
    ns(Msg_payload_end_as_root(&builder));
  ns(Msg_end_as_root(&builder));

  size_t bufSize;
  uint8_t *bufPtr = (uint8_t*)flatcc_builder_get_direct_buffer(&builder, &bufSize);
  ns(Msg_table_t) msgTable = ns(Msg_as_root(bufPtr));
  ns(Payload_table_t) nestedPayload = ns(Msg_payload_as_root(msgTable));

  uint32_t someId = ns(Payload_someId(nestedPayload)); // <--- Why a segfault here?
  printf("someId: %u\n", someId);

  return 0;
}

With the following schema:

namespace QQQ;

table Payload
{
  someId:uint;
}

table Msg
{
  payload:[ubyte] (nested_flatbuffer: "Payload");
}

root_type Msg;

I'm segfaulting on the uint32_t someId = ns(Payload_someId(nestedPayload)); line:

bt
#0  0x00000000004008a3 in __flatbuffers_soffset_read_from_pe (p=0x10060b9c0) at flatcc/include/flatcc/flatcc_endian.h:83
#1  0x0000000000400a41 in QQQ_Payload_someId (t=0x10060b9c0) at qmsg.h:943
#2  0x0000000000400fdf in main () at fbs_crash.c:24

Would you happen to know why? I've basically tried to follow this example:
https://github.com/dvidelabs/flatcc/blob/master/test/monster_test/monster_test.c#L1736
And as far as I can see, I have done so. Hopefully I'm missing something silly?

Also, my build script (linux):

FLATBUFFERS_DIR=../flatcc
FLATCC_EXE=../flatcc/bin/flatcc
FLATCC_INCLUDE=$FLATBUFFERS_DIR/include
FLATCC_LIB=$FLATBUFFERS_DIR/lib

$FLATCC_EXE --stdout -a qmsg.fbs > qmsg.h || exit 1
gcc -I$FLATCC_INCLUDE -g -o fbs_crash fbs_crash.c -L$FLATCC_LIB -lflatcc -lflatccrt || exit 1
gdb --args ./fbs_crash

[mikkelfj]

From looking at the generated buffer it appears that the nested buffer is not inserting a 4-byte length field for the ubyte vector containing the nested buffer.

EDIT: root cause assumption incorrect - looking further

Table unions is an alterative approach that could be used as a workaround until I get this sorted.

Some additional hints -
gcc -I$FLATCC_INCLUDE -g -o fbs_crash fbs_crash.c -L$FLATCC_LIB -lflatcc -lflatccrt || exit 1

You likely only need -lflatccrt unless you want to compile schema in you application, and for debug purposes you want -lflatccrt_d because it has runtime assertions for things like failing to close a table - though it wouldn't have helped here because you use the interface correctly.

[AndrewJDR]

@mikkelfj Thanks!

I'm actually switching away from a table union toward nested flatbuffers because the table union approach was no longer serving my needs (basically I'm doing a lot of things like #49 and nesting just makes more sense at this point). I'm needing to do a lot of copying around of flatbuffers, storing them in databases, pulling from the the db and reassembling them in different ways, etc.

If you have any ideas for quick fixes, I'm comfortable using it in my codebase and testing it out before you roll it out to master.

[AndrewJDR]

as a side question:
Can a nested_flatbuffer property be set to a union type (e.g. nested_flatbuffer: "SomeUnion")?

[mikkelfj]

If you have any ideas for quick fixes, I'm comfortable using it in my codebase and testing it out.

Just add a member to the root table table in the schema and set the value before starting the nested buffer - no guarantee. But the problem is that buffer_mark can have a 0 value correctly, even for nested buffers.

A more detailed fix - (guessing) - but I can't do it without impacting performance - but you could try it as workaround:

The buffer_mark MUST have the correct value, so we cannot change it to mark an inner buffer. We also cannot use B->level because a it can vary. The is_top_buffer macro MUST return 1 or 0 because it is hack mapping to a flag with value 1.

A possibly quick fix is to add a buffer level counter in the builder in buffer start and buffer end. Then check this counter in is_top_buffer. It must return 1 both before and after entering the top-level buffer. So buffer_counter <= 1.

(For posterity - these lines won't remain valid after a fix but)

#define is_top_buffer(B) (B->buffer_mark == 0)
 https://github.com/dvidelabs/flatcc/blob/master/src/runtime/builder.c#L181

B->buffer_mark = B->level == 1 ? 0 : B->emit_start;
https://github.com/dvidelabs/flatcc/blob/master/src/runtime/builder.c#L779

[mikkelfj]

as a side question:
Can a nested_flatbuffer property be set to a union type (e.g. nested_flatbuffer: "SomeUnion")?

A nested buffer doesn't really exist. It is just a ubyte vector with the exception that alignment is different. A nested buffer is an independent binary entity stored in this vector. All the accessors are just sugar around this fact. However, getting the alignment right is tricky, which is what the builder does more than anything else.

As such, this buffer must contain a table and not a union. FlatCC also permits use of structs at as buffer roots, but most other bindings do not not, AFAIK.

[mikkelfj]

Can a nested_flatbuffer property be set to a union type (e.g. nested_flatbuffer: "SomeUnion")?

You can do something else though: there are variant of start_as_root_with_identifier and as_typed_root. The latter stores the type hash identifier for a given table type. This is in effect a schema independent union assuming you namespace appropriately.

EDIT: this is incorrect: you CAN get the buffer verified by using as_typed_root.

Note that retrieving a buffer will NOT verify the identifier, but you can still retrieve it because it is a standard buffer. Ther is a small risk of type hash conflicts, but a switch statement would then error, and you can change the namespace slightly. Let me know if you need more details.

What you cannot do is to verify a buffer with a nested buffer and make sure the nested buffer has a specific identifier. This is because there can be many embedded buffers with different identifiers. They will still be verified. And upon access you can make sure the type hash is as expected per the above.

[AndrewJDR]

Just add a member to the root table table in the schema and set the value before starting the nested buffer - no guarantee. But the problem is that buffer_mark can have a 0 value correctly, even for nested buffers.

Assuming you mean:

  ns(Msg_start_as_root(&builder));
    ns(Msg_firstint_add(&builder, 222));
    ns(Msg_payload_start_as_root(&builder));
      ns(Payload_someId_add(&builder, 456));
    ns(Msg_payload_end_as_root(&builder));
  ns(Msg_end_as_root(&builder));

and mod the schema to be like this:

table Msg
{
  firstint:uint;
  payload:[ubyte] (nested_flatbuffer: "Payload");
}

I already tried it, and it still segfaults :(
I'll try the more extensive fix tomorrow... thanks again.

[mikkelfj]

I think it is because the table is still pending on the stack, so emit_start does not move. If you create string, it will likely work - you probably don't even have to store it.

[mikkelfj]

  ns(Msg_start_as_root(&builder));
    flatcc_builder_create_string_str(&builder, "");
    ns(Msg_payload_start_as_root(&builder));
      ns(Payload_someId_add(&builder, 456));
    ns(Msg_payload_end_as_root(&builder));
  ns(Msg_end_as_root(&builder));

Seems to work for me.

[mikkelfj]

On nested buffers as unions:

https://github.com/dvidelabs/flatcc#file-and-type-identifiers

[mikkelfj]

The original idea of the type hash was to avoid having a containing table/buffer and detect a message directly based on the type hash.

[mikkelfj]

A made a correction to buffers with type hashes - see EDIT above.

[mikkelfj]

Fixed - it was actually a bit tricky because buffer_mark was used to identify top-level buffers and to ensure that vtables are not shared across different buffers. Therefore I had to introduce nest_id.
Added a test case for this issue in monster_test.

[AndrewJDR]

It's working! Thank you!