Bug 4269 - gtk-demo crash in "Color Selector" + Solution
Summary: gtk-demo crash in "Color Selector" + Solution
Status: RESOLVED FIXED
Alias: None
Product: pixman
Classification: Unclassified
Component: pixman (show other bugs)
Version: 0.1.3
Hardware: x86 (IA32) Linux (All)
: high normal
Assignee: Carl Worth
QA Contact: cairo-bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-08-27 08:37 UTC by Ronald Wahl
Modified: 2005-09-05 06:29 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
Fix for segfault (1.64 KB, patch)
2005-08-27 08:37 UTC, Ronald Wahl
Details | Splinter Review
Fix segfault (updated - missed %esi in clobber list) (1.85 KB, patch)
2005-08-27 08:42 UTC, Ronald Wahl
Details | Splinter Review
Patch applied to libpixman (2.79 KB, patch)
2005-08-27 18:36 UTC, Owen Taylor
Details | Splinter Review

Description Ronald Wahl 2005-08-27 08:37:01 UTC
(this report is against cairo-1.0.0 - bugzilla gui is not up to date!)

gtk-demo crashes in "Color Selector" after the "Change the above color" is
pressed. The backtrace showed me a crash in detectCPUFeatures() which is located
in cairo-1.0.0/pixman/src/fbmmx.c. Since I had a similar problem for quite some
time I know the solution: If you change the stack pointer (here by pushing ebx)
then the arguments (%0 %1 ...) are not correctly accessed anymore. It might only
occure when compiled with -fomit-frame-pointer but I have not tried w/o this
switch yet. I have attached a fix to this. I hope it gets applied - thanks!
Comment 1 Ronald Wahl 2005-08-27 08:37:57 UTC
Created attachment 3061 [details] [review]
Fix for segfault
Comment 2 Ronald Wahl 2005-08-27 08:42:14 UTC
Created attachment 3062 [details] [review]
Fix segfault (updated - missed %esi in clobber list)
Comment 3 Owen Taylor 2005-08-27 10:44:25 UTC
I had trouble understanding the description provided, but looking at 
the patch, I think the idea of it is:

 While generally gcc will try to avoid reusing %ebx (since it is
 the "pic" register and used as the base offset for accessing global
 data), with some combinations of of compiler flags it's possible
 that it *will* be used for one of the destination arguments, so 
 doing:

             "mov %%ebx, %1\n"
             "mov %%edx, %2\n"
             "mov %%ecx, %3\n"
[...]
             "mov %%edx, %0\n"

 doesn't work. The obvious fix would be to mark %ebx as clobbered,
 but that doesn't work either since because %ebx is the pic register,
 it's not allowed to be clobbered. So, the patch is careful to restore
 ebx at the points where the destination registeres are accessed.

The patch looks OK to me, though it seems to me that %eax could have been
used rather than adding %esi as another clobber. (While it's unlikely 
here, if you have too many clobbers, it's conceivable that the compiler
will be unable to satisfy the constraints.)
Comment 4 Ronald Wahl 2005-08-27 11:41:08 UTC
If you have no frame pointer in ebp then the compiler uses esp to access local
variables. Since esp is modified behind compilers back by pushing ebx on the stack
the compiler will fail to access the right storage place of the local variable(s).

Using of eax instead of esi is ok.
Comment 5 Owen Taylor 2005-08-27 18:36:15 UTC
Created attachment 3069 [details] [review]
Patch applied to libpixman

I'm applying the patch (with a comment added and the esi => eax change)
to libpixman. Billy - can you take care of making sure the fix is
propagated as appropriate in case someone wants to try and compile
their X server with -fomit-frmae-pointer?
Comment 6 Billy Biggs 2005-08-27 19:05:34 UTC
Fix applied to xserver, I'll add it to the list that should go to Xorg.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.