dvidelabs / flatcc

FlatBuffers Compiler and Library in C for C
Apache License 2.0
632 stars 180 forks source link

buffer containing large vector #220

Closed hinxx closed 2 years ago

hinxx commented 2 years ago

I'm creating a vector of doubles inside the table, inside the union. If vector is 100 elements big, verification passes. When I use 1000 vector elements the verification fails with: buffer header too small.

The code is this:

int ret;
LogData_start_as_root(B);
flatbuffers_double_vec_ref_t vec = flatbuffers_double_vec_create(B, (double *)channel->dataPtr, channel->dataCount);
ArrayDouble_ref_t val = ArrayDouble_create(B, vec);
Value_union_ref_t un = Value_as_ArrayDouble(val);
ret = LogData_source_name_create_str(B, channel->name);
printf("%d: ret = %d\n",__LINE__, ret);
ret = LogData_timestamp_add(B, channel->timeStamp.secPastEpoch);
printf("%d: ret = %d\n",__LINE__, ret);
ret = LogData_value_add_value(B, un);
printf("%d: ret = %d\n",__LINE__, ret);
ret = LogData_value_add_type(B, un.type);
printf("%d: ret = %d\n",__LINE__, ret);
ret = LogData_severity_add(B, AlarmSeverity_MAJOR);
printf("%d: ret = %d\n",__LINE__, ret);
ret = LogData_status_add(B, AlarmStatus_HIHI);
printf("%d: ret = %d\n",__LINE__, ret);
LogData_end_as_root(B);

All the return statuses are 0.

Am I supposed to manually resize the buffer of something?

I'm struggling to figure out which macros / functions from the generated header to use in order to pull this off.

mikkelfj commented 2 years ago

I can't see anything obviously wrong. You can create arrays using a source array to copy from, like you do with create. In this case the size is allocated immediately to the specified size, and should not be wrong. It effectively works the same as creating strings.

You can also start and end an array, especially if you do not know the total size yet, or do not want to maintain you own full size source array. In this case you have to be more careful because you get a pointer to a raw buffer that only has a reserved size and is only valid until other operations are called. You can reserve more memory in the temp buffer and get a new pointer if you dynamically run out of space. The push method simplifies this by reserving one element extra on each push, but is a bit slower.

Everything works like a stack: tables, strings, vectors. When the end is callled, the stack is popped and content is written (emitted) to the output holding buffer. You can resize the top stack element. Create calls create a temp stack element (with multiple array elements for vectors) and immediately pops it again after endian conversion, or bypasses the stack entirely.

mikkelfj commented 2 years ago

One thing though, when you add the union you can either add the union as whole in one call, or you can add the union value and the union type separately. It seems like you do a combination of adding the whole union and the type as well. The union ref is a struct of two fields: a value ref, and a type code. I don't think this is related to your problem, you just overwrite the type field with the same value while it is still valid to do so.

hinxx commented 2 years ago

I must be doing something terribly wrong. Here are bugreport sources amended with a simple vector of int64_t that works for 100 elements but fails with 200 elements.

I get assert() on returned buf or this if assert() is removed:

running build/myissue
myissue:
  |��v��|
Eclectic.FooBar buffer for myissue:
  |��v��|
could not verify Electic.FooBar table, got buffer header too small
failed

bugreport.zip

mikkelfj commented 2 years ago

I will have a look at your report when I find some time, thanks.

Meanwhile, I recalled an old outstanding bug that is presumed to be bug in the verifier related to large double arrays: https://github.com/dvidelabs/flatcc/issues/210

However, it does not seem to match the error you are getting?

You could try to verify with C++.

hinxx commented 2 years ago

You could try to verify with C++.

Can't do because flatcc_builder_get_direct_buffer() returns buf == NULL and size == 0. That is why the assert is raised in the bugreport code above.

mikkelfj commented 2 years ago

Oh, that explains it all.

Get direct buffer is only for small buffers. If the buffer is too large, it returns null. It is because the stack explained earlier, is pop'ed and emitted to a temporary ringbuffer. If the buffer is small enough, a single page in the ringbuffer covers the full buffer. Otherwise dynamic allocation is required to convert the buffer into contigous memory. You can either ask for such a buffer or provide your own after asking for the required size.

mikkelfj commented 2 years ago

https://github.com/dvidelabs/flatcc/blob/bca3b5fcd77c3e990fef75fae615b8f62f66af0e/include/flatcc/flatcc_builder.h#L1790-L1814

hinxx commented 2 years ago

OK, using flatcc_builder_finalize_buffer() instead I can work with large vectors!! Thank you for the help!!