[PATCH] Make init-order checker accept access to already initialized globals

[Alexey Samsonov]

Hi kcc,

This change adds ASan runtime option "strict-init-order" (off by default)
that makes init-order checker bark if global initializer accesses any global from different
translation unit (even if the latter is already initialized). strict init-order checking
doesn't play well with, e.g. LLVM registration machineries, and causes issue 
https://code.google.com/p/address-sanitizer/issues/detail?id=178.

http://llvm-reviews.chandlerc.com/D679

Files:
  lib/asan/lit_tests/SharedLibs/init-order-dlopen-so.cc
  lib/asan/lit_tests/Linux/initialization-bug-any-order.cc
  lib/asan/lit_tests/init-order-dlopen.cc
  lib/asan/asan_flags.h
  lib/asan/asan_rtl.cc
  lib/asan/asan_globals.cc

Index: lib/asan/lit_tests/SharedLibs/init-order-dlopen-so.cc
===================================================================
--- /dev/null
+++ lib/asan/lit_tests/SharedLibs/init-order-dlopen-so.cc
@@ -0,0 +1,12 @@
+#include <stdio.h>
+#include <unistd.h>
+
+void inc_global();
+
+int slow_init() {
+  sleep(1);
+  inc_global();
+  return 42;
+}
+
+int slowly_init_glob = slow_init();
Index: lib/asan/lit_tests/Linux/initialization-bug-any-order.cc
===================================================================
--- lib/asan/lit_tests/Linux/initialization-bug-any-order.cc
+++ lib/asan/lit_tests/Linux/initialization-bug-any-order.cc
@@ -1,12 +1,13 @@
 // Test to make sure basic initialization order errors are caught.
 // Check that on Linux initialization order bugs are caught
-// independently on order in which we list source files.
+// independently on order in which we list source files (if we specify
+// strict init-order checking).
 
 // RUN: %clangxx_asan -m64 -O0 %s %p/../Helpers/initialization-bug-extra.cc -o %t
-// RUN: ASAN_OPTIONS=check_initialization_order=true %t 2>&1 \
+// RUN: ASAN_OPTIONS=check_initialization_order=true:strict_init_order=true %t 2>&1 \
 // RUN:    | %symbolize | FileCheck %s
 // RUN: %clangxx_asan -m64 -O0 %p/../Helpers/initialization-bug-extra.cc %s -o %t
-// RUN: ASAN_OPTIONS=check_initialization_order=true %t 2>&1 \
+// RUN: ASAN_OPTIONS=check_initialization_order=true:strict_init_order=true %t 2>&1 \
 // RUN:    | %symbolize | FileCheck %s
 
 // Do not test with optimization -- the error may be optimized away.
Index: lib/asan/lit_tests/init-order-dlopen.cc
===================================================================
--- /dev/null
+++ lib/asan/lit_tests/init-order-dlopen.cc
@@ -0,0 +1,47 @@
+// Regression test for
+// https://code.google.com/p/address-sanitizer/issues/detail?id=178
+
+// RUN: %clangxx_asan -m64 -O0 %p/SharedLibs/init-order-dlopen-so.cc \
+// RUN:     -fPIC -shared -o %t-so.so
+// RUN: %clangxx_asan -m64 -O0 %s -o %t -Wl,--export-dynamic
+// RUN: ASAN_OPTIONS=check_initialization_order=true %t 2>&1 | FileCheck %s
+#include <dlfcn.h>
+#include <pthread.h>
+#include <stdio.h>
+
+#include <string>
+
+using std::string;
+
+int foo() {
+  return 42;
+}
+int global = foo();
+
+__attribute__((visibility("default")))
+void inc_global() {
+  global++;
+}
+
+void *global_poller(void *arg) {
+  while (true) {
+    if (global != 42)
+      break;
+    usleep(100);
+  }
+  return 0;
+}
+
+int main(int argc, char *argv[]) {
+  pthread_t p;
+  pthread_create(&p, 0, global_poller, 0);
+  string path = string(argv[0]) + "-so.so";
+  if (0 == dlopen(path.c_str(), RTLD_NOW)) {
+    fprintf(stderr, "dlerror: %s\n", dlerror());
+    return 1;
+  }
+  pthread_join(p, 0);
+  printf("PASSED\n");
+  // CHECK: PASSED
+  return 0;
+}
Index: lib/asan/asan_flags.h
===================================================================
--- lib/asan/asan_flags.h
+++ lib/asan/asan_flags.h
@@ -113,6 +113,9 @@
   // If true, assume that memcmp(p1, p2, n) always reads n bytes before
   // comparing p1 and p2.
   bool strict_memcmp;
