summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--compiler/verifier_deps_test.cc10
-rw-r--r--runtime/verifier/method_verifier-inl.h2
-rw-r--r--runtime/verifier/method_verifier.cc71
-rw-r--r--runtime/verifier/method_verifier.h11
-rw-r--r--test/954-invoke-polymorphic-verifier/smali/Unresolved.smali2
5 files changed, 69 insertions, 27 deletions
diff --git a/compiler/verifier_deps_test.cc b/compiler/verifier_deps_test.cc
index 65389252e2..5c097da16f 100644
--- a/compiler/verifier_deps_test.cc
+++ b/compiler/verifier_deps_test.cc
@@ -624,7 +624,7 @@ TEST_F(VerifierDepsTest, ConstClass_Resolved) {
}
TEST_F(VerifierDepsTest, ConstClass_Unresolved) {
- ASSERT_TRUE(VerifyMethod("ConstClass_Unresolved"));
+ ASSERT_FALSE(VerifyMethod("ConstClass_Unresolved"));
ASSERT_TRUE(HasClass("LUnresolvedClass;", false));
}
@@ -634,7 +634,7 @@ TEST_F(VerifierDepsTest, CheckCast_Resolved) {
}
TEST_F(VerifierDepsTest, CheckCast_Unresolved) {
- ASSERT_TRUE(VerifyMethod("CheckCast_Unresolved"));
+ ASSERT_FALSE(VerifyMethod("CheckCast_Unresolved"));
ASSERT_TRUE(HasClass("LUnresolvedClass;", false));
}
@@ -644,7 +644,7 @@ TEST_F(VerifierDepsTest, InstanceOf_Resolved) {
}
TEST_F(VerifierDepsTest, InstanceOf_Unresolved) {
- ASSERT_TRUE(VerifyMethod("InstanceOf_Unresolved"));
+ ASSERT_FALSE(VerifyMethod("InstanceOf_Unresolved"));
ASSERT_TRUE(HasClass("LUnresolvedClass;", false));
}
@@ -654,12 +654,12 @@ TEST_F(VerifierDepsTest, NewInstance_Resolved) {
}
TEST_F(VerifierDepsTest, NewInstance_Unresolved) {
- ASSERT_TRUE(VerifyMethod("NewInstance_Unresolved"));
+ ASSERT_FALSE(VerifyMethod("NewInstance_Unresolved"));
ASSERT_TRUE(HasClass("LUnresolvedClass;", false));
}
TEST_F(VerifierDepsTest, NewArray_Unresolved) {
- ASSERT_TRUE(VerifyMethod("NewArray_Unresolved"));
+ ASSERT_FALSE(VerifyMethod("NewArray_Unresolved"));
ASSERT_TRUE(HasClass("[LUnresolvedClass;", false));
}
diff --git a/runtime/verifier/method_verifier-inl.h b/runtime/verifier/method_verifier-inl.h
index 6c832e3492..9bb875c033 100644
--- a/runtime/verifier/method_verifier-inl.h
+++ b/runtime/verifier/method_verifier-inl.h
@@ -77,7 +77,7 @@ inline bool MethodVerifier::HasFailures() const {
inline const RegType& MethodVerifier::ResolveCheckedClass(dex::TypeIndex class_idx) {
DCHECK(!HasFailures());
- const RegType& result = ResolveClassAndCheckAccess(class_idx);
+ const RegType& result = ResolveClass<CheckAccess::kYes>(class_idx);
DCHECK(!HasFailures());
return result;
}
diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc
index 312d8dfc18..9a876077d4 100644
--- a/runtime/verifier/method_verifier.cc
+++ b/runtime/verifier/method_verifier.cc
@@ -1765,7 +1765,9 @@ bool MethodVerifier::SetTypesFromSignature() {
// it's effectively considered initialized the instant we reach here (in the sense that we
// can return without doing anything or call virtual methods).
{
- const RegType& reg_type = ResolveClassAndCheckAccess(iterator.GetTypeIdx());
+ // Note: don't check access. No error would be thrown for declaring or passing an
+ // inaccessible class. Only actual accesses to fields or methods will.
+ const RegType& reg_type = ResolveClass<CheckAccess::kNo>(iterator.GetTypeIdx());
if (!reg_type.IsNonZeroReferenceTypes()) {
DCHECK(HasFailures());
return false;
@@ -2322,7 +2324,7 @@ bool MethodVerifier::CodeFlowVerifyInstruction(uint32_t* start_guess) {
case Instruction::CONST_CLASS: {
// Get type from instruction if unresolved then we need an access check
// TODO: check Compiler::CanAccessTypeWithoutChecks returns false when res_type is unresolved
- const RegType& res_type = ResolveClassAndCheckAccess(dex::TypeIndex(inst->VRegB_21c()));
+ const RegType& res_type = ResolveClass<CheckAccess::kYes>(dex::TypeIndex(inst->VRegB_21c()));
// Register holds class, ie its type is class, on error it will hold Conflict.
work_line_->SetRegisterType<LockOp::kClear>(
this, inst->VRegA_21c(), res_type.IsConflict() ? res_type
@@ -2393,7 +2395,7 @@ bool MethodVerifier::CodeFlowVerifyInstruction(uint32_t* start_guess) {
*/
const bool is_checkcast = (inst->Opcode() == Instruction::CHECK_CAST);
const dex::TypeIndex type_idx((is_checkcast) ? inst->VRegB_21c() : inst->VRegC_22c());
- const RegType& res_type = ResolveClassAndCheckAccess(type_idx);
+ const RegType& res_type = ResolveClass<CheckAccess::kYes>(type_idx);
if (res_type.IsConflict()) {
// If this is a primitive type, fail HARD.
ObjPtr<mirror::Class> klass =
@@ -2463,7 +2465,7 @@ bool MethodVerifier::CodeFlowVerifyInstruction(uint32_t* start_guess) {
break;
}
case Instruction::NEW_INSTANCE: {
- const RegType& res_type = ResolveClassAndCheckAccess(dex::TypeIndex(inst->VRegB_21c()));
+ const RegType& res_type = ResolveClass<CheckAccess::kYes>(dex::TypeIndex(inst->VRegB_21c()));
if (res_type.IsConflict()) {
DCHECK_NE(failures_.size(), 0U);
break; // bad class
@@ -2675,7 +2677,7 @@ bool MethodVerifier::CodeFlowVerifyInstruction(uint32_t* start_guess) {
// ensure that subsequent merges don't lose type information - such as becoming an
// interface from a class that would lose information relevant to field checks.
const RegType& orig_type = work_line_->GetRegisterType(this, instance_of_inst->VRegB_22c());
- const RegType& cast_type = ResolveClassAndCheckAccess(
+ const RegType& cast_type = ResolveClass<CheckAccess::kYes>(
dex::TypeIndex(instance_of_inst->VRegC_22c()));
if (!orig_type.Equals(cast_type) &&
@@ -3723,7 +3725,8 @@ inline bool MethodVerifier::IsInstantiableOrPrimitive(mirror::Class* klass) {
return klass->IsInstantiable() || klass->IsPrimitive();
}
-const RegType& MethodVerifier::ResolveClassAndCheckAccess(dex::TypeIndex class_idx) {
+template <MethodVerifier::CheckAccess C>
+const RegType& MethodVerifier::ResolveClass(dex::TypeIndex class_idx) {
mirror::Class* klass = can_load_classes_
? Runtime::Current()->GetClassLinker()->ResolveType(
*dex_file_, class_idx, dex_cache_, class_loader_)
@@ -3760,13 +3763,15 @@ const RegType& MethodVerifier::ResolveClassAndCheckAccess(dex::TypeIndex class_i
// Record result of class resolution attempt.
VerifierDeps::MaybeRecordClassResolution(*dex_file_, class_idx, klass);
- // Check if access is allowed. Unresolved types use xxxWithAccessCheck to
- // check at runtime if access is allowed and so pass here. If result is
- // primitive, skip the access check.
- if (result->IsNonZeroReferenceTypes() && !result->IsUnresolvedTypes()) {
+ // If requested, check if access is allowed. Unresolved types are included in this check, as the
+ // interpreter only tests whether access is allowed when a class is not pre-verified and runs in
+ // the access-checks interpreter. If result is primitive, skip the access check.
+ //
+ // Note: we do this for unresolved classes to trigger re-verification at runtime.
+ if (C == CheckAccess::kYes && result->IsNonZeroReferenceTypes()) {
const RegType& referrer = GetDeclaringClass();
- if (!referrer.IsUnresolvedTypes() && !referrer.CanAccess(*result)) {
- Fail(VERIFY_ERROR_ACCESS_CLASS) << "illegal class access: '"
+ if (!referrer.CanAccess(*result)) {
+ Fail(VERIFY_ERROR_ACCESS_CLASS) << "(possibly) illegal class access: '"
<< referrer << "' -> '" << *result << "'";
}
}
@@ -3785,7 +3790,8 @@ const RegType& MethodVerifier::GetCaughtExceptionType() {
if (!iterator.GetHandlerTypeIndex().IsValid()) {
common_super = &reg_types_.JavaLangThrowable(false);
} else {
- const RegType& exception = ResolveClassAndCheckAccess(iterator.GetHandlerTypeIndex());
+ const RegType& exception =
+ ResolveClass<CheckAccess::kYes>(iterator.GetHandlerTypeIndex());
if (!reg_types_.JavaLangThrowable(false).IsAssignableFrom(exception, this)) {
DCHECK(!exception.IsUninitializedTypes()); // Comes from dex, shouldn't be uninit.
if (exception.IsUnresolvedTypes()) {
@@ -3827,7 +3833,7 @@ const RegType& MethodVerifier::GetCaughtExceptionType() {
ArtMethod* MethodVerifier::ResolveMethodAndCheckAccess(
uint32_t dex_method_idx, MethodType method_type) {
const DexFile::MethodId& method_id = dex_file_->GetMethodId(dex_method_idx);
- const RegType& klass_type = ResolveClassAndCheckAccess(method_id.class_idx_);
+ const RegType& klass_type = ResolveClass<CheckAccess::kYes>(method_id.class_idx_);
if (klass_type.IsConflict()) {
std::string append(" in attempt to access method ");
append += dex_file_->GetMethodName(method_id);
@@ -4598,7 +4604,7 @@ void MethodVerifier::VerifyNewArray(const Instruction* inst, bool is_filled, boo
DCHECK_EQ(inst->Opcode(), Instruction::FILLED_NEW_ARRAY_RANGE);
type_idx = dex::TypeIndex(inst->VRegB_3rc());
}
- const RegType& res_type = ResolveClassAndCheckAccess(type_idx);
+ const RegType& res_type = ResolveClass<CheckAccess::kYes>(type_idx);
if (res_type.IsConflict()) { // bad class
DCHECK_NE(failures_.size(), 0U);
} else {
@@ -4812,7 +4818,7 @@ void MethodVerifier::VerifyAPut(const Instruction* inst,
ArtField* MethodVerifier::GetStaticField(int field_idx) {
const DexFile::FieldId& field_id = dex_file_->GetFieldId(field_idx);
// Check access to class
- const RegType& klass_type = ResolveClassAndCheckAccess(field_id.class_idx_);
+ const RegType& klass_type = ResolveClass<CheckAccess::kYes>(field_id.class_idx_);
if (klass_type.IsConflict()) { // bad class
AppendToLastFailMessage(StringPrintf(" in attempt to access static field %d (%s) in %s",
field_idx, dex_file_->GetFieldName(field_id),
@@ -4820,6 +4826,9 @@ ArtField* MethodVerifier::GetStaticField(int field_idx) {
return nullptr;
}
if (klass_type.IsUnresolvedTypes()) {
+ // Accessibility checks depend on resolved fields.
+ DCHECK(klass_type.Equals(GetDeclaringClass()) || !failures_.empty());
+
return nullptr; // Can't resolve Class so no more to do here, will do checking at runtime.
}
ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
@@ -4850,7 +4859,7 @@ ArtField* MethodVerifier::GetStaticField(int field_idx) {
ArtField* MethodVerifier::GetInstanceField(const RegType& obj_type, int field_idx) {
const DexFile::FieldId& field_id = dex_file_->GetFieldId(field_idx);
// Check access to class.
- const RegType& klass_type = ResolveClassAndCheckAccess(field_id.class_idx_);
+ const RegType& klass_type = ResolveClass<CheckAccess::kYes>(field_id.class_idx_);
if (klass_type.IsConflict()) {
AppendToLastFailMessage(StringPrintf(" in attempt to access instance field %d (%s) in %s",
field_idx, dex_file_->GetFieldName(field_id),
@@ -4858,6 +4867,9 @@ ArtField* MethodVerifier::GetInstanceField(const RegType& obj_type, int field_id
return nullptr;
}
if (klass_type.IsUnresolvedTypes()) {
+ // Accessibility checks depend on resolved fields.
+ DCHECK(klass_type.Equals(GetDeclaringClass()) || !failures_.empty());
+
return nullptr; // Can't resolve Class so no more to do here
}
ClassLinker* class_linker = Runtime::Current()->GetClassLinker();
@@ -4994,6 +5006,31 @@ void MethodVerifier::VerifyISFieldAccess(const Instruction* inst, const RegType&
DCHECK(!can_load_classes_ || self_->IsExceptionPending());
self_->ClearException();
}
+ } else {
+ // If we don't have the field (it seems we failed resolution) and this is a PUT, we need to
+ // redo verification at runtime as the field may be final, unless the field id shows it's in
+ // the same class.
+ //
+ // For simplicity, it is OK to not distinguish compile-time vs runtime, and post this an
+ // ACCESS_FIELD failure at runtime. This has the same effect as NO_FIELD - punting the class
+ // to the access-checks interpreter.
+ //
+ // Note: see b/34966607. This and above may be changed in the future.
+ if (kAccType == FieldAccessType::kAccPut) {
+ const DexFile::FieldId& field_id = dex_file_->GetFieldId(field_idx);
+ const char* field_class_descriptor = dex_file_->GetFieldDeclaringClassDescriptor(field_id);
+ const RegType* field_class_type = &reg_types_.FromDescriptor(GetClassLoader(),
+ field_class_descriptor,
+ false);
+ if (!field_class_type->Equals(GetDeclaringClass())) {
+ Fail(VERIFY_ERROR_ACCESS_FIELD) << "could not check field put for final field modify of "
+ << field_class_descriptor
+ << "."
+ << dex_file_->GetFieldName(field_id)
+ << " from other class "
+ << GetDeclaringClass();
+ }
+ }
}
if (field_type == nullptr) {
const DexFile::FieldId& field_id = dex_file_->GetFieldId(field_idx);
diff --git a/runtime/verifier/method_verifier.h b/runtime/verifier/method_verifier.h
index ea8729cb3e..da4102a786 100644
--- a/runtime/verifier/method_verifier.h
+++ b/runtime/verifier/method_verifier.h
@@ -576,9 +576,14 @@ class MethodVerifier {
void VerifyQuickFieldAccess(const Instruction* inst, const RegType& insn_type, bool is_primitive)
REQUIRES_SHARED(Locks::mutator_lock_);
- // Resolves a class based on an index and performs access checks to ensure the referrer can
- // access the resolved class.
- const RegType& ResolveClassAndCheckAccess(dex::TypeIndex class_idx)
+ enum class CheckAccess { // private.
+ kYes,
+ kNo,
+ };
+ // Resolves a class based on an index and, if C is kYes, performs access checks to ensure
+ // the referrer can access the resolved class.
+ template <CheckAccess C>
+ const RegType& ResolveClass(dex::TypeIndex class_idx)
REQUIRES_SHARED(Locks::mutator_lock_);
/*
diff --git a/test/954-invoke-polymorphic-verifier/smali/Unresolved.smali b/test/954-invoke-polymorphic-verifier/smali/Unresolved.smali
index 882f0e9256..26044726ad 100644
--- a/test/954-invoke-polymorphic-verifier/smali/Unresolved.smali
+++ b/test/954-invoke-polymorphic-verifier/smali/Unresolved.smali
@@ -23,7 +23,7 @@
.line 23
invoke-direct {p0}, Ljava/lang/Object;-><init>()V
# Get an unresolvable instance (abstract class)
- invoke-static {}, LAbstract;->getUnresolvedInstance()Lother/thing/Foo;
+ invoke-static {}, LUnresolved;->getUnresolvedInstance()Lother/thing/Foo;
move-result-object v0
const-string v1, "1"
const-string v2, "2"
generated by cgit v1.2.3-59-g8ed1b (git 2.39.0) at 2026-01-05 13:43:32 +0000