Skip to content
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

trash, in-process: fix segfault with glib 2.77.1 #658

Merged
merged 1 commit into from
Aug 13, 2023

Conversation

lukefromdc
Copy link
Member

*the animation on this applet isn't visible anyway even out of process where it still runs

*the animation on this applet isn't visible anyway even out of process where it still runs
@lukefromdc
Copy link
Member Author

With the trash applet in process, calling xstuff_zoom_animate() crashes with glib 2.77.1, oddly even when the function is emptied! Does not crash with glib 2.74, does for sure with glib 2.77, no idea what happens with glib 2.76 which would now be difficult to install

This is probably supposed to be the same animation as the launchers. Launchers and a few other applets shrink a tiny bit and pop back to fullsize when clicked, but I cannot recall ever seeing the trash applet's icon do that, so probably this animation for the trash has been broken for years, possiblly all the way back to the gtk2 -> gtk3 transition.

It's not crashing out of process, though probably not doing anything either, so for now I've limited it to the x11 and out of process case(it cannot be used in wayland). OK with removing it entirely since it doesn't seem to work if others agree.

@lukefromdc lukefromdc requested a review from a team August 13, 2023 05:36
@raveit65
Copy link
Member

Confirmed, the applet crashes with glib2-2.76.4-3.fc38.x86_64 in x11 but not in wayland session.

@raveit65
Copy link
Member

raveit65 commented Aug 13, 2023

Note, the crash happens in x11 and build-in-process because of xstuff_zoom_animate () function.

Core was generated by `mate-panel'.
Program terminated with signal SIGSEGV, Segmentation fault.
warning: Section `.reg-xstate/3165' in core file too small.
#0  xstuff_zoom_animate (widget=0x556313b67f40, surface=0x0, orientation=(PANEL_ORIENTATION_RIGHT | PANEL_ORIENTATION_BOTTOM), opt_rect=0x556645daae9e) at /usr/src/debug/mate-panel-1.27.1-3.fc38.x86_64/mate-panel/xstuff.c:353
353			rect = *opt_rect;
[Current thread is 1 (Thread 0x7f298b3e0ac0 (LWP 3165))]

Thread 1 (Thread 0x7f298b3e0ac0 (LWP 3165)):
#0  xstuff_zoom_animate (widget=0x556313b67f40, surface=0x0, orientation=(PANEL_ORIENTATION_RIGHT | PANEL_ORIENTATION_BOTTOM), opt_rect=0x556645daae9e) at /usr/src/debug/mate-panel-1.27.1-3.fc38.x86_64/mate-panel/xstuff.c:353
        gscreen = <optimized out>
        rect = {x = 309267792, y = 21859, width = -120, height = -1}
        dest = {x = 11, y = 0, width = -544541440, height = -2110984539}
        allocation = {x = 1600522218, y = 32553, width = -120, height = -1}
#1  0x00007f295f65f27d in trash_applet_button_release (widget=0x556313b67f40, event=<optimized out>) at /usr/src/debug/mate-applets-1.27.0-2.fc38.x86_64/trashapplet/src/trashapplet.c:269
        applet = 0x556313b67f40
        settings = 0x556313460500
#2  0x00007f298cc99fe7 in _gtk_marshal_BOOLEAN__BOXEDv (closure=0x5563126fa280, return_value=0x7ffe0a9b91c0, instance=<optimized out>, args=<optimized out>, marshal_data=<optimized out>, n_params=<optimized out>, param_types=0x5563126fa2b0) at gtk/gtkmarshalers.c:130
        data1 = <optimized out>
        data2 = 0x5563126f0d50
        callback = 0x7f295f65f1d0 <trash_applet_button_release>
        v_return = <optimized out>
        arg0 = 0x556313985ab0
        args_copy = {{gp_offset = 32, fp_offset = 48, overflow_arg_area = 0x7ffe0a9b9350, reg_save_area = 0x7ffe0a9b9290}}
        __func__ = "_gtk_marshal_BOOLEAN__BOXEDv"
