Subzero: Fixed deadlock when _start is first function

src/IceASanInstrumentation.cpp

@@ -74,6 +74,7 @@ bool ASanInstrumentation::isInstrumentable(Cfg *Func) {
 // types of the redzones and their associated globals match so that they are
 // laid out together in memory.
 void ASanInstrumentation::instrumentGlobals(VariableDeclarationList &Globals) {
+  std::unique_lock<std::mutex> _(GlobalsMutex);
   if (DidProcessGlobals)
     return;
   VariableDeclarationList NewGlobals;
@@ -135,7 +136,6 @@ void ASanInstrumentation::instrumentGlobals(VariableDeclarationList &Globals) {
   Globals.clear();
   Globals.merge(&NewGlobals);
   DidProcessGlobals = true;
-  GlobalsDoneCV.notify_all();
 
   // Log the new set of globals
   if (BuildDefs::dump() && (getFlags().getVerbose() & IceV_GlobalInit)) {
@@ -332,13 +332,8 @@ void ASanInstrumentation::instrumentStart(Cfg *Func) {
   auto *Call = InstCall::create(Func, NumArgs, Void, ShadowMemInit, NoTailCall);
   Func->getEntryNode()->getInsts().push_front(Call);
 
-  // wait to get the final count of global redzones
-  if (!DidProcessGlobals) {
-    GlobalsLock.lock();
-    while (!DidProcessGlobals)
-      GlobalsDoneCV.wait(GlobalsLock);
-    GlobalsLock.release();
-  }
+  instrumentGlobals(*getGlobals());
+
   Call->addArg(ConstantInteger32::create(Ctx, IceType_i32, RzGlobalsNum));
   Call->addArg(Ctx->getConstantSym(0, Ctx->getGlobalString(RzArrayName)));
   Call->addArg(Ctx->getConstantSym(0, Ctx->getGlobalString(RzSizesName)));

src/IceASanInstrumentation.h

@@ -23,8 +23,6 @@
 #include "IceGlobalInits.h"
 #include "IceInstrumentation.h"
 
-#include <condition_variable>
-
 namespace Ice {
 
 class ASanInstrumentation : public Instrumentation {
@@ -33,9 +31,7 @@ class ASanInstrumentation : public Instrumentation {
   ASanInstrumentation &operator=(const ASanInstrumentation &) = delete;
 
 public:
-  ASanInstrumentation(GlobalContext *Ctx)
-      : Instrumentation(Ctx), RzNum(0),
-        GlobalsLock(GlobalsMutex, std::defer_lock) {
+  ASanInstrumentation(GlobalContext *Ctx) : Instrumentation(Ctx), RzNum(0) {
     ICE_TLS_INIT_FIELD(LocalDtors);
   }
   void instrumentGlobals(VariableDeclarationList &Globals) override;
@@ -57,8 +53,6 @@ private:
   bool DidProcessGlobals = false;
   SizeT RzGlobalsNum = 0;
   std::mutex GlobalsMutex;
-  std::unique_lock<std::mutex> GlobalsLock;
-  std::condition_variable GlobalsDoneCV;
 };
 } // end of namespace Ice
 

src/IceGlobalContext.h

@@ -482,6 +482,10 @@ public:
     return LockedPtr<StringPool>(Strings.get(), &StringsLock);
   }
 
+  LockedPtr<VariableDeclarationList> getGlobals() {
+    return LockedPtr<VariableDeclarationList>(&Globals, &InitAllocLock);
+  }
+
   /// Number of function blocks that can be queued before waiting for
   /// translation
   /// threads to consume.
@@ -612,6 +616,8 @@ private:
     LockedPtr<VariableDeclarationList> _(&Globals, &InitAllocLock);
     if (Globls != nullptr) {
       Globals.merge(Globls.get());
+      if (!BuildDefs::minimal() && Instrumentor != nullptr)
+        Instrumentor->setHasSeenGlobals();
     }
   }
 

src/IceInstrumentation.cpp

@@ -118,4 +118,18 @@ void Instrumentation::instrumentInst(LoweringContext &Context) {
   }
 }
 
+void Instrumentation::setHasSeenGlobals() {
+  {
+    std::unique_lock<std::mutex> _(GlobalsSeenMutex);
+    HasSeenGlobals = true;
+  }
+  GlobalsSeenCV.notify_all();
+}
+
+LockedPtr<VariableDeclarationList> Instrumentation::getGlobals() {
+  std::unique_lock<std::mutex> GlobalsLock(GlobalsSeenMutex);
+  GlobalsSeenCV.wait(GlobalsLock, [this] { return HasSeenGlobals; });
+  return Ctx->getGlobals();
+}
+
 } // end of namespace Ice

src/IceInstrumentation.h

@@ -30,6 +30,8 @@
 
 #include "IceDefs.h"
 
+#include <condition_variable>
+
 namespace Ice {
 
 class LoweringContext;
@@ -41,11 +43,14 @@ class Instrumentation {
 
 public:
   Instrumentation(GlobalContext *Ctx) : Ctx(Ctx) {}
+  virtual ~Instrumentation() = default;
   virtual void instrumentGlobals(VariableDeclarationList &) {}
   void instrumentFunc(Cfg *Func);
+  void setHasSeenGlobals();
 
 protected:
   virtual void instrumentInst(LoweringContext &Context);
+  LockedPtr<VariableDeclarationList> getGlobals();
 
 private:
   virtual bool isInstrumentable(Cfg *) { return true; }
@@ -78,6 +83,11 @@ private:
 
 protected:
   GlobalContext *Ctx;
+
+private:
+  bool HasSeenGlobals = false;
+  std::mutex GlobalsSeenMutex;
+  std::condition_variable GlobalsSeenCV;
 };
 
 } // end of namespace Ice

tests_lit/asan_tests/no_globals.ll

+; Check that Subzero can instrument _start when there are no globals.
+; Previously Subzero would deadlock when _start was the first function. Also
+; test that instrumenting start does not deadlock waiting for nonexistent
+; global initializers to be lowered.
+
+; REQUIRES: no_minimal_build
+
+; RUN: %p2i -i %s --args -verbose=inst -fsanitize-address \
+; RUN:     | FileCheck --check-prefix=DUMP %s
+
+; RUN: %p2i -i %s --args -verbose=inst -fsanitize-address -threads=0 \
+; RUN:     | FileCheck --check-prefix=DUMP %s
+
+
+define void @_start(i32 %arg) {
+  ret void
+}
+
+; DUMP: __asan_init