From e89e1f5049d3b78542fcd69b7d6106dd4bdafc39 Mon Sep 17 00:00:00 2001
From: Adam Jackson <ajax@redhat.com>
Date: Wed, 21 Jul 2021 12:02:35 -0400
Subject: [PATCH] glx: Fix error handling yet again in CreateContextAttribs

Unlike the legacy CreateContext path, we would try to send the
GLXCreateContextAttribs request regardless of whether we'd successfully
created the client context state. And there's not a lot on the server
side to go wrong besides BadAlloc, so if the request succeeded but
the client side didn't we'd need to destroy the server context and
synthesize an X error. Since that itself involves more X protocol it's
tricky to get the request number right in the error, and tests and apps
can notice when you get it wrong.

Since we have now fixed client-side validation to generate the right
errors at the right times, this patch does something simpler, we match
CreateContext and fail early if the client-side setup fails. Now there's
no question of what request number to use, because we haven't sent any
protocol, the error is for the request as if it'd been sent.

Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/4763
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/12006>
---
 src/glx/create_context.c | 48 ++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 26 deletions(-)

diff --git a/src/glx/create_context.c b/src/glx/create_context.c
index fcf468f33ab6e..f89ceec30beb3 100644
--- a/src/glx/create_context.c
+++ b/src/glx/create_context.c
@@ -54,7 +54,7 @@ glXCreateContextAttribsARB(Display *dpy, GLXFBConfig config,
    struct glx_screen *psc;
    xcb_generic_error_t *err;
    xcb_void_cookie_t cookie;
-   unsigned dummy_err = 0;
+   unsigned error = BadImplementation;
    uint32_t xid, share_xid;
    int screen = -1;
 
@@ -100,25 +100,30 @@ glXCreateContextAttribsARB(Display *dpy, GLXFBConfig config,
       direct = true;
    }
 
-
-   if (direct && psc->vtable->create_context_attribs) {
-      /* GLX drops the error returned by the driver.  The expectation is that
-       * an error will also be returned by the server.  The server's error
-       * will be delivered to the application.
-       */
-      gc = psc->vtable->create_context_attribs(psc, cfg, share, num_attribs,
-                                               (const uint32_t *) attrib_list,
-                                               &dummy_err);
-   } 
 #ifdef GLX_USE_APPLEGL
-   else if (gc == NULL) {
-      gc = applegl_create_context(psc, cfg, share, 0);
-   }
-#endif
-   else if (!direct) {
+   gc = applegl_create_context(psc, cfg, share, 0);
+#else
+   if (direct && psc->vtable->create_context_attribs) {
+      gc = psc->vtable->create_context_attribs(psc, cfg, share, num_attribs,
+					       (const uint32_t *) attrib_list,
+					       &error);
+   } else if (!direct) {
       gc = indirect_create_context_attribs(psc, cfg, share, num_attribs,
                                            (const uint32_t *) attrib_list,
-                                           &dummy_err);
+                                           &error);
+   }
+#endif
+
+   if (gc == NULL) {
+      /* -1 isn't a legal XID, which is sort of the point, we've failed
+       * before we even got to XID allocation.
+       */
+      if (error == GLXBadContext || error == GLXBadFBConfig ||
+          error == GLXBadProfileARB)
+         __glXSendError(dpy, error, -1, 0, False);
+      else
+         __glXSendError(dpy, error, -1, 0, True);
+      return NULL;
    }
 
    xid = xcb_generate_id(c);
@@ -150,15 +155,6 @@ glXCreateContextAttribsARB(Display *dpy, GLXFBConfig config,
 
       __glXSendErrorForXcb(dpy, err);
       free(err);
-   } else if (!gc) {
-      /* the server thought the context description was okay, but we failed
-       * somehow on the client side. clean up the server resource and panic.
-       */
-      xcb_glx_destroy_context(c, xid);
-      /* increment dpy->request in order to give a unique serial number to the
-       * error */
-      XNoOp(dpy);
-      __glXSendError(dpy, GLXBadFBConfig, xid, 0, False);
    } else {
       gc->xid = xid;
       gc->share_xid = share_xid;