#3  0x00007f298c8b7dba in _g_closure_invoke_va (param_types=0x5563126fa2b0, n_params=<optimized out>, args=0x7ffe0a9b9270, instance=0x556313b67f40, return_value=0x7ffe0a9b91c0, closure=0x5563126fa280) at ../gobject/gclosure.c:895
        marshal = <optimized out>
        marshal_data = <optimized out>
        in_marshal = 0
        real_closure = 0x5563126fa260
        return_accu = 0x7ffe0a9b91c0
        accu = {g_type = 0x14, data = {{v_int = 0, v_uint = 0, v_long = 0, v_ulong = 0, v_int64 = 0, v_uint64 = 0, v_float = 0, v_double = 0, v_pointer = 0x0}, {v_int = 0, v_uint = 0, v_long = 0, v_ulong = 0, v_int64 = 0, v_uint64 = 0, v_float = 0, v_double = 0, v_pointer = 0x0}}}
        accumulator = 0x5563126fa340
        emission = Python Exception <class 'TypeError'>: can only concatenate str (not "NoneType") to str
{next = 0x0, instance = 0x556313b67f40, ihint = {signal_id = 80, detail = 0, run_type = (G_SIGNAL_RUN_LAST | G_SIGNAL_ACCUMULATOR_FIRST_RUN)}, state = EMISSION_RUN, chain_type = }
        instance_type = Python Exception <class 'TypeError'>: can only concatenate str (not "NoneType") to str

        emission_return = {g_type = 0x14, data = {{v_int = 0, v_uint = 0, v_long = 0, v_ulong = 0, v_int64 = 0, v_uint64 = 0, v_float = 0, v_double = 0, v_pointer = 0x0}, {v_int = 0, v_uint = 0, v_long = 0, v_ulong = 0, v_int64 = 0, v_uint64 = 0, v_float = 0, v_double = 0, v_pointer = 0x0}}}
        rtype = 0x14
        static_scope = 0
        fastpath_handler = <optimized out>
        closure = <optimized out>
        run_type = <optimized out>
        hlist = <optimized out>
        l = <optimized out>
        fastpath = <optimized out>
        instance_and_params = <optimized out>
        signal_return_type = <optimized out>
        param_values = <optimized out>
        node = <optimized out>
        i = <optimized out>
        n_params = <optimized out>
        __func__ = "g_signal_emit_valist"
#4  g_signal_emit_valist (instance=0x556313b67f40, signal_id=80, detail=0, var_args=var_args@entry=0x7ffe0a9b9270) at ../gobject/gsignal.c:3472
        return_accu = 0x7ffe0a9b91c0
        accu = {g_type = 0x14, data = {{v_int = 0, v_uint = 0, v_long = 0, v_ulong = 0, v_int64 = 0, v_uint64 = 0, v_float = 0, v_double = 0, v_pointer = 0x0}, {v_int = 0, v_uint = 0, v_long = 0, v_ulong = 0, v_int64 = 0, v_uint64 = 0, v_float = 0, v_double = 0, v_pointer = 0x0}}}
        accumulator = 0x5563126fa340
        emission = Python Exception <class 'TypeError'>: can only concatenate str (not "NoneType") to str
{next = 0x0, instance = 0x556313b67f40, ihint = {signal_id = 80, detail = 0, run_type = (G_SIGNAL_RUN_LAST | G_SIGNAL_ACCUMULATOR_FIRST_RUN)}, state = EMISSION_RUN, chain_type = }
        instance_type = Python Exception <class 'TypeError'>: can only concatenate str (not "NoneType") to str

        emission_return = {g_type = 0x14, data = {{v_int = 0, v_uint = 0, v_long = 0, v_ulong = 0, v_int64 = 0, v_uint64 = 0, v_float = 0, v_double = 0, v_pointer = 0x0}, {v_int = 0, v_uint = 0, v_long = 0, v_ulong = 0, v_int64 = 0, v_uint64 = 0, v_float = 0, v_double = 0, v_pointer = 0x0}}}
        rtype = 0x14
        static_scope = 0
        fastpath_handler = <optimized out>
        closure = <optimized out>
        run_type = <optimized out>
        hlist = <optimized out>
        l = <optimized out>
        fastpath = <optimized out>
        instance_and_params = <optimized out>
        signal_return_type = <optimized out>
        param_values = <optimized out>
        node = <optimized out>
        i = <optimized out>
        n_params = <optimized out>
        __func__ = "g_signal_emit_valist"
