util: It's not thread safe to set initialized=true before get the real GALLIUM_PRINT_...

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>

diff --git a/src/util/u_debug.c b/src/util/u_debug.c
index b0ce238..4ff018b 100644 (file)
--- 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"
@@ -101,20 +101,50 @@ 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");