After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 677977 - gnome-shell 3.5.2 crashed
gnome-shell 3.5.2 crashed
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
3.5.x
Other Linux
: Normal critical
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2012-06-12 21:23 UTC by Cosimo Cecchi
Modified: 2012-06-25 18:07 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
meta-window-actor: Fix a potential crash in the window shaping code (2.50 KB, patch)
2012-06-13 17:53 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review

Description Cosimo Cecchi 2012-06-12 21:23:08 UTC
Clearing out some recent shell crash reports from the abrt queue; not sure how I triggered this.

gnome-shell 3.5.2


Thread 1 (Thread 0x7fc93b6e29c0 (LWP 1063))

  • #0 INT_cairo_region_create_rectangles
    at cairo-region.c line 259
  • #1 check_needs_reshape
    at compositor/meta-window-actor.c line 2262
  • #2 meta_window_actor_pre_paint
    at compositor/meta-window-actor.c line 2353
  • #3 meta_window_actor_get_paint_volume
    at compositor/meta-window-actor.c line 694
  • #4 _clutter_actor_get_paint_volume_real
    at ./clutter-actor.c line 15705
  • #5 _clutter_actor_get_paint_volume_mutable
    at ./clutter-actor.c line 15781
  • #6 _clutter_actor_finish_queue_redraw
    at ./clutter-actor.c line 7473
  • #7 _clutter_stage_maybe_finish_queue_redraws
    at ./clutter-stage.c line 4099
  • #8 _clutter_stage_do_update
    at ./clutter-stage.c line 1223
  • #9 master_clock_update_stages
    at ./clutter-master-clock.c line 386
  • #10 clutter_clock_dispatch
    at ./clutter-master-clock.c line 519
  • #11 g_main_dispatch
    at gmain.c line 2539
  • #12 g_main_context_dispatch
    at gmain.c line 3075
  • #13 g_main_context_iterate
    at gmain.c line 3146
  • #14 g_main_loop_run
    at gmain.c line 3340
  • #15 meta_run
    at core/main.c line 548
  • #16 main
    at main.c line 396

Comment 1 Jasper St. Pierre (not reading bugmail) 2012-06-12 21:32:09 UTC
Ot looks like a bug in the X server: it says that there are two rectangles, but returns NULL. We do have an existing branch to test if rects is NULL, and not taking that path will cause a crash. I figured that count would always be 0 in that case, though.
Comment 2 Ray Strode [halfline] 2012-06-13 15:22:56 UTC
It's common for APIs to leave out arguments untouched on error.  It's better to just treat them as not defined on error, but you can also often get away with pre-initializing out args to 0 before making the call.
Comment 3 Jasper St. Pierre (not reading bugmail) 2012-06-13 16:29:10 UTC
But we didn't get an X error back. It succeeded.
Comment 4 Ray Strode [halfline] 2012-06-13 17:44:45 UTC
so the code i see does:

/* n_rects is uninitialized */
int n_rects;

/* Not using _with_return variants of of meta_error_trap so not checking for specific X errors */

meta_error_trap_push (display);
rects = XShapeGetRectangles (..., &n_rects);
meta_error_trap_pop (display);

if (rects)
  {
     /* some stuff here */
  }

/* regardless of error from XShapeGetRectangles, regardless of whether rects is NULL, use potentially uninitialized n_rects variable */
region = cairo_region_create_rectangles (..., n_rects);
Comment 5 Jasper St. Pierre (not reading bugmail) 2012-06-13 17:53:35 UTC
Created attachment 216337 [details] [review]
meta-window-actor: Fix a potential crash in the window shaping code

There was a potential case where we were trying to use uninitialized memory,
in the case where the X server threw an error during XShapeGetRectangles.
In this case, we need to use the implicit shape for the window, which means
we need to rearrange code flow to make it work.
Comment 6 Ray Strode [halfline] 2012-06-13 18:04:03 UTC
Review of attachment 216337 [details] [review]:

seems right.

::: src/compositor/meta-window-actor.c
@@ +2256,3 @@
           int i;
+          cairo_rectangle_int_t *cairo_rects = g_new (cairo_rectangle_int_t, n_rects);
+

I think it's more common in mutter to split this into two separate lines, but there seems to be a mix of both conventions sprinkled throughout the codebase so probably doesn't matter.
Comment 7 Jasper St. Pierre (not reading bugmail) 2012-06-25 18:07:50 UTC
Attachment 216337 [details] pushed as a2f2e07 - meta-window-actor: Fix a potential crash in the window shaping code