#5  0x00007f298c8b7f33 in g_signal_emit (instance=instance@entry=0x556313b67f40, signal_id=<optimized out>, detail=detail@entry=0) at ../gobject/gsignal.c:3622
        var_args = {{gp_offset = 24, fp_offset = 48, overflow_arg_area = 0x7ffe0a9b9350, reg_save_area = 0x7ffe0a9b9290}}
#6  0x00007f298cf71174 in gtk_widget_event_internal.part.0.lto_priv.0 (widget=0x556313b67f40, event=0x556313985ab0) at ../gtk/gtkwidget.c:7812
        signal_num = <optimized out>
        return_val = <optimized out>
        handled = 0
#7  0x00007f298ce08580 in propagate_event_up (topmost=<optimized out>, event=<optimized out>, widget=0x556313b67f40) at ../gtk/gtkmain.c:2588
        tmp = <optimized out>
        handled_event = <optimized out>
        handled_event = 0
#8  propagate_event (widget=widget@entry=0x556313b67f40, event=event@entry=0x556313985ab0, captured=captured@entry=0, topmost=topmost@entry=0x0) at ../gtk/gtkmain.c:2691
        handled_event = 0
#9  0x00007f298ce086af in gtk_propagate_event (event=0x556313985ab0, widget=0x556313b67f40) at ../gtk/gtkmain.c:2725
        __func__ = "gtk_propagate_event"
#10 0x00007f298ce0911a in gtk_main_do_event (event=0x556313985ab0) at ../gtk/gtkmain.c:1921
        grab_widget = 0x556313b67f40
        window_group = 0x556312b346e0
        rewritten_event = <optimized out>
        device = <optimized out>
        tmp_list = <optimized out>
        event_widget = 0x556313b67f40
        topmost_widget = <optimized out>
        __func__ = "gtk_main_do_event"
#11 gtk_main_do_event (event=<optimized out>) at ../gtk/gtkmain.c:1691
        __func__ = "gtk_main_do_event"
#12 0x00007f298d4c7427 in _gdk_event_emit (event=0x556313985ab0) at ../gdk/gdkevents.c:73
#13 _gdk_event_emit (event=0x556313985ab0) at ../gdk/gdkevents.c:67
#14 0x00007f298d52082e in gdk_event_source_dispatch.lto_priv () at ../gdk/x11/gdkeventsource.c:354
#15 0x00007f298c79748c in g_main_dispatch (context=0x5563126ce7d0) at ../glib/gmain.c:3460
        dispatch = 0x7f298d520800 <gdk_event_source_dispatch.lto_priv>
        prev_source = 0x0
        begin_time_nsec = 5220801091955
        was_in_call = 0
        user_data = 0x0
        callback = 0x0
        cb_funcs = 0x0
        cb_data = 0x0
        need_destroy = <optimized out>
        source = 0x5563126c3830
        current = 0x5563126ea410
        i = 0
#16 g_main_context_dispatch (context=0x5563126ce7d0) at ../glib/gmain.c:4200
#17 0x00007f298c7f5648 in g_main_context_iterate.isra.0 (context=0x5563126ce7d0, block=1, dispatch=1, self=<optimized out>) at ../glib/gmain.c:4276
        max_priority = 2147483647
        timeout = 146
        some_ready = 1
        nfds = 9
        allocated_nfds = <optimized out>
        fds = <optimized out>
        begin_time_nsec = 5220794732776
#18 0x00007f298c796a8f in g_main_loop_run (loop=0x556312867a30) at ../glib/gmain.c:4479
        __func__ = "g_main_loop_run"
#19 0x00007f298ce06975 in gtk_main () at ../gtk/gtkmain.c:1329
        loop = 0x556312867a30
