[compiler-rt] 167fb10 - compiler-rt/cpu_model: Ensure constructor priority is set and align with GCC

[Keno Fischer via llvm-commits]

Author: Keno Fischer
Date: 2022-05-30T20:31:49Z
New Revision: 167fb106d28ff026f53a3f3468bdcb914e094bd1

URL: https://github.com/llvm/llvm-project/commit/167fb106d28ff026f53a3f3468bdcb914e094bd1
DIFF: https://github.com/llvm/llvm-project/commit/167fb106d28ff026f53a3f3468bdcb914e094bd1.diff

LOG: compiler-rt/cpu_model: Ensure constructor priority is set and align with GCC

GCC recently started setting constructor priority on init_have_lse_atomics [1]
to avoid undefined initialization order with respect to other initializers,
causing accidental use of ll/sc intrinsics on targets where this was not
intended (which presents a minor performance problem as well as a
compatibility problem for users wanting to use the rr debugger). I initially
thought compiler-rt does not have the same issue as libgcc, since it looks
like we're already setting init priority on the constructor.

Unfortuantely, it does not appear that the HAVE_INIT_PRIORITY check is ever
performed anyway, so despite appearances the init priority was not actually
applied. Fix that by applying the init priority unconditionally. It has been
supported in clang ever since it was first introduced and in any case for
more than 14 years in both gcc and clang. MSVC is already excluded from this
code path and we're already using constructors with init priority elsewhere
in compiler-rt without additional check (though mostly in the sanitizer
runtime, which may have more narrow target support). Regardless, I believe
that for our supported compilers, if they support the constructor attribute,
they should also support init priorities.

While we're here, change the init priority from 101, which is the highest
priority for end user applications, to instead use one of the priority levels
reserved for implementations (1-100; lower integers are higher priority).
GCC ended up using `90`, so this commit aligns the value in compiler-rt
to the same value to ensure that there are no subtle initialization order
differences between libgcc and compiler-rt.

[1] https://github.com/gcc-mirror/gcc/commit/75c4e4909ae2667f56487434f99c2915b4570794

Differential Revision: https://reviews.llvm.org/D126424

Added: 
    

Modified: 
    compiler-rt/lib/builtins/cpu_model.c

Removed: 
    


################################################################################
diff  --git a/compiler-rt/lib/builtins/cpu_model.c b/compiler-rt/lib/builtins/cpu_model.c
index e52f2aa610a6..228be67acb89 100644
--- a/compiler-rt/lib/builtins/cpu_model.c
+++ b/compiler-rt/lib/builtins/cpu_model.c
@@ -17,10 +17,17 @@
 #define __has_attribute(attr) 0
 #endif
 
-#if defined(HAVE_INIT_PRIORITY)
-#define CONSTRUCTOR_ATTRIBUTE __attribute__((__constructor__ 101))
-#elif __has_attribute(__constructor__)
-#define CONSTRUCTOR_ATTRIBUTE __attribute__((__constructor__))
+#if __has_attribute(constructor)
+#if __GNUC__ >= 9
+// Ordinarily init priorities below 101 are disallowed as they are reserved for the
+// implementation. However, we are the implementation, so silence the diagnostic,
+// since it doesn't apply to us.
+#pragma GCC diagnostic ignored "-Wprio-ctor-dtor"
+#endif
+// We're choosing init priority 90 to force our constructors to run before any
+// constructors in the end user application (starting at priority 101). This value
+// matches the libgcc choice for the same functions.
+#define CONSTRUCTOR_ATTRIBUTE __attribute__((constructor(90)))
 #else
 // FIXME: For MSVC, we should make a function pointer global in .CRT$X?? so that
 // this runs during initialization.