From db55a1121b2437765e732c8bbedf914f8a52f624 Mon Sep 17 00:00:00 2001 From: Alex Light Date: Thu, 17 Oct 2019 10:32:47 -0700 Subject: Class redefinition sometimes needs to update verification In cases where class redefinition moves a class from having no verification failures to having soft verification failures we need to update the methods with new verification class flags. For example if a method is modified to have unbalanced monitors we need to make sure that future invokes of that method count locks and use the interpreter. Previously we would simply keep the same verification state as the original implementation, causing us to try to compile in situations the compiler cannot handle or leave monitors in inconsistent states. Test: ./test.py --host Bug: 142876078 Change-Id: I8adf59158639bdf237d691b20fad223f0a34db1f --- runtime/art_method.h | 6 ++++ runtime/mirror/class.cc | 9 +++++ runtime/mirror/class.h | 3 ++ runtime/verifier/class_verifier.cc | 70 +++++++++++++++++++++++++++++++++++++ runtime/verifier/class_verifier.h | 30 ++++++++++++++++ runtime/verifier/method_verifier.cc | 8 +++-- runtime/verifier/method_verifier.h | 2 ++ 7 files changed, 126 insertions(+), 2 deletions(-) (limited to 'runtime') diff --git a/runtime/art_method.h b/runtime/art_method.h index d84ea7c4bc..d4fb5d7b90 100644 --- a/runtime/art_method.h +++ b/runtime/art_method.h @@ -335,6 +335,11 @@ class ArtMethod final { DCHECK(!IsNative()); AddAccessFlags(kAccSkipAccessChecks); } + void ClearSkipAccessChecks() { + // SkipAccessChecks() is applicable only to non-native methods. + DCHECK(!IsNative()); + ClearAccessFlags(kAccSkipAccessChecks); + } bool PreviouslyWarm() { if (IsIntrinsic()) { @@ -363,6 +368,7 @@ class ArtMethod final { void SetMustCountLocks() { AddAccessFlags(kAccMustCountLocks); + ClearAccessFlags(kAccSkipAccessChecks); } // Returns true if this method could be overridden by a default method. diff --git a/runtime/mirror/class.cc b/runtime/mirror/class.cc index 455f98d489..a0e8a237c5 100644 --- a/runtime/mirror/class.cc +++ b/runtime/mirror/class.cc @@ -1031,6 +1031,15 @@ ArtField* Class::FindField(Thread* self, return nullptr; } +void Class::ClearSkipAccessChecksFlagOnAllMethods(PointerSize pointer_size) { + DCHECK(IsVerified()); + for (auto& m : GetMethods(pointer_size)) { + if (!m.IsNative() && m.IsInvokable()) { + m.ClearSkipAccessChecks(); + } + } +} + void Class::SetSkipAccessChecksFlagOnAllMethods(PointerSize pointer_size) { DCHECK(IsVerified()); for (auto& m : GetMethods(pointer_size)) { diff --git a/runtime/mirror/class.h b/runtime/mirror/class.h index 6ed20ed02f..d925a96d9c 100644 --- a/runtime/mirror/class.h +++ b/runtime/mirror/class.h @@ -1134,6 +1134,9 @@ class MANAGED Class final : public Object { static ObjPtr GetPrimitiveClass(ObjPtr name) REQUIRES_SHARED(Locks::mutator_lock_); + // Clear the kAccSkipAccessChecks flag on each method, for class redefinition. + void ClearSkipAccessChecksFlagOnAllMethods(PointerSize pointer_size) + REQUIRES_SHARED(Locks::mutator_lock_); // When class is verified, set the kAccSkipAccessChecks flag on each method. void SetSkipAccessChecksFlagOnAllMethods(PointerSize pointer_size) REQUIRES_SHARED(Locks::mutator_lock_); diff --git a/runtime/verifier/class_verifier.cc b/runtime/verifier/class_verifier.cc index 1df11ada50..66f5801634 100644 --- a/runtime/verifier/class_verifier.cc +++ b/runtime/verifier/class_verifier.cc @@ -20,6 +20,7 @@ #include #include "art_method-inl.h" +#include "base/enums.h" #include "base/systrace.h" #include "base/utils.h" #include "class_linker.h" @@ -28,11 +29,13 @@ #include "dex/class_reference.h" #include "dex/descriptors_names.h" #include "dex/dex_file-inl.h" +#include "handle.h" #include "handle_scope-inl.h" #include "method_verifier-inl.h" #include "mirror/class-inl.h" #include "mirror/dex_cache.h" #include "runtime.h" +#include "thread.h" namespace art { namespace verifier { @@ -43,6 +46,30 @@ using android::base::StringPrintf; // sure we only print this once. static bool gPrintedDxMonitorText = false; +FailureKind ClassVerifier::ReverifyClass(Thread* self, + ObjPtr klass, + HardFailLogMode log_level, + uint32_t api_level, + std::string* error) { + DCHECK(!Runtime::Current()->IsAotCompiler()); + StackHandleScope<1> hs(self); + Handle h_klass(hs.NewHandle(klass)); + ScopedAssertNoThreadSuspension sants(__FUNCTION__); + FailureKind res = CommonVerifyClass(self, + h_klass.Get(), + /*callbacks=*/nullptr, + /*allow_soft_failures=*/false, + log_level, + api_level, + /*can_allocate=*/ false, + error); + if (res == FailureKind::kSoftFailure) { + // We cannot skip access checks since there was a soft failure. + h_klass->ClearSkipAccessChecksFlagOnAllMethods(kRuntimePointerSize); + } + return res; +} + FailureKind ClassVerifier::VerifyClass(Thread* self, ObjPtr klass, CompilerCallbacks* callbacks, @@ -53,6 +80,23 @@ FailureKind ClassVerifier::VerifyClass(Thread* self, if (klass->IsVerified()) { return FailureKind::kNoFailure; } + return CommonVerifyClass(self, + klass, + callbacks, + allow_soft_failures, + log_level, + api_level, + /*can_allocate=*/ true, + error); +} +FailureKind ClassVerifier::CommonVerifyClass(Thread* self, + ObjPtr klass, + CompilerCallbacks* callbacks, + bool allow_soft_failures, + HardFailLogMode log_level, + uint32_t api_level, + bool can_allocate, + std::string* error) { bool early_failure = false; std::string failure_message; const DexFile& dex_file = klass->GetDexFile(); @@ -89,6 +133,30 @@ FailureKind ClassVerifier::VerifyClass(Thread* self, allow_soft_failures, log_level, api_level, + can_allocate, + error); +} + +FailureKind ClassVerifier::VerifyClass(Thread* self, + const DexFile* dex_file, + Handle dex_cache, + Handle class_loader, + const dex::ClassDef& class_def, + CompilerCallbacks* callbacks, + bool allow_soft_failures, + HardFailLogMode log_level, + uint32_t api_level, + std::string* error) { + return VerifyClass(self, + dex_file, + dex_cache, + class_loader, + class_def, + callbacks, + allow_soft_failures, + log_level, + api_level, + /*can_allocate=*/!Runtime::Current()->IsAotCompiler(), error); } @@ -101,6 +169,7 @@ FailureKind ClassVerifier::VerifyClass(Thread* self, bool allow_soft_failures, HardFailLogMode log_level, uint32_t api_level, + bool can_allocate, std::string* error) { // A class must not be abstract and final. if ((class_def.access_flags_ & (kAccAbstract | kAccFinal)) == (kAccAbstract | kAccFinal)) { @@ -156,6 +225,7 @@ FailureKind ClassVerifier::VerifyClass(Thread* self, /*need_precise_constants=*/ false, api_level, Runtime::Current()->IsAotCompiler(), + can_allocate, &hard_failure_msg); if (result.kind == FailureKind::kHardFailure) { if (failure_data.kind == FailureKind::kHardFailure) { diff --git a/runtime/verifier/class_verifier.h b/runtime/verifier/class_verifier.h index db5e4f592d..c97ea24799 100644 --- a/runtime/verifier/class_verifier.h +++ b/runtime/verifier/class_verifier.h @@ -50,6 +50,14 @@ namespace verifier { // Verifier that ensures the complete class is OK. class ClassVerifier { public: + // Redo verification on a loaded class. This is for use by class redefinition. Since the class is + // already loaded and in use this can only be performed with the mutator lock held. + static FailureKind ReverifyClass(Thread* self, + ObjPtr klass, + HardFailLogMode log_level, + uint32_t api_level, + std::string* error) + REQUIRES(Locks::mutator_lock_); // Verify a class. Returns "kNoFailure" on success. static FailureKind VerifyClass(Thread* self, ObjPtr klass, @@ -70,6 +78,18 @@ class ClassVerifier { uint32_t api_level, std::string* error) REQUIRES_SHARED(Locks::mutator_lock_); + static FailureKind VerifyClass(Thread* self, + const DexFile* dex_file, + Handle dex_cache, + Handle class_loader, + const dex::ClassDef& class_def, + CompilerCallbacks* callbacks, + bool allow_soft_failures, + HardFailLogMode log_level, + uint32_t api_level, + bool can_allocate, + std::string* error) + REQUIRES_SHARED(Locks::mutator_lock_); static void Init(ClassLinker* class_linker) REQUIRES_SHARED(Locks::mutator_lock_); static void Shutdown(); @@ -78,6 +98,16 @@ class ClassVerifier { REQUIRES_SHARED(Locks::mutator_lock_); private: + static FailureKind CommonVerifyClass(Thread* self, + ObjPtr klass, + CompilerCallbacks* callbacks, + bool allow_soft_failures, + HardFailLogMode log_level, + uint32_t api_level, + bool can_allocate, + std::string* error) + REQUIRES_SHARED(Locks::mutator_lock_); + DISALLOW_COPY_AND_ASSIGN(ClassVerifier); }; diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc index 839491ec3e..29bc40c30c 100644 --- a/runtime/verifier/method_verifier.cc +++ b/runtime/verifier/method_verifier.cc @@ -5112,6 +5112,7 @@ MethodVerifier::FailureData MethodVerifier::VerifyMethod(Thread* self, bool need_precise_constants, uint32_t api_level, bool aot_mode, + bool allow_suspension, std::string* hard_failure_msg) { if (VLOG_IS_ON(verifier_debug)) { return VerifyMethod(self, @@ -5131,6 +5132,7 @@ MethodVerifier::FailureData MethodVerifier::VerifyMethod(Thread* self, need_precise_constants, api_level, aot_mode, + allow_suspension, hard_failure_msg); } else { return VerifyMethod(self, @@ -5150,6 +5152,7 @@ MethodVerifier::FailureData MethodVerifier::VerifyMethod(Thread* self, need_precise_constants, api_level, aot_mode, + allow_suspension, hard_failure_msg); } } @@ -5172,6 +5175,7 @@ MethodVerifier::FailureData MethodVerifier::VerifyMethod(Thread* self, bool need_precise_constants, uint32_t api_level, bool aot_mode, + bool allow_suspension, std::string* hard_failure_msg) { MethodVerifier::FailureData result; uint64_t start_ns = kTimeVerifyMethod ? NanoTime() : 0; @@ -5182,8 +5186,8 @@ MethodVerifier::FailureData MethodVerifier::VerifyMethod(Thread* self, dex_file, code_item, method_idx, - /* can_load_classes= */ true, - /* allow_thread_suspension= */ true, + /* can_load_classes= */ allow_suspension, + /* allow_thread_suspension= */ allow_suspension, allow_soft_failures, aot_mode, dex_cache, diff --git a/runtime/verifier/method_verifier.h b/runtime/verifier/method_verifier.h index aab6ee55c1..09d384a069 100644 --- a/runtime/verifier/method_verifier.h +++ b/runtime/verifier/method_verifier.h @@ -253,6 +253,7 @@ class MethodVerifier { bool need_precise_constants, uint32_t api_level, bool aot_mode, + bool allow_suspension, std::string* hard_failure_msg) REQUIRES_SHARED(Locks::mutator_lock_); @@ -274,6 +275,7 @@ class MethodVerifier { bool need_precise_constants, uint32_t api_level, bool aot_mode, + bool allow_suspension, std::string* hard_failure_msg) REQUIRES_SHARED(Locks::mutator_lock_); -- cgit v1.2.3-59-g8ed1b