[JIT] Close code cache race window
See http://b/issue?id=4271784 for details.
Three fixes:
1. Verify the code cache version hasn't changed between completion
of the translation and registering it in JitTable
2. When code cache full detected during translating a trace, mark
the "discard" flag on the work order.
3. [The actual cause of the bug] When doing a code cache flush,
traverse the thread least and cancel any trace selections in
progress.
Change-Id: Ifea70416d7d91637fb742fc8de11044a89358caa
diff --git a/vm/compiler/Compiler.c b/vm/compiler/Compiler.c
index dfa6589..4f2261b 100644
--- a/vm/compiler/Compiler.c
+++ b/vm/compiler/Compiler.c
@@ -266,7 +266,8 @@
int inJit = 0;
int byteUsed = gDvmJit.codeCacheByteUsed;
- /* If any thread is found stuck in the JIT state, don't reset the cache */
+ /* If any thread is found stuck in the JIT state, don't reset the cache */
+ dvmLockThreadList(NULL);
for (thread = gDvm.threadList; thread != NULL; thread = thread->next) {
/*
* Crawl the stack to wipe out the returnAddr field so that
@@ -278,7 +279,11 @@
if (thread->inJitCodeCache) {
inJit++;
}
+ /* Cancel any ongoing trace selection */
+ dvmUpdateInterpBreak(thread, kInterpJitBreak, kSubModeJitTraceBuild,
+ false /* clear */);
}
+ dvmUnlockThreadList();
if (inJit) {
LOGD("JIT code cache reset delayed (%d bytes %d/%d)",
@@ -675,13 +680,23 @@
bool aborted = setjmp(jmpBuf);
if (!aborted) {
bool codeCompiled = dvmCompilerDoWork(&work);
- if (codeCompiled && !work.result.discardResult &&
- work.result.codeAddress) {
+ /*
+ * Make sure we are still operating with the
+ * same translation cache version. See
+ * Issue 4271784 for details.
+ */
+ dvmLockMutex(&gDvmJit.compilerLock);
+ if ((work.result.cacheVersion ==
+ gDvmJit.cacheVersion) &&
+ codeCompiled &&
+ !work.result.discardResult &&
+ work.result.codeAddress) {
dvmJitSetCodeAddr(work.pc, work.result.codeAddress,
work.result.instructionSet,
false, /* not method entry */
work.result.profileCodeSize);
}
+ dvmUnlockMutex(&gDvmJit.compilerLock);
}
dvmCompilerArenaReset();
}
diff --git a/vm/compiler/codegen/arm/Assemble.c b/vm/compiler/codegen/arm/Assemble.c
index 4f2c10a..83fec33 100644
--- a/vm/compiler/codegen/arm/Assemble.c
+++ b/vm/compiler/codegen/arm/Assemble.c
@@ -1426,7 +1426,7 @@
if (gDvmJit.codeCacheByteUsed + cUnit->totalSize > gDvmJit.codeCacheSize) {
gDvmJit.codeCacheFull = true;
- cUnit->baseAddr = NULL;
+ info->discardResult = true;
return;
}
@@ -1434,7 +1434,7 @@
cUnit->codeBuffer = (unsigned char *)dvmCompilerNew(chainCellOffset, true);
if (cUnit->codeBuffer == NULL) {
LOGE("Code buffer allocation failure\n");
- cUnit->baseAddr = NULL;
+ info->discardResult = true;
return;
}
diff --git a/vm/interp/Jit.c b/vm/interp/Jit.c
index 936bbc8..c32e922 100644
--- a/vm/interp/Jit.c
+++ b/vm/interp/Jit.c
@@ -1094,13 +1094,14 @@
* Register the translated code pointer into the JitTable.
* NOTE: Once a codeAddress field transitions from initial state to
* JIT'd code, it must not be altered without first halting all
- * threads. This routine should only be called by the compiler
- * thread. We defer the setting of the profile prefix size until
+ * threads. We defer the setting of the profile prefix size until
* after the new code address is set to ensure that the prefix offset
* is never applied to the initial interpret-only translation. All
* translations with non-zero profile prefixes will still be correct
* if entered as if the profile offset is 0, but the interpret-only
* template cannot handle a non-zero prefix.
+ * NOTE: JitTable must not be in danger of reset while this
+ * code is executing. see Issue 4271784 for details.
*/
void dvmJitSetCodeAddr(const u2* dPC, void *nPC, JitInstructionSetType set,
bool isMethodEntry, int profilePrefixSize)
@@ -1108,12 +1109,13 @@
JitEntryInfoUnion oldValue;
JitEntryInfoUnion newValue;
/*
- * Method-based JIT doesn't go through the normal profiling phase, so use
- * lookupAndAdd here to request a new entry in the table.
+ * Get the JitTable slot for this dPC (or create one if JitTable
+ * has been reset between the time the trace was requested and
+ * now.
*/
JitEntry *jitEntry = isMethodEntry ?
- lookupAndAdd(dPC, false /* caller locked */, true) :
- dvmJitFindEntry(dPC, isMethodEntry);
+ lookupAndAdd(dPC, false /* caller holds tableLock */, isMethodEntry) :
+ dvmJitFindEntry(dPC, isMethodEntry);
assert(jitEntry);
/* Note: order of update is important */
do {
