From da8ea41a542787691ea1120e5c8c7dc3182cbea5 Mon Sep 17 00:00:00 2001 From: Maarten Maathuis Date: Sat, 28 Feb 2009 21:59:09 +0100 Subject: [PATCH] exa: increase/rework safety checks in Prepare/FinishAccess. --- exa/exa.c | 81 ++++++++++++++++++++++++++++++++++++++++---------- exa/exa.h | 5 ++++ exa/exa_priv.h | 16 ++++++++++ 3 files changed, 86 insertions(+), 16 deletions(-) diff --git a/exa/exa.c b/exa/exa.c index f4fba57f4..0ecbcf743 100644 --- a/exa/exa.c +++ b/exa/exa.c @@ -450,15 +450,28 @@ exaModifyPixmapHeader(PixmapPtr pPixmap, int width, int height, int depth, } } - if (pExaScr->info->ModifyPixmapHeader) { ret = pExaScr->info->ModifyPixmapHeader(pPixmap, width, height, depth, bitsPerPixel, devKind, pPixData); + /* For EXA_HANDLES_PIXMAPS, we set pPixData to NULL. + * If pPixmap->devPrivate.ptr is non-NULL, then we've got a non-offscreen pixmap. + * We need to store the pointer, because PrepareAccess won't be called. + */ + if (!pPixData && pPixmap->devPrivate.ptr && pPixmap->devKind) { + pExaPixmap->sys_ptr = pPixmap->devPrivate.ptr; + pExaPixmap->sys_pitch = pPixmap->devKind; + } if (ret == TRUE) - return ret; + goto out; } - return pExaScr->SavedModifyPixmapHeader(pPixmap, width, height, depth, + ret = pExaScr->SavedModifyPixmapHeader(pPixmap, width, height, depth, bitsPerPixel, devKind, pPixData); + +out: + /* Always NULL this, we don't want lingering pointers. */ + pPixmap->devPrivate.ptr = NULL; + + return ret; } /** @@ -523,16 +536,43 @@ exaGetOffscreenPixmap (DrawablePtr pDrawable, int *xp, int *yp) Bool ExaDoPrepareAccess(DrawablePtr pDrawable, int index) { - ScreenPtr pScreen = pDrawable->pScreen; - ExaScreenPriv (pScreen); - PixmapPtr pPixmap = exaGetDrawablePixmap (pDrawable); - Bool offscreen = exaPixmapIsOffscreen(pPixmap); + ScreenPtr pScreen = pDrawable->pScreen; + ExaScreenPriv (pScreen); + PixmapPtr pPixmap = exaGetDrawablePixmap (pDrawable); + ExaPixmapPriv(pPixmap); + Bool offscreen; - /* Unhide pixmap pointer */ - if (pPixmap->devPrivate.ptr == NULL && !(pExaScr->info->flags & EXA_HANDLES_PIXMAPS)) { - pPixmap->devPrivate.ptr = ExaGetPixmapAddress(pPixmap); + if (!(pExaScr->info->flags & EXA_OFFSCREEN_PIXMAPS)) + return FALSE; + + if (pExaPixmap == NULL) + EXA_FatalErrorDebugWithRet(("EXA bug: ExaDoPrepareAccess was called on a non-exa pixmap.\n"), FALSE); + + /* Check if we're dealing SRC == DST or similar. + * In that case the first PrepareAccess has already set pPixmap->devPrivate.ptr. + */ + if (pPixmap->devPrivate.ptr != NULL) { + int i; + for (i = 0; i < 6; i++) + if (pExaScr->prepare_access[i] == pPixmap) + break; + + /* No known PrepareAccess or double prepare on the same index. */ + if (i == 6 || i == index) + EXA_FatalErrorDebug(("EXA bug: pPixmap->devPrivate.ptr was %p, but should have been NULL.\n", + pPixmap->devPrivate.ptr)); } + offscreen = exaPixmapIsOffscreen(pPixmap); + + if (offscreen) + pPixmap->devPrivate.ptr = pExaPixmap->fb_ptr; + else + pPixmap->devPrivate.ptr = pExaPixmap->sys_ptr; + + /* Store so we can check SRC and DEST being the same. */ + pExaScr->prepare_access[index] = pPixmap; + if (!offscreen) return FALSE; @@ -548,9 +588,8 @@ ExaDoPrepareAccess(DrawablePtr pDrawable, int index) } if (!(*pExaScr->info->PrepareAccess) (pPixmap, index)) { - ExaPixmapPriv (pPixmap); if (pExaPixmap->score == EXA_PIXMAP_SCORE_PINNED) - FatalError("Driver failed PrepareAccess on a pinned pixmap\n"); + FatalError("Driver failed PrepareAccess on a pinned pixmap.\n"); exaMoveOutPixmap (pPixmap); return FALSE; @@ -604,10 +643,20 @@ exaFinishAccess(DrawablePtr pDrawable, int index) PixmapPtr pPixmap = exaGetDrawablePixmap (pDrawable); ExaPixmapPriv (pPixmap); - /* Rehide pixmap pointer if we're doing that. */ - if (pExaPixmap && !(pExaScr->info->flags & EXA_HANDLES_PIXMAPS)) { - pPixmap->devPrivate.ptr = NULL; - } + if (!(pExaScr->info->flags & EXA_OFFSCREEN_PIXMAPS)) + return; + + if (pExaPixmap == NULL) + EXA_FatalErrorDebugWithRet(("EXA bug: exaFinishAccesss was called on a non-exa pixmap.\n"),); + + /* Avoid mismatching indices. */ + if (pExaScr->prepare_access[index] != pPixmap) + EXA_FatalErrorDebug(("EXA bug: Calling FinishAccess on pixmap %p with index %d while " + "it should have been %p.\n", pPixmap, index, pExaScr->prepare_access[index])); + pExaScr->prepare_access[index] = NULL; + + /* We always hide the devPrivate.ptr. */ + pPixmap->devPrivate.ptr = NULL; if (pExaScr->info->FinishAccess == NULL) return; diff --git a/exa/exa.h b/exa/exa.h index 8339a3e44..d7219f053 100644 --- a/exa/exa.h +++ b/exa/exa.h @@ -695,6 +695,11 @@ typedef struct _ExaDriver { /* Hooks to allow driver to its own pixmap memory management */ void *(*CreatePixmap)(ScreenPtr pScreen, int size, int align); void (*DestroyPixmap)(ScreenPtr pScreen, void *driverPriv); + /** + * Returning a pixmap with non-NULL devPrivate.ptr implies a pixmap which is + * not offscreen, which will never be accelerated and Prepare/FinishAccess won't + * be called. + */ Bool (*ModifyPixmapHeader)(PixmapPtr pPixmap, int width, int height, int depth, int bitsPerPixel, int devKind, pointer pPixData); diff --git a/exa/exa_priv.h b/exa/exa_priv.h index a037eb031..9efbbc98c 100644 --- a/exa/exa_priv.h +++ b/exa/exa_priv.h @@ -85,6 +85,18 @@ exaDrawableLocation(DrawablePtr pDrawable); #define EXA_MAX_FB FB_OVERLAY_MAX #endif +#ifdef DEBUG +#define EXA_FatalErrorDebug(x) FatalError x +#define EXA_FatalErrorDebugWithRet(x, ret) FatalError x +#else +#define EXA_FatalErrorDebug(x) ErrorF x +#define EXA_FatalErrorDebugWithRet(x, ret) \ +do { \ + ErrorF x; \ + return ret; \ +} while (0) +#endif + /** * This is the list of migration heuristics supported by EXA. See * exaDoMigration() for what their implementations do. @@ -158,6 +170,10 @@ typedef struct { unsigned disableFbCount; Bool optimize_migration; unsigned offScreenCounter; + + /* Store all accessed pixmaps, so we can check for duplicates. */ + PixmapPtr prepare_access[6]; + /* Holds information on fallbacks that cannot be relayed otherwise. */ unsigned int fallback_flags;