ART: Disallow uses of uninitialized references
The following instructions accepted uninitialized reference types
as their arguments:
- instance-of
- check-cast
- throw
- iput-object (stored value argument)
- sput-object
- invoke-* (non-this arguments)
Monitor-enter and monitor-exit are allowed.
Bug: 26594149
Change-Id: I2a4decb1fba274b8969b17bc237ac0fd19b93c80
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index 5f40123..56154c6 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -2076,7 +2076,7 @@
} else if (reg_type.IsConflict()) {
Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "returning register with conflict";
} else if (reg_type.IsUninitializedTypes()) {
- Fail(VERIFY_ERROR_BAD_CLASS_SOFT) << "returning uninitialized object '"
+ Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "returning uninitialized object '"
<< reg_type << "'";
} else if (!reg_type.IsReferenceTypes()) {
// We really do expect a reference here.
@@ -2233,7 +2233,6 @@
opcode_flags &= ~Instruction::kThrow;
work_line_->PopMonitor(this, inst->VRegA_11x());
break;
-
case Instruction::CHECK_CAST:
case Instruction::INSTANCE_OF: {
/*
@@ -2279,6 +2278,14 @@
} else {
Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "instance-of on non-reference in v" << orig_type_reg;
}
+ } else if (orig_type.IsUninitializedTypes()) {
+ if (is_checkcast) {
+ Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "check-cast on uninitialized reference in v"
+ << orig_type_reg;
+ } else {
+ Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "instance-of on uninitialized reference in v"
+ << orig_type_reg;
+ }
} else {
if (is_checkcast) {
work_line_->SetRegisterType<LockOp::kKeep>(this, inst->VRegA_21c(), res_type);
@@ -2373,8 +2380,12 @@
case Instruction::THROW: {
const RegType& res_type = work_line_->GetRegisterType(this, inst->VRegA_11x());
if (!reg_types_.JavaLangThrowable(false).IsAssignableFrom(res_type)) {
- Fail(res_type.IsUnresolvedTypes() ? VERIFY_ERROR_NO_CLASS : VERIFY_ERROR_BAD_CLASS_SOFT)
- << "thrown class " << res_type << " not instanceof Throwable";
+ if (res_type.IsUninitializedTypes()) {
+ Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "thrown exception not initialized";
+ } else {
+ Fail(res_type.IsUnresolvedTypes() ? VERIFY_ERROR_NO_CLASS : VERIFY_ERROR_BAD_CLASS_SOFT)
+ << "thrown class " << res_type << " not instanceof Throwable";
+ }
}
break;
}
@@ -3596,6 +3607,7 @@
} else {
const RegType& exception = ResolveClassAndCheckAccess(iterator.GetHandlerTypeIndex());
if (!reg_types_.JavaLangThrowable(false).IsAssignableFrom(exception)) {
+ DCHECK(!exception.IsUninitializedTypes()); // Comes from dex, shouldn't be uninit.
if (exception.IsUnresolvedTypes()) {
// We don't know enough about the type. Fail here and let runtime handle it.
Fail(VERIFY_ERROR_NO_CLASS) << "unresolved exception class " << exception;
@@ -3786,7 +3798,8 @@
CHECK(have_pending_hard_failure_);
return nullptr;
}
- if (actual_arg_type.IsUninitializedReference()) {
+ bool is_init = false;
+ if (actual_arg_type.IsUninitializedTypes()) {
if (res_method) {
if (!res_method->IsConstructor()) {
Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "'this' arg must be initialized";
@@ -3800,8 +3813,12 @@
return nullptr;
}
}
+ is_init = true;
}
- if (method_type != METHOD_INTERFACE && !actual_arg_type.IsZero()) {
+ const RegType& adjusted_type = is_init
+ ? GetRegTypeCache()->FromUninitialized(actual_arg_type)
+ : actual_arg_type;
+ if (method_type != METHOD_INTERFACE && !adjusted_type.IsZero()) {
const RegType* res_method_class;
// Miranda methods have the declaring interface as their declaring class, not the abstract
// class. It would be wrong to use this for the type check (interface type checks are
@@ -3819,10 +3836,12 @@
dex_file_->StringByTypeIdx(class_idx),
false);
}
- if (!res_method_class->IsAssignableFrom(actual_arg_type)) {
- Fail(actual_arg_type.IsUnresolvedTypes() ? VERIFY_ERROR_NO_CLASS:
- VERIFY_ERROR_BAD_CLASS_SOFT) << "'this' argument '" << actual_arg_type
- << "' not instance of '" << *res_method_class << "'";
+ if (!res_method_class->IsAssignableFrom(adjusted_type)) {
+ Fail(adjusted_type.IsUnresolvedTypes()
+ ? VERIFY_ERROR_NO_CLASS
+ : VERIFY_ERROR_BAD_CLASS_SOFT)
+ << "'this' argument '" << actual_arg_type << "' not instance of '"
+ << *res_method_class << "'";
// Continue on soft failures. We need to find possible hard failures to avoid problems in
// the compiler.
if (have_pending_hard_failure_) {
@@ -4083,7 +4102,8 @@
* For an interface class, we don't do the full interface merge (see JoinClass), so we can't do a
* rigorous check here (which is okay since we have to do it at runtime).
*/
- if (actual_arg_type.IsUninitializedReference() && !res_method->IsConstructor()) {
+ // Note: given an uninitialized type, this should always fail. Constructors aren't virtual.
+ if (actual_arg_type.IsUninitializedTypes() && !res_method->IsConstructor()) {
Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "'this' arg must be initialized";
return nullptr;
}
@@ -4093,8 +4113,11 @@
const RegType& res_method_class =
FromClass(klass->GetDescriptor(&temp), klass, klass->CannotBeAssignedFromOtherTypes());
if (!res_method_class.IsAssignableFrom(actual_arg_type)) {
- Fail(actual_arg_type.IsUnresolvedTypes() ? VERIFY_ERROR_NO_CLASS :
- VERIFY_ERROR_BAD_CLASS_SOFT) << "'this' argument '" << actual_arg_type
+ Fail(actual_arg_type.IsUninitializedTypes() // Just overcautious - should have never
+ ? VERIFY_ERROR_BAD_CLASS_HARD // quickened this.
+ : actual_arg_type.IsUnresolvedTypes()
+ ? VERIFY_ERROR_NO_CLASS
+ : VERIFY_ERROR_BAD_CLASS_SOFT) << "'this' argument '" << actual_arg_type
<< "' not instance of '" << res_method_class << "'";
return nullptr;
}
@@ -4421,15 +4444,20 @@
const RegType& field_klass =
FromClass(dex_file_->GetFieldDeclaringClassDescriptor(field_id),
klass, klass->CannotBeAssignedFromOtherTypes());
- if (obj_type.IsUninitializedTypes() &&
- (!IsConstructor() || GetDeclaringClass().Equals(obj_type) ||
- !field_klass.Equals(GetDeclaringClass()))) {
+ if (obj_type.IsUninitializedTypes()) {
// Field accesses through uninitialized references are only allowable for constructors where
- // the field is declared in this class
- Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "cannot access instance field " << PrettyField(field)
- << " of a not fully initialized object within the context"
- << " of " << PrettyMethod(dex_method_idx_, *dex_file_);
- return nullptr;
+ // the field is declared in this class.
+ // Note: this IsConstructor check is technically redundant, as UninitializedThis should only
+ // appear in constructors.
+ if (!obj_type.IsUninitializedThisReference() ||
+ !IsConstructor() ||
+ !field_klass.Equals(GetDeclaringClass())) {
+ Fail(VERIFY_ERROR_BAD_CLASS_HARD) << "cannot access instance field " << PrettyField(field)
+ << " of a not fully initialized object within the context"
+ << " of " << PrettyMethod(dex_method_idx_, *dex_file_);
+ return nullptr;
+ }
+ return field;
} else if (!field_klass.IsAssignableFrom(obj_type)) {
// Trying to access C1.field1 using reference of type C2, which is neither C1 or a sub-class
// of C1. For resolution to occur the declared class of the field must be compatible with
@@ -4452,7 +4480,18 @@
field = GetStaticField(field_idx);
} else {
const RegType& object_type = work_line_->GetRegisterType(this, inst->VRegB_22c());
- field = GetInstanceField(object_type, field_idx);
+
+ // One is not allowed to access fields on uninitialized references, except to write to
+ // fields in the constructor (before calling another constructor).
+ // GetInstanceField does an assignability check which will fail for uninitialized types.
+ // 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();
+ const RegType& adjusted_type = should_adjust
+ ? GetRegTypeCache()->FromUninitialized(object_type)
+ : object_type;
+ field = GetInstanceField(adjusted_type, field_idx);
if (UNLIKELY(have_pending_hard_failure_)) {
return;
}
diff --git a/runtime/verifier/reg_type-inl.h b/runtime/verifier/reg_type-inl.h
index 11a53e5..861db3c 100644
--- a/runtime/verifier/reg_type-inl.h
+++ b/runtime/verifier/reg_type-inl.h
@@ -93,6 +93,10 @@
return true; // All reference types can be assigned null.
} else if (!rhs.IsReferenceTypes()) {
return false; // Expect rhs to be a reference type.
+ } else if (lhs.IsUninitializedTypes() || rhs.IsUninitializedTypes()) {
+ // Uninitialized types are only allowed to be assigned to themselves.
+ // TODO: Once we have a proper "reference" super type, this needs to be extended.
+ return false;
} else if (lhs.IsJavaLangObject()) {
return true; // All reference types can be assigned to Object.
} else if (!strict && !lhs.IsUnresolvedTypes() && lhs.GetClass()->IsInterface()) {
diff --git a/runtime/verifier/register_line-inl.h b/runtime/verifier/register_line-inl.h
index 57fb701..08f85b3 100644
--- a/runtime/verifier/register_line-inl.h
+++ b/runtime/verifier/register_line-inl.h
@@ -147,6 +147,9 @@
if (!check_type.IsNonZeroReferenceTypes() || !src_type.IsNonZeroReferenceTypes()) {
// Hard fail if one of the types is primitive, since they are concretely known.
fail_type = VERIFY_ERROR_BAD_CLASS_HARD;
+ } else if (check_type.IsUninitializedTypes() || src_type.IsUninitializedTypes()) {
+ // Hard fail for uninitialized types, which don't match anything but themselves.
+ fail_type = VERIFY_ERROR_BAD_CLASS_HARD;
} else if (check_type.IsUnresolvedTypes() || src_type.IsUnresolvedTypes()) {
fail_type = VERIFY_ERROR_NO_CLASS;
} else {
diff --git a/test/800-smali/expected.txt b/test/800-smali/expected.txt
index 2e66af5..73ce307 100644
--- a/test/800-smali/expected.txt
+++ b/test/800-smali/expected.txt
@@ -50,4 +50,12 @@
b/21869691
b/26143249
b/26579108
+b/26594149 (1)
+b/26594149 (2)
+b/26594149 (3)
+b/26594149 (4)
+b/26594149 (5)
+b/26594149 (6)
+b/26594149 (7)
+b/26594149 (8)
Done!
diff --git a/test/800-smali/smali/b_26594149_1.smali b/test/800-smali/smali/b_26594149_1.smali
new file mode 100644
index 0000000..c465859
--- /dev/null
+++ b/test/800-smali/smali/b_26594149_1.smali
@@ -0,0 +1,26 @@
+# 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 LB26594149_1;
+.super Ljava/lang/Object;
+
+.method public static run()V
+ .registers 2
+ new-instance v0, Ljava/lang/String;
+
+ # Illegal operation.
+ instance-of v1, v0, Ljava/lang/String;
+
+ return-void
+ .end method
diff --git a/test/800-smali/smali/b_26594149_2.smali b/test/800-smali/smali/b_26594149_2.smali
new file mode 100644
index 0000000..765afe2
--- /dev/null
+++ b/test/800-smali/smali/b_26594149_2.smali
@@ -0,0 +1,26 @@
+# 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 LB26594149_2;
+.super Ljava/lang/Object;
+
+.method public static run()V
+ .registers 2
+ new-instance v0, Ljava/lang/String;
+
+ # Illegal operation.
+ check-cast v0, Ljava/lang/String;
+
+ return-void
+ .end method
diff --git a/test/800-smali/smali/b_26594149_3.smali b/test/800-smali/smali/b_26594149_3.smali
new file mode 100644
index 0000000..42b5675
--- /dev/null
+++ b/test/800-smali/smali/b_26594149_3.smali
@@ -0,0 +1,28 @@
+# 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 LB26594149_3;
+.super Ljava/lang/Object;
+
+.field public static field:Ljava/lang/String;
+
+.method public static run()V
+ .registers 2
+ new-instance v0, Ljava/lang/String;
+
+ # Illegal operation.
+ sput-object v0, LB26594149_3;->field:Ljava/lang/String;
+
+ return-void
+ .end method
diff --git a/test/800-smali/smali/b_26594149_4.smali b/test/800-smali/smali/b_26594149_4.smali
new file mode 100644
index 0000000..5b2f99b
--- /dev/null
+++ b/test/800-smali/smali/b_26594149_4.smali
@@ -0,0 +1,38 @@
+# 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 LB26594149_4;
+.super Ljava/lang/Object;
+
+.field public field:Ljava/lang/String;
+
+.method public constructor <init>()V
+ .registers 4
+ invoke-direct {p0}, Ljava/lang/Object;-><init>()V
+ return-void
+.end method
+
+.method public static run()V
+ .registers 4
+
+ new-instance v1, LB26594149_4;
+ invoke-direct {v1}, LB26594149_4;-><init>()V
+
+ new-instance v0, Ljava/lang/String;
+
+ # Illegal operation.
+ iput-object v0, v1, LB26594149_4;->field:Ljava/lang/String;
+
+ return-void
+ .end method
diff --git a/test/800-smali/smali/b_26594149_5.smali b/test/800-smali/smali/b_26594149_5.smali
new file mode 100644
index 0000000..27d6255
--- /dev/null
+++ b/test/800-smali/smali/b_26594149_5.smali
@@ -0,0 +1,28 @@
+# 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 LB26594149_5;
+.super Ljava/lang/Object;
+
+.method public static run()V
+ .registers 4
+
+ new-instance v0, Ljava/lang/Object;
+
+ # Allowed operation on uninitialized objects.
+ monitor-enter v0
+ monitor-exit v0
+
+ return-void
+ .end method
diff --git a/test/800-smali/smali/b_26594149_6.smali b/test/800-smali/smali/b_26594149_6.smali
new file mode 100644
index 0000000..8d26ee8
--- /dev/null
+++ b/test/800-smali/smali/b_26594149_6.smali
@@ -0,0 +1,24 @@
+# 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 LB26594149_6;
+.super Ljava/lang/Object;
+
+.method public static run()V
+ .registers 4
+
+ new-instance v0, Ljava/lang/Exception;
+ throw v0
+
+ .end method
diff --git a/test/800-smali/smali/b_26594149_7.smali b/test/800-smali/smali/b_26594149_7.smali
new file mode 100644
index 0000000..f624d1a
--- /dev/null
+++ b/test/800-smali/smali/b_26594149_7.smali
@@ -0,0 +1,30 @@
+# 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 LB26594149_7;
+.super Ljava/lang/Object;
+
+.method private static foo(Ljava/lang/Object;)V
+ .registers 1
+ return-void
+.end method
+
+.method public static run()V
+ .registers 4
+
+ new-instance v0, Ljava/lang/Object;
+ invoke-static {v0}, LB26594149_7;->foo(Ljava/lang/Object;)V
+ return-void
+
+ .end method
diff --git a/test/800-smali/smali/b_26594149_8.smali b/test/800-smali/smali/b_26594149_8.smali
new file mode 100644
index 0000000..e366de4
--- /dev/null
+++ b/test/800-smali/smali/b_26594149_8.smali
@@ -0,0 +1,24 @@
+# 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 LB26594149_8;
+.super Ljava/lang/Object;
+
+.method public static run()Ljava/lang/Object;
+ .registers 4
+
+ new-instance v0, Ljava/lang/Object;
+ return-object v0
+
+ .end method
diff --git a/test/800-smali/src/Main.java b/test/800-smali/src/Main.java
index 38aa58d..b0eff5d 100644
--- a/test/800-smali/src/Main.java
+++ b/test/800-smali/src/Main.java
@@ -145,6 +145,21 @@
new AbstractMethodError(), null));
testCases.add(new TestCase("b/26579108", "B26579108", "run", null, new VerifyError(),
null));
+ testCases.add(new TestCase("b/26594149 (1)", "B26594149_1", "run", null, new VerifyError(),
+ null));
+ testCases.add(new TestCase("b/26594149 (2)", "B26594149_2", "run", null, new VerifyError(),
+ null));
+ testCases.add(new TestCase("b/26594149 (3)", "B26594149_3", "run", null, new VerifyError(),
+ null));
+ testCases.add(new TestCase("b/26594149 (4)", "B26594149_4", "run", null, new VerifyError(),
+ null));
+ testCases.add(new TestCase("b/26594149 (5)", "B26594149_5", "run", null, null, null));
+ testCases.add(new TestCase("b/26594149 (6)", "B26594149_6", "run", null, new VerifyError(),
+ null));
+ testCases.add(new TestCase("b/26594149 (7)", "B26594149_7", "run", null, new VerifyError(),
+ null));
+ testCases.add(new TestCase("b/26594149 (8)", "B26594149_8", "run", null, new VerifyError(),
+ null));
}
public void runTests() {