xyzzy42 / tg

A program for timing mechanical watches
GNU General Public License v2.0
34 stars 10 forks source link

ScaleButton.value-changed Windows/Linux parameter mismatch #3

Closed jakelewis3d closed 1 year ago

jakelewis3d commented 1 year ago

static void handle_zoom(GtkScaleButton b, struct output_panel op)

While this function's parameter's work in the Linux build, they segfault in the Windows build under msys. Checking the documentation for ScaleButton.value-changed, the correct parameters are

void value_changed ( GtkScaleButton* self, gdouble value, gpointer user_data)

and this is what the Windows build is receiving. The value parameter is a bit redundant as that can be extracted from the self parameter, I guess that's why the functionally similar Range.value-changed only has the two parameters. I haven't checked the Mac build, but to cross-compile on Linux and Windows I added this ugly #ifdef

static void handle_zoom(GtkScaleButton *b,

ifdef G_OS_WIN32

                    gdouble value,

endif

                    struct output_panel *op)

                    Regards, Jake
xyzzy42 commented 1 year ago

Thanks for finding this. I was aware there was some Windows bug, probably with the zoom support, but I don't do any windows development and couldn't replicate this under MacOS or Linux. I originally used a Range and never noticed the callback changed when I switched to a scale button. And unfortunately there's no type checking of this function because it's always cast to the signature of a generic prototype.

There's no need to check for the OS. Having the value parameter there is always correct.

The reason it only crashes on Windows is that Linux and MacOS use a newer, much superior, calling convention.

On Linux, the arguments to a function are placed into registers and only if there are more than registers used for this are they placed on the stack. So the two pointer arguments are passed in two of the general purpose registers, rdi and then rsi, while the double argument is passed in an SSE register. Since the double goes in a floating point register, the user_data argument will be put into rsi both with and without the double in the function arguments.

But Windows uses a much older standard dating back to the days of DOS, from back when 8086 systems had fewer registers and no floating point support at all and compilers were much simpler. It passes all argument by placing them on the stack. So the missing double argument means the callback is looking for user_data on the stack in the place where value was placed. And of course turning the double into a pointer will crash when it's used.

xyzzy42 commented 1 year ago

Should be fixed in 4834501f064d6224b0cd95512d809f6e328905ef