+  // If true, assume that dynamic initializers can never access globals from
+  // other modules, even if the latter are already initialized.
+  bool strict_init_order;
 };
 
 extern Flags asan_flags_dont_use_directly;
Index: lib/asan/asan_rtl.cc
===================================================================
--- lib/asan/asan_rtl.cc
+++ lib/asan/asan_rtl.cc
@@ -125,6 +125,7 @@
   ParseFlag(str, &f->alloc_dealloc_mismatch, "alloc_dealloc_mismatch");
   ParseFlag(str, &f->use_stack_depot, "use_stack_depot");
   ParseFlag(str, &f->strict_memcmp, "strict_memcmp");
+  ParseFlag(str, &f->strict_init_order, "strict_init_order");
 }
 
 static const char *asan_external_symbolizer;
@@ -170,6 +171,7 @@
   f->alloc_dealloc_mismatch = (SANITIZER_MAC == 0);;
   f->use_stack_depot = true;  // Only affects allocator2.
   f->strict_memcmp = true;
+  f->strict_init_order = false;
 
   // Override from compile definition.
   ParseFlagsFromString(f, MaybeUseAsanDefaultOptionsCompileDefiniton());
Index: lib/asan/asan_globals.cc
===================================================================
--- lib/asan/asan_globals.cc
+++ lib/asan/asan_globals.cc
@@ -37,7 +37,11 @@
 static ListOfGlobals *list_of_all_globals;
 
 static const int kDynamicInitGlobalsInitialCapacity = 512;
-typedef InternalVector<Global> VectorOfGlobals;
+struct DynInitGlobal {
+  Global g;
+  bool initialized;
+};
+typedef InternalVector<DynInitGlobal> VectorOfGlobals;
 // Lazy-initialized and never deleted.
 static VectorOfGlobals *dynamic_init_globals;
 
@@ -101,7 +105,8 @@
       dynamic_init_globals = new(mem)
           VectorOfGlobals(kDynamicInitGlobalsInitialCapacity);
     }
-    dynamic_init_globals->push_back(*g);
+    DynInitGlobal dyn_global = { *g, false };
+    dynamic_init_globals->push_back(dyn_global);
   }
 }
 
@@ -150,16 +155,22 @@
   if (!flags()->check_initialization_order ||
       !flags()->poison_heap)
     return;
+  bool strict_init_order = flags()->strict_init_order;
   CHECK(dynamic_init_globals);
   CHECK(module_name);
   CHECK(asan_inited);
   BlockingMutexLock lock(&mu_for_globals);
   if (flags()->report_globals >= 3)
     Printf("DynInitPoison module: %s\n", module_name);
   for (uptr i = 0, n = dynamic_init_globals->size(); i < n; ++i) {
-    const Global *g = &(*dynamic_init_globals)[i];
-    if (g->module_name != module_name)
+    DynInitGlobal &dyn_g = (*dynamic_init_globals)[i];
+    const Global *g = &dyn_g.g;
+    if (dyn_g.initialized)
+      continue;
+    else if (g->module_name != module_name)
       PoisonShadowForGlobal(g, kAsanInitializationOrderMagic);
+    else if (!strict_init_order)
+      dyn_g.initialized = true;
   }
 }
 
@@ -174,10 +185,13 @@
   BlockingMutexLock lock(&mu_for_globals);
   // FIXME: Optionally report that we're unpoisoning globals from a module.
   for (uptr i = 0, n = dynamic_init_globals->size(); i < n; ++i) {
-    const Global *g = &(*dynamic_init_globals)[i];
-    // Unpoison the whole global.
-    PoisonShadowForGlobal(g, 0);
-    // Poison redzones back.
-    PoisonRedZones(*g);
+    DynInitGlobal &dyn_g = (*dynamic_init_globals)[i];
+    const Global *g = &dyn_g.g;
+    if (!dyn_g.initialized) {
+      // Unpoison the whole global.
+      PoisonShadowForGlobal(g, 0);
+      // Poison redzones back.
+      PoisonRedZones(*g);
+    }
   }
 }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D679.1.patch
Type: text/x-patch
Size: 6441 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130416/09ddf05e/attachment.bin>