[TSan] Yet another idea about whitelisting data races (#5310)

* Introduce racing.h
- introduce the `RacingIncrement* ()` functions
- test the `RacingIncrement* ()` functions with races of `mono_stats` in class.c
- add racing.h to Makefile.am, libmonoutils.vcxproj and msvc/libmonoutils.vcxproj.filters

* [fixup!] Rename "racing" to "unlocked"
- rename racing.h to unlocked.h
- update new file name in Makefile.am and *.vcxproj* files
- rename the functions from Racing* to Unlocked*
- update the macro logic

diff --git a/mono/metadata/class-internals.h b/mono/metadata/class-internals.h
index 3ff6e77cbb990f3d2b15b3252b2cb22628ebe4ac..909e950c6c934c642b55c47aa2eeb21800faa031 100644 (file)
--- a/mono/metadata/class-internals.h
+++ b/mono/metadata/class-internals.h
@@ -757,17 +757,17 @@ typedef struct {
 
 typedef struct {
        guint64 new_object_count;
-       size_t initialized_class_count;
-       size_t generic_vtable_count;
+       gsize initialized_class_count;
+       gsize generic_vtable_count;
        size_t used_class_count;
        size_t method_count;
        size_t class_vtable_size;
        size_t class_static_data_size;
        size_t generic_instance_count;
-       size_t generic_class_count;
-       size_t inflated_method_count;
+       gsize generic_class_count;
+       gsize inflated_method_count;
        size_t inflated_method_count_2;
-       size_t inflated_type_count;
+       gsize inflated_type_count;
        size_t generics_metadata_size;
        size_t delegate_creations;
        size_t imt_tables_size;

diff --git a/mono/metadata/class.c b/mono/metadata/class.c
index cd3f8bf703b433073663ebb089962c6306f760cf..2e3dcd73856b25bb5cf6694a8b4594c04ca3ce1d 100644 (file)
--- a/mono/metadata/class.c
+++ b/mono/metadata/class.c
@@ -46,6 +46,7 @@
 #include <mono/utils/mono-logger-internals.h>
 #include <mono/utils/mono-memory-model.h>
 #include <mono/utils/atomic.h>
+#include <mono/utils/unlocked.h>
 #include <mono/utils/bsearch.h>
 #include <mono/utils/checked-build.h>
 
@@ -864,7 +865,7 @@ mono_class_inflate_generic_type_with_mempool (MonoImage *image, MonoType *type,
                }
        }
 
-       mono_stats.inflated_type_count++;
+       UnlockedIncrementSize (&mono_stats.inflated_type_count);
        return inflated;
 }
 
@@ -928,7 +929,7 @@ mono_class_inflate_generic_type_no_copy (MonoImage *image, MonoType *type, MonoG
        if (!inflated)
                return type;
 
-       mono_stats.inflated_type_count++;
+       UnlockedIncrementSize (&mono_stats.inflated_type_count);
        return inflated;
 }
 
@@ -1089,7 +1090,7 @@ mono_class_inflate_generic_method_full_checked (MonoMethod *method, MonoClass *k
                return (MonoMethod*)cached;
        }
 
-       mono_stats.inflated_method_count++;
+       UnlockedIncrementSize (&mono_stats.inflated_method_count);
 
        inflated_methods_size += sizeof (MonoMethodInflated);
 
@@ -3568,7 +3569,7 @@ mono_class_setup_vtable_full (MonoClass *klass, GList *in_setup)
                return;
        }
 
-       mono_stats.generic_vtable_count ++;
+       UnlockedIncrementSize (&mono_stats.generic_vtable_count);
        in_setup = g_list_prepend (in_setup, klass);
 
        if (mono_class_is_ginst (klass)) {
@@ -4902,7 +4903,7 @@ mono_class_init (MonoClass *klass)
                        goto leave;
        }
 
-       mono_stats.initialized_class_count++;
+       UnlockedIncrementSize (&mono_stats.initialized_class_count);
 
        if (mono_class_is_ginst (klass) && !mono_class_get_generic_class (klass)->is_dynamic) {
                MonoClass *gklass = mono_class_get_generic_class (klass)->container_class;
@@ -5047,10 +5048,10 @@ mono_class_init (MonoClass *klass)
                return !mono_class_has_failure (klass);
        }
 
-       mono_stats.initialized_class_count++;
+       UnlockedIncrementSize (&mono_stats.initialized_class_count);
 
        if (mono_class_is_ginst (klass) && !mono_class_get_generic_class (klass)->is_dynamic)
-               mono_stats.generic_class_count++;
+               UnlockedIncrementSize (&mono_stats.generic_class_count);
 
        if (mono_class_is_ginst (klass) || image_is_dynamic (klass->image) || !klass->type_token || (has_cached_info && !cached_info.has_nested_classes))
                klass->nested_classes_inited = TRUE;

diff --git a/mono/utils/Makefile.am b/mono/utils/Makefile.am
index e16931a28881757afdbc91db5950eb92841fda37..b277688859176ae54ff3a4d3c5b16ff574eccbb5 100644 (file)
--- a/mono/utils/Makefile.am
+++ b/mono/utils/Makefile.am
@@ -175,7 +175,8 @@ monoutils_sources = \
        checked-build.h \
        os-event.h \
        refcount.h      \
