Fix crash stemming from mismatch of the C++ and Rust interfaces bug #389419
Needs ReviewPublic

Authored by illis on Mar 5 2018, 10:59 PM.



Fix for Bug #389419
"MyExe" received signal SIGSEGV, Segmentation fault

Diff Detail

R881 Rust Qt Binding Generator
Lint Skipped
Unit Tests Skipped
illis requested review of this revision.Mar 5 2018, 10:59 PM
illis created this revision.
illis edited the summary of this revision. (Show Details)Mar 5 2018, 11:01 PM
illis added a reviewer: vandenoever.
illis retitled this revision from Fix crash steming from mismatch of the C++ and Rust interfaces bug #389419 to Fix crash stemming from mismatch of the C++ and Rust interfaces bug #389419.Mar 6 2018, 3:40 AM

After testing, I found that this patch crashes the demo in my docker on a 64 bit machine.

I suspect that the struct qbytearray_t and struct qstring_t should be placed in extern "C" { ... }.

That in turn means that the structs cannot have member functions.

qbytearray_t and qstring_t are passed to C, so they should be PODs. This requires quite large changes.

I did test the demo using docker - crashed without the patch for me, and works fine with the patch - Arch Linux, 4.15.7-1-ck-haswell, x86_64
I suspect I might need to fire up an actual virtual machine to replicate.

Most of Qt classes are same size as void* to keep ABI compatibility so you can try:

typedef void (*qstring_set)(QString*, void*);
void set_qstring(QString* v, void* val) {
    static_assert(sizeof(void*) >= sizeof(QString), "QString has a bigger size than void*, code below will crash");
    *v = *(QString*)&val;
illis added a comment.Mar 11 2018, 7:31 AM

@anthonyfieroni Just tried the copy pasting your void* idea in. No dice on my machine. Seg faulted trying to run the demo. Captured some valgrind output (attached), in case its of any use. I had to keep my qbytearray segment of the patch in, else it wouldn't even get that far (attached valgrind output for that too).

I'd have thought the same docker on the same hardware would give the same compiled code. I'll have a go at making a version of the patch where there are no qbytearray_t and no qstring_t.

See D11232 for a different approach.