Fix the mutex diagnostics, and other targets of opportunity.
Three changes for the price of one:
1. Fix the mutex diagnostics so they work right during startup and shutdown.
2. Fix a memory leak in common_test.
3. Fix memory corruption in the compiler; we were calling memset(3) on a struct
with non-POD members.
Thanks, as usual, to valgrind(1) for the latter two (and several bugs in
earlier attempts at the former).
Change-Id: I15e1ffb01e73e4c56a5bbdcaa7233a4b5221e08a
diff --git a/src/common_test.h b/src/common_test.h
index 74056c9..3d5b53b 100644
--- a/src/common_test.h
+++ b/src/common_test.h
@@ -366,7 +366,8 @@
}
}
class_linker_->FixupDexCaches(runtime_->GetResolutionMethod());
- compiler_.reset(new Compiler(instruction_set, true, 2, false, new std::set<std::string>,
+ image_classes_.reset(new std::set<std::string>);
+ compiler_.reset(new Compiler(instruction_set, true, 2, false, image_classes_.get(),
true, true));
#if defined(ART_USE_LLVM_COMPILER)
compiler_->EnableAutoElfLoading();
@@ -413,6 +414,7 @@
(*icu_cleanup_fn)();
compiler_.reset();
+ image_classes_.reset();
STLDeleteElements(&opened_dex_files_);
Runtime::Current()->GetHeap()->VerifyHeap(); // Check for heap corruption after the test
@@ -512,6 +514,7 @@
// Owned by the runtime
ClassLinker* class_linker_;
UniquePtr<Compiler> compiler_;
+ UniquePtr<std::set<std::string> > image_classes_;
private:
std::vector<const DexFile*> opened_dex_files_;
diff --git a/src/compiler/Compiler.h b/src/compiler/Compiler.h
index 4b4e83d..6d62f29 100644
--- a/src/compiler/Compiler.h
+++ b/src/compiler/Compiler.h
@@ -111,8 +111,6 @@
kNumBitMapKinds
};
-extern uint32_t compilerOptimizerDisableFlags;
-
/* Force code generation paths for testing */
enum debugControlVector {
kDebugDisplayMissingTargets,
@@ -131,14 +129,6 @@
kDebugCountOpcodes,
};
-extern uint32_t compilerDebugFlags;
-
-/* If non-empty, apply optimizer/debug flags only to matching methods */
-extern std::string compilerMethodMatch;
-
-/* Flips sense of compilerMethodMatch - apply flags if doesn't match */
-extern bool compilerFlipMatch;
-
enum OatMethodAttributes {
kIsCallee = 0, /* Code is part of a callee (invoked by a hot trace) */
kIsHot, /* Code is part of a hot trace */
diff --git a/src/compiler/CompilerIR.h b/src/compiler/CompilerIR.h
index 621cccc..0fc26de 100644
--- a/src/compiler/CompilerIR.h
+++ b/src/compiler/CompilerIR.h
@@ -290,7 +290,100 @@
#define NOTVISITED (-1)
struct CompilationUnit {
- int numInsts;
+ CompilationUnit()
+ : numBlocks(0),
+ compiler(NULL),
+ class_linker(NULL),
+ dex_file(NULL),
+ dex_cache(NULL),
+ class_loader(NULL),
+ method_idx(0),
+ code_item(NULL),
+ access_flags(0),
+ shorty(NULL),
+ firstLIRInsn(NULL),
+ lastLIRInsn(NULL),
+ literalList(NULL),
+ methodLiteralList(NULL),
+ codeLiteralList(NULL),
+ classPointerList(NULL),
+ numClassPointers(0),
+ chainCellOffsetLIR(NULL),
+ disableOpt(0),
+ enableDebug(0),
+ headerSize(0),
+ dataOffset(0),
+ totalSize(0),
+ assemblerStatus(kSuccess),
+ assemblerRetries(0),
+ genDebugger(false),
+ printMe(false),
+ hasClassLiterals(false),
+ hasLoop(false),
+ hasInvoke(false),
+ heapMemOp(false),
+ qdMode(false),
+ usesLinkRegister(false),
+ methodTraceSupport(false),
+ regPool(NULL),
+ optRound(0),
+ instructionSet(kNone),
+ numSSARegs(0),
+ ssaBaseVRegs(NULL),
+ ssaSubscripts(NULL),
+ vRegToSSAMap(NULL),
+ SSALastDefs(NULL),
+ isConstantV(NULL),
+ constantValues(NULL),
+ phiAliasMap(NULL),
+ phiList(NULL),
+ regLocation(NULL),
+ sequenceNumber(0),
+ promotionMap(NULL),
+ methodSReg(0),
+ switchOverflowPad(NULL),
+ numReachableBlocks(0),
+ numDalvikRegisters(0),
+ entryBlock(NULL),
+ exitBlock(NULL),
+ curBlock(NULL),
+ nextCodegenBlock(NULL),
+ iDomList(NULL),
+ tryBlockAddr(NULL),
+ defBlockMatrix(NULL),
+ tempBlockV(NULL),
+ tempDalvikRegisterV(NULL),
+ tempSSARegisterV(NULL),
+ printSSANames(false),
+ blockLabelList(NULL),
+ quitLoopMode(false),
+ preservedRegsUsed(0),
+ numIns(0),
+ numOuts(0),
+ numRegs(0),
+ numCoreSpills(0),
+ numFPSpills(0),
+ numCompilerTemps(0),
+ frameSize(0),
+ coreSpillMask(0U),
+ fpSpillMask(0U),
+ attrs(0U),
+ currentDalvikOffset(0),
+ insns(NULL),
+ insnsSize(0U),
+ disableDataflow(false),
+ defCount(0),
+ compilerFlipMatch(false),
+ arenaHead(NULL),
+ currentArena(NULL),
+ numArenaBlocks(0),
+ mstats(NULL),
+ opcodeCount(NULL) {
+#if !defined(NDEBUG)
+ liveSReg = 0;
+#endif
+ }
+
int numBlocks;
GrowableList blockList;
Compiler* compiler; // Compiler driving this compiler
@@ -420,22 +513,25 @@
* The low-level LIR creation utilites will pull it from here. Should
* be rewritten.
*/
- int currentDalvikOffset;
- GrowableList switchTables;
- GrowableList fillArrayData;
- const u2* insns;
- u4 insnsSize;
- bool disableDataflow; // Skip dataflow analysis if possible
- std::map<unsigned int, BasicBlock*> blockMap; // findBlock lookup cache
- std::map<unsigned int, LIR*> boundaryMap; // boundary lookup cache
- int defCount; // Used to estimate number of SSA names
- std::string* compilerMethodMatch;
- bool compilerFlipMatch;
- struct ArenaMemBlock* arenaHead;
- struct ArenaMemBlock* currentArena;
- int numArenaBlocks;
- struct Memstats* mstats;
- int* opcodeCount; // Count Dalvik opcodes for tuning
+ int currentDalvikOffset;
+ GrowableList switchTables;
+ GrowableList fillArrayData;
+ const u2* insns;
+ u4 insnsSize;
+ bool disableDataflow; // Skip dataflow analysis if possible
+ std::map<unsigned int, BasicBlock*> blockMap; // findBlock lookup cache
+ std::map<unsigned int, LIR*> boundaryMap; // boundary lookup cache
+ int defCount; // Used to estimate number of SSA names
+
+ // If non-empty, apply optimizer/debug flags only to matching methods.
+ std::string compilerMethodMatch;
+ // Flips sense of compilerMethodMatch - apply flags if doesn't match.
+ bool compilerFlipMatch;
+ struct ArenaMemBlock* arenaHead;
+ struct ArenaMemBlock* currentArena;
+ int numArenaBlocks;
+ struct Memstats* mstats;
+ int* opcodeCount; // Count Dalvik opcodes for tuning
#ifndef NDEBUG
/*
* Sanity checking for the register temp tracking. The same ssa
diff --git a/src/compiler/CompilerUtility.h b/src/compiler/CompilerUtility.h
index 41f6cf1..f7b9b0e 100644
--- a/src/compiler/CompilerUtility.h
+++ b/src/compiler/CompilerUtility.h
@@ -43,11 +43,14 @@
void oatArenaReset(CompilationUnit *cUnit);
struct GrowableList {
- size_t numAllocated;
- size_t numUsed;
- intptr_t *elemList;
+ GrowableList() : numAllocated(0), numUsed(0), elemList(NULL) {
+ }
+
+ size_t numAllocated;
+ size_t numUsed;
+ intptr_t* elemList;
#ifdef WITH_MEMSTATS
- oatListKind kind;
+ oatListKind kind;
#endif
};
diff --git a/src/compiler/Frontend.cc b/src/compiler/Frontend.cc
index 74c13d4..309bcf8 100644
--- a/src/compiler/Frontend.cc
+++ b/src/compiler/Frontend.cc
@@ -24,7 +24,7 @@
namespace art {
/* Default optimizer/debug setting for the compiler. */
-uint32_t compilerOptimizerDisableFlags = 0 | // Disable specific optimizations
+static uint32_t kCompilerOptimizerDisableFlags = 0 | // Disable specific optimizations
//(1 << kLoadStoreElimination) |
//(1 << kLoadHoisting) |
//(1 << kSuppressLoads) |
@@ -38,7 +38,7 @@
//(1 << kPromoteCompilerTemps) |
0;
-uint32_t compilerDebugFlags = 0 | // Enable debug/testing modes
+static uint32_t kCompilerDebugFlags = 0 | // Enable debug/testing modes
//(1 << kDebugDisplayMissingTargets) |
//(1 << kDebugVerbose) |
//(1 << kDebugDumpCFG) |
@@ -751,7 +751,6 @@
ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
UniquePtr<CompilationUnit> cUnit(new CompilationUnit);
- memset(cUnit.get(), 0, sizeof(*cUnit));
oatInit(cUnit.get(), compiler);
@@ -771,20 +770,14 @@
cUnit->numOuts = code_item->outs_size_;
/* Adjust this value accordingly once inlining is performed */
cUnit->numDalvikRegisters = code_item->registers_size_;
- cUnit->blockMap = std::map<unsigned int, BasicBlock*>();
- cUnit->blockMap.clear();
- cUnit->boundaryMap = std::map<unsigned int, LIR*>();
- cUnit->boundaryMap.clear();
- // TODO: set these from command line
- cUnit->compilerMethodMatch = new std::string("");
+ // TODO: set this from command line
cUnit->compilerFlipMatch = false;
- bool useMatch = cUnit->compilerMethodMatch->length() != 0;
+ bool useMatch = !cUnit->compilerMethodMatch.empty();
bool match = useMatch && (cUnit->compilerFlipMatch ^
- (PrettyMethod(method_idx, dex_file).find(*cUnit->compilerMethodMatch)
- != std::string::npos));
+ (PrettyMethod(method_idx, dex_file).find(cUnit->compilerMethodMatch) != std::string::npos));
if (!useMatch || match) {
- cUnit->disableOpt = compilerOptimizerDisableFlags;
- cUnit->enableDebug = compilerDebugFlags;
+ cUnit->disableOpt = kCompilerOptimizerDisableFlags;
+ cUnit->enableDebug = kCompilerDebugFlags;
cUnit->printMe = VLOG_IS_ON(compiler) || (cUnit->enableDebug & (1 << kDebugVerbose));
}
if (cUnit->instructionSet == kX86) {
@@ -806,7 +799,7 @@
}
/* Gathering opcode stats? */
- if (compilerDebugFlags & (1 << kDebugCountOpcodes)) {
+ if (kCompilerDebugFlags & (1 << kDebugCountOpcodes)) {
cUnit->opcodeCount = (int*)oatNew(cUnit.get(),
kNumPackedOpcodes * sizeof(int), true, kAllocMisc);
}
diff --git a/src/mutex.cc b/src/mutex.cc
index b56f1ef..1e30543 100644
--- a/src/mutex.cc
+++ b/src/mutex.cc
@@ -54,20 +54,14 @@
if (rank == -1) {
return;
}
- Thread* self = Thread::Current();
- if (self != NULL) {
- self->CheckSafeToLockOrUnlock(rank, is_locking);
- }
+ Thread::Current()->CheckSafeToLockOrUnlock(rank, is_locking);
}
static inline void CheckSafeToWait(MutexRank rank) {
if (!kIsDebugBuild) {
return;
}
- Thread* self = Thread::Current();
- if (self != NULL) {
- self->CheckSafeToWait(rank);
- }
+ Thread::Current()->CheckSafeToWait(rank);
}
Mutex::Mutex(const char* name, MutexRank rank) : name_(name), rank_(rank) {
diff --git a/src/runtime.cc b/src/runtime.cc
index 3aa7e6e..bedeb27 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -88,11 +88,9 @@
Trace::Shutdown();
}
- Dbg::StopJdwp();
-
// Make sure our internal threads are dead before we start tearing down things they're using.
+ Dbg::StopJdwp();
delete signal_catcher_;
- // TODO: GC thread.
// Make sure all other non-daemon threads have terminated, and all daemon threads are suspended.
delete thread_list_;
diff --git a/src/thread_list.cc b/src/thread_list.cc
index a64e217..706a2ef 100644
--- a/src/thread_list.cc
+++ b/src/thread_list.cc
@@ -25,7 +25,8 @@
namespace art {
ThreadList::ThreadList()
- : thread_list_lock_("thread list lock", kThreadListLock),
+ : allocated_ids_lock_("allocated thread ids lock"),
+ thread_list_lock_("thread list lock", kThreadListLock),
thread_start_cond_("thread_start_cond_"),
thread_exit_cond_("thread_exit_cond_"),
thread_suspend_count_lock_("thread suspend count lock", kThreadSuspendCountLock),
@@ -34,13 +35,15 @@
}
ThreadList::~ThreadList() {
- // Detach the current thread if necessary.
+ WaitForOtherNonDaemonThreadsToExit();
+ SuspendAllDaemonThreads();
+
+ // Detach the current thread if necessary. If we failed to start, there might not be any threads.
+ // We don't deal with the current thread before this point because the thread/thread-list
+ // diagnostics and debugging machinery requires the current thread to be attached.
if (Contains(Thread::Current())) {
Runtime::Current()->DetachCurrentThread();
}
-
- WaitForNonDaemonThreadsToExit();
- SuspendAllDaemonThreads();
}
bool ThreadList::Contains(Thread* thread) {
@@ -335,13 +338,13 @@
ScopedThreadListLock thread_list_lock;
CHECK(Contains(self));
list_.remove(self);
-
- // Delete the Thread* and release the thin lock id.
- uint32_t thin_lock_id = self->thin_lock_id_;
- delete self;
- ReleaseThreadId(thin_lock_id);
}
+ // Delete the Thread* and release the thin lock id.
+ uint32_t thin_lock_id = self->thin_lock_id_;
+ delete self;
+ ReleaseThreadId(thin_lock_id);
+
// Clear the TLS data, so that the underlying native thread is recognizably detached.
// (It may wish to reattach later.)
CHECK_PTHREAD_CALL(pthread_setspecific, (Thread::pthread_key_self_, NULL), "detach self");
@@ -432,20 +435,21 @@
self->SetState(Thread::kRunnable);
}
-bool ThreadList::AllThreadsAreDaemons() {
+bool ThreadList::AllOtherThreadsAreDaemons() {
for (It it = list_.begin(), end = list_.end(); it != end; ++it) {
// TODO: there's a race here with thread exit that's being worked around by checking if the peer
// is null.
- if ((*it)->GetPeer() != NULL && !(*it)->IsDaemon()) {
+ Thread* thread = *it;
+ if (thread != Thread::Current() && thread->GetPeer() != NULL && !thread->IsDaemon()) {
return false;
}
}
return true;
}
-void ThreadList::WaitForNonDaemonThreadsToExit() {
+void ThreadList::WaitForOtherNonDaemonThreadsToExit() {
ScopedThreadListLock thread_list_lock;
- while (!AllThreadsAreDaemons()) {
+ while (!AllOtherThreadsAreDaemons()) {
thread_exit_cond_.Wait(thread_list_lock_);
}
}
@@ -453,13 +457,14 @@
void ThreadList::SuspendAllDaemonThreads() {
ScopedThreadListLock thread_list_lock;
- // Tell all the daemons it's time to suspend. (At this point, we know
- // all threads are daemons.)
+ // Tell all the daemons it's time to suspend.
{
MutexLock mu(thread_suspend_count_lock_);
for (It it = list_.begin(), end = list_.end(); it != end; ++it) {
Thread* thread = *it;
- ++thread->suspend_count_;
+ if (thread != Thread::Current()) {
+ ++thread->suspend_count_;
+ }
}
}
@@ -470,7 +475,7 @@
bool all_suspended = true;
for (It it = list_.begin(), end = list_.end(); it != end; ++it) {
Thread* thread = *it;
- if (thread->GetState() == Thread::kRunnable) {
+ if (thread != Thread::Current() && thread->GetState() == Thread::kRunnable) {
if (!have_complained) {
LOG(WARNING) << "daemon thread not yet suspended: " << *thread;
have_complained = true;
@@ -485,7 +490,8 @@
}
uint32_t ThreadList::AllocThreadId() {
- ScopedThreadListLock thread_list_lock;
+ MutexLock mu(allocated_ids_lock_);
+ //ScopedThreadListLock thread_list_lock;
for (size_t i = 0; i < allocated_ids_.size(); ++i) {
if (!allocated_ids_[i]) {
allocated_ids_.set(i);
@@ -497,7 +503,7 @@
}
void ThreadList::ReleaseThreadId(uint32_t id) {
- thread_list_lock_.AssertHeld();
+ MutexLock mu(allocated_ids_lock_);
--id; // Zero is reserved to mean "invalid".
DCHECK(allocated_ids_[id]) << id;
allocated_ids_.reset(id);
diff --git a/src/thread_list.h b/src/thread_list.h
index fbcd9bc..0691808 100644
--- a/src/thread_list.h
+++ b/src/thread_list.h
@@ -60,17 +60,21 @@
private:
typedef std::list<Thread*>::const_iterator It; // TODO: C++0x auto
- bool AllThreadsAreDaemons();
uint32_t AllocThreadId();
- bool Contains(Thread* thread);
void ReleaseThreadId(uint32_t id);
+
+ bool Contains(Thread* thread);
+
+ bool AllOtherThreadsAreDaemons();
void SuspendAllDaemonThreads();
- void WaitForNonDaemonThreadsToExit();
+ void WaitForOtherNonDaemonThreadsToExit();
static void ModifySuspendCount(Thread* thread, int delta, bool for_debugger);
- mutable Mutex thread_list_lock_;
+ mutable Mutex allocated_ids_lock_;
std::bitset<kMaxThreadId> allocated_ids_;
+
+ mutable Mutex thread_list_lock_;
std::list<Thread*> list_;
ConditionVariable thread_start_cond_;