Handle unresolved field type in compiler.
Make behavior consistent with interpreter, by only resolving field types
when the stored value is not null.
Note that this differs from RI behavior which throws a
NoClassDefFoundError when loading the BadField class.
Bug: 79751666
Test: 173-missing-field-type
Change-Id: I1e584f3129fd651bee1c9635c90bc30e13190a90
diff --git a/compiler/optimizing/instruction_builder.cc b/compiler/optimizing/instruction_builder.cc
index df236c1..cd68b2a 100644
--- a/compiler/optimizing/instruction_builder.cc
+++ b/compiler/optimizing/instruction_builder.cc
@@ -34,6 +34,7 @@
#include "oat_file.h"
#include "optimizing_compiler_stats.h"
#include "quicken_info.h"
+#include "reflective_handle_scope-inl.h"
#include "scoped_thread_state_change-inl.h"
#include "sharpening.h"
#include "ssa_builder.h"
@@ -1934,7 +1935,14 @@
return nullptr;
}
- return resolved_field;
+ StackArtFieldHandleScope<1> rhs(soa.Self());
+ ReflectiveHandle<ArtField> resolved_field_handle(rhs.NewHandle(resolved_field));
+ if (resolved_field->ResolveType().IsNull()) {
+ // ArtField::ResolveType() may fail as evidenced with a dexing bug (b/78788577).
+ soa.Self()->ClearException();
+ return nullptr; // Failure
+ }
+ return resolved_field_handle.Get();
}
void HInstructionBuilder::BuildStaticFieldAccess(const Instruction& instruction,
diff --git a/runtime/entrypoints/quick/quick_field_entrypoints.cc b/runtime/entrypoints/quick/quick_field_entrypoints.cc
index 56232c5..bfe015f 100644
--- a/runtime/entrypoints/quick/quick_field_entrypoints.cc
+++ b/runtime/entrypoints/quick/quick_field_entrypoints.cc
@@ -118,25 +118,32 @@
ArtField* field = FindFieldFast( \
field_idx, referrer, Static ## PrimitiveOrObject ## Write, \
sizeof(PrimitiveType)); \
- if (LIKELY(field != nullptr)) { \
- field->Set ## Kind <false>(field->GetDeclaringClass(), new_value); \
- return 0; \
+ if (UNLIKELY(field == nullptr)) { \
+ if (IsObject) { \
+ StackHandleScope<1> hs(self); \
+ HandleWrapper<mirror::Object> h_obj(hs.NewHandleWrapper( \
+ reinterpret_cast<mirror::Object**>(&new_value))); \
+ field = FindFieldFromCode<Static ## PrimitiveOrObject ## Write, true>( \
+ field_idx, referrer, self, sizeof(PrimitiveType)); \
+ } else { \
+ field = FindFieldFromCode<Static ## PrimitiveOrObject ## Write, true>( \
+ field_idx, referrer, self, sizeof(PrimitiveType)); \
+ } \
+ if (UNLIKELY(field == nullptr)) { \
+ return -1; \
+ } \
} \
- if (IsObject) { \
- StackHandleScope<1> hs(self); \
- HandleWrapper<mirror::Object> h_obj(hs.NewHandleWrapper( \
- reinterpret_cast<mirror::Object**>(&new_value))); \
- field = FindFieldFromCode<Static ## PrimitiveOrObject ## Write, true>( \
- field_idx, referrer, self, sizeof(PrimitiveType)); \
- } else { \
- field = FindFieldFromCode<Static ## PrimitiveOrObject ## Write, true>( \
- field_idx, referrer, self, sizeof(PrimitiveType)); \
+ if (!referrer->SkipAccessChecks() && IsObject && new_value != 0) { \
+ StackArtFieldHandleScope<1> rhs(self); \
+ ReflectiveHandle<ArtField> field_handle(rhs.NewHandle(field)); \
+ if (field->ResolveType().IsNull()) { \
+ self->AssertPendingException(); \
+ return -1; \
+ } \
+ field = field_handle.Get(); \
} \
- if (LIKELY(field != nullptr)) { \
- field->Set ## Kind <false>(field->GetDeclaringClass(), new_value); \
- return 0; \
- } \
- return -1; \
+ field->Set ## Kind <false>(field->GetDeclaringClass(), new_value); \
+ return 0; \
} \
\
extern "C" int artSet ## Kind ## InstanceFromCode(uint32_t field_idx, \
@@ -149,33 +156,42 @@
ArtField* field = FindFieldFast( \
field_idx, referrer, Instance ## PrimitiveOrObject ## Write, \
sizeof(PrimitiveType)); \
- if (LIKELY(field != nullptr && obj != nullptr)) { \
- field->Set ## Kind <false>(obj, new_value); \
- return 0; \
+ if (UNLIKELY(field == nullptr || obj == nullptr)) { \
+ if (IsObject) { \
+ StackHandleScope<1> hs(self); \
+ HandleWrapper<mirror::Object> h_obj(hs.NewHandleWrapper( \
+ reinterpret_cast<mirror::Object**>(&new_value))); \
+ field = \
+ FindInstanceField<Instance ## PrimitiveOrObject ## Write, true>( \
+ field_idx, \
+ referrer, \
+ self, \
+ sizeof(PrimitiveType), \
+ &obj); \
+ } else { \
+ field = \
+ FindInstanceField<Instance ## PrimitiveOrObject ## Write, true>( \
+ field_idx, \
+ referrer, \
+ self, \
+ sizeof(PrimitiveType), \
+ &obj); \
+ } \
+ if (UNLIKELY(field == nullptr)) { \
+ return -1; \
+ } \
} \
- if (IsObject) { \
- StackHandleScope<1> hs(self); \
- HandleWrapper<mirror::Object> h_obj(hs.NewHandleWrapper( \
- reinterpret_cast<mirror::Object**>(&new_value))); \
- field = FindInstanceField<Instance ## PrimitiveOrObject ## Write, true>( \
- field_idx, \
- referrer, \
- self, \
- sizeof(PrimitiveType), \
- &obj); \
- } else { \
- field = FindInstanceField<Instance ## PrimitiveOrObject ## Write, true>( \
- field_idx, \
- referrer, \
- self, \
- sizeof(PrimitiveType), \
- &obj); \
+ if (!referrer->SkipAccessChecks() && IsObject && new_value != 0) { \
+ StackArtFieldHandleScope<1> rhs(self); \
+ ReflectiveHandle<ArtField> field_handle(rhs.NewHandle(field)); \
+ if (field->ResolveType().IsNull()) { \
+ self->AssertPendingException(); \
+ return -1; \
+ } \
+ field = field_handle.Get(); \
} \
- if (LIKELY(field != nullptr)) { \
- field->Set ## Kind<false>(obj, new_value); \
- return 0; \
- } \
- return -1; \
+ field->Set ## Kind<false>(obj, new_value); \
+ return 0; \
} \
\
extern "C" RetType artGet ## Kind ## StaticFromCompiledCode( \
diff --git a/test/173-missing-field-type/expected.txt b/test/173-missing-field-type/expected.txt
index b0aad4d..e69de29 100644
--- a/test/173-missing-field-type/expected.txt
+++ b/test/173-missing-field-type/expected.txt
@@ -1 +0,0 @@
-passed
diff --git a/test/173-missing-field-type/smali/BadField.smali b/test/173-missing-field-type/smali/BadField.smali
index 851e8fe..7593983 100644
--- a/test/173-missing-field-type/smali/BadField.smali
+++ b/test/173-missing-field-type/smali/BadField.smali
@@ -16,8 +16,9 @@
.class public LBadField;
.super Ljava/lang/Object;
-# This is a bad field since there is no class Widget in this test.
+# These are bad fields since there is no class Widget in this test.
.field public static widget:LWidget;
+.field public iwidget:LWidget;
.method public constructor <init>()V
.registers 2
@@ -25,10 +26,36 @@
return-void
.end method
-.method public static constructor <clinit>()V
+.method public static storeStaticObject()V
.registers 1
new-instance v0, Ljava/lang/Object;
invoke-direct {v0}, Ljava/lang/Object;-><init>()V
sput-object v0, LBadField;->widget:LWidget;
return-void
-.end method
\ No newline at end of file
+.end method
+
+.method public static storeStaticNull()V
+ .registers 1
+ const/4 v0, 0
+ sput-object v0, LBadField;->widget:LWidget;
+ return-void
+.end method
+
+.method public static storeInstanceObject()V
+ .registers 2
+ new-instance v1, LBadField;
+ invoke-direct {v1}, LBadField;-><init>()V
+ new-instance v0, Ljava/lang/Object;
+ invoke-direct {v0}, Ljava/lang/Object;-><init>()V
+ iput-object v0, v1, LBadField;->iwidget:LWidget;
+ return-void
+.end method
+
+.method public static storeInstanceNull()V
+ .registers 2
+ new-instance v1, LBadField;
+ invoke-direct {v1}, LBadField;-><init>()V
+ const/4 v0, 0
+ iput-object v0, v1, LBadField;->iwidget:LWidget;
+ return-void
+.end method
diff --git a/test/173-missing-field-type/src-art/Main.java b/test/173-missing-field-type/src-art/Main.java
index 61bb918..625cf25 100644
--- a/test/173-missing-field-type/src-art/Main.java
+++ b/test/173-missing-field-type/src-art/Main.java
@@ -14,21 +14,36 @@
* limitations under the License.
*/
+import java.lang.reflect.InvocationTargetException;
public class Main {
public static void main(String[] args) throws Throwable {
+ // Class BadField is defined in BadField.smali.
+ Class<?> c = Class.forName("BadField");
+
+ // Storing null is OK.
+ c.getMethod("storeStaticNull").invoke(null);
+ c.getMethod("storeInstanceNull").invoke(null);
+
+ testStoreObject(c, "storeStaticObject");
+ testStoreObject(c, "storeInstanceObject");
+ }
+
+ public static void testStoreObject(Class<?> c, String methodName) throws Throwable{
try {
- // Class BadField is defined in BadField.smali.
- Class<?> c = Class.forName("BadField");
- System.out.println("Not reached");
- c.newInstance();
- } catch (NoClassDefFoundError expected) {
+ // Storing anything else should throw an exception.
+ c.getMethod(methodName).invoke(null);
+ throw new Error("Expected NoClassDefFoundError");
+ } catch (InvocationTargetException expected) {
+ Throwable e = expected.getCause();
+ if (e instanceof NoClassDefFoundError) {
// The NoClassDefFoundError is for the field widget in class BadField.
- if (expected.getMessage().equals("Failed resolution of: LWidget;")) {
- System.out.println("passed");
- } else {
- System.out.println("failed: " + expected.getMessage());
+ if (!e.getMessage().equals("Failed resolution of: LWidget;")) {
+ throw new Error("Unexpected " + e);
}
+ } else {
+ throw new Error("Unexpected " + e);
+ }
}
}
}
diff --git a/test/173-missing-field-type/src/Main.java b/test/173-missing-field-type/src/Main.java
index 172d6a6..b821c96 100644
--- a/test/173-missing-field-type/src/Main.java
+++ b/test/173-missing-field-type/src/Main.java
@@ -17,6 +17,5 @@
public class Main {
// Allow test to pass on RI without adding to knownfailures.json file.
public static void main(String args[]) throws Exception {
- System.out.println("passed");
}
}