Fix bug with verification of constructors

We would incorrectly allow the storing of values into superclass
fields before the superclass constructor was called.

Bug: 26965384

Change-Id: I45b824fbdbfc133663ed6d3306853595b5dc9262

diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index 8d5e6ea..1d31408 100644 (file)
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -4526,6 +4526,19 @@ void MethodVerifier::VerifyISFieldAccess(const Instruction* inst, const RegType&
     if (UNLIKELY(have_pending_hard_failure_)) {
       return;
     }
+    if (should_adjust) {
+      if (field == nullptr) {
+        Fail(VERIFY_ERROR_BAD_CLASS_SOFT) << "Might be accessing a superclass instance field prior "
+                                          << "to the superclass being initialized in "
+                                          << PrettyMethod(dex_method_idx_, *dex_file_);
+      } else if (field->GetDeclaringClass() != GetDeclaringClass().GetClass()) {
+        Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "cannot access superclass instance field "
+                                          << PrettyField(field) << " of a not fully initialized "
+                                          << "object within the context of "
+                                          << PrettyMethod(dex_method_idx_, *dex_file_);
+        return;
+      }
+    }
   }
   const RegType* field_type = nullptr;
   if (field != nullptr) {

diff --git a/test/800-smali/expected.txt b/test/800-smali/expected.txt
index edbb7b5..8808a50 100644 (file)
--- a/test/800-smali/expected.txt
+++ b/test/800-smali/expected.txt
@@ -59,4 +59,5 @@ b/26594149 (6)
 b/26594149 (7)
 b/26594149 (8)
 b/27148248
+b/26965384
 Done!

diff --git a/test/800-smali/smali/b_26965384.smali b/test/800-smali/smali/b_26965384.smali
new file mode 100644 (file)
index 0000000..47ed418
--- /dev/null
+++ b/test/800-smali/smali/b_26965384.smali
@@ -0,0 +1,20 @@
+.class public LB26965384;
+.super LB26965384Super;
+
+.method public constructor <init>()V
+    .locals 1
+    const v0, 0
+    iput v0, p0, LB26965384;->a:I
+    invoke-direct {p0}, LB26965384Super;-><init>()V
+    return-void
+.end method
+
+
+# Just by loading this class we should fail. It doesn't really matter what's in
+# this method.
+.method public static run()V
+    .registers 4
+    new-instance v0, LB26965384;
+    invoke-direct {v0}, LB26965384;-><init>()V
+    return-void
+.end method

diff --git a/test/800-smali/smali/b_26965384Super.smali b/test/800-smali/smali/b_26965384Super.smali
new file mode 100644 (file)
index 0000000..32faea7
--- /dev/null
+++ b/test/800-smali/smali/b_26965384Super.smali
@@ -0,0 +1,10 @@
+.class public LB26965384Super;
+.super Ljava/lang/Object;
+
+.field public a:I
+
+.method public constructor <init>()V
+    .locals 0
+    invoke-direct {p0}, Ljava/lang/Object;-><init>()V
+    return-void
+.end method

diff --git a/test/800-smali/src/Main.java b/test/800-smali/src/Main.java
index 2ea3367..4e6de46 100644 (file)
--- a/test/800-smali/src/Main.java
+++ b/test/800-smali/src/Main.java
@@ -162,6 +162,8 @@ public class Main {
                 null));
         testCases.add(new TestCase("b/27148248", "B27148248", "run", null, new VerifyError(),
                 null));
+        testCases.add(new TestCase("b/26965384", "B26965384", "run", null, new VerifyError(),
+                null));
     }
 
     public void runTests() {