Change the way breakpoints work.
This replaces the breakpoint mechanism with a more efficient approach.
We now insert breakpoint instructions into the bytecode stream instead of
maintaining a table. This requires mapping DEX files as private instead
of shared, which allows copy-on-write to work. mprotect() is used to
guard the pages against inadvertent writes.
Unused opcode EC is now OP_BREAKPOINT. It's not recognized by dexdump or
any interpreter except portdbg, but it can be encountered by the bytecode
verifier (the debugger can request breakpoints in unverified code).
Breakpoint changes are blocked while the verifier runs to avoid races.
This eliminates method->debugBreakpointCount, which is no longer needed.
(Also, it clashed with LinearAlloc's read-only mode.)
The deferred verification error mechanism was using a code-copying
approach to modify the bytecode stream. That has been changed to use
the same copy-on-write modification mechanism.
Also, normalized all PAGE_SIZE/PAGESIZE references to a single
SYSTEM_PAGE_SIZE define.
Simple Fibonacci computation test times (opal-eng):
JIT, no debugger: 10.6ms
Fast interp, no debugger: 36ms
Portable interp, no debugger: 43.8ms
ORIG debug interp, no breakpoints set: 458ms
ORIG debug interp, breakpoint set nearby: 697ms
NEW debug interp, no breakpoints set: 341ms
NEW debug interp, breakpoints set nearby: 341ms
Where "nearby" means there's a breakpoint in the method doing the
computation that isn't actually hit -- the VM had an optimization where
it flagged methods with breakpoints and skipped some of the processing
when possible.
The bottom line is that code should run noticeably faster while a
debugger is attached.
diff --git a/dexdump/OpCodeNames.c b/dexdump/OpCodeNames.c
index 378dcf7..6a1a52a 100644
--- a/dexdump/OpCodeNames.c
+++ b/dexdump/OpCodeNames.c
@@ -296,8 +296,8 @@
"UNUSED",
"UNUSED",
"UNUSED",
- "UNUSED",
- "UNUSED",
+ "^breakpoint", // does not appear in DEX files
+ "^throw-verification-error", // does not appear in DEX files
"+execute-inline",
"UNUSED",
diff --git a/libdex/InstrUtils.c b/libdex/InstrUtils.c
index 93e1f00..06e26bc 100644
--- a/libdex/InstrUtils.c
+++ b/libdex/InstrUtils.c
@@ -306,7 +306,7 @@
width = -3;
break;
- /* these should never appear */
+ /* these should never appear when scanning bytecode */
case OP_UNUSED_3E:
case OP_UNUSED_3F:
case OP_UNUSED_40:
@@ -325,7 +325,7 @@
case OP_UNUSED_E9:
case OP_UNUSED_EA:
case OP_UNUSED_EB:
- case OP_UNUSED_EC:
+ case OP_BREAKPOINT:
case OP_UNUSED_EF:
case OP_UNUSED_F1:
case OP_UNUSED_FC:
@@ -635,7 +635,7 @@
flags = kInstrCanContinue | kInstrCanThrow | kInstrInvoke;
break;
- /* these should never appear */
+ /* these should never appear when scanning code */
case OP_UNUSED_3E:
case OP_UNUSED_3F:
case OP_UNUSED_40:
@@ -654,7 +654,7 @@
case OP_UNUSED_E9:
case OP_UNUSED_EA:
case OP_UNUSED_EB:
- case OP_UNUSED_EC:
+ case OP_BREAKPOINT:
case OP_UNUSED_EF:
case OP_UNUSED_F1:
case OP_UNUSED_FC:
@@ -989,7 +989,7 @@
fmt = kFmt35c;
break;
- /* these should never appear */
+ /* these should never appear when scanning code */
case OP_UNUSED_3E:
case OP_UNUSED_3F:
case OP_UNUSED_40:
@@ -1008,7 +1008,7 @@
case OP_UNUSED_E9:
case OP_UNUSED_EA:
case OP_UNUSED_EB:
- case OP_UNUSED_EC:
+ case OP_BREAKPOINT:
case OP_UNUSED_EF:
case OP_UNUSED_F1:
case OP_UNUSED_FC:
diff --git a/libdex/OpCode.h b/libdex/OpCode.h
index 1272231..c3ed476 100644
--- a/libdex/OpCode.h
+++ b/libdex/OpCode.h
@@ -330,7 +330,15 @@
OP_UNUSED_E9 = 0xe9,
OP_UNUSED_EA = 0xea,
OP_UNUSED_EB = 0xeb,
- OP_UNUSED_EC = 0xec,
+
+ /*
+ * The "breakpoint" instruction is special, in that it should never
+ * be seen by anything but the debug interpreter. During debugging
+ * it takes the place of an arbitrary opcode, which means operations
+ * like "tell me the opcode width so I can find the next instruction"
+ * aren't possible. (This is correctable, but probably not useful.)
+ */
+ OP_BREAKPOINT = 0xec,
/* optimizer output -- these are never generated by "dx" */
OP_THROW_VERIFICATION_ERROR = 0xed,
@@ -358,6 +366,7 @@
#define kNumDalvikInstructions 256
+
/*
* Switch-statement signatures are a "NOP" followed by a code. (A true NOP
* is 0x0000.)
@@ -627,7 +636,7 @@
H(OP_UNUSED_E9), \
H(OP_UNUSED_EA), \
H(OP_UNUSED_EB), \
- H(OP_UNUSED_EC), \
+ H(OP_BREAKPOINT), \
H(OP_THROW_VERIFICATION_ERROR), \
H(OP_EXECUTE_INLINE), \
H(OP_UNUSED_EF), \
diff --git a/libdex/SysUtil.c b/libdex/SysUtil.c
index bf1be88..08dc67c 100644
--- a/libdex/SysUtil.c
+++ b/libdex/SysUtil.c
@@ -155,7 +155,7 @@
/*
* Map a file (from fd's current offset) into a shared, read-only memory
- * segment. The file offset must be a multiple of the page size.
+ * segment. The file offset must be a multiple of the system page size.
*
* On success, returns 0 and fills out "pMap". On failure, returns a nonzero
* value and does not disturb "pMap".
@@ -172,12 +172,26 @@
if (getFileStartAndLength(fd, &start, &length) < 0)
return -1;
- memPtr = mmap(NULL, length, PROT_READ, MAP_FILE | MAP_SHARED, fd, start);
+ /*
+ * This was originally (PROT_READ, MAP_SHARED), but we want to be able
+ * to make local edits for verification errors and debugger breakpoints.
+ * So we map it read-write and private, but use mprotect to mark the
+ * pages read-only. This should yield identical results so long as the
+ * pages are left read-only.
+ */
+ memPtr = mmap(NULL, length, PROT_READ | PROT_WRITE, MAP_FILE | MAP_PRIVATE,
+ fd, start);
if (memPtr == MAP_FAILED) {
- LOGW("mmap(%d, R, FILE|SHARED, %d, %d) failed: %s\n", (int) length,
+ LOGW("mmap(%d, R/W, FILE|PRIVATE, %d, %d) failed: %s\n", (int) length,
fd, (int) start, strerror(errno));
return -1;
}
+ if (mprotect(memPtr, length, PROT_READ) < 0) {
+ LOGW("mprotect(%p, %d, PROT_READ) failed: %s\n",
+ memPtr, length, strerror(errno));
+ (void) munmap(memPtr, length);
+ return -1;
+ }
pMap->baseAddr = pMap->addr = memPtr;
pMap->baseLength = pMap->length = length;
@@ -270,6 +284,45 @@
}
/*
+ * Change the access rights on one or more pages to read-only or read-write.
+ *
+ * Returns 0 on success.
+ */
+int sysChangeMapAccess(void* addr, size_t length, int wantReadWrite,
+ MemMapping* pMap)
+{
+ /*
+ * Verify that "addr" is part of this mapping file.
+ */
+ if (addr < pMap->baseAddr ||
+ (u1*)addr >= (u1*)pMap->baseAddr + pMap->baseLength)
+ {
+ LOGE("Attempted to change %p; map is %p - %p\n",
+ addr, pMap->baseAddr, (u1*)pMap->baseAddr + pMap->baseLength);
+ return -1;
+ }
+
+ /*
+ * Align "addr" to a page boundary and adjust "length" appropriately.
+ * (The address must be page-aligned, the length doesn't need to be,
+ * but we do need to ensure we cover the same range.)
+ */
+ u1* alignAddr = (u1*) ((int) addr & ~(SYSTEM_PAGE_SIZE-1));
+ size_t alignLength = length + ((u1*) addr - alignAddr);
+
+ //LOGI("%p/%zd --> %p/%zd\n", addr, length, alignAddr, alignLength);
+ int prot = wantReadWrite ? (PROT_READ|PROT_WRITE) : (PROT_READ);
+ if (mprotect(alignAddr, alignLength, prot) != 0) {
+ int err = errno;
+ LOGW("mprotect (%p,%zd,%d) failed: %s\n",
+ alignAddr, alignLength, prot, strerror(errno));
+ return (errno != 0) ? errno : -1;
+ }
+
+ return 0;
+}
+
+/*
* Release a memory mapping.
*/
void sysReleaseShmem(MemMapping* pMap)
diff --git a/libdex/SysUtil.h b/libdex/SysUtil.h
index 8b80503..6ee352b 100644
--- a/libdex/SysUtil.h
+++ b/libdex/SysUtil.h
@@ -23,6 +23,17 @@
#include <sys/types.h>
/*
+ * System page size. Normally you're expected to get this from
+ * sysconf(_SC_PAGESIZE) or some system-specific define (usually PAGESIZE
+ * or PAGE_SIZE). If we use a simple #define the compiler can generate
+ * appropriate masks directly, so we define it here and verify it as the
+ * VM is starting up.
+ *
+ * Must be a power of 2.
+ */
+#define SYSTEM_PAGE_SIZE 4096
+
+/*
* Use this to keep track of mapped segments.
*/
typedef struct MemMapping {
@@ -71,6 +82,15 @@
int sysCreatePrivateMap(size_t length, MemMapping* pMap);
/*
+ * Change the access rights on one or more pages. If "wantReadWrite" is
+ * zero, the pages will be made read-only; otherwise they will be read-write.
+ *
+ * Returns 0 on success.
+ */
+int sysChangeMapAccess(void* addr, size_t length, int wantReadWrite,
+ MemMapping* pmap);
+
+/*
* Release the pages associated with a shared memory segment.
*
* This does not free "pMap"; it just releases the memory.
diff --git a/vm/Debugger.c b/vm/Debugger.c
index eec8176..97e7b00 100644
--- a/vm/Debugger.c
+++ b/vm/Debugger.c
@@ -97,6 +97,9 @@
*/
bool dvmDebuggerStartup(void)
{
+ if (!dvmBreakpointStartup())
+ return false;
+
gDvm.dbgRegistry = dvmHashTableCreate(1000, NULL);
return (gDvm.dbgRegistry != NULL);
}
@@ -108,6 +111,7 @@
{
dvmHashTableFree(gDvm.dbgRegistry);
gDvm.dbgRegistry = NULL;
+ dvmBreakpointShutdown();
}
diff --git a/vm/Debugger.h b/vm/Debugger.h
index 189e2be..eb9c439 100644
--- a/vm/Debugger.h
+++ b/vm/Debugger.h
@@ -13,6 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
+
/*
* Dalvik-specific side of debugger support. (The JDWP code is intended to
* be relatively generic.)
@@ -32,7 +33,7 @@
struct Thread;
/*
- * used by StepControl to track a set of addresses associated with
+ * Used by StepControl to track a set of addresses associated with
* a single line.
*/
typedef struct AddressSet {
diff --git a/vm/DvmDex.c b/vm/DvmDex.c
index 6740632..20a4376 100644
--- a/vm/DvmDex.c
+++ b/vm/DvmDex.c
@@ -13,11 +13,13 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
+
/*
* VM-specific state associated with a DEX file.
*/
#include "Dalvik.h"
+
/*
* Create auxillary data structures.
*
@@ -217,3 +219,73 @@
free(pDvmDex);
}
+
+/*
+ * Change the byte at the specified address to a new value. If the location
+ * already has the new value, do nothing.
+ *
+ * This requires changing the access permissions to read-write, updating
+ * the value, and then resetting the permissions.
+ *
+ * This does not make any synchronization guarantees. It's important for the
+ * caller(s) to work out mutual exclusion, at least on a page granularity,
+ * to avoid a race where one threads sets read-write, another thread sets
+ * read-only, and then the first thread does a write.
+ *
+ * TODO: if we're back to the original state of the page, use
+ * madvise(MADV_DONTNEED) to release the private/dirty copy.
+ *
+ * Returns "true" on success.
+ */
+bool dvmDexChangeDex1(DvmDex* pDvmDex, u1* addr, u1 newVal)
+{
+ if (*addr == newVal) {
+ LOGV("+++ byte at %p is already 0x%02x\n", addr, newVal);
+ return true;
+ }
+
+ LOGV("+++ change byte at %p from 0x%02x to 0x%02x\n", addr, *addr, newVal);
+ if (sysChangeMapAccess(addr, 1, true, &pDvmDex->memMap) < 0) {
+ LOGE("access change failed\n");
+ return false;
+ }
+
+ *addr = newVal;
+
+ if (sysChangeMapAccess(addr, 1, false, &pDvmDex->memMap) < 0) {
+ LOGW("WARNING: unable to restore read-only access on mapping\n");
+ /* not fatal, keep going */
+ }
+
+ return true;
+}
+
+/*
+ * Change the 2-byte value at the specified address to a new value. If the
+ * location already has the new value, do nothing.
+ *
+ * Otherwise works like dvmDexChangeDex1.
+ */
+bool dvmDexChangeDex2(DvmDex* pDvmDex, u2* addr, u2 newVal)
+{
+ if (*addr == newVal) {
+ LOGV("+++ value at %p is already 0x%04x\n", addr, newVal);
+ return true;
+ }
+
+ LOGV("+++ change 2byte at %p from 0x%04x to 0x%04x\n", addr, *addr, newVal);
+ if (sysChangeMapAccess(addr, 2, true, &pDvmDex->memMap) < 0) {
+ LOGE("access change failed\n");
+ return false;
+ }
+
+ *addr = newVal;
+
+ if (sysChangeMapAccess(addr, 2, false, &pDvmDex->memMap) < 0) {
+ LOGW("WARNING: unable to restore read-only access on mapping\n");
+ /* not fatal, keep going */
+ }
+
+ return true;
+}
+
diff --git a/vm/DvmDex.h b/vm/DvmDex.h
index be31af3..9f3903a 100644
--- a/vm/DvmDex.h
+++ b/vm/DvmDex.h
@@ -13,6 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
+
/*
* The VM wraps some additional data structures around the DexFile. These
* are defined here.
@@ -81,6 +82,21 @@
void dvmDexFileFree(DvmDex* pDvmDex);
+/*
+ * Change the 1- or 2-byte value at the specified address to a new value. If
+ * the location already has the new value, do nothing.
+ *
+ * This does not make any synchronization guarantees. The caller must
+ * ensure exclusivity vs. other callers.
+ *
+ * For the 2-byte call, the pointer should have 16-bit alignment.
+ *
+ * Returns "true" on success.
+ */
+bool dvmDexChangeDex1(DvmDex* pDvmDex, u1* addr, u1 newVal);
+bool dvmDexChangeDex2(DvmDex* pDvmDex, u2* addr, u2 newVal);
+
+
#if DVM_RESOLVER_CACHE == DVM_RC_DISABLED
/* 1:1 mapping */
diff --git a/vm/Globals.h b/vm/Globals.h
index 152008e..0a983c3 100644
--- a/vm/Globals.h
+++ b/vm/Globals.h
@@ -34,8 +34,9 @@
#define MAX_BREAKPOINTS 20 /* used for a debugger optimization */
-// fwd
-typedef struct GcHeap GcHeap; /* heap internal structure */
+/* private structures */
+typedef struct GcHeap GcHeap;
+typedef struct BreakpointSet BreakpointSet;
/*
* One of these for each -ea/-da/-esa/-dsa on the command line.
@@ -528,12 +529,9 @@
HashTable* dbgRegistry;
/*
- * Breakpoint optimization table. This is global and NOT explicitly
- * synchronized, but all operations that modify the table are made
- * from relatively-synchronized functions. False-positives are
- * possible, false-negatives (i.e. missing a breakpoint) should not be.
+ * Debugger breakpoint table.
*/
- const u2* debugBreakAddr[MAX_BREAKPOINTS];
+ BreakpointSet* breakpointSet;
/*
* Single-step control struct. We currently only allow one thread to
diff --git a/vm/Init.c b/vm/Init.c
index f77dba6..66c1ad1 100644
--- a/vm/Init.c
+++ b/vm/Init.c
@@ -1135,6 +1135,13 @@
if (!gDvm.reduceSignals)
blockSignals();
+ /* verify system page size */
+ if (sysconf(_SC_PAGESIZE) != SYSTEM_PAGE_SIZE) {
+ LOGE("ERROR: expected page size %d, got %d\n",
+ SYSTEM_PAGE_SIZE, (int) sysconf(_SC_PAGESIZE));
+ goto fail;
+ }
+
/* mterp setup */
LOGV("Using executionMode %d\n", gDvm.executionMode);
dvmCheckAsmConstants();
diff --git a/vm/Jni.c b/vm/Jni.c
index 1dcea10..1029cdd 100644
--- a/vm/Jni.c
+++ b/vm/Jni.c
@@ -441,6 +441,7 @@
#else
dvmClearReferenceTable(&gDvm.jniGlobalRefTable);
#endif
+ dvmClearReferenceTable(&gDvm.jniPinRefTable);
}
diff --git a/vm/LinearAlloc.c b/vm/LinearAlloc.c
index 8a18af3..a8ed3ab 100644
--- a/vm/LinearAlloc.c
+++ b/vm/LinearAlloc.c
@@ -79,11 +79,6 @@
#define LENGTHFLAG_RW 0x40000000
#define LENGTHFLAG_MASK (~(LENGTHFLAG_FREE|LENGTHFLAG_RW))
-/* in case limits.h doesn't have it; must be a power of 2 */
-#ifndef PAGESIZE
-# define PAGESIZE 4096
-#endif
-
/* fwd */
static void checkAllFree(Object* classLoader);
@@ -130,7 +125,8 @@
* chunk of data will be properly aligned.
*/
assert(BLOCK_ALIGN >= HEADER_EXTRA);
- pHdr->curOffset = pHdr->firstOffset = (BLOCK_ALIGN-HEADER_EXTRA) + PAGESIZE;
+ pHdr->curOffset = pHdr->firstOffset =
+ (BLOCK_ALIGN-HEADER_EXTRA) + SYSTEM_PAGE_SIZE;
pHdr->mapLength = DEFAULT_MAX_LENGTH;
#ifdef USE_ASHMEM
@@ -168,7 +164,7 @@
#endif /*USE_ASHMEM*/
/* region expected to begin on a page boundary */
- assert(((int) pHdr->mapAddr & (PAGESIZE-1)) == 0);
+ assert(((int) pHdr->mapAddr & (SYSTEM_PAGE_SIZE-1)) == 0);
/* the system should initialize newly-mapped memory to zero */
assert(*(u4*) (pHdr->mapAddr + pHdr->curOffset) == 0);
@@ -195,7 +191,7 @@
free(pHdr);
return NULL;
}
- if (mprotect(pHdr->mapAddr + PAGESIZE, PAGESIZE,
+ if (mprotect(pHdr->mapAddr + SYSTEM_PAGE_SIZE, SYSTEM_PAGE_SIZE,
ENFORCE_READ_ONLY ? PROT_READ : PROT_READ|PROT_WRITE) != 0)
{
LOGW("LinearAlloc init mprotect #2 failed: %s\n", strerror(errno));
@@ -205,7 +201,7 @@
if (ENFORCE_READ_ONLY) {
/* allocate the per-page ref count */
- int numPages = (pHdr->mapLength+PAGESIZE-1) / PAGESIZE;
+ int numPages = (pHdr->mapLength+SYSTEM_PAGE_SIZE-1) / SYSTEM_PAGE_SIZE;
pHdr->writeRefCount = calloc(numPages, sizeof(short));
if (pHdr->writeRefCount == NULL) {
free(pHdr);
@@ -332,7 +328,7 @@
* See if we are starting on or have crossed into a new page. If so,
* call mprotect on the page(s) we're about to write to. We have to
* page-align the start address, but don't have to make the length a
- * PAGESIZE multiple (but we do it anyway).
+ * SYSTEM_PAGE_SIZE multiple (but we do it anyway).
*
* Note that "startOffset" is not the last *allocated* byte, but rather
* the offset of the first *unallocated* byte (which we are about to
@@ -341,9 +337,9 @@
* If ENFORCE_READ_ONLY is enabled, we have to call mprotect even if
* we've written to this page before, because it might be read-only.
*/
- lastGoodOff = (startOffset-1) & ~(PAGESIZE-1);
- firstWriteOff = startOffset & ~(PAGESIZE-1);
- lastWriteOff = (nextOffset-1) & ~(PAGESIZE-1);
+ lastGoodOff = (startOffset-1) & ~(SYSTEM_PAGE_SIZE-1);
+ firstWriteOff = startOffset & ~(SYSTEM_PAGE_SIZE-1);
+ lastWriteOff = (nextOffset-1) & ~(SYSTEM_PAGE_SIZE-1);
LOGVV("--- lastGood=0x%04x firstWrite=0x%04x lastWrite=0x%04x\n",
lastGoodOff, firstWriteOff, lastWriteOff);
if (lastGoodOff != lastWriteOff || ENFORCE_READ_ONLY) {
@@ -351,7 +347,7 @@
start = firstWriteOff;
assert(start <= nextOffset);
- len = (lastWriteOff - firstWriteOff) + PAGESIZE;
+ len = (lastWriteOff - firstWriteOff) + SYSTEM_PAGE_SIZE;
LOGVV("--- calling mprotect(start=%d len=%d RW)\n", start, len);
cc = mprotect(pHdr->mapAddr + start, len, PROT_READ | PROT_WRITE);
@@ -367,8 +363,8 @@
if (ENFORCE_READ_ONLY) {
int i, start, end;
- start = firstWriteOff / PAGESIZE;
- end = lastWriteOff / PAGESIZE;
+ start = firstWriteOff / SYSTEM_PAGE_SIZE;
+ end = lastWriteOff / SYSTEM_PAGE_SIZE;
LOGVV("--- marking pages %d-%d RW (alloc %d at %p)\n",
start, end, size, pHdr->mapAddr + startOffset + HEADER_EXTRA);
@@ -465,8 +461,8 @@
u4 len = *pLen & LENGTHFLAG_MASK;
int firstPage, lastPage;
- firstPage = ((u1*)pLen - (u1*)pHdr->mapAddr) / PAGESIZE;
- lastPage = ((u1*)mem - (u1*)pHdr->mapAddr + (len-1)) / PAGESIZE;
+ firstPage = ((u1*)pLen - (u1*)pHdr->mapAddr) / SYSTEM_PAGE_SIZE;
+ lastPage = ((u1*)mem - (u1*)pHdr->mapAddr + (len-1)) / SYSTEM_PAGE_SIZE;
LOGVV("--- updating pages %d-%d (%d)\n", firstPage, lastPage, direction);
int i, cc;
@@ -496,7 +492,8 @@
pHdr->writeRefCount[i]--;
if (pHdr->writeRefCount[i] == 0) {
LOGVV("--- prot page %d RO\n", i);
- cc = mprotect(pHdr->mapAddr + PAGESIZE * i, PAGESIZE, PROT_READ);
+ cc = mprotect(pHdr->mapAddr + SYSTEM_PAGE_SIZE * i,
+ SYSTEM_PAGE_SIZE, PROT_READ);
assert(cc == 0);
}
} else {
@@ -509,8 +506,8 @@
}
if (pHdr->writeRefCount[i] == 0) {
LOGVV("--- prot page %d RW\n", i);
- cc = mprotect(pHdr->mapAddr + PAGESIZE * i, PAGESIZE,
- PROT_READ | PROT_WRITE);
+ cc = mprotect(pHdr->mapAddr + SYSTEM_PAGE_SIZE * i,
+ SYSTEM_PAGE_SIZE, PROT_READ | PROT_WRITE);
assert(cc == 0);
}
pHdr->writeRefCount[i]++;
@@ -616,7 +613,7 @@
& ~(BLOCK_ALIGN-1));
LOGI(" %p (%3d): %clen=%d%s\n", pHdr->mapAddr + off + HEADER_EXTRA,
- (int) ((off + HEADER_EXTRA) / PAGESIZE),
+ (int) ((off + HEADER_EXTRA) / SYSTEM_PAGE_SIZE),
(rawLen & LENGTHFLAG_FREE) != 0 ? '*' : ' ',
rawLen & LENGTHFLAG_MASK,
(rawLen & LENGTHFLAG_RW) != 0 ? " [RW]" : "");
@@ -627,7 +624,7 @@
if (ENFORCE_READ_ONLY) {
LOGI("writeRefCount map:\n");
- int numPages = (pHdr->mapLength+PAGESIZE-1) / PAGESIZE;
+ int numPages = (pHdr->mapLength+SYSTEM_PAGE_SIZE-1) / SYSTEM_PAGE_SIZE;
int zstart = 0;
int i;
diff --git a/vm/LinearAlloc.h b/vm/LinearAlloc.h
index a390ee3..aa33fe1 100644
--- a/vm/LinearAlloc.h
+++ b/vm/LinearAlloc.h
@@ -22,7 +22,6 @@
/*
* If this is set, we create additional data structures and make many
* additional mprotect() calls.
- * (this breaks the debugger because the debugBreakpointCount cannot be updated)
*/
#define ENFORCE_READ_ONLY false
diff --git a/vm/Profile.c b/vm/Profile.c
index 7f39f1a..393440c 100644
--- a/vm/Profile.c
+++ b/vm/Profile.c
@@ -33,9 +33,6 @@
#ifdef HAVE_ANDROID_OS
# define UPDATE_MAGIC_PAGE 1
-# ifndef PAGESIZE
-# define PAGESIZE 4096
-# endif
#endif
/*
@@ -183,7 +180,7 @@
if (fd < 0) {
LOGV("Unable to open /dev/qemu_trace\n");
} else {
- gDvm.emulatorTracePage = mmap(0, PAGESIZE, PROT_READ|PROT_WRITE,
+ gDvm.emulatorTracePage = mmap(0, SYSTEM_PAGE_SIZE, PROT_READ|PROT_WRITE,
MAP_SHARED, fd, 0);
close(fd);
if (gDvm.emulatorTracePage == MAP_FAILED) {
@@ -207,7 +204,7 @@
{
#ifdef UPDATE_MAGIC_PAGE
if (gDvm.emulatorTracePage != NULL)
- munmap(gDvm.emulatorTracePage, PAGESIZE);
+ munmap(gDvm.emulatorTracePage, SYSTEM_PAGE_SIZE);
#endif
free(gDvm.executedInstrCounts);
}
diff --git a/vm/alloc/HeapBitmap.c b/vm/alloc/HeapBitmap.c
index 2c75678..778fd87 100644
--- a/vm/alloc/HeapBitmap.c
+++ b/vm/alloc/HeapBitmap.c
@@ -23,11 +23,8 @@
#define HB_ASHMEM_NAME "dalvik-heap-bitmap"
-#ifndef PAGE_SIZE
-#define PAGE_SIZE 4096
-#endif
#define ALIGN_UP_TO_PAGE_SIZE(p) \
- (((size_t)(p) + (PAGE_SIZE - 1)) & ~(PAGE_SIZE - 1))
+ (((size_t)(p) + (SYSTEM_PAGE_SIZE - 1)) & ~(SYSTEM_PAGE_SIZE - 1))
#define LIKELY(exp) (__builtin_expect((exp) != 0, true))
#define UNLIKELY(exp) (__builtin_expect((exp) != 0, false))
diff --git a/vm/alloc/HeapSource.c b/vm/alloc/HeapSource.c
index 830e5d7..e792dca 100644
--- a/vm/alloc/HeapSource.c
+++ b/vm/alloc/HeapSource.c
@@ -32,13 +32,10 @@
static void snapIdealFootprint(void);
static void setIdealFootprint(size_t max);
-#ifndef PAGE_SIZE
-#define PAGE_SIZE 4096
-#endif
#define ALIGN_UP_TO_PAGE_SIZE(p) \
- (((size_t)(p) + (PAGE_SIZE - 1)) & ~(PAGE_SIZE - 1))
+ (((size_t)(p) + (SYSTEM_PAGE_SIZE - 1)) & ~(SYSTEM_PAGE_SIZE - 1))
#define ALIGN_DOWN_TO_PAGE_SIZE(p) \
- ((size_t)(p) & ~(PAGE_SIZE - 1))
+ ((size_t)(p) & ~(SYSTEM_PAGE_SIZE - 1))
#define HEAP_UTILIZATION_MAX 1024
#define DEFAULT_HEAP_UTILIZATION 512 // Range 1..HEAP_UTILIZATION_MAX
@@ -1286,7 +1283,7 @@
* We also align the end address.
*/
start = (void *)ALIGN_UP_TO_PAGE_SIZE(start);
- end = (void *)((size_t)end & ~(PAGE_SIZE - 1));
+ end = (void *)((size_t)end & ~(SYSTEM_PAGE_SIZE - 1));
if (start < end) {
size_t length = (char *)end - (char *)start;
madvise(start, length, MADV_DONTNEED);
diff --git a/vm/alloc/MarkSweep.c b/vm/alloc/MarkSweep.c
index 634cfda..fda4e6d 100644
--- a/vm/alloc/MarkSweep.c
+++ b/vm/alloc/MarkSweep.c
@@ -68,11 +68,8 @@
#define LOGV_SWEEP(...) LOGVV_GC("SWEEP: " __VA_ARGS__)
#define LOGV_REF(...) LOGVV_GC("REF: " __VA_ARGS__)
-#ifndef PAGE_SIZE
-#define PAGE_SIZE 4096
-#endif
#define ALIGN_UP_TO_PAGE_SIZE(p) \
- (((size_t)(p) + (PAGE_SIZE - 1)) & ~(PAGE_SIZE - 1))
+ (((size_t)(p) + (SYSTEM_PAGE_SIZE - 1)) & ~(SYSTEM_PAGE_SIZE - 1))
/* Do not cast the result of this to a boolean; the only set bit
* may be > 1<<8.
diff --git a/vm/analysis/CodeVerify.c b/vm/analysis/CodeVerify.c
index f945f23..911775c 100644
--- a/vm/analysis/CodeVerify.c
+++ b/vm/analysis/CodeVerify.c
@@ -2926,6 +2926,9 @@
* receive a "nop". The instruction's length will be left unchanged
* in "insnFlags".
*
+ * The verifier explicitly locks out breakpoint activity, so there should
+ * be no clashes with the debugger.
+ *
* IMPORTANT: this may replace meth->insns with a pointer to a new copy of
* the instructions.
*
@@ -2939,7 +2942,7 @@
u2 oldInsn = *oldInsns;
bool result = false;
- dvmMakeCodeReadWrite(meth);
+ //dvmMakeCodeReadWrite(meth);
//LOGD(" was 0x%04x\n", oldInsn);
u2* newInsns = (u2*) meth->insns + insnIdx;
@@ -3018,7 +3021,8 @@
/* nothing to do */
break;
case 3:
- newInsns[2] = OP_NOP;
+ dvmDexChangeDex2(meth->clazz->pDvmDex, newInsns+2, OP_NOP);
+ //newInsns[2] = OP_NOP;
break;
default:
/* whoops */
@@ -3028,13 +3032,15 @@
}
/* encode the opcode, with the failure code in the high byte */
- newInsns[0] = OP_THROW_VERIFICATION_ERROR |
+ u2 newVal = OP_THROW_VERIFICATION_ERROR |
(failure << 8) | (refType << (8 + kVerifyErrorRefTypeShift));
+ //newInsns[0] = newVal;
+ dvmDexChangeDex2(meth->clazz->pDvmDex, newInsns, newVal);
result = true;
bail:
- dvmMakeCodeReadOnly(meth);
+ //dvmMakeCodeReadOnly(meth);
return result;
}
@@ -5420,7 +5426,7 @@
failure = VERIFY_ERROR_GENERIC;
break;
- /* these should never appear */
+ /* these should never appear during verification */
case OP_UNUSED_3E:
case OP_UNUSED_3F:
case OP_UNUSED_40:
@@ -5439,7 +5445,7 @@
case OP_UNUSED_E9:
case OP_UNUSED_EA:
case OP_UNUSED_EB:
- case OP_UNUSED_EC:
+ case OP_BREAKPOINT:
case OP_UNUSED_EF:
case OP_UNUSED_F1:
case OP_UNUSED_FC:
diff --git a/vm/analysis/DexVerify.c b/vm/analysis/DexVerify.c
index 10251db..c490691 100644
--- a/vm/analysis/DexVerify.c
+++ b/vm/analysis/DexVerify.c
@@ -151,6 +151,67 @@
}
/*
+ * Temporarily "undo" any breakpoints found in this method. There is no risk
+ * of confusing the interpreter, because unverified code cannot be executed.
+ *
+ * Breakpoints can be set after a class is loaded but before it has been
+ * verified.
+ *
+ * The "breakpoint" opcode can replace any other opcode, leaving no
+ * indication of the original instruction's width or purpose in the
+ * instruction stream. We either have to quietly undo the breakpoints
+ * before verification, or look up the original opcode whenever we need it.
+ * The latter is more efficient since we only slow down on code that
+ * actually has breakpoints, but it requires explicit handling in every
+ * function that examines the instruction stream.
+ *
+ * We need to ensure that the debugger doesn't insert any additional
+ * breakpoints while we work. This either requires holding a lock on the
+ * breakpoint set throughout the verification of this method, or adding a
+ * "do not touch anything on these pages" list to the set. Either way,
+ * the caller of this method must ensure that it calls "redo" to release
+ * state.
+ *
+ * A debugger could connect while we work, so we return without doing
+ * anything if a debugger doesn't happen to be connected now. We can only
+ * avoid doing work if the debugger thread isn't running (dexopt, zygote,
+ * or debugging not configured).
+ *
+ * Returns "false" if we did nothing, "true" if we did stuff (and, hence,
+ * need to call "redo" at some point).
+ */
+static bool undoBreakpoints(Method* meth)
+{
+#ifdef WITH_DEBUGGER
+ if (gDvm.optimizing || gDvm.zygote || !gDvm.jdwpConfigured)
+ return false;
+ dvmUndoBreakpoints(meth);
+ return true;
+#else
+ return false;
+#endif
+}
+
+/*
+ * Restore any breakpoints we undid previously. Also has to update the
+ * stored "original opcode" value for any instruction that we replaced
+ * with a throw-verification-error op.
+ */
+static void redoBreakpoints(Method* meth)
+{
+#ifdef WITH_DEBUGGER
+ if (gDvm.optimizing || gDvm.zygote || !gDvm.jdwpConfigured) {
+ /* should not be here */
+ assert(false);
+ return;
+ }
+ dvmRedoBreakpoints(meth);
+#else
+ assert(false);
+#endif
+}
+
+/*
* Perform verification on a single method.
*
* We do this in three passes:
@@ -177,6 +238,9 @@
UninitInstanceMap* uninitMap = NULL;
InsnFlags* insnFlags = NULL;
int i, newInstanceCount;
+ bool undidBreakpoints;
+
+ undidBreakpoints = undoBreakpoints(meth);
/*
* If there aren't any instructions, make sure that's expected, then
@@ -258,6 +322,8 @@
result = true;
bail:
+ if (undidBreakpoints)
+ redoBreakpoints(meth);
dvmFreeUninitInstanceMap(uninitMap);
free(insnFlags);
return result;
diff --git a/vm/analysis/RegisterMap.c b/vm/analysis/RegisterMap.c
index d1f2600..bc314a2 100644
--- a/vm/analysis/RegisterMap.c
+++ b/vm/analysis/RegisterMap.c
@@ -3034,7 +3034,7 @@
case OP_UNUSED_E9:
case OP_UNUSED_EA:
case OP_UNUSED_EB:
- case OP_UNUSED_EC:
+ case OP_BREAKPOINT:
case OP_UNUSED_ED:
case OP_UNUSED_EF:
case OP_UNUSED_F1:
diff --git a/vm/compiler/Dataflow.c b/vm/compiler/Dataflow.c
index 8525540..fc5ecb9 100644
--- a/vm/compiler/Dataflow.c
+++ b/vm/compiler/Dataflow.c
@@ -737,7 +737,7 @@
// EB OP_UNUSED_EB
DF_NOP,
- // EC OP_UNUSED_EC
+ // EC OP_BREAKPOINT
DF_NOP,
// ED OP_THROW_VERIFICATION_ERROR
diff --git a/vm/compiler/codegen/arm/Codegen.c b/vm/compiler/codegen/arm/Codegen.c
index 5dea431..7ca905c 100644
--- a/vm/compiler/codegen/arm/Codegen.c
+++ b/vm/compiler/codegen/arm/Codegen.c
@@ -2316,7 +2316,7 @@
{
OpCode dalvikOpCode = mir->dalvikInsn.opCode;
if (((dalvikOpCode >= OP_UNUSED_3E) && (dalvikOpCode <= OP_UNUSED_43)) ||
- ((dalvikOpCode >= OP_UNUSED_E3) && (dalvikOpCode <= OP_UNUSED_EC))) {
+ ((dalvikOpCode >= OP_UNUSED_E3) && (dalvikOpCode <= OP_UNUSED_EB))) {
LOGE("Codegen: got unused opcode 0x%x\n",dalvikOpCode);
return true;
}
diff --git a/vm/interp/Interp.c b/vm/interp/Interp.c
index 658b771..10b9bb1 100644
--- a/vm/interp/Interp.c
+++ b/vm/interp/Interp.c
@@ -35,16 +35,322 @@
* ===========================================================================
*/
+// fwd
+static BreakpointSet* dvmBreakpointSetAlloc(void);
+static void dvmBreakpointSetFree(BreakpointSet* pSet);
+
/*
- * Initialize the breakpoint address lookup table when the debugger attaches.
+ * Initialize global breakpoint structures.
+ */
+bool dvmBreakpointStartup(void)
+{
+#ifdef WITH_DEBUGGER
+ gDvm.breakpointSet = dvmBreakpointSetAlloc();
+ return (gDvm.breakpointSet != NULL);
+#else
+ return true;
+#endif
+}
+
+/*
+ * Free resources.
+ */
+void dvmBreakpointShutdown(void)
+{
+#ifdef WITH_DEBUGGER
+ dvmBreakpointSetFree(gDvm.breakpointSet);
+#endif
+}
+
+
+#ifdef WITH_DEBUGGER
+/*
+ * This represents a breakpoint inserted in the instruction stream.
*
- * This shouldn't be necessary -- the global area is initially zeroed out,
- * and the events should be cleaning up after themselves.
+ * The debugger may ask us to create the same breakpoint multiple times.
+ * We only remove the breakpoint when the last instance is cleared.
+ */
+typedef struct {
+ u2* addr; /* absolute memory address */
+ u1 originalOpCode; /* original 8-bit opcode value */
+ int setCount; /* #of times this breakpoint was set */
+} Breakpoint;
+
+/*
+ * Set of breakpoints.
+ */
+struct BreakpointSet {
+ /* grab lock before reading or writing anything else in here */
+ pthread_mutex_t lock;
+
+ /* vector of breakpoint structures */
+ int alloc;
+ int count;
+ Breakpoint* breakpoints;
+};
+
+/*
+ * Initialize a BreakpointSet. Initially empty.
+ */
+static BreakpointSet* dvmBreakpointSetAlloc(void)
+{
+ BreakpointSet* pSet = (BreakpointSet*) calloc(1, sizeof(*pSet));
+
+ dvmInitMutex(&pSet->lock);
+ /* leave the rest zeroed -- will alloc on first use */
+
+ return pSet;
+}
+
+/*
+ * Free storage associated with a BreakpointSet.
+ */
+static void dvmBreakpointSetFree(BreakpointSet* pSet)
+{
+ if (pSet == NULL)
+ return;
+
+ free(pSet->breakpoints);
+ free(pSet);
+}
+
+/*
+ * Lock the breakpoint set.
+ */
+static void dvmBreakpointSetLock(BreakpointSet* pSet)
+{
+ dvmLockMutex(&pSet->lock);
+}
+
+/*
+ * Unlock the breakpoint set.
+ */
+static void dvmBreakpointSetUnlock(BreakpointSet* pSet)
+{
+ dvmUnlockMutex(&pSet->lock);
+}
+
+/*
+ * Return the #of breakpoints.
+ */
+static int dvmBreakpointSetCount(const BreakpointSet* pSet)
+{
+ return pSet->count;
+}
+
+/*
+ * See if we already have an entry for this address.
+ *
+ * The BreakpointSet's lock must be acquired before calling here.
+ *
+ * Returns the index of the breakpoint entry, or -1 if not found.
+ */
+static int dvmBreakpointSetFind(const BreakpointSet* pSet, const u2* addr)
+{
+ int i;
+
+ for (i = 0; i < pSet->count; i++) {
+ Breakpoint* pBreak = &pSet->breakpoints[i];
+ if (pBreak->addr == addr)
+ return i;
+ }
+
+ return -1;
+}
+
+/*
+ * Retrieve the opcode that was originally at the specified location.
+ *
+ * The BreakpointSet's lock must be acquired before calling here.
+ *
+ * Returns "true" with the opcode in *pOrig on success.
+ */
+static bool dvmBreakpointSetOriginalOpCode(const BreakpointSet* pSet,
+ const u2* addr, u1* pOrig)
+{
+ int idx = dvmBreakpointSetFind(pSet, addr);
+ if (idx < 0)
+ return false;
+
+ *pOrig = pSet->breakpoints[idx].originalOpCode;
+ return true;
+}
+
+/*
+ * Add a breakpoint at a specific address. If the address is already
+ * present in the table, this just increments the count.
+ *
+ * For a new entry, this will extract and preserve the current opcode from
+ * the instruction stream, and replace it with a breakpoint opcode.
+ *
+ * The BreakpointSet's lock must be acquired before calling here.
+ *
+ * Returns "true" on success.
+ */
+static bool dvmBreakpointSetAdd(BreakpointSet* pSet, Method* method,
+ unsigned int instrOffset)
+{
+ const int kBreakpointGrowth = 10;
+ const u2* addr = method->insns + instrOffset;
+ int idx = dvmBreakpointSetFind(pSet, addr);
+ Breakpoint* pBreak;
+
+ if (idx < 0) {
+ if (pSet->count == pSet->alloc) {
+ int newSize = pSet->alloc + kBreakpointGrowth;
+ Breakpoint* newVec;
+
+ LOGV("+++ increasing breakpoint set size to %d\n", newSize);
+
+ /* pSet->breakpoints will be NULL on first entry */
+ newVec = realloc(pSet->breakpoints, newSize * sizeof(Breakpoint));
+ if (newVec == NULL)
+ return false;
+
+ pSet->breakpoints = newVec;
+ pSet->alloc = newSize;
+ }
+
+ pBreak = &pSet->breakpoints[pSet->count++];
+ pBreak->addr = (u2*)addr;
+ pBreak->originalOpCode = *(u1*)addr;
+ pBreak->setCount = 1;
+
+ /*
+ * Change the opcode. We must ensure that the BreakpointSet
+ * updates happen before we change the opcode.
+ */
+ MEM_BARRIER();
+ assert(*(u1*)addr != OP_BREAKPOINT);
+ dvmDexChangeDex1(method->clazz->pDvmDex, (u1*)addr, OP_BREAKPOINT);
+ } else {
+ pBreak = &pSet->breakpoints[idx];
+ pBreak->setCount++;
+
+ /* verify instruction stream has break op */
+ assert(*(u1*)addr == OP_BREAKPOINT);
+ }
+
+ return true;
+}
+
+/*
+ * Remove one instance of the specified breakpoint. When the count
+ * reaches zero, the entry is removed from the table, and the original
+ * opcode is restored.
+ *
+ * The BreakpointSet's lock must be acquired before calling here.
+ */
+static void dvmBreakpointSetRemove(BreakpointSet* pSet, Method* method,
+ unsigned int instrOffset)
+{
+ const u2* addr = method->insns + instrOffset;
+ int idx = dvmBreakpointSetFind(pSet, addr);
+
+ if (idx < 0) {
+ /* breakpoint not found in set -- unexpected */
+ if (*(u1*)addr == OP_BREAKPOINT) {
+ LOGE("Unable to restore breakpoint opcode (%s.%s +%u)\n",
+ method->clazz->descriptor, method->name, instrOffset);
+ dvmAbort();
+ } else {
+ LOGW("Breakpoint was already restored? (%s.%s +%u)\n",
+ method->clazz->descriptor, method->name, instrOffset);
+ }
+ } else {
+ Breakpoint* pBreak = &pSet->breakpoints[idx];
+ if (pBreak->setCount == 1) {
+ /*
+ * Must restore opcode before removing set entry.
+ */
+ dvmDexChangeDex1(method->clazz->pDvmDex, (u1*)addr,
+ pBreak->originalOpCode);
+ MEM_BARRIER();
+
+ if (idx != pSet->count-1) {
+ /* shift down */
+ memmove(&pSet->breakpoints[idx], &pSet->breakpoints[idx+1],
+ (pSet->count-1 - idx) * sizeof(pSet->breakpoints[0]));
+ }
+ pSet->count--;
+ pSet->breakpoints[pSet->count].addr = (u2*) 0xdecadead; // debug
+ } else {
+ pBreak->setCount--;
+ assert(pBreak->setCount > 0);
+ }
+ }
+}
+
+/*
+ * Restore the original opcode on any breakpoints that are in the specified
+ * method. The breakpoints are NOT removed from the set.
+ *
+ * The BreakpointSet's lock must be acquired before calling here.
+ */
+static void dvmBreakpointSetUndo(BreakpointSet* pSet, Method* method)
+{
+ const u2* start = method->insns;
+ const u2* end = method->insns + dvmGetMethodInsnsSize(method);
+
+ int i;
+ for (i = 0; i < pSet->count; i++) {
+ Breakpoint* pBreak = &pSet->breakpoints[i];
+ if (pBreak->addr >= start && pBreak->addr < end) {
+ LOGV("UNDO %s.%s [%d]\n",
+ method->clazz->descriptor, method->name, i);
+ dvmDexChangeDex1(method->clazz->pDvmDex, (u1*)pBreak->addr,
+ pBreak->originalOpCode);
+ }
+ }
+}
+
+/*
+ * Put the breakpoint opcode back into the instruction stream, and check
+ * to see if the original opcode has changed.
+ *
+ * The BreakpointSet's lock must be acquired before calling here.
+ */
+static void dvmBreakpointSetRedo(BreakpointSet* pSet, Method* method)
+{
+ const u2* start = method->insns;
+ const u2* end = method->insns + dvmGetMethodInsnsSize(method);
+
+ int i;
+ for (i = 0; i < pSet->count; i++) {
+ Breakpoint* pBreak = &pSet->breakpoints[i];
+ if (pBreak->addr >= start && pBreak->addr < end) {
+ LOGV("REDO %s.%s [%d]\n",
+ method->clazz->descriptor, method->name, i);
+ u1 currentOpCode = *(u1*)pBreak->addr;
+ if (pBreak->originalOpCode != currentOpCode) {
+ /* verifier can drop in a throw-verification-error */
+ LOGD("NOTE: updating originalOpCode from 0x%02x to 0x%02x\n",
+ pBreak->originalOpCode, currentOpCode);
+ pBreak->originalOpCode = currentOpCode;
+ }
+ dvmDexChangeDex1(method->clazz->pDvmDex, (u1*)pBreak->addr,
+ OP_BREAKPOINT);
+ }
+ }
+}
+
+#endif /*WITH_DEBUGGER*/
+
+
+/*
+ * Do any debugger-attach-time initialization.
*/
void dvmInitBreakpoints(void)
{
#ifdef WITH_DEBUGGER
- memset(gDvm.debugBreakAddr, 0, sizeof(gDvm.debugBreakAddr));
+ /* quick sanity check */
+ BreakpointSet* pSet = gDvm.breakpointSet;
+ dvmBreakpointSetLock(pSet);
+ if (dvmBreakpointSetCount(pSet) != 0) {
+ LOGW("WARNING: %d leftover breakpoints\n", dvmBreakpointSetCount(pSet));
+ /* generally not good, but we can keep going */
+ }
+ dvmBreakpointSetUnlock(pSet);
#else
assert(false);
#endif
@@ -64,29 +370,13 @@
*
* "addr" is the absolute address of the breakpoint bytecode.
*/
-void dvmAddBreakAddr(Method* method, int instrOffset)
+void dvmAddBreakAddr(Method* method, unsigned int instrOffset)
{
#ifdef WITH_DEBUGGER
- const u2* addr = method->insns + instrOffset;
- const u2** ptr = gDvm.debugBreakAddr;
- int i;
-
- LOGV("BKP: add %p %s.%s (%s:%d)\n",
- addr, method->clazz->descriptor, method->name,
- dvmGetMethodSourceFile(method), dvmLineNumFromPC(method, instrOffset));
-
- method->debugBreakpointCount++;
- for (i = 0; i < MAX_BREAKPOINTS; i++, ptr++) {
- if (*ptr == NULL) {
- *ptr = addr;
- break;
- }
- }
- if (i == MAX_BREAKPOINTS) {
- /* no room; size is too small or we're not cleaning up properly */
- LOGE("ERROR: max breakpoints exceeded\n");
- assert(false);
- }
+ BreakpointSet* pSet = gDvm.breakpointSet;
+ dvmBreakpointSetLock(pSet);
+ dvmBreakpointSetAdd(pSet, method, instrOffset);
+ dvmBreakpointSetUnlock(pSet);
#else
assert(false);
#endif
@@ -102,35 +392,72 @@
* synchronized, so it should not be possible for two threads to be
* updating breakpoints at the same time.
*/
-void dvmClearBreakAddr(Method* method, int instrOffset)
+void dvmClearBreakAddr(Method* method, unsigned int instrOffset)
{
#ifdef WITH_DEBUGGER
- const u2* addr = method->insns + instrOffset;
- const u2** ptr = gDvm.debugBreakAddr;
- int i;
+ BreakpointSet* pSet = gDvm.breakpointSet;
+ dvmBreakpointSetLock(pSet);
+ dvmBreakpointSetRemove(pSet, method, instrOffset);
+ dvmBreakpointSetUnlock(pSet);
- LOGV("BKP: clear %p %s.%s (%s:%d)\n",
- addr, method->clazz->descriptor, method->name,
- dvmGetMethodSourceFile(method), dvmLineNumFromPC(method, instrOffset));
-
- method->debugBreakpointCount--;
- assert(method->debugBreakpointCount >= 0);
- for (i = 0; i < MAX_BREAKPOINTS; i++, ptr++) {
- if (*ptr == addr) {
- *ptr = NULL;
- break;
- }
- }
- if (i == MAX_BREAKPOINTS) {
- /* didn't find it */
- LOGE("ERROR: breakpoint on %p not found\n", addr);
- assert(false);
- }
#else
assert(false);
#endif
}
+#ifdef WITH_DEBUGGER
+/*
+ * Get the original opcode from under a breakpoint.
+ */
+u1 dvmGetOriginalOpCode(const u2* addr)
+{
+ BreakpointSet* pSet = gDvm.breakpointSet;
+ u1 orig = 0;
+
+ dvmBreakpointSetLock(pSet);
+ if (!dvmBreakpointSetOriginalOpCode(pSet, addr, &orig)) {
+ orig = *(u1*)addr;
+ if (orig == OP_BREAKPOINT) {
+ LOGE("GLITCH: can't find breakpoint, opcode is still set\n");
+ dvmAbort();
+ }
+ }
+ dvmBreakpointSetUnlock(pSet);
+
+ return orig;
+}
+
+/*
+ * Temporarily "undo" any breakpoints set in a specific method. Used
+ * during verification.
+ *
+ * Locks the breakpoint set, and leaves it locked.
+ */
+void dvmUndoBreakpoints(Method* method)
+{
+ BreakpointSet* pSet = gDvm.breakpointSet;
+
+ dvmBreakpointSetLock(pSet);
+ dvmBreakpointSetUndo(pSet, method);
+ /* lock remains held */
+}
+
+/*
+ * "Redo" the breakpoints cleared by a previous "undo", re-inserting the
+ * breakpoint opcodes and updating the "original opcode" values.
+ *
+ * Unlocks the breakpoint set, which must be held by a previous "undo".
+ */
+void dvmRedoBreakpoints(Method* method)
+{
+ BreakpointSet* pSet = gDvm.breakpointSet;
+
+ /* lock already held */
+ dvmBreakpointSetRedo(pSet, method);
+ dvmBreakpointSetUnlock(pSet);
+}
+#endif
+
/*
* Add a single step event. Currently this is a global item.
*
diff --git a/vm/interp/Interp.h b/vm/interp/Interp.h
index cd4c7ec..015c3db 100644
--- a/vm/interp/Interp.h
+++ b/vm/interp/Interp.h
@@ -13,6 +13,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
+
/*
* Dalvik interpreter public definitions.
*/
@@ -34,12 +35,41 @@
void dvmThrowVerificationError(const Method* method, int kind, int ref);
/*
- * Breakpoint optimization table.
+ * One-time initialization and shutdown.
+ */
+bool dvmBreakpointStartup(void);
+void dvmBreakpointShutdown(void);
+
+/*
+ * Breakpoint implementation.
*/
void dvmInitBreakpoints();
-void dvmAddBreakAddr(Method* method, int instrOffset);
-void dvmClearBreakAddr(Method* method, int instrOffset);
+void dvmAddBreakAddr(Method* method, unsigned int instrOffset);
+void dvmClearBreakAddr(Method* method, unsigned int instrOffset);
bool dvmAddSingleStep(Thread* thread, int size, int depth);
void dvmClearSingleStep(Thread* thread);
+#ifdef WITH_DEBUGGER
+/*
+ * Recover the opcode that was replaced by a breakpoint.
+ */
+u1 dvmGetOriginalOpCode(const u2* addr);
+
+/*
+ * Temporarily "undo" any breakpoints set in a specific method. Used
+ * during verification.
+ *
+ * Locks the breakpoint set, and leaves it locked.
+ */
+void dvmUndoBreakpoints(Method* method);
+
+/*
+ * "Redo" the breakpoints cleared by a previous "undo", re-inserting the
+ * breakpoint opcodes and updating the "original opcode" values.
+ *
+ * Unlocks the breakpoint set, which must be held by a previous "undo".
+ */
+void dvmRedoBreakpoints(Method* method);
+#endif
+
#endif /*_DALVIK_INTERP_INTERP*/
diff --git a/vm/mterp/armv5te/OP_UNUSED_EC.S b/vm/mterp/armv5te/OP_BREAKPOINT.S
similarity index 100%
rename from vm/mterp/armv5te/OP_UNUSED_EC.S
rename to vm/mterp/armv5te/OP_BREAKPOINT.S
diff --git a/vm/mterp/c/OP_BREAKPOINT.c b/vm/mterp/c/OP_BREAKPOINT.c
new file mode 100644
index 0000000..5a1b543
--- /dev/null
+++ b/vm/mterp/c/OP_BREAKPOINT.c
@@ -0,0 +1,29 @@
+HANDLE_OPCODE(OP_BREAKPOINT)
+#if (INTERP_TYPE == INTERP_DBG) && defined(WITH_DEBUGGER)
+ {
+ /*
+ * Restart this instruction with the original opcode. We do
+ * this by simply jumping to the handler.
+ *
+ * It's probably not necessary to update "inst", but we do it
+ * for the sake of anything that needs to do disambiguation in a
+ * common handler with INST_INST.
+ *
+ * The breakpoint itself is handled over in updateDebugger(),
+ * because we need to detect other events (method entry, single
+ * step) and report them in the same event packet, and we're not
+ * yet handling those through breakpoint instructions. By the
+ * time we get here, the breakpoint has already been handled and
+ * the thread resumed.
+ */
+ u1 originalOpCode = dvmGetOriginalOpCode(pc);
+ LOGV("+++ break 0x%02x (0x%04x -> 0x%04x)\n", originalOpCode, inst,
+ INST_REPLACE_OP(inst, originalOpCode));
+ inst = INST_REPLACE_OP(inst, originalOpCode);
+ FINISH_BKPT(originalOpCode);
+ }
+#else
+ LOGE("Breakpoint hit in non-debug interpreter\n");
+ dvmAbort();
+#endif
+OP_END
diff --git a/vm/mterp/c/OP_UNUSED_EC.c b/vm/mterp/c/OP_UNUSED_EC.c
deleted file mode 100644
index fcb8c2e..0000000
--- a/vm/mterp/c/OP_UNUSED_EC.c
+++ /dev/null
@@ -1,2 +0,0 @@
-HANDLE_OPCODE(OP_UNUSED_EC)
-OP_END
diff --git a/vm/mterp/c/header.c b/vm/mterp/c/header.c
index 174c226..341d24b 100644
--- a/vm/mterp/c/header.c
+++ b/vm/mterp/c/header.c
@@ -295,6 +295,11 @@
#define INST_INST(_inst) ((_inst) & 0xff)
/*
+ * Replace the opcode (used when handling breakpoints). _opcode is a u1.
+ */
+#define INST_REPLACE_OP(_inst, _opcode) (((_inst) & 0xff00) | _opcode)
+
+/*
* Extract the "vA, vB" 4-bit registers from the instruction word (_inst is u2).
*/
#define INST_A(_inst) (((_inst) >> 8) & 0x0f)
diff --git a/vm/mterp/out/InterpAsm-armv4t.S b/vm/mterp/out/InterpAsm-armv4t.S
index 56d0a26..a4e5c43 100644
--- a/vm/mterp/out/InterpAsm-armv4t.S
+++ b/vm/mterp/out/InterpAsm-armv4t.S
@@ -7618,8 +7618,8 @@
/* ------------------------------ */
.balign 64
-.L_OP_UNUSED_EC: /* 0xec */
-/* File: armv5te/OP_UNUSED_EC.S */
+.L_OP_BREAKPOINT: /* 0xec */
+/* File: armv5te/OP_BREAKPOINT.S */
/* File: armv5te/unused.S */
bl common_abort
diff --git a/vm/mterp/out/InterpAsm-armv5te-vfp.S b/vm/mterp/out/InterpAsm-armv5te-vfp.S
index c7926f6..3bb5409 100644
--- a/vm/mterp/out/InterpAsm-armv5te-vfp.S
+++ b/vm/mterp/out/InterpAsm-armv5te-vfp.S
@@ -7278,8 +7278,8 @@
/* ------------------------------ */
.balign 64
-.L_OP_UNUSED_EC: /* 0xec */
-/* File: armv5te/OP_UNUSED_EC.S */
+.L_OP_BREAKPOINT: /* 0xec */
+/* File: armv5te/OP_BREAKPOINT.S */
/* File: armv5te/unused.S */
bl common_abort
diff --git a/vm/mterp/out/InterpAsm-armv5te.S b/vm/mterp/out/InterpAsm-armv5te.S
index 3c5c496..52e536b 100644
--- a/vm/mterp/out/InterpAsm-armv5te.S
+++ b/vm/mterp/out/InterpAsm-armv5te.S
@@ -7618,8 +7618,8 @@
/* ------------------------------ */
.balign 64
-.L_OP_UNUSED_EC: /* 0xec */
-/* File: armv5te/OP_UNUSED_EC.S */
+.L_OP_BREAKPOINT: /* 0xec */
+/* File: armv5te/OP_BREAKPOINT.S */
/* File: armv5te/unused.S */
bl common_abort
diff --git a/vm/mterp/out/InterpAsm-armv7-a.S b/vm/mterp/out/InterpAsm-armv7-a.S
index aaf85d9..401bb96 100644
--- a/vm/mterp/out/InterpAsm-armv7-a.S
+++ b/vm/mterp/out/InterpAsm-armv7-a.S
@@ -7222,8 +7222,8 @@
/* ------------------------------ */
.balign 64
-.L_OP_UNUSED_EC: /* 0xec */
-/* File: armv5te/OP_UNUSED_EC.S */
+.L_OP_BREAKPOINT: /* 0xec */
+/* File: armv5te/OP_BREAKPOINT.S */
/* File: armv5te/unused.S */
bl common_abort
diff --git a/vm/mterp/out/InterpAsm-x86.S b/vm/mterp/out/InterpAsm-x86.S
index c25071e..4e6623c 100644
--- a/vm/mterp/out/InterpAsm-x86.S
+++ b/vm/mterp/out/InterpAsm-x86.S
@@ -5797,8 +5797,8 @@
/* ------------------------------ */
.balign 64
-.L_OP_UNUSED_EC: /* 0xec */
-/* File: x86/OP_UNUSED_EC.S */
+.L_OP_BREAKPOINT: /* 0xec */
+/* File: x86/OP_BREAKPOINT.S */
/* File: x86/unused.S */
jmp common_abort
diff --git a/vm/mterp/out/InterpC-allstubs.c b/vm/mterp/out/InterpC-allstubs.c
index 04d7c32..0ca466b 100644
--- a/vm/mterp/out/InterpC-allstubs.c
+++ b/vm/mterp/out/InterpC-allstubs.c
@@ -302,6 +302,11 @@
#define INST_INST(_inst) ((_inst) & 0xff)
/*
+ * Replace the opcode (used when handling breakpoints). _opcode is a u1.
+ */
+#define INST_REPLACE_OP(_inst, _opcode) (((_inst) & 0xff00) | _opcode)
+
+/*
* Extract the "vA, vB" 4-bit registers from the instruction word (_inst is u2).
*/
#define INST_A(_inst) (((_inst) >> 8) & 0x0f)
@@ -2783,8 +2788,35 @@
HANDLE_OPCODE(OP_UNUSED_EB)
OP_END
-/* File: c/OP_UNUSED_EC.c */
-HANDLE_OPCODE(OP_UNUSED_EC)
+/* File: c/OP_BREAKPOINT.c */
+HANDLE_OPCODE(OP_BREAKPOINT)
+#if (INTERP_TYPE == INTERP_DBG) && defined(WITH_DEBUGGER)
+ {
+ /*
+ * Restart this instruction with the original opcode. We do
+ * this by simply jumping to the handler.
+ *
+ * It's probably not necessary to update "inst", but we do it
+ * for the sake of anything that needs to do disambiguation in a
+ * common handler with INST_INST.
+ *
+ * The breakpoint itself is handled over in updateDebugger(),
+ * because we need to detect other events (method entry, single
+ * step) and report them in the same event packet, and we're not
+ * yet handling those through breakpoint instructions. By the
+ * time we get here, the breakpoint has already been handled and
+ * the thread resumed.
+ */
+ u1 originalOpCode = dvmGetOriginalOpCode(pc);
+ LOGV("+++ break 0x%02x (0x%04x -> 0x%04x)\n", originalOpCode, inst,
+ INST_REPLACE_OP(inst, originalOpCode));
+ inst = INST_REPLACE_OP(inst, originalOpCode);
+ FINISH_BKPT(originalOpCode);
+ }
+#else
+ LOGE("Breakpoint hit in non-debug interpreter\n");
+ dvmAbort();
+#endif
OP_END
/* File: c/OP_THROW_VERIFICATION_ERROR.c */
diff --git a/vm/mterp/out/InterpC-armv4t.c b/vm/mterp/out/InterpC-armv4t.c
index 6b82bc8..6541684 100644
--- a/vm/mterp/out/InterpC-armv4t.c
+++ b/vm/mterp/out/InterpC-armv4t.c
@@ -302,6 +302,11 @@
#define INST_INST(_inst) ((_inst) & 0xff)
/*
+ * Replace the opcode (used when handling breakpoints). _opcode is a u1.
+ */
+#define INST_REPLACE_OP(_inst, _opcode) (((_inst) & 0xff00) | _opcode)
+
+/*
* Extract the "vA, vB" 4-bit registers from the instruction word (_inst is u2).
*/
#define INST_A(_inst) (((_inst) >> 8) & 0x0f)
diff --git a/vm/mterp/out/InterpC-armv5te-vfp.c b/vm/mterp/out/InterpC-armv5te-vfp.c
index 7312700..0db6ce7 100644
--- a/vm/mterp/out/InterpC-armv5te-vfp.c
+++ b/vm/mterp/out/InterpC-armv5te-vfp.c
@@ -302,6 +302,11 @@
#define INST_INST(_inst) ((_inst) & 0xff)
/*
+ * Replace the opcode (used when handling breakpoints). _opcode is a u1.
+ */
+#define INST_REPLACE_OP(_inst, _opcode) (((_inst) & 0xff00) | _opcode)
+
+/*
* Extract the "vA, vB" 4-bit registers from the instruction word (_inst is u2).
*/
#define INST_A(_inst) (((_inst) >> 8) & 0x0f)
diff --git a/vm/mterp/out/InterpC-armv5te.c b/vm/mterp/out/InterpC-armv5te.c
index ea11551..b2038f6 100644
--- a/vm/mterp/out/InterpC-armv5te.c
+++ b/vm/mterp/out/InterpC-armv5te.c
@@ -302,6 +302,11 @@
#define INST_INST(_inst) ((_inst) & 0xff)
/*
+ * Replace the opcode (used when handling breakpoints). _opcode is a u1.
+ */
+#define INST_REPLACE_OP(_inst, _opcode) (((_inst) & 0xff00) | _opcode)
+
+/*
* Extract the "vA, vB" 4-bit registers from the instruction word (_inst is u2).
*/
#define INST_A(_inst) (((_inst) >> 8) & 0x0f)
diff --git a/vm/mterp/out/InterpC-armv7-a.c b/vm/mterp/out/InterpC-armv7-a.c
index 97799ec..e1872e1 100644
--- a/vm/mterp/out/InterpC-armv7-a.c
+++ b/vm/mterp/out/InterpC-armv7-a.c
@@ -302,6 +302,11 @@
#define INST_INST(_inst) ((_inst) & 0xff)
/*
+ * Replace the opcode (used when handling breakpoints). _opcode is a u1.
+ */
+#define INST_REPLACE_OP(_inst, _opcode) (((_inst) & 0xff00) | _opcode)
+
+/*
* Extract the "vA, vB" 4-bit registers from the instruction word (_inst is u2).
*/
#define INST_A(_inst) (((_inst) >> 8) & 0x0f)
diff --git a/vm/mterp/out/InterpC-portdbg.c b/vm/mterp/out/InterpC-portdbg.c
index 4d15b45..062eadc 100644
--- a/vm/mterp/out/InterpC-portdbg.c
+++ b/vm/mterp/out/InterpC-portdbg.c
@@ -302,6 +302,11 @@
#define INST_INST(_inst) ((_inst) & 0xff)
/*
+ * Replace the opcode (used when handling breakpoints). _opcode is a u1.
+ */
+#define INST_REPLACE_OP(_inst, _opcode) (((_inst) & 0xff00) | _opcode)
+
+/*
* Extract the "vA, vB" 4-bit registers from the instruction word (_inst is u2).
*/
#define INST_A(_inst) (((_inst) >> 8) & 0x0f)
@@ -448,6 +453,8 @@
*
* Assumes the existence of "const u2* pc" and (for threaded operation)
* "u2 inst".
+ *
+ * TODO: remove "switch" version.
*/
#ifdef THREADED_INTERP
# define H(_op) &&op_##_op
@@ -460,9 +467,13 @@
if (CHECK_JIT()) GOTO_bail_switch(); \
goto *handlerTable[INST_INST(inst)]; \
}
+# define FINISH_BKPT(_opcode) { \
+ goto *handlerTable[_opcode]; \
+ }
#else
# define HANDLE_OPCODE(_op) case _op:
# define FINISH(_offset) { ADJUST_PC(_offset); break; }
+# define FINISH_BKPT(opcode) { > not implemented < }
#endif
#define OP_END
@@ -1170,26 +1181,6 @@
/* code in here is only included in portable-debug interpreter */
/*
- * Determine if an address is "interesting" to the debugger. This allows
- * us to avoid scanning the entire event list before every instruction.
- *
- * The "debugBreakAddr" table is global and not synchronized.
- */
-static bool isInterestingAddr(const u2* pc)
-{
- const u2** ptr = gDvm.debugBreakAddr;
- int i;
-
- for (i = 0; i < MAX_BREAKPOINTS; i++, ptr++) {
- if (*ptr == pc) {
- LOGV("BKP: hit on %p\n", pc);
- return true;
- }
- }
- return false;
-}
-
-/*
* Update the debugger on interesting events, such as hitting a breakpoint
* or a single-step point. This is called from the top of the interpreter
* loop, before the current instruction is processed.
@@ -1235,14 +1226,10 @@
*
* Depending on the "mods" associated with event(s) on this address,
* we may or may not actually send a message to the debugger.
- *
- * Checking method->debugBreakpointCount is slower on the device than
- * just scanning the table (!). We could probably work something out
- * where we just check it on method entry/exit and remember the result,
- * but that's more fragile and requires passing more stuff around.
*/
#ifdef WITH_DEBUGGER
- if (method->debugBreakpointCount > 0 && isInterestingAddr(pc)) {
+ if (INST_INST(*pc) == OP_BREAKPOINT) {
+ LOGV("+++ breakpoint hit at %p\n", pc);
eventFlags |= DBG_BREAKPOINT;
}
#endif
@@ -3164,8 +3151,35 @@
HANDLE_OPCODE(OP_UNUSED_EB)
OP_END
-/* File: c/OP_UNUSED_EC.c */
-HANDLE_OPCODE(OP_UNUSED_EC)
+/* File: c/OP_BREAKPOINT.c */
+HANDLE_OPCODE(OP_BREAKPOINT)
+#if (INTERP_TYPE == INTERP_DBG) && defined(WITH_DEBUGGER)
+ {
+ /*
+ * Restart this instruction with the original opcode. We do
+ * this by simply jumping to the handler.
+ *
+ * It's probably not necessary to update "inst", but we do it
+ * for the sake of anything that needs to do disambiguation in a
+ * common handler with INST_INST.
+ *
+ * The breakpoint itself is handled over in updateDebugger(),
+ * because we need to detect other events (method entry, single
+ * step) and report them in the same event packet, and we're not
+ * yet handling those through breakpoint instructions. By the
+ * time we get here, the breakpoint has already been handled and
+ * the thread resumed.
+ */
+ u1 originalOpCode = dvmGetOriginalOpCode(pc);
+ LOGV("+++ break 0x%02x (0x%04x -> 0x%04x)\n", originalOpCode, inst,
+ INST_REPLACE_OP(inst, originalOpCode));
+ inst = INST_REPLACE_OP(inst, originalOpCode);
+ FINISH_BKPT(originalOpCode);
+ }
+#else
+ LOGE("Breakpoint hit in non-debug interpreter\n");
+ dvmAbort();
+#endif
OP_END
/* File: c/OP_THROW_VERIFICATION_ERROR.c */
diff --git a/vm/mterp/out/InterpC-portstd.c b/vm/mterp/out/InterpC-portstd.c
index 6ea6a56..52014f4 100644
--- a/vm/mterp/out/InterpC-portstd.c
+++ b/vm/mterp/out/InterpC-portstd.c
@@ -302,6 +302,11 @@
#define INST_INST(_inst) ((_inst) & 0xff)
/*
+ * Replace the opcode (used when handling breakpoints). _opcode is a u1.
+ */
+#define INST_REPLACE_OP(_inst, _opcode) (((_inst) & 0xff00) | _opcode)
+
+/*
* Extract the "vA, vB" 4-bit registers from the instruction word (_inst is u2).
*/
#define INST_A(_inst) (((_inst) >> 8) & 0x0f)
@@ -443,6 +448,8 @@
*
* Assumes the existence of "const u2* pc" and (for threaded operation)
* "u2 inst".
+ *
+ * TODO: remove "switch" version.
*/
#ifdef THREADED_INTERP
# define H(_op) &&op_##_op
@@ -455,9 +462,13 @@
if (CHECK_JIT()) GOTO_bail_switch(); \
goto *handlerTable[INST_INST(inst)]; \
}
+# define FINISH_BKPT(_opcode) { \
+ goto *handlerTable[_opcode]; \
+ }
#else
# define HANDLE_OPCODE(_op) case _op:
# define FINISH(_offset) { ADJUST_PC(_offset); break; }
+# define FINISH_BKPT(opcode) { > not implemented < }
#endif
#define OP_END
@@ -2880,8 +2891,35 @@
HANDLE_OPCODE(OP_UNUSED_EB)
OP_END
-/* File: c/OP_UNUSED_EC.c */
-HANDLE_OPCODE(OP_UNUSED_EC)
+/* File: c/OP_BREAKPOINT.c */
+HANDLE_OPCODE(OP_BREAKPOINT)
+#if (INTERP_TYPE == INTERP_DBG) && defined(WITH_DEBUGGER)
+ {
+ /*
+ * Restart this instruction with the original opcode. We do
+ * this by simply jumping to the handler.
+ *
+ * It's probably not necessary to update "inst", but we do it
+ * for the sake of anything that needs to do disambiguation in a
+ * common handler with INST_INST.
+ *
+ * The breakpoint itself is handled over in updateDebugger(),
+ * because we need to detect other events (method entry, single
+ * step) and report them in the same event packet, and we're not
+ * yet handling those through breakpoint instructions. By the
+ * time we get here, the breakpoint has already been handled and
+ * the thread resumed.
+ */
+ u1 originalOpCode = dvmGetOriginalOpCode(pc);
+ LOGV("+++ break 0x%02x (0x%04x -> 0x%04x)\n", originalOpCode, inst,
+ INST_REPLACE_OP(inst, originalOpCode));
+ inst = INST_REPLACE_OP(inst, originalOpCode);
+ FINISH_BKPT(originalOpCode);
+ }
+#else
+ LOGE("Breakpoint hit in non-debug interpreter\n");
+ dvmAbort();
+#endif
OP_END
/* File: c/OP_THROW_VERIFICATION_ERROR.c */
diff --git a/vm/mterp/out/InterpC-x86.c b/vm/mterp/out/InterpC-x86.c
index ed42355..30a4e1c 100644
--- a/vm/mterp/out/InterpC-x86.c
+++ b/vm/mterp/out/InterpC-x86.c
@@ -302,6 +302,11 @@
#define INST_INST(_inst) ((_inst) & 0xff)
/*
+ * Replace the opcode (used when handling breakpoints). _opcode is a u1.
+ */
+#define INST_REPLACE_OP(_inst, _opcode) (((_inst) & 0xff00) | _opcode)
+
+/*
* Extract the "vA, vB" 4-bit registers from the instruction word (_inst is u2).
*/
#define INST_A(_inst) (((_inst) >> 8) & 0x0f)
diff --git a/vm/mterp/portable/debug.c b/vm/mterp/portable/debug.c
index 449d49b..6716aba 100644
--- a/vm/mterp/portable/debug.c
+++ b/vm/mterp/portable/debug.c
@@ -1,26 +1,6 @@
/* code in here is only included in portable-debug interpreter */
/*
- * Determine if an address is "interesting" to the debugger. This allows
- * us to avoid scanning the entire event list before every instruction.
- *
- * The "debugBreakAddr" table is global and not synchronized.
- */
-static bool isInterestingAddr(const u2* pc)
-{
- const u2** ptr = gDvm.debugBreakAddr;
- int i;
-
- for (i = 0; i < MAX_BREAKPOINTS; i++, ptr++) {
- if (*ptr == pc) {
- LOGV("BKP: hit on %p\n", pc);
- return true;
- }
- }
- return false;
-}
-
-/*
* Update the debugger on interesting events, such as hitting a breakpoint
* or a single-step point. This is called from the top of the interpreter
* loop, before the current instruction is processed.
@@ -66,14 +46,10 @@
*
* Depending on the "mods" associated with event(s) on this address,
* we may or may not actually send a message to the debugger.
- *
- * Checking method->debugBreakpointCount is slower on the device than
- * just scanning the table (!). We could probably work something out
- * where we just check it on method entry/exit and remember the result,
- * but that's more fragile and requires passing more stuff around.
*/
#ifdef WITH_DEBUGGER
- if (method->debugBreakpointCount > 0 && isInterestingAddr(pc)) {
+ if (INST_INST(*pc) == OP_BREAKPOINT) {
+ LOGV("+++ breakpoint hit at %p\n", pc);
eventFlags |= DBG_BREAKPOINT;
}
#endif
diff --git a/vm/mterp/portable/stubdefs.c b/vm/mterp/portable/stubdefs.c
index 717e746..29258fc 100644
--- a/vm/mterp/portable/stubdefs.c
+++ b/vm/mterp/portable/stubdefs.c
@@ -19,6 +19,8 @@
*
* Assumes the existence of "const u2* pc" and (for threaded operation)
* "u2 inst".
+ *
+ * TODO: remove "switch" version.
*/
#ifdef THREADED_INTERP
# define H(_op) &&op_##_op
@@ -31,9 +33,13 @@
if (CHECK_JIT()) GOTO_bail_switch(); \
goto *handlerTable[INST_INST(inst)]; \
}
+# define FINISH_BKPT(_opcode) { \
+ goto *handlerTable[_opcode]; \
+ }
#else
# define HANDLE_OPCODE(_op) case _op:
# define FINISH(_offset) { ADJUST_PC(_offset); break; }
+# define FINISH_BKPT(opcode) { > not implemented < }
#endif
#define OP_END
diff --git a/vm/mterp/x86/OP_UNUSED_EC.S b/vm/mterp/x86/OP_BREAKPOINT.S
similarity index 100%
rename from vm/mterp/x86/OP_UNUSED_EC.S
rename to vm/mterp/x86/OP_BREAKPOINT.S
diff --git a/vm/oo/Class.c b/vm/oo/Class.c
index cc461e7..9565eb0 100644
--- a/vm/oo/Class.c
+++ b/vm/oo/Class.c
@@ -2117,6 +2117,7 @@
}
}
+#if 0 /* replaced with private/read-write mapping */
/*
* We usually map bytecode directly out of the DEX file, which is mapped
* shared read-only. If we want to be able to modify it, we have to make
@@ -2164,6 +2165,7 @@
LOGV("+++ marking %p read-only\n", methodDexCode);
dvmLinearReadOnly(meth->clazz->classLoader, methodDexCode);
}
+#endif
/*
diff --git a/vm/oo/Object.h b/vm/oo/Object.h
index 788ec13..298c230 100644
--- a/vm/oo/Object.h
+++ b/vm/oo/Object.h
@@ -553,9 +553,6 @@
#ifdef WITH_PROFILER
bool inProfile;
#endif
-#ifdef WITH_DEBUGGER
- short debugBreakpointCount;
-#endif
};
/*
