[clang] [llvm] [Hashing] Use a non-deterministic seed if LLVM_ENABLE_ABI_BREAKING_CHECKS (PR #96282)

Christian Sigg via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 1 23:03:31 PDT 2024


================
@@ -322,24 +306,20 @@ struct hash_state {
   }
 };
 
-
-/// A global, fixed seed-override variable.
-///
-/// This variable can be set using the \see llvm::set_fixed_execution_seed
-/// function. See that function for details. Do not, under any circumstances,
-/// set or read this variable.
-extern uint64_t fixed_seed_override;
-
+/// In LLVM_ENABLE_ABI_BREAKING_CHECKS builds, the seed is non-deterministic
+/// (address of a variable) to prevent having users depend on the particular
+/// hash values. On platforms without ASLR, this is still likely
+/// non-deterministic per build.
 inline uint64_t get_execution_seed() {
-  // FIXME: This needs to be a per-execution seed. This is just a placeholder
-  // implementation. Switching to a per-execution seed is likely to flush out
-  // instability bugs and so will happen as its own commit.
-  //
-  // However, if there is a fixed seed override set the first time this is
-  // called, return that instead of the per-execution seed.
-  const uint64_t seed_prime = 0xff51afd7ed558ccdULL;
-  static uint64_t seed = fixed_seed_override ? fixed_seed_override : seed_prime;
-  return seed;
+  // Work around x86-64 negative offset folding for old Clang -fno-pic
+  // https://reviews.llvm.org/D93931
+#if LLVM_ENABLE_ABI_BREAKING_CHECKS &&                                         \
+    (!defined(__clang__) || __clang_major__ > 11)
----------------
chsigg wrote:

We are still dealing with the fallout from this change in [Triton](https://github.com/triton-lang/triton/pull/4760).

It seems that breaking the ABI across different compilers is acceptable, but it would be good to detect this and error out. We already [detect mismatches](https://github.com/llvm/llvm-project/blob/bc91f3cdd57cbe4b0a456626f52960158cb3232f/llvm/include/llvm/Config/abi-breaking.h.cmake#L45) of `LLVM_ENABLE_ABI_BREAKING_CHECKS`. Would it be acceptable to just add the `clang > 11` condition there?

https://github.com/llvm/llvm-project/pull/96282


More information about the cfe-commits mailing list