[PATCH 2/6] exa: increase/rework safety checks in Prepare/FinishAccess.

[Maarten Maathuis]

---
 exa/exa.c      |   98 +++++++++++++++++++++++++++++++++++++++++++++++--------
 exa/exa.h      |    5 +++
 exa/exa_priv.h |    4 ++
 3 files changed, 92 insertions(+), 15 deletions(-)

diff --git a/exa/exa.c b/exa/exa.c
index f4fba57..6e8a18d 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,53 @@ 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;
+
+    if (!(pExaScr->info->flags & EXA_OFFSCREEN_PIXMAPS))
+	return FALSE;
+
+    if (pExaPixmap == NULL) {
+#ifdef DEBUG
+	FatalError("EXA bug: ExaDoPrepareAccess was called on a non-exa pixmap.\n");
+#else
+	ErrorF("EXA bug: ExaDoPrepareAccess was called on a non-exa pixmap.\n");
+	return FALSE;
+#endif
+    }
 
-    /* Unhide pixmap pointer */
-    if (pPixmap->devPrivate.ptr == NULL && !(pExaScr->info->flags & EXA_HANDLES_PIXMAPS)) {
-	pPixmap->devPrivate.ptr = ExaGetPixmapAddress(pPixmap);
+    /* 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) {
+	    ErrorF("EXA bug: pPixmap->devPrivate.ptr was %p, but should have been NULL.\n",
+		pPixmap->devPrivate.ptr);
+#ifdef DEBUG
+	    assert(pPixmap->devPrivate.ptr == NULL);
+#endif
+	}
     }
 
+    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 +598,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,11 +653,30 @@ 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) {
+#ifdef DEBUG
+	FatalError("EXA bug: exaFinishAccesss was called on a non-exa pixmap.\n");
+#else
+	ErrorF("EXA bug: exaFinishAccess was called on a non-exa pixmap.\n");
+	return;
+#endif
     }
 
+    /* Avoid mismatching indices. */
+    if (pExaScr->prepare_access[index] != pPixmap)
+	ErrorF("EXA bug: Calling FinishAccess on pixmap %p with index %d while "
+			"it should have been %p.\n", pPixmap, index, pExaScr->prepare_access[index]);
+#ifdef DEBUG
+    assert(pExaScr->prepare_access[index] == pPixmap);
+#endif
+    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 8339a3e..d7219f0 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 a037eb0..ce2cf48 100644
--- a/exa/exa_priv.h
+++ b/exa/exa_priv.h
@@ -158,6 +158,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;
 
-- 
1.6.1.3