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
diff --git a/runtime/class_linker-inl.h b/runtime/class_linker-inl.h
index 1306546..875efbb 100644
--- a/runtime/class_linker-inl.h
+++ b/runtime/class_linker-inl.h
@@ -156,7 +156,7 @@
}
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 ed3592c..3bb47d4 100644
--- a/runtime/dex_file.cc
+++ b/runtime/dex_file.cc
@@ -206,19 +206,20 @@
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 47696f9..fede2f8 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 @@
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 @@
// 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::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 bb26780..8b66a6f 100644
--- a/runtime/fault_handler.h
+++ b/runtime/fault_handler.h
@@ -39,10 +39,17 @@
~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 69f618c..e54d0e0 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 @@
#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 89ad505..4b90324 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -139,9 +139,6 @@
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 @@
// 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 @@
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 @@
// 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 a0993ca..9df1453 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -463,16 +463,8 @@
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 @@
// 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_;