commit dc2ad6d066e0a997f0f41c3e9da14d76c5549aa0
author Corentin Wallez <cwallez@chromium.org> Mon Jul 08 17:25:54 2024 +0000
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Mon Jul 08 17:25:54 2024 +0000
tree 1f61b23eaaf1850bb575a8d7222f5768932219a1
parent 2234b2051644bbe216f36dfa3504ca60ca933ada

Add synchronous debug functionality for X11.

X11 errors can be very difficult to debug because of the asynchronous
nature of the protocol. Add a helper that makes all X11 calls
synchronous and triggers a breakpoint if an error is detect, such that
looking at the backtrace gives hints as to what went wrong.

Bug: None
Change-Id: I5e92878a4cd62a426a8faf81beecbd596c47fcad
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/189703
Reviewed-by: Stephen White <senorblanco@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>

src/dawn/native/X11Functions.cpp

diff --git a/src/dawn/native/X11Functions.cpp b/src/dawn/native/X11Functions.cpp
index 436854e..d5dd14e 100644
--- a/src/dawn/native/X11Functions.cpp
+++ b/src/dawn/native/X11Functions.cpp
@@ -27,11 +27,16 @@
 
 #include "dawn/native/X11Functions.h"
 
+#include <memory>
+
+#include "dawn/common/Log.h"
+
 namespace dawn::native {
 
 X11Functions::X11Functions() {
     if (!mX11Lib.Open("libX11.so.6") || !mX11Lib.GetProc(&xSetErrorHandler, "XSetErrorHandler") ||
-        !mX11Lib.GetProc(&xGetWindowAttributes, "XGetWindowAttributes")) {
+        !mX11Lib.GetProc(&xGetWindowAttributes, "XGetWindowAttributes") ||
+        !mX11Lib.GetProc(&xSynchronize, "XSynchronize")) {
         mX11Lib.Close();
     }
 
@@ -50,4 +55,39 @@
     return mX11XcbLib.Valid();
 }
 
+struct DebugX11 {
+    static DebugX11* sDebug;
+    static void Init(Display* display) {
+        if (sDebug == nullptr) {
+            auto debug = std::make_unique<DebugX11>();
+            if (!debug->x.IsX11Loaded()) {
+                return;
+            }
+
+            debug->previousHandler = debug->x.xSetErrorHandler(&HandleError);
+            sDebug = debug.release();
+        }
+
+        // Make all X11 calls synchronous so that the error handler is called immediately.
+        sDebug->x.xSynchronize(display, true);
+    }
+
+    static int HandleError(Display* d, XErrorEvent* e) {
+        dawn::ErrorLog()
+            << "An X11 error happened, triggering a breakpoint, the culprit will be in the stack.";
+        dawn::BreakPoint();
+
+        int result = sDebug->previousHandler(d, e);
+        return result;
+    }
+
+    X11Functions x;
+    XErrorHandler previousHandler = nullptr;
+};
+DebugX11* DebugX11::sDebug = nullptr;
+
+void SynchronouslyDebugX11(Display* display) {
+    DebugX11::Init(display);
+}
+
 }  // namespace dawn::native

src/dawn/native/X11Functions.h

diff --git a/src/dawn/native/X11Functions.h b/src/dawn/native/X11Functions.h
index 3b54bbb..160431c 100644
--- a/src/dawn/native/X11Functions.h
+++ b/src/dawn/native/X11Functions.h
@@ -51,6 +51,9 @@
     decltype(&::XSetErrorHandler) xSetErrorHandler = nullptr;
     decltype(&::XGetWindowAttributes) xGetWindowAttributes = nullptr;
 
+    // Calling XSynchronize(display, true) can help debug "X Error of failed request" messages.
+    decltype(&::XSynchronize) xSynchronize = nullptr;
+
     // Functions from x11-xcb
     decltype(&::XGetXCBConnection) xGetXCBConnection = nullptr;
 
@@ -59,6 +62,9 @@
     DynamicLib mX11XcbLib;
 };
 
+// Make future X11 calls synchronously trigger a breakpoint if they cause an X11 error.
+void SynchronouslyDebugX11(Display* display);
+
 }  // namespace dawn::native
 
 #endif  // SRC_DAWN_NATIVE_X11FUNCTIONS_H_