Closed madebr closed 3 years ago
The monster test is a huge test with many test cases. The first step is to find out which test case drives the output. If the test doesn't already, it would help printing the test title in the output.
The second step is to figure out what might cause it. Most likely it is an uninitialised struct. Flatcc will zero structs and other data, but it also allow you to write more directly in some cases. If the user program doesn't zero structs, there could be uninitialised fields or padding. I'm not sure that is the case here. But it is a possibility.
Also, there is a theoretical possibility that the C compiler uses padding within structs for temporary data when optimising because flatcc uses alignas to arrange structs. Even if a struct is properly memset to zero, the optimiser is allowed to change that. This is a known concern, but never observed.
diff:
--- output1.txt 2020-12-21 03:43:38.028780149 +0100
+++ output2.txt 2020-12-21 03:43:38.044780240 +0100
@@ -30,7 +30,7 @@
00000180 a0 ff ff ff 04 00 00 00 05 00 00 00 4a 6f 6b 65 |............Joke|
00000190 72 00 00 00 b4 ff ff ff 04 00 00 00 07 00 00 00 |r...............|
000001a0 54 77 6f 46 61 63 65 00 05 00 00 00 10 00 20 00 |TwoFace....... .|
-000001b0 30 00 40 00 50 00 60 00 70 00 80 00 91 01 91 81 |0.@.P.`.p.......|
+000001b0 30 00 40 00 50 00 60 00 70 00 80 00 91 01 91 57 |0.@.P.`.p......W|
000001c0 0a 00 00 00 00 01 02 03 04 05 06 07 08 09 00 00 |................|
000001d0 09 00 00 00 4d 79 4d 6f 6e 73 74 65 72 00 00 00 |....MyMonster...|
000001e0 0c 00 08 00 00 00 00 00 00 00 04 00 16 00 10 00 |................|
@@ -70,7 +70,7 @@
00000180 a0 ff ff ff 04 00 00 00 05 00 00 00 4a 6f 6b 65 |............Joke|
00000190 72 00 00 00 b4 ff ff ff 04 00 00 00 07 00 00 00 |r...............|
000001a0 54 77 6f 46 61 63 65 00 05 00 00 00 10 00 20 00 |TwoFace....... .|
-000001b0 30 00 40 00 50 00 60 00 70 00 80 00 91 01 91 7e |0.@.P.`.p......~|
+000001b0 30 00 40 00 50 00 60 00 70 00 80 00 91 01 91 54 |0.@.P.`.p......T|
000001c0 0a 00 00 00 00 01 02 03 04 05 06 07 08 09 00 00 |................|
000001d0 09 00 00 00 4d 79 4d 6f 6e 73 74 65 72 00 00 00 |....MyMonster...|
000001e0 0c 00 08 00 00 00 00 00 00 00 04 00 16 00 10 00 |................|
So it is one of the early tests. The function monster_test() calls gen_monster()
It seems that the variable x is a struct name Test:
x isn't zero initialised and has a 16 bit and an 8 bit field, and thus 8 bits of padding. The test doesn't zero the struct first so the padding is undefined as I suspected. Changing the declaration of x to x = { 0 } might fix the problem, but the compiler is not required to zero the padding.
Note that this doesn't create incompatible buffers, but the practice of not zero padding is frowned upon, exactly because it results in unexpected diffs, and because it can leak information.
Or better ns(Test_clear(&x));
which will memset the struct.
Excerpt from documentation:
Because padding can carry noise and unintended information, structs should be cleared before assignment - but if used as a source to copy the padding is not copied so only the destination need to be zeroed.
If a struct is nested, the assign operation includes all fields as if the struct was flattened:
typedef struct Plane Plane_t;
struct Plane {
Vec3_t direction;
Vec3_t normal;
};
Plane_t plane;
Plane_clear(&plane);
Plane_assign(&plane, 1, 2, 3, 7, 8, 9);
Yup, adding a ns(Test_clear(&x));
prior to assignment makes the test fully reproducible.
Fixed on master.
Thanks for the fix!
When running the monster_test twice in a row, its output (stderr) is not the same. Run the following commands to reproduce:
The output is:
The diff is always in the same spot. I can also reproduce this on CI: https://github.com/madebr/flatcc/runs/1586518323?check_suite_focus=true