-
Notifications
You must be signed in to change notification settings - Fork 184
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
Table above 64 KB fails, was Mismatched in create_cached_vtable() #235
Comments
Well, that doesn't sound quite right, but for a start, why do you have id:0 on multiple table table fields? And why do you use id's at all? And why does flatcc not raise an error when id is reused? As to pointing into the middle of something: that may be a problem but it does not have to be: However, I'm not sure that vtables are placed on the same stack as tempory table fields so it could be a problem. |
If there is a bug, ignoring the id:0 issue, it is possibly triggered by unusually large amounts of struct data that could trigger a stack reallocation that isn't handled correctly in that situation. In general reallocation should not be an issue, but there have been bugs there before. |
Table data is limited to a little under 64 KB because vtable offsets are 16 bits unsigned. I don't think you are hitting that limit, but as far as I can tell, you haven't provided all details of the schema, so you might be hitting that issue. You should get an error then, but who knows? Not likely the issue here, but worth mentioning. |
There is no id:0 issue. Sorry it's my mistake (copy paste). I corrected it. Anyway, removing the ids doesn't help. After calling Msg_Message_create_as_root(B, msg_payload), I call flatcc_builder_finalize_buffer(B, size_ptr) and the return buffer is my flatbuffer binary. The final binary buffer has a size of 96,278 Bytes, a little bit more than 94KB |
OK, I suspected id:0 wasn't the issue. Either way, I suggest you check the return values of all operations to see if any of them fail. There are also options to compile the builder with some debug assertions, look at the source. Once return errors have been ruled out, it may be necessary to debug. For that I would need a fully reproducible project. It can happen over a private channel if you prefer. |
One potential cause of the error is that large structs (or really any large table field) might not be written correctly to the ring buffer in the emitter. vtables are placed at the end of flatcc binary buffer, but during construction that is in the middle of the first page of buffer. vtables are appended, and other data is prepended, and the ring buffer can grow in both directions. If the struct data is somehow allowed to expand into the vtable area of the first page, that could explain what you are seeing. The ring buffer is a doubly linked list with about 2 KB pages by default, AFAIR. You could debug the emit calls to see if that happens. |
https://github.com/dvidelabs/flatcc/blob/master/src/runtime/emitter.c This is the default implementation, you can also add your own, here one that only prints information and stores nothing: |
To further understand this, the emitter uses address 0 to refer to the middle of the first page. Ordinary ref_t values are negative values relative to this location. If the offset is wrong, data might land in the wrong place. |
"It appears that you only really store any significant amount of data in the single Configuration table." " It can happen over a private channel if you prefer" |
You have my email in my profile. |
Anyway, if you are writing that much data, that would be your problem. So yes, maybe there should have been an error check for that, but it would not make your problem go away. Also, it may be better to use table fields for configuration data, especially if you generally only use a subset of those fields (I do that in some project). The vtables will be larger, but you will store much less data because unused fields do not take up space, unlike structs. Your are ultimately still limited by the table size if you use all data, so you may have to split up the data regardless of what you do. |
Alternatively, you can store data vectors of structs. These are limited to 2 GB. This is different from fixed length arrays used inside structs. |
"so you may have to split up the data regardless of what you do" |
No, I mean using multiple tables in the same buffer. The table_add method calls push_ds_field which computes the size of the table: Line 1811 in 5ff2e60
Line 267 in 5ff2e60
You might be able to instrument this to see if you hit the ds_limit.
That null is forwarded by table_add, and hopefully all the way out to end user calls? |
To be clear on splitting: |
Actually I need to send the all configuration (the all 96K) so it won't help to split right ? |
It will, but it may not help to use table fields over struct fields. |
However, I don't think it is a good idea to have such large structs: they must also be valid in all C compilers, and they could easily have a limit of, say 128 KB that you might hit sooner that you would like. https://stackoverflow.com/questions/7724790/are-there-any-size-limitations-for-c-structures#7724909 |
Do we have the same problem with flatc ? |
flatc also represents structs as native C++ structs as far as I recall. But it seems to me that a vector of smaller structs would be much better way to deal with your problem. The memory layout can even be the same since vectors of scalars and structs are largely C arrays. The only exception is that you need to use the generated API accessors if you are on big endian systems, but that also applies in your current setup. Regardless, I suggest
If you cannot confirm the problem, we may have to consider debugging flatcc. |
This is what I get by adding prints in push_ds_field:
It seems there is no error |
I'm not sure this is a size issue. I checked the sizes and I get the following:
When CoreCfgField6_Table is a table containing 6 structs of the following sizes:
So there is no structs more than 64K |
Thanks for data. The problem is that the sum of the individual fields reach more than 64 KB. The main problem is that vtable offsets are 2 bytes so they cannot point beyond that. But in principle the end of the last field could go beyond that. However, the table size is also stored in 2 bytes so at the very least the buffer should fail verification. Therefore the builder should also reject such large tables. I am not sure why you do not get an error, but this area probably hasn't be used or tested much since you are not allowed to do it anyway. My next suggestion: Reduce the size of the table gradually until you get below 64 KB just for testing. Run the verifier on buffer each time you reduce, and see if you can confirm success once you below the limit. If you can show the buffer is valid below 64 KB and if you also confirm that none of the data is corrupted (the verifier does not check everything), then:
We already know you are doing something wrong right now, but that does not rule out a bug in flatcc also. I think the data you posted in the previous mail is interesting. Please keep the code for that project as is, even if you make changes. We might not to add more debugging to see if and where error reporting fails. |
To be clear: CoreCfg has 1 field of size 92 KB, but that is a child table so it does not count. Also, the problem with maximum struct size is more C compiler issue than a FlatBuffer issue. In principle very large structs could be stored in vectors even if they do not fit in tables. |
I think I know why the error reporting fails: The check in push_ds_table is only to see if the internal builder stack has enough space. If not, it allocates more memory. If that fails, it returns 0. This is fine, but it doesn't check the table size, that is not the purpose of that check. So I think an error would be appropriate, but not worth runtime nor development effort. This leaves us with you confirming that there is no problem when you go below 64 KB in a table. |
If I change all CoreCfgField6_Table fields to tables it will be OK ? |
Yes, that should be fine, at least with the sizes you currently have. |
This is a bit too involved for me to test, but I added a runtime check to fail and optionally assert if the table is too large for the FlatBuffers voffset_t data type. I think this was the assumed root cause of this issue. Fixed in f8c346f . |
Hello
I have a quit complex shema like this:
To build Message as root I create structs and tables, including CoreCfg table like this:
When calling Cfg_Configuration_create() it appears that field54 in struct FilterConfig is corrupt.
Looking inside I found that it occurs in flatcc_builder_create_cached_vtable()
Calling stack:
Cfg_Configuration_create() -> Cfg_Configuration_end() -> flatcc_builder_end_table() -> flatcc_builder_create_cached_vtable()
In flatcc_builder_create_cached_vtable() there is a memcpy(vt_, vt, vt_size); and vt_ points in the middle of struct FilterConfig (field54)
It looks like a bug but I may do something wrong.
Thanks
The text was updated successfully, but these errors were encountered: