Make wide-volatile loads and stores atomic.
This implements the four wide-volatile instructions added in a previous
change, and modifies the verifier to substitute the opcodes into the
instruction stream when appropriate.
For mterp, the ARM wide get/put instructions now have conditional code
that replaces ldrd/strd with a call to the quasiatomic functions. The
C version does essentially the same thing. ARMv4T lacks ldrd/stdrd, and
uses separate implementations for the wide field accesses, so those were
updated as well. x86 will just use stubs.
The JIT should punt these to the interpreter.
Change-Id: Ife88559ed1a698c3267d43c454896f6b12081c0f
Also:
- We don't seem to be using the negative widths in the instruction
table. Not sure they're useful anymore.
- Tabs -> spaces in x86-atom throw-verification-error impl.
diff --git a/vm/analysis/CodeVerify.c b/vm/analysis/CodeVerify.c
index 7ef559b..8187354 100644
--- a/vm/analysis/CodeVerify.c
+++ b/vm/analysis/CodeVerify.c
@@ -2921,6 +2921,10 @@
* Replace an instruction with "throw-verification-error". This allows us to
* defer error reporting until the code path is first used.
*
+ * This is expected to be called during "just in time" verification, not
+ * from within dexopt. (Verification failures in dexopt will result in
+ * postponement of verification to first use of the class.)
+ *
* The throw-verification-error instruction requires two code units. Some
* of the replaced instructions require three; the third code unit will
* receive a "nop". The instruction's length will be left unchanged
@@ -2942,8 +2946,6 @@
u2 oldInsn = *oldInsns;
bool result = false;
- //dvmMakeCodeReadWrite(meth);
-
//LOGD(" was 0x%04x\n", oldInsn);
u2* newInsns = (u2*) meth->insns + insnIdx;
@@ -3040,10 +3042,53 @@
result = true;
bail:
- //dvmMakeCodeReadOnly(meth);
return result;
}
+/*
+ * Replace {iget,iput,sget,sput}-wide with the -wide-volatile form.
+ *
+ * If this is called during dexopt, we can modify the instruction in
+ * place. If this happens during just-in-time verification, we need to
+ * use the DEX read/write page feature.
+ *
+ * NOTE:
+ * This shouldn't really be tied to verification. It ought to be a
+ * separate pass that is run before or after the verifier. However, that
+ * requires a bunch of extra code, and the only advantage of doing so is
+ * that the feature isn't disabled when verification is turned off. At
+ * some point we may need to revisit this choice.
+ */
+static void replaceVolatileInstruction(Method* meth, InsnFlags* insnFlags,
+ int insnIdx)
+{
+ u2* oldInsns = (u2*)meth->insns + insnIdx;
+ u2 oldInsn = *oldInsns;
+ u2 newVal;
+
+ switch (oldInsn & 0xff) {
+ case OP_IGET_WIDE: newVal = OP_IGET_WIDE_VOLATILE; break;
+ case OP_IPUT_WIDE: newVal = OP_IPUT_WIDE_VOLATILE; break;
+ case OP_SGET_WIDE: newVal = OP_SGET_WIDE_VOLATILE; break;
+ case OP_SPUT_WIDE: newVal = OP_SPUT_WIDE_VOLATILE; break;
+ default:
+ LOGE("wide-volatile op mismatch (0x%x)\n", oldInsn);
+ dvmAbort();
+ return; // in-lieu-of noreturn attribute
+ }
+
+ /* merge new opcode into 16-bit code unit */
+ newVal |= (oldInsn & 0xff00);
+
+ if (gDvm.optimizing) {
+ /* dexopt time, alter the output */
+ *oldInsns = newVal;
+ } else {
+ /* runtime, make the page read/write */
+ dvmDexChangeDex2(meth->clazz->pDvmDex, oldInsns, newVal);
+ }
+}
+
/*
* ===========================================================================
@@ -4472,6 +4517,7 @@
}
break;
case OP_IGET_WIDE:
+ case OP_IGET_WIDE_VOLATILE:
{
RegType dstType;
ClassObject* fieldClass;
@@ -4506,6 +4552,13 @@
setRegisterType(workRegs, insnRegCount, decInsn.vA,
dstType, &failure);
}
+ if (VERIFY_OK(failure)) {
+ if (decInsn.opCode != OP_IGET_WIDE_VOLATILE &&
+ dvmIsVolatileField(&instField->field))
+ {
+ replaceVolatileInstruction(meth, insnFlags, insnIdx);
+ }
+ }
}
break;
case OP_IGET_OBJECT:
@@ -4602,6 +4655,7 @@
}
break;
case OP_IPUT_WIDE:
+ case OP_IPUT_WIDE_VOLATILE:
tmpType = getRegisterType(workRegs, insnRegCount, decInsn.vA, &failure);
if (VERIFY_OK(failure)) {
RegType typeHi =
@@ -4639,6 +4693,13 @@
failure = VERIFY_ERROR_GENERIC;
break;
}
+ if (VERIFY_OK(failure)) {
+ if (decInsn.opCode != OP_IPUT_WIDE_VOLATILE &&
+ dvmIsVolatileField(&instField->field))
+ {
+ replaceVolatileInstruction(meth, insnFlags, insnIdx);
+ }
+ }
}
break;
case OP_IPUT_OBJECT:
@@ -4748,6 +4809,7 @@
}
break;
case OP_SGET_WIDE:
+ case OP_SGET_WIDE_VOLATILE:
{
StaticField* staticField;
RegType dstType;
@@ -4775,6 +4837,13 @@
setRegisterType(workRegs, insnRegCount, decInsn.vA,
dstType, &failure);
}
+ if (VERIFY_OK(failure)) {
+ if (decInsn.opCode != OP_SGET_WIDE_VOLATILE &&
+ dvmIsVolatileField(&staticField->field))
+ {
+ replaceVolatileInstruction(meth, insnFlags, insnIdx);
+ }
+ }
}
break;
case OP_SGET_OBJECT:
@@ -4864,6 +4933,7 @@
}
break;
case OP_SPUT_WIDE:
+ case OP_SPUT_WIDE_VOLATILE:
tmpType = getRegisterType(workRegs, insnRegCount, decInsn.vA, &failure);
if (VERIFY_OK(failure)) {
RegType typeHi =
@@ -4894,6 +4964,13 @@
failure = VERIFY_ERROR_GENERIC;
break;
}
+ if (VERIFY_OK(failure)) {
+ if (decInsn.opCode != OP_SPUT_WIDE_VOLATILE &&
+ dvmIsVolatileField(&staticField->field))
+ {
+ replaceVolatileInstruction(meth, insnFlags, insnIdx);
+ }
+ }
}
break;
case OP_SPUT_OBJECT:
@@ -5420,10 +5497,6 @@
case OP_IPUT_QUICK:
case OP_IPUT_WIDE_QUICK:
case OP_IPUT_OBJECT_QUICK:
- case OP_IGET_WIDE_VOLATILE:
- case OP_IPUT_WIDE_VOLATILE:
- case OP_SGET_WIDE_VOLATILE:
- case OP_SPUT_WIDE_VOLATILE:
case OP_INVOKE_VIRTUAL_QUICK:
case OP_INVOKE_VIRTUAL_QUICK_RANGE:
case OP_INVOKE_SUPER_QUICK:
diff --git a/vm/analysis/DexOptimize.c b/vm/analysis/DexOptimize.c
index a5b8b6f..ae1edfc 100644
--- a/vm/analysis/DexOptimize.c
+++ b/vm/analysis/DexOptimize.c
@@ -1667,7 +1667,7 @@
u4 len = insns[2] | (((u4)insns[3]) << 16);
width = 4 + (elemWidth * len + 1) / 2;
} else {
- width = dexGetInstrWidth(gDvm.instrWidth, inst);
+ width = dexGetInstrWidthAbs(gDvm.instrWidth, inst);
}
assert(width > 0);