dstep icon indicating copy to clipboard operation
dstep copied to clipboard

Automatically add unittests to generated bindings that validate that C and D struct sizes match

Open Cogitri opened this issue 4 years ago • 5 comments

Otherwise programs using the bindings can SIGSEGV when using the structs due to size mismatches.

Cogitri avatar May 05 '20 09:05 Cogitri

I'm not sure I understand. The only reason the struct sizes would differ is if there's a bug in DStep. The whole point of DStep is to automate this to make sure it's correct.

jacob-carlborg avatar May 11 '20 17:05 jacob-carlborg

The only reason the struct sizes would differ is if there's a bug in DStep.

Yes, the point of having these would be to make sure that in case DStep does have a bug it's caught immediately, since debugging size mismatches in the bindings tends to be rather hard to do.

E.g. dstep misgenerated the following C struct for me:

#define APK_ARRAY(array_type_name, elem_type_name)                      \
        struct array_type_name {                                        \
                size_t num;                                             \
                elem_type_name item[];                                  \
        }; 

APK_ARRAY(apk_string_array, char*);

It generated the following D code for that:

struct apk_string_array {
    size_t num;
    char*[] item;
}

Which is actually 8 bytes too big (16 bytes instead of 8). The correct definition would be:

struct apk_string_array {
    size_t num;
    char*[0] item;
}

If there was a

version(X86_64) {
    static assert(apk_string_array.sizeof == 8);
}

generated by dstep for me as well this would've been an easy fix and instantly spotted upon building the bindings the first time, but because dstep currently doesn't generate these I just happened to notice random SIGSEGVs in my program and had a rather hard time to actually debug them. I guess the tests would be kind of a safety fallback in case dstep does generate wrong code, so bugs are found more easily. FWIW other bindings generators, such as rust-bindgen, generate these checks automatically. I had a few more places where the sizes mismatched in my generated bindings and it'd be very nice if these were detected automatically.

If you think it'd be a good idea to add these, I'll try to look into that in a bit - but exam time starts here in a bit, so maybe I'd try to make dstep my semester holiday project :)

Cogitri avatar May 11 '20 17:05 Cogitri

Hmm, that might be useful.

DStep would only be able to generate the tests for the platform it currently generates the bindings.

jacob-carlborg avatar May 11 '20 17:05 jacob-carlborg

Yup, but that'd still be very useful.

Cogitri avatar May 11 '20 18:05 Cogitri

If you think it'd be a good idea to add these, I'll try to look into that in a bit

Yes, please do. It should be possible to enable/disable with a flag.

I'd try to make dstep my semester holiday project

That sounds like a great idea 😄.

jacob-carlborg avatar May 12 '20 17:05 jacob-carlborg