fixup some data races

compiler/compiler-core.cpp

@@ -490,11 +490,11 @@ VarPtr CompilerCore::create_var(const std::string &name, VarData::Type type) {
 VarPtr CompilerCore::get_global_var(const std::string &name, VertexPtr init_val) {
   auto *node = globals_ht.at(vk::std_hash(name));
 
-  if (!node->data) {
+  {
     AutoLocker<Lockable *> locker(node);
     if (!node->data) {
       node->data = create_var(name, VarData::var_global_t);
-      node->data->init_val = init_val;
+      node->data->init_val = init_val.clone();
       node->data->is_builtin_runtime = VarData::does_name_eq_any_builtin_runtime(name);
     }
   }
@@ -505,11 +505,11 @@ VarPtr CompilerCore::get_global_var(const std::string &name, VertexPtr init_val)
 VarPtr CompilerCore::get_constant_var(const std::string &name, VertexPtr init_val, bool *is_new_inserted) {
   auto *node = constants_ht.at(vk::std_hash(name));
   VarPtr new_var;
-  if (!node->data) {
+  { 
     AutoLocker<Lockable *> locker(node);
     if (!node->data) {
       new_var = create_var(name, VarData::var_const_t);
-      new_var->init_val = init_val;
+      new_var->init_val = init_val.clone();
       node->data = new_var;
     }
   }

compiler/data/class-members.cpp

@@ -60,7 +60,7 @@ inline ClassMemberStaticField::ClassMemberStaticField(ClassPtr klass, VertexAdap
   std::string global_var_name = replace_backslashes(klass->name) + "$$" + root->get_string();
   var = G->get_global_var(global_var_name, def_val);
   root->var_id = var;
-  var->init_val = def_val;
+  var->init_val = def_val.clone();
   var->class_id = klass;
 }
 
@@ -101,7 +101,7 @@ ClassMemberInstanceField::ClassMemberInstanceField(ClassPtr klass, VertexAdaptor
   std::string local_var_name = root->get_string();
   var = G->create_var(local_var_name, VarData::var_instance_t);
   root->var_id = var;
-  var->init_val = def_val;
+  var->init_val = def_val.clone();
   var->class_id = klass;
   var->marked_as_const = klass->is_immutable || (phpdoc && phpdoc->has_tag(PhpDocType::kphp_const));
 }

compiler/data/function-data.h

@@ -56,7 +56,7 @@ class FunctionData {
   std::vector<VarPtr> local_var_ids, global_var_ids, static_var_ids, param_ids;
   std::unordered_set<VarPtr> *bad_vars = nullptr;     // for check ub and safe operations wrapper, see comments in check-ub.cpp
   std::set<VarPtr> implicit_const_var_ids, explicit_const_var_ids, explicit_header_const_var_ids;
-  std::vector<FunctionPtr> dep;
+  std::vector<FunctionPtr> dep; // should wait on them in some passes; may be deadlock?
   std::set<ClassPtr> class_dep;
   std::set<ClassPtr> exceptions_thrown; // exceptions that can be thrown by this function
   bool tl_common_h_dep = false;

compiler/data/var-data.h

@@ -4,6 +4,7 @@
 
 #pragma once
 
+#include "common/wrappers/copyable-atomic.h"
 #include <cstdint>
 #include <string>
 
@@ -46,13 +47,17 @@ class VarData {
   bool is_read_only = true;
   bool is_foreach_reference = false;
   bool is_builtin_runtime = false;        // $_SERVER, $argv, etc., see PhpScriptBuiltInSuperGlobals in runtime
-  int dependency_level = 0;               // for constants only (c_str$, c_arr$, etc)
+  vk::copyable_atomic<int> dependency_level = 0;               // for constants only (c_str$, c_arr$, etc)
   int offset_in_linear_mem = -1;          // for globals only (offset in g_linear_mem)
   int batch_idx = -1;                     // for constants and globals, a number [0;N), see const-globals-batched-mem.h
 
   void set_uninited_flag(bool f);
   bool get_uninited_flag();
 
+  VarData(const VarData &) = default;
+  VarData(VarData &&) = default;
+  VarData &operator=(const VarData &) = default;
+  VarData &operator=(VarData &&) = default;
   explicit VarData(Type type);
 
   inline Type &type() { return type_; }

compiler/inferring/node.cpp

@@ -1,16 +1,16 @@
 // Compiler for PHP (aka KPHP)
-// Copyright (c) 2020 LLC «V Kontakte»
+// Copyright (c) 2024 LLC «V Kontakte»
 // Distributed under the GPL v3 License, see LICENSE.notice.txt
 
 #include "compiler/inferring/node.h"
 
 #include "compiler/inferring/type-data.h"
-#include "compiler/stage.h"
+#include <atomic>
 
 namespace tinf {
 
 std::string Node::as_human_readable() const {
-  return type_->as_human_readable(false);
+  return type_.load(std::memory_order_relaxed)->as_human_readable(false);
 }
 
 void Node::register_edge_from_this(const tinf::Edge *edge) {
@@ -28,16 +28,20 @@ bool Node::try_start_recalc() {
     int recalc_state_copy = recalc_state_;
     int once_finished_flag = recalc_state_copy & recalc_bit_at_least_once;
     switch (recalc_state_copy & 15) {   // preserve bit 16 in transformations
-      case recalc_st_waiting:
-        if (__sync_bool_compare_and_swap(&recalc_state_, recalc_st_waiting | once_finished_flag, recalc_st_need_relaunch | once_finished_flag)) {
+      case recalc_st_waiting: {
+        int old = recalc_st_waiting | once_finished_flag;
+        if (recalc_state_.compare_exchange_strong(old, recalc_st_need_relaunch | once_finished_flag)) {
           return true;
         }
         break;
-      case recalc_st_processing:
-        if (__sync_bool_compare_and_swap(&recalc_state_, recalc_st_processing | once_finished_flag, recalc_st_need_relaunch | once_finished_flag)) {
+      }
+      case recalc_st_processing: {
+        int old = recalc_st_processing | once_finished_flag;
+        if (recalc_state_.compare_exchange_strong(old, recalc_st_need_relaunch | once_finished_flag)) {
           return false;
         }
         break;
+      }
       case recalc_st_need_relaunch:
         return false;
       default:
@@ -49,29 +53,36 @@ bool Node::try_start_recalc() {
 
 void Node::start_recalc() {
   int once_finished_flag = recalc_state_ & recalc_bit_at_least_once;  // preserve bit 16 in transformation
-  bool swapped = __sync_bool_compare_and_swap(&recalc_state_, recalc_st_need_relaunch | once_finished_flag, recalc_st_processing | once_finished_flag);
+  int old = recalc_st_need_relaunch | once_finished_flag;
+  bool swapped = recalc_state_.compare_exchange_strong(old, recalc_st_processing | once_finished_flag);
   kphp_assert(swapped);
 }
 
 bool Node::try_finish_recalc() {
   while (true) {
     int recalc_state_copy = recalc_state_;
     switch (recalc_state_copy) {  // always set bit 16 in transformations
-      case recalc_st_processing:
-        if (__sync_bool_compare_and_swap(&recalc_state_, recalc_st_processing, recalc_st_waiting | recalc_bit_at_least_once)) {
+      case recalc_st_processing: {
+        int old = recalc_st_processing;
+        if (recalc_state_.compare_exchange_strong(old, recalc_st_waiting | recalc_bit_at_least_once)) {
           return true;
         }
         break;
-      case recalc_st_processing | recalc_bit_at_least_once:
-        if (__sync_bool_compare_and_swap(&recalc_state_, recalc_st_processing | recalc_bit_at_least_once, recalc_st_waiting | recalc_bit_at_least_once)) {
+      }
+      case recalc_st_processing | recalc_bit_at_least_once: {
+        int old = recalc_st_processing | recalc_bit_at_least_once;
+        if (recalc_state_.compare_exchange_strong(old, recalc_st_waiting | recalc_bit_at_least_once)) {
           return true;
         }
         break;
-      case recalc_st_need_relaunch:
-        if (__sync_bool_compare_and_swap(&recalc_state_, recalc_st_need_relaunch, recalc_st_need_relaunch | recalc_bit_at_least_once)) {
+      }
+      case recalc_st_need_relaunch: {
+        int old = recalc_st_need_relaunch;
+        if (recalc_state_.compare_exchange_strong(old, recalc_st_need_relaunch | recalc_bit_at_least_once)) {
           return false; // false here, unlike above, but like below
         }
         break;
+      }
       case recalc_st_need_relaunch | recalc_bit_at_least_once:
         return false;
       default:

compiler/inferring/node.h

@@ -4,6 +4,8 @@
 
 #pragma once
 
+#include "common/wrappers/copyable-atomic.h"
+#include <atomic>
 #include <string>
 #include <forward_list>
 
@@ -39,11 +41,11 @@ class Node : public Lockable {
     recalc_bit_at_least_once = 16,
   };
 
-  const TypeData *type_{TypeData::get_type(tp_any)};
+  vk::copyable_atomic<const TypeData *> type_{TypeData::get_type(tp_any)};
 
   // this field is a finite-state automation for multithreading synchronization, see enum above
   // if should be placed here (after TypeData*) to make it join with the next int field in memory
-  int recalc_state_{recalc_st_waiting};
+  vk::copyable_atomic<int> recalc_state_{recalc_st_waiting};
 
 public:
 
@@ -54,11 +56,11 @@ class Node : public Lockable {
   std::string as_human_readable() const;
 
   bool was_recalc_started_at_least_once() const {
-    return recalc_state_ > recalc_st_waiting;
+    return recalc_state_.load() > recalc_st_waiting;
   }
 
   bool was_recalc_finished_at_least_once() const {
-    return recalc_state_ >= recalc_bit_at_least_once;
+    return recalc_state_.load() >= recalc_bit_at_least_once;
   }
 
   void register_edge_from_this(const tinf::Edge *edge);
@@ -77,11 +79,11 @@ class Node : public Lockable {
   bool try_finish_recalc();
 
   const TypeData *get_type() const {
-    return type_;
+    return type_.load(std::memory_order_relaxed);
   }
 
   void set_type(const TypeData *type) {
-    type_ = type;
+    type_.store(type, std::memory_order_relaxed);
   }
 
   virtual void recalc(TypeInferer *inferer) = 0;

compiler/inferring/type-node.cpp

@@ -6,9 +6,10 @@
 
 #include "compiler/inferring/type-data.h"
 #include "compiler/stage.h"
+#include <atomic>
 
 std::string tinf::TypeNode::get_description() {
-  return "TypeNode at " + location_.as_human_readable() + " : " + type_->as_human_readable();
+  return "TypeNode at " + location_.as_human_readable() + " : " + type_.load(std::memory_order_relaxed)->as_human_readable();
 }
 
 const Location &tinf::TypeNode::get_location() const {

compiler/pipes/check-func-calls-and-vararg.cpp

@@ -250,7 +250,7 @@ VertexPtr CheckFuncCallsAndVarargPass::on_func_call(VertexAdaptor<op_func_call>
                fmt_format("Can not call an abstract method {}()", f->as_human_readable()));
   }
 
-  VertexRange func_params = f->get_params();
+  VertexRange func_params = f->get_params(); // read here, f is concurrently processed in this pass
   VertexRange call_params = call->args();
 
   if (call_params.size() < func_params.size()) {

compiler/pipes/collect-const-vars.cpp

@@ -10,6 +10,7 @@
 #include "compiler/const-manipulations.h"
 #include "compiler/compiler-core.h"
 #include "compiler/name-gen.h"
+#include <atomic>
 
 namespace {
 
@@ -218,10 +219,10 @@ int get_expr_dep_level(VertexPtr vertex) {
 }
 
 void set_var_dep_level(VarPtr var_id) {
-  if (!IsComposite::visit(var_id->init_val)) {
-    var_id->dependency_level = 0;
-  } else {
-    var_id->dependency_level = 1 + get_expr_dep_level(var_id->init_val);
+  if (IsComposite::visit(var_id->init_val)) {
+    int old = var_id->dependency_level.load(std::memory_order_relaxed);
+    int cur = 1 + get_expr_dep_level(var_id->init_val);
+    var_id->dependency_level.store(std::max(old, cur), std::memory_order_relaxed);
   }
 }
 

compiler/pipes/deduce-implicit-types-and-casts.cpp

@@ -605,7 +605,7 @@ void DeduceImplicitTypesAndCastsPass::on_func_call(VertexAdaptor<op_func_call> c
 
   FunctionPtr f_called = call->func_id;
   auto call_args = call->args();
-  auto f_called_params = f_called->get_params();
+  auto f_called_params = f_called->get_params(); // f_called is concurrently processed in TransformToSmartInstanceofPass
 
   // if we are calling `f<T>`, then `f` has not been instantiated yet at this point, so we have a generic func call
   // at first, we need to know all generic types (call->reifiedTs)

compiler/pipes/register-variables.cpp

@@ -77,7 +77,7 @@ void RegisterVariablesPass::register_function_static_var(VertexAdaptor<op_var> v
 
   if (default_value) {
     if (!kphp_error(is_const(default_value), fmt_format("Default value of [{}] is not constant", name))) {
-      var->init_val = default_value;
+      var->init_val = default_value.clone();
     }
   }
   var_vertex->var_id = var;
@@ -91,7 +91,7 @@ void RegisterVariablesPass::register_param_var(VertexAdaptor<op_var> var_vertex,
   kphp_assert (var);
   if (default_value) {
     kphp_error_return(is_const(default_value) || current_function->is_extern(), fmt_format("Default value of [{}] is not constant", name));
-    var->init_val = default_value;
+    var->init_val = default_value.clone();
   }
   var_vertex->var_id = var;
 }

compiler/pipes/sort-and-inherit-classes.cpp

@@ -367,7 +367,7 @@ void SortAndInheritClassesF::execute(ClassPtr klass, MultipleDataStreams<Functio
   }
 
   node->data.waiting.clear();
-  node->data.done = true;
+  node->data.done = true; // write
 }
 
 void SortAndInheritClassesF::check_on_finish(DataStream<FunctionPtr> &os) {

compiler/pipes/sort-and-inherit-classes.h

@@ -4,6 +4,7 @@
 
 #pragma once
 
+#include "common/wrappers/copyable-atomic.h"
 #include <forward_list>
 
 #include "compiler/data/class-members.h"
@@ -14,7 +15,7 @@
 class SortAndInheritClassesF {
 private:
   struct wait_list {
-    bool done;
+    vk::copyable_atomic<bool> done;
     std::forward_list<ClassPtr> waiting;
   };
 

compiler/threading/hash-table.h

@@ -4,6 +4,7 @@
 
 #pragma once
 
+#include <atomic>
 #include <cassert>
 #include <vector>
 
@@ -13,7 +14,7 @@ template<class T, int N = 1000000>
 class TSHashTable {
 public:
   struct HTNode : Lockable {
-    unsigned long long hash;
+    std::atomic<unsigned long long> hash;
     T data;
 
     HTNode() :
@@ -24,7 +25,7 @@ class TSHashTable {
 
 private:
   HTNode *nodes;
-  int used_size;
+  std::atomic<int> used_size;
 public:
   TSHashTable() :
     nodes(new HTNode[N]),
@@ -34,14 +35,16 @@ class TSHashTable {
   HTNode *at(unsigned long long hash) {
     int i = (unsigned)hash % (unsigned)N;
     while (true) {
-      while (nodes[i].hash != 0 && nodes[i].hash != hash) {
+      while (nodes[i].hash.load(std::memory_order_acquire) != 0 && nodes[i].hash.load(std::memory_order_relaxed) != hash) {
         i++;
         if (i == N) {
           i = 0;
         }
       }
-      if (nodes[i].hash == 0 && !__sync_bool_compare_and_swap(&nodes[i].hash, 0, hash)) {
-        int id = __sync_fetch_and_add(&used_size, 1);
+      unsigned long long expected = 0;
+
+      if (nodes[i].hash.load(std::memory_order_acquire) == expected && !nodes[i].hash.compare_exchange_strong(expected, hash, std::memory_order_acq_rel)) {
+        int id = used_size.fetch_add(1, std::memory_order_relaxed);
         assert(id * 2 < N);
         continue;
       }
@@ -52,21 +55,21 @@ class TSHashTable {
 
   const T *find(unsigned long long hash) {
     int i = (unsigned)hash % (unsigned)N;
-    while (nodes[i].hash != 0 && nodes[i].hash != hash) {
+    while (nodes[i].hash.load(std::memory_order_acquire) != 0 && nodes[i].hash.load(std::memory_order_relaxed) != hash) {
       i++;
       if (i == N) {
         i = 0;
       }
     }
 
-    return nodes[i].hash == hash ? &nodes[i].data : nullptr;
+    return nodes[i].hash.load(std::memory_order_relaxed) == hash ? &nodes[i].data : nullptr;
   }
 
   std::vector<T> get_all() {
     std::vector<T> res;
     res.reserve(used_size);
     for (int i = 0; i < N; i++) {
-      if (nodes[i].hash != 0) {
+      if (nodes[i].hash.load(std::memory_order_acquire) != 0) {
         res.push_back(nodes[i].data);
       }
     }
@@ -77,7 +80,7 @@ class TSHashTable {
   std::vector<T> get_all_if(const CondF &callbackF) {
     std::vector<T> res;
     for (int i = 0; i < N; i++) {
-      if (nodes[i].hash != 0 && callbackF(nodes[i].data)) {
+      if (nodes[i].hash.load(std::memory_order_acquire) != 0 && callbackF(nodes[i].data)) {
         res.push_back(nodes[i].data);
       }
     }