Do not place null check from unresolved field access.
Rationale:
These accesses go though the runtime anyway where various
checks are done, including null check. Since particular
checks, like access checks, need to occur prior to the
null check (to ensure link errors are not masked by
a null reference), the explicit null check should not
occur in the HIR.
BUG=29068831
Change-Id: I30fc9cb8cf4993e4176e235ceba3a38aef98d503
diff --git a/compiler/optimizing/instruction_builder.cc b/compiler/optimizing/instruction_builder.cc
index aaddc01..5e691c7 100644
--- a/compiler/optimizing/instruction_builder.cc
+++ b/compiler/optimizing/instruction_builder.cc
@@ -1204,7 +1204,12 @@
compiler_driver_->ComputeInstanceFieldInfo(field_index, dex_compilation_unit_, is_put, soa);
- HInstruction* object = LoadNullCheckedLocal(obj_reg, dex_pc);
+ // Generate an explicit null check on the reference, unless the field access
+ // is unresolved. In that case, we rely on the runtime to perform various
+ // checks first, followed by a null check.
+ HInstruction* object = (resolved_field == nullptr)
+ ? LoadLocal(obj_reg, Primitive::kPrimNot)
+ : LoadNullCheckedLocal(obj_reg, dex_pc);
Primitive::Type field_type = (resolved_field == nullptr)
? GetFieldAccessType(*dex_file_, field_index)
diff --git a/test/529-checker-unresolved/src/Main.java b/test/529-checker-unresolved/src/Main.java
index 872fa6d..a934377 100644
--- a/test/529-checker-unresolved/src/Main.java
+++ b/test/529-checker-unresolved/src/Main.java
@@ -114,6 +114,21 @@
expectEquals(o, c.instanceObject);
}
+ static public void callUnresolvedNull(UnresolvedClass c) {
+ int x = 0;
+ try {
+ x = c.instanceInt;
+ throw new Error("Expected NPE");
+ } catch (NullPointerException e) {
+ }
+ expectEquals(0, x);
+ try {
+ c.instanceInt = -1;
+ throw new Error("Expected NPE");
+ } catch (NullPointerException e) {
+ }
+ }
+
static public void testInstanceOf(Object o) {
if (o instanceof UnresolvedSuperClass) {
System.out.println("instanceof ok");
@@ -136,6 +151,7 @@
callInvokeUnresolvedSuper(m);
callUnresolvedStaticFieldAccess();
callUnresolvedInstanceFieldAccess(c);
+ callUnresolvedNull(null);
testInstanceOf(m);
testCheckCast(m);
testLicm(2);
@@ -185,7 +201,7 @@
}
}
- public static void expectEquals(float expected, float result) {
+ public static void expectEquals(float expected, float result) {
if (expected != result) {
throw new Error("Expected: " + expected + ", found: " + result);
}
diff --git a/test/600-verifier-fails/expected.txt b/test/600-verifier-fails/expected.txt
index e06fe1d..010f1b7 100644
--- a/test/600-verifier-fails/expected.txt
+++ b/test/600-verifier-fails/expected.txt
@@ -1,2 +1,3 @@
passed A
passed B
+passed C
diff --git a/test/600-verifier-fails/info.txt b/test/600-verifier-fails/info.txt
index 781b922..677b477 100644
--- a/test/600-verifier-fails/info.txt
+++ b/test/600-verifier-fails/info.txt
@@ -4,9 +4,11 @@
(1) b/28908555:
soft verification fail (on the final field modification) should
not hide the hard verification fail (on the type mismatch) to
- avoid compiler crash later on.
+ avoid compiler crash later on
(2) b/29070461:
hard failure (not calling super in constructor) should bail
immediately and not allow soft fails to pile up behind it to
avoid fatal message later on
+(3) b/29068831:
+ access validation should occur prior to null reference check
diff --git a/test/600-verifier-fails/smali/iput.smali b/test/600-verifier-fails/smali/iput.smali
new file mode 100644
index 0000000..bd8b928
--- /dev/null
+++ b/test/600-verifier-fails/smali/iput.smali
@@ -0,0 +1,25 @@
+#
+# Copyright (C) 2016 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+.class public LC;
+.super Ljava/lang/Object;
+
+.method public constructor <init>()V
+ .registers 2
+ invoke-direct {v1}, Ljava/lang/Object;-><init>()V
+ const v0, 0
+ iput-object v0, v0, LMain;->staticPrivateField:Ljava/lang/String;
+ return-void
+.end method
diff --git a/test/600-verifier-fails/smali/sput.smali b/test/600-verifier-fails/smali/sput.smali
index 87f3799..e8e56ac 100644
--- a/test/600-verifier-fails/smali/sput.smali
+++ b/test/600-verifier-fails/smali/sput.smali
@@ -18,6 +18,6 @@
.method public foo(I)V
.registers 2
- sput v1, LMain;->staticField:Ljava/lang/String;
+ sput v1, LMain;->staticFinalField:Ljava/lang/String;
return-void
.end method
diff --git a/test/600-verifier-fails/src/Main.java b/test/600-verifier-fails/src/Main.java
index 70873b2..0a8c5a1 100644
--- a/test/600-verifier-fails/src/Main.java
+++ b/test/600-verifier-fails/src/Main.java
@@ -16,18 +16,22 @@
public class Main {
- public static final String staticField = null;
+ public static final String staticFinalField = null;
+
+ private static String staticPrivateField = null;
+
+ private static void test(String name) throws Exception {
+ try {
+ Class<?> a = Class.forName(name);
+ a.newInstance();
+ } catch (java.lang.LinkageError e) {
+ System.out.println("passed " + name);
+ }
+ }
public static void main(String[] args) throws Exception {
- try {
- Class<?> a = Class.forName("A");
- } catch (java.lang.VerifyError e) {
- System.out.println("passed A");
- }
- try {
- Class<?> a = Class.forName("B");
- } catch (java.lang.VerifyError e) {
- System.out.println("passed B");
- }
+ test("A");
+ test("B");
+ test("C");
}
}