Fix getter/setter special case codegen

The special-purpose code generators for simple methods that get
or set and instance field and then return require that no throws
are possible.  The previous code incorrectly relied on the first
argument being a "this" pointer, and thus previously null-checked.
This did not take into account the possibility of a static method
which happened to pass an object referece as it's first argument.

The fix is to avoid making any assumptions, but rather rely
solely on the results of the null-check elimination pass which
will correctly recoginize the "this" case.

Change-Id: Icf001a10a19234cf3f4d87cf1baede93fdf0360c

2 files changed, 18 insertions, 16 deletions

diff --git a/src/compiler/codegen/arm/Thumb2/Gen.cc b/src/compiler/codegen/arm/Thumb2/Gen.cc
index d5a4efc36d..4319ec874f 100644
--- a/src/compiler/codegen/arm/Thumb2/Gen.cc
+++ b/src/compiler/codegen/arm/Thumb2/Gen.cc
@@ -149,18 +149,12 @@ MIR* specialIGet(CompilationUnit* cUnit, BasicBlock** bb, MIR* mir,
     uint32_t fieldIdx = mir->dalvikInsn.vC;
     bool fastPath = fastInstance(cUnit, fieldIdx, fieldOffset, isVolatile,
                                  false);
-    if (!fastPath) {
+    if (!fastPath || !(mir->optimizationFlags & MIR_IGNORE_NULL_CHECK)) {
         return NULL;
     }
     RegLocation rlObj = oatGetSrc(cUnit, mir, 0);
     lockLiveArgs(cUnit, mir);
     rlObj = argLoc(cUnit, rlObj);
-    // Reject if object reference is not "this"
-    if ((rlObj.location == kLocInvalid) ||
-        (inPosition(cUnit, rlObj.sRegLow) != 0)) {
-        oatResetRegPool(cUnit);
-        return NULL;
-    }
     RegLocation rlDest;
     if (longOrDouble) {
         rlDest = oatGetReturnWide(cUnit, false);
@@ -168,7 +162,6 @@ MIR* specialIGet(CompilationUnit* cUnit, BasicBlock** bb, MIR* mir,
         rlDest = oatGetReturn(cUnit, false);
     }
     // Point of no return - no aborts after this
-    mir->optimizationFlags |= MIR_IGNORE_NULL_CHECK;
     genPrintLabel(cUnit, mir);
     rlObj = loadArg(cUnit, rlObj);
     genIGet(cUnit, mir, size, rlDest, rlObj, longOrDouble, isObject);
@@ -183,7 +176,7 @@ MIR* specialIPut(CompilationUnit* cUnit, BasicBlock** bb, MIR* mir,
     uint32_t fieldIdx = mir->dalvikInsn.vC;
     bool fastPath = fastInstance(cUnit, fieldIdx, fieldOffset, isVolatile,
                                  false);
-    if (!fastPath) {
+    if (!fastPath || !(mir->optimizationFlags & MIR_IGNORE_NULL_CHECK)) {
         return NULL;
     }
     RegLocation rlSrc;
@@ -198,15 +191,12 @@ MIR* specialIPut(CompilationUnit* cUnit, BasicBlock** bb, MIR* mir,
     }
     rlSrc = argLoc(cUnit, rlSrc);
     rlObj = argLoc(cUnit, rlObj);
-    // Reject if object reference is not "this"
-    if ((rlObj.location == kLocInvalid) ||
-        (inPosition(cUnit, rlObj.sRegLow) != 0) ||
-        (rlSrc.location == kLocInvalid)) {
+    // Reject if source is split across registers & frame
+    if (rlObj.location == kLocInvalid) {
         oatResetRegPool(cUnit);
         return NULL;
     }
     // Point of no return - no aborts after this
-    mir->optimizationFlags |= MIR_IGNORE_NULL_CHECK;
     genPrintLabel(cUnit, mir);
     rlObj = loadArg(cUnit, rlObj);
     rlSrc = loadArg(cUnit, rlSrc);
diff --git a/test/083-compiler-regressions/src/Main.java b/test/083-compiler-regressions/src/Main.java
index efd8aa9cfa..f3e84cc910 100644
--- a/test/083-compiler-regressions/src/Main.java
+++ b/test/083-compiler-regressions/src/Main.java
@@ -128,12 +128,19 @@ public class Main {
         foo.setBar4(0,0,0,sum);
         sum += foo.getBar5(1,2,3,4,5);
         foo.setBar5(0,0,0,0,sum);
-        if (foo.getBar0() == 39488) {
+        Foo nullFoo = null;
+        try {
+            sum += Foo.barBar(nullFoo);
+        } catch(NullPointerException npe) {
+            sum += 404;
+        }
+        foo.setBar1(sum);
+        if (foo.getBar0() == 39892) {
             System.out.println("getterSetterTest passes");
         }
         else {
             System.out.println("getterSetterTest fails: " + foo.getBar0() +
-                               " (expecting 39488)");
+                               " (expecting 39892)");
         }
     }
 
@@ -8278,6 +8285,11 @@ class Foo {
     private int bar = 1234;
     private long lbar = 1234;
 
+    // Looks similar to a direct method, make sure we're null checking
+    static int barBar(Foo foo) {
+        return foo.bar;
+    }
+
     public int iConst0x1234() {
         return 0x1234;
     }