Don't cache a field if storing to it can throw.
This aligns nterp behavior with swith interpreter and compiler.
Test: 173-missing-field-type
Change-Id: Iec9714ce5528e3de04df3270862429bd681e83a0
diff --git a/runtime/interpreter/mterp/nterp.cc b/runtime/interpreter/mterp/nterp.cc
index 93b33ba..5922efd 100644
--- a/runtime/interpreter/mterp/nterp.cc
+++ b/runtime/interpreter/mterp/nterp.cc
@@ -437,13 +437,14 @@
const Instruction* inst = Instruction::At(dex_pc_ptr);
uint16_t field_index = inst->VRegB_21c();
ClassLinker* const class_linker = Runtime::Current()->GetClassLinker();
+ bool is_put = IsInstructionSPut(inst->Opcode());
ArtField* resolved_field = ResolveFieldWithAccessChecks(
self,
class_linker,
field_index,
caller,
/* is_static */ true,
- /* is_put */ IsInstructionSPut(inst->Opcode()),
+ is_put,
resolve_field_type);
if (resolved_field == nullptr) {
@@ -466,7 +467,16 @@
// check for it.
return reinterpret_cast<size_t>(resolved_field) | 1;
} else {
- UpdateCache(self, dex_pc_ptr, resolved_field);
+ // Try to resolve the field type even if we were not requested to. Only if
+ // the field type is successfully resolved can we update the cache. If we
+ // fail to resolve the type, we clear the exception to keep interpreter
+ // semantics of not throwing when null is stored.
+ if (is_put && resolve_field_type == 0 && resolved_field->ResolveType() == nullptr) {
+ DCHECK(self->IsExceptionPending());
+ self->ClearException();
+ } else {
+ UpdateCache(self, dex_pc_ptr, resolved_field);
+ }
return reinterpret_cast<size_t>(resolved_field);
}
}
@@ -480,13 +490,14 @@
const Instruction* inst = Instruction::At(dex_pc_ptr);
uint16_t field_index = inst->VRegC_22c();
ClassLinker* const class_linker = Runtime::Current()->GetClassLinker();
+ bool is_put = IsInstructionIPut(inst->Opcode());
ArtField* resolved_field = ResolveFieldWithAccessChecks(
self,
class_linker,
field_index,
caller,
/* is_static */ false,
- /* is_put */ IsInstructionIPut(inst->Opcode()),
+ is_put,
resolve_field_type);
if (resolved_field == nullptr) {
DCHECK(self->IsExceptionPending());
@@ -497,7 +508,16 @@
// of volatile.
return -resolved_field->GetOffset().Uint32Value();
}
- UpdateCache(self, dex_pc_ptr, resolved_field->GetOffset().Uint32Value());
+ // Try to resolve the field type even if we were not requested to. Only if
+ // the field type is successfully resolved can we update the cache. If we
+ // fail to resolve the type, we clear the exception to keep interpreter
+ // semantics of not throwing when null is stored.
+ if (is_put && resolve_field_type == 0 && resolved_field->ResolveType() == nullptr) {
+ DCHECK(self->IsExceptionPending());
+ self->ClearException();
+ } else {
+ UpdateCache(self, dex_pc_ptr, resolved_field->GetOffset().Uint32Value());
+ }
return resolved_field->GetOffset().Uint32Value();
}
diff --git a/test/173-missing-field-type/smali/BadField.smali b/test/173-missing-field-type/smali/BadField.smali
index 7593983..1734034 100644
--- a/test/173-missing-field-type/smali/BadField.smali
+++ b/test/173-missing-field-type/smali/BadField.smali
@@ -41,6 +41,18 @@
return-void
.end method
+.method public static storeStatic(Ljava/lang/Object;)V
+ .registers 1
+ sput-object p0, LBadField;->widget:LWidget;
+ return-void
+.end method
+
+.method public static storeInstance(LBadField;Ljava/lang/Object;)V
+ .registers 2
+ iput-object p1, p0, LBadField;->iwidget:LWidget;
+ return-void
+.end method
+
.method public static storeInstanceObject()V
.registers 2
new-instance v1, LBadField;
diff --git a/test/173-missing-field-type/src-art/Main.java b/test/173-missing-field-type/src-art/Main.java
index 35dbbea..8c5a741 100644
--- a/test/173-missing-field-type/src-art/Main.java
+++ b/test/173-missing-field-type/src-art/Main.java
@@ -15,6 +15,7 @@
*/
import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
public class Main {
public static void main(String[] args) throws Throwable {
@@ -24,10 +25,16 @@
// Storing null is OK.
c.getMethod("storeStaticNull").invoke(null);
c.getMethod("storeInstanceNull").invoke(null);
+ c.getMethod("storeStatic", Object.class).invoke(null, new Object[]{ null });
+ c.getMethod("storeInstance", c, Object.class).invoke(
+ null, new Object[]{ c.newInstance(), null });
// Storing anything else should throw an exception.
- testStoreObject(c, "storeStaticObject");
- testStoreObject(c, "storeInstanceObject");
+ testStoreObject(c.getMethod("storeStaticObject"));
+ testStoreObject(c.getMethod("storeInstanceObject"));
+ testStoreObject(c.getMethod("storeStatic", Object.class), new Object());
+ testStoreObject(
+ c.getMethod("storeInstance", c, Object.class), c.newInstance(), new Object());
// Loading is OK.
c = Class.forName("BadFieldGet");
@@ -38,9 +45,9 @@
c.getMethod(methodName).invoke(null);
}
- public static void testStoreObject(Class<?> c, String methodName) throws Throwable {
+ public static void testStoreObject(Method method, Object... arguments) throws Throwable {
try {
- c.getMethod(methodName).invoke(null);
+ method.invoke(null, arguments);
throw new Error("Expected NoClassDefFoundError");
} catch (InvocationTargetException expected) {
Throwable e = expected.getCause();