-       w32api.h
+       w32api.h        \
+       unlocked.h
 
 arch_sources = 
 

diff --git a/mono/utils/mono-compiler.h b/mono/utils/mono-compiler.h
index f4409e384c5166d5a3c753439734a6a4f6e771a9..746c84fb5131455564551cf4186402cbf41820f0 100644 (file)
--- a/mono/utils/mono-compiler.h
+++ b/mono/utils/mono-compiler.h
@@ -113,13 +113,19 @@ typedef SSIZE_T ssize_t;
 #define MONO_GNUC_VERSION (__GNUC__ * 10000 + __GNUC_MINOR__ * 100 + __GNUC_PATCHLEVEL__)
 #endif
 
-/* Used to tell clang's ThreadSanitizer to not report data races that occur within a certain function */
 #if defined(__has_feature)
 #if __has_feature(thread_sanitizer)
-#define MONO_NO_SANITIZE_THREAD __attribute__ ((no_sanitize("thread")))
+#define MONO_HAS_CLANG_THREAD_SANITIZER 1
 #else
-#define MONO_NO_SANITIZE_THREAD
+#define MONO_HAS_CLANG_THREAD_SANITIZER 0
+#endif
+#else
+#define MONO_HAS_CLANG_THREAD_SANITIZER 0
 #endif
+
+/* Used to tell Clang's ThreadSanitizer to not report data races that occur within a certain function */
+#if MONO_HAS_CLANG_THREAD_SANITIZER
+#define MONO_NO_SANITIZE_THREAD __attribute__ ((no_sanitize("thread")))
 #else
 #define MONO_NO_SANITIZE_THREAD
 #endif

diff --git a/mono/utils/unlocked.h b/mono/utils/unlocked.h
new file mode 100644 (file)
index 0000000..5b812ac
--- /dev/null
+++ b/mono/utils/unlocked.h
@@ -0,0 +1,42 @@
+/**
+ * \file
+ * Contains inline functions to explicitly mark data races that should not be changed.
+ * This way, instruments like Clang's ThreadSanitizer can be told to ignore very specific instructions.
+ *
+ * Licensed under the MIT license. See LICENSE file in the project root for full license information.
+ */
+
+#ifndef _UNLOCKED_H_
+#define _UNLOCKED_H_
+
+#include <glib.h>
+#include <mono/utils/mono-compiler.h>
+
+#if MONO_HAS_CLANG_THREAD_SANITIZER
+#define MONO_UNLOCKED_ATTRS MONO_NO_SANITIZE_THREAD MONO_NEVER_INLINE static
+#else
+#define MONO_UNLOCKED_ATTRS MONO_ALWAYS_INLINE static inline
+#endif
+
+MONO_UNLOCKED_ATTRS
+gint32
+UnlockedIncrement (gint32 *val)
+{
+       return ++*val;
+}
+
+MONO_UNLOCKED_ATTRS
+gint64
+UnlockedIncrement64 (gint64 *val)
+{
+       return ++*val;
+}
+
+MONO_UNLOCKED_ATTRS
+gsize
+UnlockedIncrementSize (gsize *val)
+{
+       return ++*val;
+}
+
+#endif /* _UNLOCKED_H_ */

diff --git a/msvc/libmonoutils.vcxproj b/msvc/libmonoutils.vcxproj
index 29903290d3381140bf7742288fbc2128591c5f4a..350a54654e475e39aa163f1c125cbaff0c19b47b 100644 (file)
--- a/msvc/libmonoutils.vcxproj
+++ b/msvc/libmonoutils.vcxproj
@@ -189,6 +189,7 @@
     <ClInclude Include="..\mono\utils\strenc.h" />\r
     <ClInclude Include="..\mono\utils\valgrind.h" />\r
     <ClInclude Include="..\mono\utils\atomic.h" />\r
+    <ClInclude Include="..\mono\utils\unlocked.h" />
     <ClInclude Include="..\mono\utils\mono-hwcap.h" />\r
     <ClInclude Include="..\mono\utils\mono-hwcap-x86.h" />\r
     <ClInclude Include="..\mono\utils\bsearch.h" />\r

diff --git a/msvc/libmonoutils.vcxproj.filters b/msvc/libmonoutils.vcxproj.filters
index 30b819626b6fc91f3f5f3cea2ffabec06f416384..30badee3648d03e88c2a0eec33f164a87902f672 100644 (file)
--- a/msvc/libmonoutils.vcxproj.filters
+++ b/msvc/libmonoutils.vcxproj.filters
@@ -426,6 +426,9 @@
     <ClInclude Include="..\mono\utils\os-event.h">\r
       <Filter>Header Files</Filter>\r
     </ClInclude>\r
+    <ClInclude Include="..\mono\utils\unlocked.h">
+      <Filter>Header Files</Filter>
+    </ClInclude>
   </ItemGroup>\r
   <ItemGroup>\r
     <Filter Include="Header Files">\r