Bug 2335 - icbltone.c compile warnings on x86_64 - possibly real bug.
Summary: icbltone.c compile warnings on x86_64 - possibly real bug.
Status: RESOLVED FIXED
Alias: None
Product: cairo
Classification: Unclassified
Component: general (show other bugs)
Version: 0.9.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-01-19 16:55 UTC by ellson
Modified: 2005-08-22 00:14 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
patch to pixman.h suggested by Keith (679 bytes, patch)
2005-01-21 19:05 UTC, ellson
Details | Splinter Review

Description ellson 2005-01-19 16:55:05 UTC
I get many warnings like this:

icbltone.c:461: warning: left shift count >= width of type

I tracked it down to a problem in Mask24Pos(x,r) when BITMAP_BIT_ORDER != MSBFirst

#if BITMAP_BIT_ORDER == MSBFirst
#define Mask24Pos(x,r) ((x)*24-(r))
#else
#define Mask24Pos(x,r) ((x)*24-((r) ? 24 - (r) : 0))
#endif

When x=2 r=0 this produces a value of 48, which in  Mask24(x,r) results
in 0xffffff << 48 which is 72bits, i.e larger than uint64.

I suspect this is the cause of the mashed PNG images reported on the cairo
development list a couple of days ago.

I haven't been able to identify a patch, because I still don't fully understand
what this is doing.  I'd spend more effort on it but I understand you have a 
major rewrite about to land in CVS and so perhaps you've fixed it already?
Comment 1 Carl Worth 2005-01-19 18:23:06 UTC
The "rewrite" is just moving some newer code over from the X server, but
only for the trapezoid rasterization, (most of what is currently in ictrap.c
will disappear). It won't affect the function you are looking at.

I don't know offhand the details of whats going on in this function, so some
investigation would be useful.
Comment 2 ellson 2005-01-21 17:59:24 UTC
Right.  The problem still exists with the latest updates.

Do you happen to know what "BITMAP_BIT_ORDER != MSBFirst" means?

Does it really imply bit-order reversal, or just byte order?

Do you happen to have a description of bit/byte ordering on x86_64
as compared with i386 ?

Comment 3 Keith Packard 2005-01-21 18:46:08 UTC
The 64-bit code in fb (pixman) is probably broken; it hasn't been used in quite
some time as PCI (and AGP) is 32-bits wide, so doing things 64-bits at a time is
a net loss.  To quickly fix this, I suggest just using 32-bit datatypes by
setting IC_SHIFT to 5 for all machines.
Comment 4 ellson 2005-01-21 19:03:22 UTC
Thanks, that fixes both the compile warnings (this bug) and the distortion
of fonts from cairo-demo/png/text.c reported to the cairo list a few days ago.

patch follows
Comment 5 ellson 2005-01-21 19:05:15 UTC
Created attachment 1733 [details] [review]
patch to pixman.h suggested by Keith
Comment 6 Carl Worth 2005-01-26 09:11:25 UTC
I've committed a fix for this, but have not tested on any 64-bit platforms.

Please test and re-open the report if necessary.
Comment 7 ellson 2005-01-26 09:42:33 UTC
Works for me on x86_64.

Do you happen to know, or can you suggest a test to find out what this does
to in-memory bitmaps?     I'm hoping it doesn't double their size?
Comment 8 Carl Worth 2005-08-22 17:14:37 UTC
Move bugs against "cvs" version to "0.9.3" so we can remove the "cvs" version.


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.