Subzero: Fixed deadlock with _start but no globals

src/IceASanInstrumentation.cpp

Index: src/IceASanInstrumentation.cpp
diff --git a/src/IceASanInstrumentation.cpp b/src/IceASanInstrumentation.cpp
index a02cb6e006f4f0b32613e7652fef30a91ac83e6e..79f57a96884de10bee81d32cc60a4b99b7b810eb 100644
--- a/src/IceASanInstrumentation.cpp
+++ b/src/IceASanInstrumentation.cpp
@@ -74,8 +74,11 @@ 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) {
-  if (DidProcessGlobals)
+  GlobalsMutex.lock();

[Jim Stichnoth, 2016/07/19 14:04:13]

Can you do this?

  std::unique_lock<std::mutex> _(GlobalsMutex);

Then you can remove the lock() and unlock() calls.

[tlively, 2016/07/19 22:27:04]

Done.

+  if (DidProcessGlobals) {
+    GlobalsMutex.unlock();
     return;
+  }
   VariableDeclarationList NewGlobals;
   // Global holding pointers to all redzones
   auto *RzArray = VariableDeclaration::create(&NewGlobals);
@@ -135,7 +138,7 @@ void ASanInstrumentation::instrumentGlobals(VariableDeclarationList &Globals) {
   Globals.clear();
   Globals.merge(&NewGlobals);
   DidProcessGlobals = true;
-  GlobalsDoneCV.notify_all();
+  GlobalsMutex.unlock();
 
   // Log the new set of globals
   if (BuildDefs::dump() && (getFlags().getVerbose() & IceV_GlobalInit)) {
@@ -332,13 +335,12 @@ 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();
+  // wait for globals to be installed
+  while (!Ctx->getGlobalsReceived()) {

[tlively, 2016/07/19 07:59:54]

This spinning could be replaced by a condition variable, but I wanted to make
sure this general direction looked good first.

[Jim Stichnoth, 2016/07/19 14:04:13]

Definitely add a TODO about properly using a condition variable.

Also, it would probably be good to do something like
  std::this_thread::yield();
inside the spin loop.

[tlively, 2016/07/19 22:27:04]

Done.

   }
+  // instrument globals if not already done
+  instrumentGlobals(*Ctx->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)));