Attachment 50248 Details for Bug 36165 – This uses memcpy, but avoids overlap as required

[patch] This uses memcpy, but avoids overlap as required

0001-fb-Fix-memcpy-abuse.patch (text/plain), 2.50 KB, created by Keith Packard on 2011-08-15 15:44:40 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Keith Packard

Created: 2011-08-15 15:44:40 UTC

Size: 2.50 KB

patch

obsolete

>From e32cc0b4c85c78cd8743a6e1680dcc79054b57ce Mon Sep 17 00:00:00 2001
>From: Adam Jackson <ajax@redhat.com>
>Date: Thu, 21 Apr 2011 16:37:11 -0400
>Subject: [PATCH] fb: Fix memcpy abuse
>
>The memcpy fast path implicitly assumes that the copy walks
>left-to-right.  That's not something memcpy guarantees, and newer glibc
>on some processors will indeed break that assumption.  Since we walk a
>line at a time, check the source and destination against the width of
>the blit to determine whether we can be sloppy enough to allow memcpy.
>(Having done this, we can remove the check for !reverse as well.)
>
>On an Intel Core i7-2630QM with an NVIDIA GeForce GTX 460M running in
>NoAccel, the broken code and various fixes for -copywinwin{10,100,500}
>gives (edited to fit in 80 columns):
>
>1: Disable the fastpath entirely
>2: Replace memcpy with memmove
>3: This fix
>4: The code before this fix
>
>  1            2                 3                 4           Operation
>------   ---------------   ---------------   ---------------   ------------
>258000   269000 (  1.04)   544000 (  2.11)   552000 (  2.14)   Copy 10x10
> 21300    23000 (  1.08)    43700 (  2.05)    47100 (  2.21)   Copy 100x100
>   960      962 (  1.00)     1990 (  2.09)     1990 (  2.07)   Copy 500x500
>
>So it's a modest performance hit, but correctness demands it, and it's
>probably worth keeping the 2x speedup from having the fast path in the
>first place.
>
>Signed-off-by: Adam Jackson <ajax@redhat.com>
>Signed-off-by: Keith Packard <keithp@keithp.com>
>---
> fb/fbblt.c |    9 +++++++--
> 1 files changed, 7 insertions(+), 2 deletions(-)
>
>diff --git a/fb/fbblt.c b/fb/fbblt.c
>index a040298..6809404 100644
>--- a/fb/fbblt.c
>+++ b/fb/fbblt.c
>@@ -65,6 +65,7 @@ fbBlt (FbBits   *srcLine,
>     int	    n, nmiddle;
>     Bool    destInvarient;
>     int	    startbyte, endbyte;
>+    int     careful;
>     FbDeclareMergeRop ();
> 
>     if (bpp == 24 && !FbCheck24Pix (pm))
>@@ -74,12 +75,16 @@ fbBlt (FbBits   *srcLine,
> 	return;
>     }
> 
>-    if (alu == GXcopy && pm == FB_ALLONES && !reverse &&
>+    careful = !((srcLine < dstLine && srcLine + width * (bpp>>3) > dstLine) ||
>+                (dstLine < srcLine && dstLine + width * (bpp>>3) > srcLine)) ||
>+              (bpp & 7);
>+
>+    if (alu == GXcopy && pm == FB_ALLONES && !careful &&
>             !(srcX & 7) && !(dstX & 7) && !(width & 7)) {
>         int i;
>         CARD8 *src = (CARD8 *) srcLine;
>         CARD8 *dst = (CARD8 *) dstLine;
>-        
>+
>         srcStride *= sizeof(FbBits);
>         dstStride *= sizeof(FbBits);
>         width >>= 3;
>-- 
>1.7.5.4
>

Actions: View | Splinter Review

Attachments on bug 36165: 45528 | 50248

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.