ART: Fix things for valgrind

Wire up valgrind gtests. Add valgrind-test-art-host, currently
only depending on valgrind-test-art-host-gtest32.

Fix an Alloc setting to allow running valgrind.

Refactor the fault handler to manage (and correctly release) the
handlers.

Fix minor failure-case leaks exposed by tests.

Failing tests:

The optimizing compiler is leaking non-arena-ed structures
(e.g., assembler buffers), as code generators are not destroyed.
The solution has been moved to a follow-up CL.

Note: All 64b tests are failing as we cannot allocate a heap.

Change-Id: I7f854cfd098d9f68107ce492363e7dba9a82b9fa

7 files changed, 54 insertions, 43 deletions

diff --git a/runtime/class_linker-inl.h b/runtime/class_linker-inl.h
index 1306546801..875efbb2a0 100644
--- a/runtime/class_linker-inl.h
+++ b/runtime/class_linker-inl.h
@@ -156,7 +156,7 @@ inline mirror::ArtField* ClassLinker::ResolveField(uint32_t field_idx, mirror::A
 }
 
 inline mirror::Object* ClassLinker::AllocObject(Thread* self) {
-  return GetClassRoot(kJavaLangObject)->Alloc<false, false>(self,
+  return GetClassRoot(kJavaLangObject)->Alloc<true, false>(self,
       Runtime::Current()->GetHeap()->GetCurrentAllocator());
 }
 
diff --git a/runtime/dex_file.cc b/runtime/dex_file.cc
index ed3592cb87..3bb47d415e 100644
--- a/runtime/dex_file.cc
+++ b/runtime/dex_file.cc
@@ -206,19 +206,20 @@ const DexFile* DexFile::OpenFile(int fd, const char* location, bool verify,
 
   const Header* dex_header = reinterpret_cast<const Header*>(map->Begin());
 
-  const DexFile* dex_file = OpenMemory(location, dex_header->checksum_, map.release(), error_msg);
-  if (dex_file == nullptr) {
+  std::unique_ptr<const DexFile> dex_file(OpenMemory(location, dex_header->checksum_, map.release(),
+                                                     error_msg));
+  if (dex_file.get() == nullptr) {
     *error_msg = StringPrintf("Failed to open dex file '%s' from memory: %s", location,
                               error_msg->c_str());
     return nullptr;
   }
 
-  if (verify && !DexFileVerifier::Verify(dex_file, dex_file->Begin(), dex_file->Size(), location,
-                                         error_msg)) {
+  if (verify && !DexFileVerifier::Verify(dex_file.get(), dex_file->Begin(), dex_file->Size(),
+                                         location, error_msg)) {
     return nullptr;
   }
 
-  return dex_file;
+  return dex_file.release();
 }
 
 const char* DexFile::kClassesDex = "classes.dex";
diff --git a/runtime/fault_handler.cc b/runtime/fault_handler.cc
index 47696f9bd0..fede2f8ec3 100644
--- a/runtime/fault_handler.cc
+++ b/runtime/fault_handler.cc
@@ -19,6 +19,7 @@
 #include <setjmp.h>
 #include <sys/mman.h>
 #include <sys/ucontext.h>
+#include "base/stl_util.h"
 #include "mirror/art_method.h"
 #include "mirror/class.h"
 #include "sigchain.h"
@@ -115,13 +116,23 @@ void FaultManager::Init() {
   initialized_ = true;
 }
 
-void FaultManager::Shutdown() {
+void FaultManager::Release() {
   if (initialized_) {
     UnclaimSignalChain(SIGSEGV);
     initialized_ = false;
   }
 }
 
+void FaultManager::Shutdown() {
+  if (initialized_) {
+    Release();
+
+    // Free all handlers.
+    STLDeleteElements(&generated_code_handlers_);
+    STLDeleteElements(&other_handlers_);
+  }
+}
+
 void FaultManager::HandleFault(int sig, siginfo_t* info, void* context) {
   // BE CAREFUL ALLOCATING HERE INCLUDING USING LOG(...)
   //
@@ -156,9 +167,9 @@ void FaultManager::HandleFault(int sig, siginfo_t* info, void* context) {
 
   // Now set up the nested signal handler.
 
-  // Shutdown the fault manager so that it will remove the signal chain for
+  // Release the fault manager so that it will remove the signal chain for
   // SIGSEGV and we call the real sigaction.
-  fault_manager.Shutdown();
+  fault_manager.Release();
 
   // The action for SIGSEGV should be the default handler now.
 
@@ -226,6 +237,7 @@ void FaultManager::HandleFault(int sig, siginfo_t* info, void* context) {
 }
 
 void FaultManager::AddHandler(FaultHandler* handler, bool generated_code) {
+  DCHECK(initialized_);
   if (generated_code) {
     generated_code_handlers_.push_back(handler);
   } else {
diff --git a/runtime/fault_handler.h b/runtime/fault_handler.h
index bb26780bd9..8b66a6f323 100644
--- a/runtime/fault_handler.h
+++ b/runtime/fault_handler.h
@@ -39,10 +39,17 @@ class FaultManager {
   ~FaultManager();
 
   void Init();
+
+  // Unclaim signals.
+  void Release();
+
+  // Unclaim signals and delete registered handlers.
   void Shutdown();
 
   void HandleFault(int sig, siginfo_t* info, void* context);
   void HandleNestedSignal(int sig, siginfo_t* info, void* context);
+
+  // Added handlers are owned by the fault handler and will be freed on Shutdown().
   void AddHandler(FaultHandler* handler, bool generated_code);
   void RemoveHandler(FaultHandler* handler);
 
diff --git a/runtime/mem_map_test.cc b/runtime/mem_map_test.cc
index 69f618c5e6..e54d0e013d 100644
--- a/runtime/mem_map_test.cc
+++ b/runtime/mem_map_test.cc
@@ -18,6 +18,8 @@
 
 #include <memory>
 
+#include <valgrind.h>
+
 #include "gtest/gtest.h"
 
 namespace art {
@@ -198,17 +200,20 @@ TEST_F(MemMapTest, RemapAtEnd32bit) {
 #endif
 
 TEST_F(MemMapTest, MapAnonymousExactAddr32bitHighAddr) {
-  uintptr_t start_addr = ART_BASE_ADDRESS + 0x1000000;
-  std::string error_msg;
-  std::unique_ptr<MemMap> map(MemMap::MapAnonymous("MapAnonymousExactAddr32bitHighAddr",
-                                             reinterpret_cast<byte*>(start_addr),
-                                             0x21000000,
-                                             PROT_READ | PROT_WRITE,
-                                             true,
-                                             &error_msg));
-  ASSERT_TRUE(map.get() != nullptr) << error_msg;
-  ASSERT_TRUE(error_msg.empty());
-  ASSERT_EQ(reinterpret_cast<uintptr_t>(BaseBegin(map.get())), start_addr);
+  // This test may not work under valgrind.
+  if (RUNNING_ON_VALGRIND == 0) {
+    uintptr_t start_addr = ART_BASE_ADDRESS + 0x1000000;
+    std::string error_msg;
+    std::unique_ptr<MemMap> map(MemMap::MapAnonymous("MapAnonymousExactAddr32bitHighAddr",
+                                                     reinterpret_cast<byte*>(start_addr),
+                                                     0x21000000,
+                                                     PROT_READ | PROT_WRITE,
+                                                     true,
+                                                     &error_msg));
+    ASSERT_TRUE(map.get() != nullptr) << error_msg;
+    ASSERT_TRUE(error_msg.empty());
+    ASSERT_EQ(reinterpret_cast<uintptr_t>(BaseBegin(map.get())), start_addr);
+  }
 }
 
 TEST_F(MemMapTest, MapAnonymousOverflow) {
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index 89ad505232..4b90324c57 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -139,9 +139,6 @@ Runtime::Runtime()
       system_class_loader_(nullptr),
       dump_gc_performance_on_shutdown_(false),
       preinitialization_transaction_(nullptr),
-      null_pointer_handler_(nullptr),
-      suspend_handler_(nullptr),
-      stack_overflow_handler_(nullptr),
       verify_(false),
       target_sdk_version_(0),
       implicit_null_checks_(false),
@@ -199,10 +196,6 @@ Runtime::~Runtime() {
   // TODO: acquire a static mutex on Runtime to avoid racing.
   CHECK(instance_ == nullptr || instance_ == this);
   instance_ = nullptr;
-
-  delete null_pointer_handler_;
-  delete suspend_handler_;
-  delete stack_overflow_handler_;
 }
 
 struct AbortState {
@@ -733,7 +726,8 @@ bool Runtime::Init(const RuntimeOptions& raw_options, bool ignore_unrecognized)
     case kArm64:
     case kX86_64:
       implicit_null_checks_ = true;
-      implicit_so_checks_ = true;
+      // Installing stack protection does not play well with valgrind.
+      implicit_so_checks_ = (RUNNING_ON_VALGRIND == 0);
       break;
     default:
       // Keep the defaults.
@@ -745,16 +739,19 @@ bool Runtime::Init(const RuntimeOptions& raw_options, bool ignore_unrecognized)
 
     // These need to be in a specific order.  The null point check handler must be
     // after the suspend check and stack overflow check handlers.
+    //
+    // Note: the instances attach themselves to the fault manager and are handled by it. The manager
+    //       will delete the instance on Shutdown().
     if (implicit_suspend_checks_) {
-      suspend_handler_ = new SuspensionHandler(&fault_manager);
+      new SuspensionHandler(&fault_manager);
     }
 
     if (implicit_so_checks_) {
-      stack_overflow_handler_ = new StackOverflowHandler(&fault_manager);
+      new StackOverflowHandler(&fault_manager);
     }
 
     if (implicit_null_checks_) {
-      null_pointer_handler_ = new NullPointerHandler(&fault_manager);
+      new NullPointerHandler(&fault_manager);
     }
 
     if (kEnableJavaStackTraceHandler) {
diff --git a/runtime/runtime.h b/runtime/runtime.h
index a0993ca91e..9df14538e9 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -463,16 +463,8 @@ class Runtime {
 
   void AddCurrentRuntimeFeaturesAsDex2OatArguments(std::vector<std::string>* arg_vector) const;
 
-  bool ExplicitNullChecks() const {
-    return null_pointer_handler_ == nullptr;
-  }
-
-  bool ExplicitSuspendChecks() const {
-    return suspend_handler_ == nullptr;
-  }
-
   bool ExplicitStackOverflowChecks() const {
-    return stack_overflow_handler_ == nullptr;
+    return !implicit_so_checks_;
   }
 
   bool IsVerificationEnabled() const {
@@ -636,9 +628,6 @@ class Runtime {
 
   // Transaction used for pre-initializing classes at compilation time.
   Transaction* preinitialization_transaction_;
-  NullPointerHandler* null_pointer_handler_;
-  SuspensionHandler* suspend_handler_;
-  StackOverflowHandler* stack_overflow_handler_;
 
   // If false, verification is disabled. True by default.
   bool verify_;