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

7 files changed, 67 insertions, 14 deletions

diff --git a/compiler/optimizing/instruction_builder.cc b/compiler/optimizing/instruction_builder.cc
index aaddc01f1f..5e691c7f5f 100644
--- a/compiler/optimizing/instruction_builder.cc
+++ b/compiler/optimizing/instruction_builder.cc
@@ -1204,7 +1204,12 @@ bool HInstructionBuilder::BuildInstanceFieldAccess(const Instruction& instructio
       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 872fa6d0dd..a934377ddf 100644
--- a/test/529-checker-unresolved/src/Main.java
+++ b/test/529-checker-unresolved/src/Main.java
@@ -114,6 +114,21 @@ public class Main extends UnresolvedSuperClass {
     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 @@ public class Main extends UnresolvedSuperClass {
     callInvokeUnresolvedSuper(m);
     callUnresolvedStaticFieldAccess();
     callUnresolvedInstanceFieldAccess(c);
+    callUnresolvedNull(null);
     testInstanceOf(m);
     testCheckCast(m);
     testLicm(2);
@@ -185,7 +201,7 @@ public class Main extends UnresolvedSuperClass {
     }
   }
 
-    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 e06fe1dea2..010f1b74b2 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 781b922716..677b4775d5 100644
--- a/test/600-verifier-fails/info.txt
+++ b/test/600-verifier-fails/info.txt
@@ -4,9 +4,11 @@ dexfuzz on the DEX files of fuzzingly random generated Java test.
 (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 0000000000..bd8b9280c0
--- /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 87f3799e02..e8e56acf13 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 70873b2291..0a8c5a179d 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;
 
-  public static void main(String[] args) throws Exception {
-    try {
-      Class<?> a = Class.forName("A");
-    } catch (java.lang.VerifyError e) {
-      System.out.println("passed A");
-    }
+  private static String staticPrivateField = null;
+
+  private static void test(String name) throws Exception {
     try {
-      Class<?> a = Class.forName("B");
-    } catch (java.lang.VerifyError e) {
-      System.out.println("passed B");
+      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 {
+    test("A");
+    test("B");
+    test("C");
+  }
 }