Better handling of unresolved fields in VerifyISFieldAccess.
Even if we cannot resolve a class / field, we know we can look in the
dex file for finding a field of the class being verified.
Test: 831-unresolved-field
Bug: 28313047
Change-Id: Ie6c3e05c8df064becc3dae913b82859875d171ef
diff --git a/dex2oat/driver/compiler_driver.cc b/dex2oat/driver/compiler_driver.cc
index 35c8dc8..a46e9ed 100644
--- a/dex2oat/driver/compiler_driver.cc
+++ b/dex2oat/driver/compiler_driver.cc
@@ -1836,8 +1836,6 @@
&error_msg);
switch (failure_kind) {
case verifier::FailureKind::kHardFailure: {
- LOG(ERROR) << "Verification failed on class " << PrettyDescriptor(descriptor)
- << " because: " << error_msg;
manager_->GetCompiler()->SetHadHardVerifierFailure();
break;
}
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index 95dd8e1..bbc6cee 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -4675,7 +4675,8 @@
// We thus modify the type if the uninitialized reference is a "this" reference (this also
// checks at the same time that we're verifying a constructor).
bool should_adjust = (kAccType == FieldAccessType::kAccPut) &&
- object_type.IsUninitializedThisReference();
+ (object_type.IsUninitializedThisReference() ||
+ object_type.IsUnresolvedAndUninitializedThisReference());
const RegType& adjusted_type = should_adjust
? GetRegTypeCache()->FromUninitialized(object_type)
: object_type;
@@ -4684,13 +4685,27 @@
return;
}
if (should_adjust) {
+ bool illegal_field_access = false;
if (field == nullptr) {
- Fail(VERIFY_ERROR_BAD_CLASS_SOFT) << "Might be accessing a superclass instance field prior "
- << "to the superclass being initialized in "
- << dex_file_->PrettyMethod(dex_method_idx_);
+ const dex::FieldId& field_id = dex_file_->GetFieldId(field_idx);
+ if (field_id.class_idx_ != GetClassDef().class_idx_) {
+ illegal_field_access = true;
+ } else {
+ ClassAccessor accessor(*dex_file_, GetClassDef());
+ illegal_field_access = (accessor.GetInstanceFields().end() ==
+ std::find_if(accessor.GetInstanceFields().begin(),
+ accessor.GetInstanceFields().end(),
+ [field_idx] (const ClassAccessor::Field& f) {
+ return f.GetIndex() == field_idx;
+ }));
+ }
} else if (field->GetDeclaringClass() != GetDeclaringClass().GetClass()) {
- Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "cannot access superclass instance field "
- << field->PrettyField() << " of a not fully initialized "
+ illegal_field_access = true;
+ }
+ if (illegal_field_access) {
+ Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "cannot access instance field "
+ << dex_file_->PrettyField(field_idx)
+ << " of a not fully initialized "
<< "object within the context of "
<< dex_file_->PrettyMethod(dex_method_idx_);
return;
diff --git a/test/831-unresolved-field/expected-stderr.txt b/test/831-unresolved-field/expected-stderr.txt
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/test/831-unresolved-field/expected-stderr.txt
diff --git a/test/831-unresolved-field/expected-stdout.txt b/test/831-unresolved-field/expected-stdout.txt
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/test/831-unresolved-field/expected-stdout.txt
diff --git a/test/831-unresolved-field/info.txt b/test/831-unresolved-field/info.txt
new file mode 100644
index 0000000..ccdceae
--- /dev/null
+++ b/test/831-unresolved-field/info.txt
@@ -0,0 +1,2 @@
+Test that the verifier handles field assignments in constructors of unresolve
+classes.
diff --git a/test/831-unresolved-field/run b/test/831-unresolved-field/run
new file mode 100644
index 0000000..23424f6
--- /dev/null
+++ b/test/831-unresolved-field/run
@@ -0,0 +1,18 @@
+#!/bin/bash
+#
+# Copyright (C) 2021 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.
+
+# Use secondary switch to add secondary dex file to class path.
+exec ${RUN} "${@}" --secondary
diff --git a/test/831-unresolved-field/smali/NonVerifiedClass.smali b/test/831-unresolved-field/smali/NonVerifiedClass.smali
new file mode 100644
index 0000000..b263607
--- /dev/null
+++ b/test/831-unresolved-field/smali/NonVerifiedClass.smali
@@ -0,0 +1,24 @@
+# Copyright (C) 2021 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 LNonVerifiedSubClass;
+.super LUnresolvedClass;
+
+.method public constructor <init>()V
+ .registers 2
+ const/4 v0, 0x1
+ iput v0, p0, LUnresolvedClass;->field:I
+ invoke-direct {p0}, LUnresolvedClass;-><init>()V
+ return-void
+.end method
diff --git a/test/831-unresolved-field/smali/VerifiedClass.smali b/test/831-unresolved-field/smali/VerifiedClass.smali
new file mode 100644
index 0000000..683181e
--- /dev/null
+++ b/test/831-unresolved-field/smali/VerifiedClass.smali
@@ -0,0 +1,26 @@
+# Copyright (C) 2021 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 LVerifiedSubClass;
+.super LUnresolvedClass;
+
+.field public localField:I
+
+.method public constructor <init>()V
+ .registers 2
+ const/4 v0, 0x1
+ iput v0, p0, LVerifiedSubClass;->localField:I
+ invoke-direct {p0}, LUnresolvedClass;-><init>()V
+ return-void
+.end method
diff --git a/test/831-unresolved-field/src-dex2oat-unresolved/UnresolvedClass.java b/test/831-unresolved-field/src-dex2oat-unresolved/UnresolvedClass.java
new file mode 100644
index 0000000..9550f40
--- /dev/null
+++ b/test/831-unresolved-field/src-dex2oat-unresolved/UnresolvedClass.java
@@ -0,0 +1,19 @@
+/*
+ * Copyright (C) 2021 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.
+ */
+
+public class UnresolvedClass {
+ public int field;
+}
diff --git a/test/831-unresolved-field/src/Main.java b/test/831-unresolved-field/src/Main.java
new file mode 100644
index 0000000..6681507
--- /dev/null
+++ b/test/831-unresolved-field/src/Main.java
@@ -0,0 +1,29 @@
+/*
+ * Copyright (C) 2021 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.
+ */
+
+public class Main {
+ public static void main(String[] args) throws Exception {
+ Class<?> cls = Class.forName("VerifiedSubClass");
+ cls.newInstance();
+
+ try {
+ Class.forName("NonVerifiedSubClass");
+ throw new Error("Expected VerifyError");
+ } catch (VerifyError e) {
+ // expected
+ }
+ }
+}
diff --git a/test/knownfailures.json b/test/knownfailures.json
index ac71c66..e7ed208 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -1158,6 +1158,7 @@
"827-resolve-method",
"830-goto-zero",
"831-unverified-bcp",
+ "831-unresolved-field",
"999-redefine-hiddenapi",
"1000-non-moving-space-stress",
"1001-app-image-regions",