-
Notifications
You must be signed in to change notification settings - Fork 182
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
Endless loop during gen_reflection_test #12
Comments
Is this a little or big endian machine? It may be worthwhile disabling the reflection test until other tests pass because it is a higher level test that depends on other things working, such as building flatbuffers. mkdir -p build/noreflection |
Big endian. Big endian rules. :) I'll try to create without reflection and see what that gets me. BTW, I dumped out the value of the next member during that while loop:
next = 50 |
That issue inside flatcc_builder_create_cached_vtable will be hard to track down without debugger access. Two possible reasons for this issue could be incorrect initialization of variables or other state which is triggered by this particular compiler, or a different hash computation that triggers a bug not present in the current test on other machines, or incorrect assumpions during pointer cast. |
I get a clean compile now, but tests fail. I'll dig deeper into why next. Here's the log from the tests:
|
OK, this is not unexpected as flatcc hasn't yet been tested with big endian, as documented. However, I suspect this particular bug may have other reasons than big endian such as non-zeroed allocation failing to terminate the hash list being traversed. |
Sure, we'll work together. I've worked with other projects to get them ported to AIX and big endian before. Get some rest, we'll approach more tomorrow. Thanks! |
Great thanks - I've done a lot big endian work on mobile devices in a former life. |
I have pushed a fix that should solve this particular issue. I was able to reproduce by pretending flatbuffers was a big endian format. My suspicion about changed hash value triggering a bug was correct, and the bug also applies to little endian. However, due to the multiplicative hash function, collisions are more likely on big endian, which triggered the problem. It was a simple pointer chaining mistake during a move to head hash optimization. This, of course, does not address all the other big endian related issues, but is comforting to know that the basic reader operation seems to work, otherwise flatc_compat wouldn't pass - it reads a pregenerated flatbuffer, and it correctly fails when I pretend flatbuffers is a big endian format. |
I also pushed a fix because vtables were only partially converted to protocol endian. I use the term native endian and protocol endian, because in flatcc_types.h it is possible to change the format: |
Downloaded current master. Still building without reflection. Improvement with the tests!
Nothing hung, I'm going to dig in shortly to see why particular tests are failing. Thanks! |
If you're curious, here's the output from the latest monster_test:
|
Thanks, this confirms what I can also see by encoding the buffer as big endian. Structs are read correctly but apparently not converted during build, at least in some cases (Vec3_t), and I also see some segfaults I have not tracked down yet. I have more problems though, because big endian encoding triggers issues not visible on little endian encoding on big endian platforms - notably the flatbuffer identifier field - I'll get back when I get this sorted. |
This might help you as well. From test_json_d:
|
thanks |
I pushed a fix to the struct issue. In general structs are supported and do pass tests without this fix, but the Vec3_create(arg list) call did a double endian conversion, which isn't effective. In more detail the builder has a concept of struct_start, user fills in returned pointer, and calls struct_end. The end call can be _end, or _end_pe if the struct is already stored endian converted. This fix ensures the end_pe version is being used because the struct was already assigned using assign_to_pe in the internal buffer. There are several other issues I will look into separately. |
Moving discussion to #13 and closing this issue - even if reflection doesn't fully work on big endian, the critical bug that affects little endian platforms has been fixed. |
Testing the latest master.
During the "Scanning dependencies of target gen_reflection_test" phase, process goes runaway.
root 1286372 872504 93 12:45:32 pts/0 2:19 ../../../../bin/flatcc_d --schema -o /work/stupc/flatcc-master/build/Debug/test/reflection_test/generated /work/stupc/flatcc-master/test/monster_test/monster_test.fbs
Attaching with the debugger shows that its stuck in an endless loop inside
flatcc_builder_create_cached_vtable().
Stack trace:
If you continue, and break again, you're still inside the same loop:
The text was updated successfully, but these errors were encountered: