From e2c3739a3f5f606d6c5055b0119b2fc40e084e0c Mon Sep 17 00:00:00 2001
From: Yonggang Luo <luoyonggang@gmail.com>
Date: Wed, 31 Aug 2022 01:26:04 +0800
Subject: [PATCH] util: It's not thread safe to set initialized=true before get
 the real GALLIUM_PRINT_OPTIONS
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Even though initialized = true can make sure have no recursion, but that's may leading to
debug_get_option_should_print return false at the second thread, but the first thread
return true. These two threads should return the same value, even though this function is for
debug only, but it's better to getting it to be correct.

Signed-off-by: Yonggang Luo <luoyonggang@gmail.com>
Acked-by: Marek Olšák <marek.olsak@amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/18764>
---
 src/util/u_debug.c | 77 ++++++++++++++++++++++++----------------------
 1 file changed, 40 insertions(+), 37 deletions(-)

diff --git a/src/util/u_debug.c b/src/util/u_debug.c
index b0ce238d7df..4ff018ba64a 100644
--- a/src/util/u_debug.c
+++ b/src/util/u_debug.c
@@ -26,7 +26,7 @@
  *
  **************************************************************************/
 
-
+#include "util/u_atomic.h"
 #include "util/u_debug.h"
 #include "util/u_string.h"
 #include "util/u_math.h"
@@ -100,21 +100,51 @@ debug_disable_win32_error_dialogs(void)
 }
 #endif /* _WIN32 */
 
+static bool
+debug_get_bool_option_direct(const char *name, bool dfault)
+{
+   const char *str = os_get_option(name);
+   bool result;
+
+   if (str == NULL)
+      result = dfault;
+   else if (!strcmp(str, "0"))
+      result = false;
+   else if (!strcasecmp(str, "n"))
+      result = false;
+   else if (!strcasecmp(str, "no"))
+      result = false;
+   else if (!strcasecmp(str, "f"))
+      result = false;
+   else if (!strcasecmp(str, "false"))
+      result = false;
+   else if (!strcmp(str, "1"))
+      result = true;
+   else if (!strcasecmp(str, "y"))
+      result = true;
+   else if (!strcasecmp(str, "yes"))
+      result = true;
+   else if (!strcasecmp(str, "t"))
+      result = true;
+   else if (!strcasecmp(str, "true"))
+      result = true;
+   else
+      result = dfault;
+   return result;
+}
+
 static bool
 debug_get_option_should_print(void)
 {
    static bool initialized = false;
    static bool value = false;
 
-   if (initialized)
-      return value;
+   if (unlikely(!p_atomic_read_relaxed(&initialized))) {
+      value = debug_get_bool_option_direct("GALLIUM_PRINT_OPTIONS", false);
+      p_atomic_set(&initialized, true);
+   }
 
-   /* Oh hey this will call into this function,
-    * but its cool since we set first to false
-    */
-   initialized = true;
-   value = debug_get_bool_option("GALLIUM_PRINT_OPTIONS", false);
-   /* XXX should we print this option? Currently it wont */
+   /* We do not print value of GALLIUM_PRINT_OPTIONS intentionally. */
    return value;
 }
 
@@ -145,34 +175,7 @@ debug_get_option(const char *name, const char *dfault)
 bool
 debug_get_bool_option(const char *name, bool dfault)
 {
-   const char *str = os_get_option(name);
-   bool result;
-
-   if (str == NULL)
-      result = dfault;
-   else if (!strcmp(str, "0"))
-      result = false;
-   else if (!strcasecmp(str, "n"))
-      result = false;
-   else if (!strcasecmp(str, "no"))
-      result = false;
-   else if (!strcasecmp(str, "f"))
-      result = false;
-   else if (!strcasecmp(str, "false"))
-      result = false;
-   else if (!strcmp(str, "1"))
-      result = true;
-   else if (!strcasecmp(str, "y"))
-      result = true;
-   else if (!strcasecmp(str, "yes"))
-      result = true;
-   else if (!strcasecmp(str, "t"))
-      result = true;
-   else if (!strcasecmp(str, "true"))
-      result = true;
-   else
-      result = dfault;
-
+   bool result = debug_get_bool_option_direct(name, dfault);
    if (debug_get_option_should_print())
       debug_printf("%s: %s = %s\n", __FUNCTION__, name,
                    result ? "TRUE" : "FALSE");