#20 0x0000556311d17eaa in main (argc=<optimized out>, argv=<optimized out>) at /usr/src/debug/mate-panel-1.27.1-3.fc38.x86_64/mate-panel/main.c:223
        desktopfile = <optimized out>
        context = <optimized out>
        error = 0x0
        display = <optimized out>
        screen = 0x5563126a4240
        css = 0x556312b04cb0
        provider = 0x556312b04cb0
        resource = 0x556311d69eb8 "/org/mate/panel/theme/mate-panel.css"
        priority = 1

I am still wondering why it doesn't crash in wayland session at my box.
Edit: Got it, it's behind a x11 definition ifdef GDK_WINDOWING_X11

Copy link
Member

@raveit65 raveit65 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM,
it fixes the crash in x11 when build-in-process.

@lukefromdc lukefromdc merged commit c5db667 into master Aug 13, 2023
2 checks passed
@lukefromdc lukefromdc deleted the glib-2.77-trashfix branch August 13, 2023 15:24
@muktupavels
Copy link
Contributor

@lukefromdc I will give a hint! Search for xstuff_zoom_animate in both - mate-panel and mate-applets and then very carefully look at stacktrace - #0 and #1.

@lukefromdc
Copy link
Member Author

Indeed-looks like we are getting negative numbers for both width and height in both the rect and dest variables in-process Those large negative numbers on the first one smack of a read of uninitiaiized memory.

What is wierd is crash even if the function was empty though, while removing the function entirely lets trash_applet_button_release finish clean. Not worth debugging though unless we first have the animation actually working when out of process, then and only then would it be worth trying to bring it back in-process.

xstuff_zoom_animate is defined in trashapplet/src/xstuff.c so the exact function should appear nowhere else but similar ones with different names causing later crashes will have to be watched for

@muktupavels
Copy link
Contributor

@lukefromdc Look again and now pay attention where called function is defined. That will explain why your empty function changes nothing.

@lukefromdc
Copy link
Member Author

That doesn't tell me anything about why it works in-process but not out of process. It's defined at the bottom of xstuff.c so anything it depends on should have been already defined. Not seeing what you are seeing

@muktupavels
Copy link
Contributor

muktupavels commented Aug 13, 2023

Your applet does not use xstuff_zoom_animate from applet, but from panel.

Basically it is something like this:

Panel process starts, it has its own xstuff_zoom_animate function. Now panel loads applet into process, it also has xstuff_zoom_animate symbol, but wait... Symbol already exists, it is literally ignored.

Basically applet calls xstuff_zoom_animate (widget, NULL, undefined, undefined). In stacktrace you can see that these undefined parameters have some value. For example opt_rect has 0x556645daae9e. So panel function thinks you passed valid rectange to it. It crash because it is not rectange, you have invalid memory access.

This merge request did not fix anyting! This can happen with other functions and not only with functions that are defined in panel, but also in other applets.

@lukefromdc
Copy link
Member Author

lukefromdc commented Aug 13, 2023 via email

@muktupavels
Copy link
Contributor

No, it does not mean that! It does not matter what function does. In case you did not see, check added/edited part in previous comment.

@lukefromdc
Copy link
Member Author

lukefromdc commented Aug 13, 2023 via email

@lukefromdc
Copy link
Member Author

lukefromdc commented Aug 13, 2023

CONFIRMED: we have an xstuff_zoom_animate() that takes four arguments in mate-panel/mate-panel/xstuff.c , and ANOTHER xstuff_zoom_animate() that takes two arguments in mate-applets/trashapplet/src/xstuff.c

When and only when they are in the same process we have a collision of names. Fix should to rename the trashapplet's version something else, thinkingtrash_zoom_animate()for a new name. Otherwise we have to remove it completely so it doesn't get called from ANOTHER in-process applet or the panel toplevel no m,atter how configured.

@lukefromdc
Copy link
Member Author

see
#659
we fix the conflicting function name, and we can return the animate option to in-process builds though we won't see an actual animation until someone fixes that part of it. Alternately, we can simply remove it. Either way removes the risk of some other applet or the panel hitting the trashapplet's version instead of the panel's own version of xstuff_zoom_animate()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants