util: Fixes thread safety of DEBUG_GET_ONCE_*_OPTION macros
authorYonggang Luo <luoyonggang@gmail.com>
Sat, 5 Nov 2022 20:32:05 +0000 (04:32 +0800)
committerMarge Bot <emma+marge@anholt.net>
Fri, 16 Dec 2022 19:30:19 +0000 (19:30 +0000)
Pick DEBUG_GET_ONCE_BOOL_OPTION as a example:
The intention of DEBUG_GET_ONCE_BOOL_OPTION are returned the same value across
thread, before this commit, on different thread call the function generated by
DEBUG_GET_ONCE_BOOL_OPTION may return different value if called setenv in the
middle of debug_get_bool_option, so use debug_get_option_cached along with
new exposed function debug_parse_bool_option to solve this issue

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/19554>

src/util/u_debug.c
src/util/u_debug.h

index 07c02b5..a5f8512 100644 (file)
@@ -100,10 +100,9 @@ debug_disable_win32_error_dialogs(void)
 }
 #endif /* _WIN32 */
 
-static bool
-debug_get_bool_option_direct(const char *name, bool dfault)
+bool
+debug_parse_bool_option(const char *str, bool dfault)
 {
-   const char *str = os_get_option(name);
    bool result;
 
    if (str == NULL)
@@ -140,7 +139,8 @@ debug_get_option_should_print(void)
    static bool value = false;
 
    if (unlikely(!p_atomic_read_relaxed(&initialized))) {
-      value = debug_get_bool_option_direct("GALLIUM_PRINT_OPTIONS", false);
+      bool parsed_value = debug_parse_bool_option(os_get_option("GALLIUM_PRINT_OPTIONS"), false);
+      p_atomic_set(&value, parsed_value);
       p_atomic_set(&initialized, true);
    }
 
@@ -192,7 +192,7 @@ debug_get_option_cached(const char *name, const char *dfault)
 bool
 debug_get_bool_option(const char *name, bool dfault)
 {
-   bool result = debug_get_bool_option_direct(name, dfault);
+   bool result = debug_parse_bool_option(os_get_option(name), dfault);
    if (debug_get_option_should_print())
       debug_printf("%s: %s = %s\n", __func__, name,
                    result ? "TRUE" : "FALSE");
@@ -202,12 +202,9 @@ debug_get_bool_option(const char *name, bool dfault)
 
 
 int64_t
-debug_get_num_option(const char *name, int64_t dfault)
+debug_parse_num_option(const char *str, int64_t dfault)
 {
    int64_t result;
-   const char *str;
-
-   str = os_get_option(name);
    if (!str) {
       result = dfault;
    } else {
@@ -219,6 +216,13 @@ debug_get_num_option(const char *name, int64_t dfault)
          result = dfault;
       }
    }
+   return result;
+}
+
+int64_t
+debug_get_num_option(const char *name, int64_t dfault)
+{
+   int64_t result = debug_parse_num_option(os_get_option(name), dfault);
 
    if (debug_get_option_should_print())
       debug_printf("%s: %s = %"PRId64"\n", __func__, name, result);
@@ -297,16 +301,15 @@ str_has_option(const char *str, const char *name)
 
 
 uint64_t
-debug_get_flags_option(const char *name,
-                       const struct debug_named_value *flags,
-                       uint64_t dfault)
+debug_parse_flags_option(const char *name,
+                         const char *str,
+                         const struct debug_named_value *flags,
+                         uint64_t dfault)
 {
    uint64_t result;
-   const char *str;
    const struct debug_named_value *orig = flags;
    unsigned namealign = 0;
 
-   str = os_get_option(name);
    if (!str)
       result = dfault;
    else if (!strcmp(str, "help")) {
@@ -328,6 +331,17 @@ debug_get_flags_option(const char *name,
       }
    }
 
+   return result;
+}
+
+uint64_t
+debug_get_flags_option(const char *name,
+                       const struct debug_named_value *flags,
+                       uint64_t dfault)
+{
+   const char *str = os_get_option(name);
+   uint64_t result = debug_parse_flags_option(name, str, flags, dfault);
+
    if (debug_get_option_should_print()) {
       if (str) {
          debug_printf("%s: %s = 0x%"PRIx64" (%s)\n",
index 61cee1c..4385be9 100644 (file)
@@ -359,12 +359,24 @@ const char *
 debug_get_option_cached(const char *name, const char *dfault);
 
 bool
+debug_parse_bool_option(const char *str, bool dfault);
+
+bool
 debug_get_bool_option(const char *name, bool dfault);
 
 int64_t
+debug_parse_num_option(const char *str, int64_t dfault);
+
+int64_t
 debug_get_num_option(const char *name, int64_t dfault);
 
 uint64_t
+debug_parse_flags_option(const char *name,
+                         const char *str,
+                         const struct debug_named_value *flags,
+                         uint64_t dfault);
+
+uint64_t
 debug_get_flags_option(const char *name,
                        const struct debug_named_value *flags,
                        uint64_t dfault);
@@ -376,7 +388,8 @@ debug_get_option_ ## suffix (void) \
    static bool initialized = false; \
    static const char * value; \
    if (unlikely(!p_atomic_read_relaxed(&initialized))) { \
-      value = debug_get_option(name, dfault); \
+      const char *str = debug_get_option_cached(name, dfault); \
+      p_atomic_set(&value, str); \
       p_atomic_set(&initialized, true); \
    } \
    return value; \
@@ -399,7 +412,9 @@ debug_get_option_ ## sufix (void) \
    static bool initialized = false; \
    static bool value; \
    if (unlikely(!p_atomic_read_relaxed(&initialized))) { \
-      value = debug_get_bool_option(name, dfault); \
+      const char *str = debug_get_option_cached(name, NULL); \
+      bool parsed_value = debug_parse_bool_option(str, dfault); \
+      p_atomic_set(&value, parsed_value); \
       p_atomic_set(&initialized, true); \
    } \
    return value; \
@@ -412,7 +427,9 @@ debug_get_option_ ## sufix (void) \
    static bool initialized = false; \
    static int64_t value; \
    if (unlikely(!p_atomic_read_relaxed(&initialized))) { \
-      value = debug_get_num_option(name, dfault); \
+      const char *str = debug_get_option_cached(name, NULL); \
+      int64_t parsed_value = debug_parse_num_option(str, dfault); \
+      p_atomic_set(&value, parsed_value); \
       p_atomic_set(&initialized, true); \
    } \
    return value; \
@@ -425,7 +442,9 @@ debug_get_option_ ## sufix (void) \
    static bool initialized = false; \
    static uint64_t value; \
    if (unlikely(!p_atomic_read_relaxed(&initialized))) { \
-      value = debug_get_flags_option(name, flags, dfault); \
+      const char *str = debug_get_option_cached(name, NULL); \
+      uint64_t parsed_value = debug_parse_flags_option(name, str, flags, dfault); \
+      p_atomic_set(&value, parsed_value); \
       p_atomic_set(&initialized, true); \
    } \
    return value; \