diff options
22 files changed, 511 insertions, 67 deletions
diff --git a/openjdkjvmti/ti_redefine.cc b/openjdkjvmti/ti_redefine.cc index df25261654..05d7de7f10 100644 --- a/openjdkjvmti/ti_redefine.cc +++ b/openjdkjvmti/ti_redefine.cc @@ -663,6 +663,11 @@ jvmtiError Redefiner::RedefineClassesDirect(ArtJvmTiEnv* env, // We don't actually need to do anything. Just return OK. return OK; } + // We need to fiddle with the verification class flags. To do this we need to make sure there are + // no concurrent redefinitions of the same class at the same time. For simplicity and because + // this is not expected to be a common occurrence we will just wrap the whole thing in a TOP-level + // lock. + // Stop JIT for the duration of this redefine since the JIT might concurrently compile a method we // are going to redefine. // TODO We should prevent user-code suspensions to make sure this isn't held for too long. @@ -1863,31 +1868,33 @@ jvmtiError Redefiner::Run() { } UnregisterAllBreakpoints(); - // Disable GC and wait for it to be done if we are a moving GC. This is fine since we are done - // allocating so no deadlocks. - ScopedDisableConcurrentAndMovingGc sdcamgc(runtime_->GetHeap(), self_); - - // Do transition to final suspension - // TODO We might want to give this its own suspended state! - // TODO This isn't right. We need to change state without any chance of suspend ideally! - art::ScopedThreadSuspension sts(self_, art::ThreadState::kNative); - art::ScopedSuspendAll ssa("Final installation of redefined Classes!", /*long_suspend=*/true); - for (RedefinitionDataIter data = holder.begin(); data != holder.end(); ++data) { - art::ScopedAssertNoThreadSuspension nts("Updating runtime objects for redefinition"); - ClassRedefinition& redef = data.GetRedefinition(); - if (data.GetSourceClassLoader() != nullptr) { - ClassLoaderHelper::UpdateJavaDexFile(data.GetJavaDexFile(), data.GetNewDexFileCookie()); + { + // Disable GC and wait for it to be done if we are a moving GC. This is fine since we are done + // allocating so no deadlocks. + ScopedDisableConcurrentAndMovingGc sdcamgc(runtime_->GetHeap(), self_); + + // Do transition to final suspension + // TODO We might want to give this its own suspended state! + // TODO This isn't right. We need to change state without any chance of suspend ideally! + art::ScopedThreadSuspension sts(self_, art::ThreadState::kNative); + art::ScopedSuspendAll ssa("Final installation of redefined Classes!", /*long_suspend=*/true); + for (RedefinitionDataIter data = holder.begin(); data != holder.end(); ++data) { + art::ScopedAssertNoThreadSuspension nts("Updating runtime objects for redefinition"); + ClassRedefinition& redef = data.GetRedefinition(); + if (data.GetSourceClassLoader() != nullptr) { + ClassLoaderHelper::UpdateJavaDexFile(data.GetJavaDexFile(), data.GetNewDexFileCookie()); + } + redef.UpdateClass(data); } - redef.UpdateClass(data); - } - RestoreObsoleteMethodMapsIfUnneeded(holder); - // TODO We should check for if any of the redefined methods are intrinsic methods here and, if any - // are, force a full-world deoptimization before finishing redefinition. If we don't do this then - // methods that have been jitted prior to the current redefinition being applied might continue - // to use the old versions of the intrinsics! - // TODO Do the dex_file release at a more reasonable place. This works but it muddles who really - // owns the DexFile and when ownership is transferred. - ReleaseAllDexFiles(); + RestoreObsoleteMethodMapsIfUnneeded(holder); + // TODO We should check for if any of the redefined methods are intrinsic methods here and, if + // any are, force a full-world deoptimization before finishing redefinition. If we don't do this + // then methods that have been jitted prior to the current redefinition being applied might + // continue to use the old versions of the intrinsics! + // TODO Do the dex_file release at a more reasonable place. This works but it muddles who really + // owns the DexFile and when ownership is transferred. + ReleaseAllDexFiles(); + } // By now the class-linker knows about all the classes so we can safetly retry verification and // update method flags. ReverifyClasses(holder); @@ -1895,7 +1902,6 @@ jvmtiError Redefiner::Run() { } void Redefiner::ReverifyClasses(RedefinitionDataHolder& holder) { - art::ScopedAssertNoThreadSuspension nts("Updating method flags"); for (RedefinitionDataIter data = holder.begin(); data != holder.end(); ++data) { data.GetRedefinition().ReverifyClass(data); } @@ -2208,6 +2214,25 @@ void Redefiner::ClassRedefinition::UpdateClass(const RedefinitionDataIter& holde } else { UpdateClassInPlace(holder); } + UpdateClassCommon(holder); +} + +void Redefiner::ClassRedefinition::UpdateClassCommon(const RedefinitionDataIter &cur_data) { + // NB This is after we've already replaced all old-refs with new-refs in the structural case. + art::ObjPtr<art::mirror::Class> klass(cur_data.GetMirrorClass()); + DCHECK(!IsStructuralRedefinition() || klass == cur_data.GetNewClassObject()); + if (!needs_reverify_) { + return; + } + // Force the most restrictive interpreter environment. We don't know what the final verification + // will allow. We will clear these after retrying verification once we drop the mutator-lock. + klass->VisitMethods([](art::ArtMethod* m) REQUIRES_SHARED(art::Locks::mutator_lock_) { + if (!m->IsNative() && m->IsInvokable() && !m->IsObsolete()) { + m->ClearSkipAccessChecks(); + m->SetDontCompile(); + m->SetMustCountLocks(); + } + }, art::kRuntimePointerSize); } // Restores the old obsolete methods maps if it turns out they weren't needed (ie there were no new diff --git a/openjdkjvmti/ti_redefine.h b/openjdkjvmti/ti_redefine.h index dad085de59..b028dee8a2 100644 --- a/openjdkjvmti/ti_redefine.h +++ b/openjdkjvmti/ti_redefine.h @@ -209,9 +209,12 @@ class Redefiner { void UpdateClass(const RedefinitionDataIter& cur_data) REQUIRES(art::Locks::mutator_lock_); - void ReverifyClass(const RedefinitionDataIter& cur_data) + void UpdateClassCommon(const RedefinitionDataIter& cur_data) REQUIRES(art::Locks::mutator_lock_); + void ReverifyClass(const RedefinitionDataIter& cur_data) + REQUIRES_SHARED(art::Locks::mutator_lock_); + void CollectNewFieldAndMethodMappings(const RedefinitionDataIter& data, std::map<art::ArtMethod*, art::ArtMethod*>* method_map, std::map<art::ArtField*, art::ArtField*>* field_map) @@ -301,7 +304,7 @@ class Redefiner { bool FinishAllRemainingAllocations(RedefinitionDataHolder& holder) REQUIRES_SHARED(art::Locks::mutator_lock_); void ReleaseAllDexFiles() REQUIRES_SHARED(art::Locks::mutator_lock_); - void ReverifyClasses(RedefinitionDataHolder& holder) REQUIRES(art::Locks::mutator_lock_); + void ReverifyClasses(RedefinitionDataHolder& holder) REQUIRES_SHARED(art::Locks::mutator_lock_); void UnregisterAllBreakpoints() REQUIRES_SHARED(art::Locks::mutator_lock_); // Restores the old obsolete methods maps if it turns out they weren't needed (ie there were no // new obsolete methods). diff --git a/runtime/art_method.h b/runtime/art_method.h index d4fb5d7b90..9647b41f3f 100644 --- a/runtime/art_method.h +++ b/runtime/art_method.h @@ -239,6 +239,10 @@ class ArtMethod final { return (GetAccessFlags() & kAccCompileDontBother) == 0; } + void ClearDontCompile() { + DCHECK(!IsMiranda()); + ClearAccessFlags(kAccCompileDontBother); + } void SetDontCompile() { DCHECK(!IsMiranda()); AddAccessFlags(kAccCompileDontBother); @@ -366,6 +370,10 @@ class ArtMethod final { return (GetAccessFlags() & kAccMustCountLocks) != 0; } + void ClearMustCountLocks() { + ClearAccessFlags(kAccMustCountLocks); + } + void SetMustCountLocks() { AddAccessFlags(kAccMustCountLocks); ClearAccessFlags(kAccSkipAccessChecks); diff --git a/runtime/mirror/class.cc b/runtime/mirror/class.cc index a0e8a237c5..e1dd54f6f7 100644 --- a/runtime/mirror/class.cc +++ b/runtime/mirror/class.cc @@ -1040,6 +1040,24 @@ void Class::ClearSkipAccessChecksFlagOnAllMethods(PointerSize pointer_size) { } } +void Class::ClearMustCountLocksFlagOnAllMethods(PointerSize pointer_size) { + DCHECK(IsVerified()); + for (auto& m : GetMethods(pointer_size)) { + if (!m.IsNative() && m.IsInvokable()) { + m.ClearMustCountLocks(); + } + } +} + +void Class::ClearDontCompileFlagOnAllMethods(PointerSize pointer_size) { + DCHECK(IsVerified()); + for (auto& m : GetMethods(pointer_size)) { + if (!m.IsNative() && m.IsInvokable()) { + m.ClearDontCompile(); + } + } +} + 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 d925a96d9c..15f7dc6177 100644 --- a/runtime/mirror/class.h +++ b/runtime/mirror/class.h @@ -1134,6 +1134,13 @@ class MANAGED Class final : public Object { static ObjPtr<mirror::Class> GetPrimitiveClass(ObjPtr<mirror::String> name) REQUIRES_SHARED(Locks::mutator_lock_); + // Clear the kAccMustCountLocks flag on each method, for class redefinition. + void ClearMustCountLocksFlagOnAllMethods(PointerSize pointer_size) + REQUIRES_SHARED(Locks::mutator_lock_); + // Clear the kAccCompileDontBother flag on each method, for class redefinition. + void ClearDontCompileFlagOnAllMethods(PointerSize pointer_size) + 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_); diff --git a/runtime/verifier/class_verifier.cc b/runtime/verifier/class_verifier.cc index fca9e16450..ed83652c8f 100644 --- a/runtime/verifier/class_verifier.cc +++ b/runtime/verifier/class_verifier.cc @@ -21,6 +21,8 @@ #include "art_method-inl.h" #include "base/enums.h" +#include "base/locks.h" +#include "base/logging.h" #include "base/systrace.h" #include "base/utils.h" #include "class_linker.h" @@ -36,6 +38,8 @@ #include "mirror/dex_cache.h" #include "runtime.h" #include "thread.h" +#include "verifier/method_verifier.h" +#include "verifier/reg_type_cache.h" namespace art { namespace verifier { @@ -46,6 +50,20 @@ using android::base::StringPrintf; // sure we only print this once. static bool gPrintedDxMonitorText = false; +class StandardVerifyCallback : public VerifierCallback { + public: + void SetDontCompile(ArtMethod* m, bool value) override REQUIRES_SHARED(Locks::mutator_lock_) { + if (value) { + m->SetDontCompile(); + } + } + void SetMustCountLocks(ArtMethod* m, bool value) override REQUIRES_SHARED(Locks::mutator_lock_) { + if (value) { + m->SetMustCountLocks(); + } + } +}; + FailureKind ClassVerifier::ReverifyClass(Thread* self, ObjPtr<mirror::Class> klass, HardFailLogMode log_level, @@ -54,19 +72,58 @@ FailureKind ClassVerifier::ReverifyClass(Thread* self, DCHECK(!Runtime::Current()->IsAotCompiler()); StackHandleScope<1> hs(self); Handle<mirror::Class> h_klass(hs.NewHandle(klass)); - ScopedAssertNoThreadSuspension sants(__FUNCTION__); + // We don't want to mess with these while other mutators are possibly looking at them. Instead we + // will wait until we can update them while everything is suspended. + class DelayedVerifyCallback : public VerifierCallback { + public: + void SetDontCompile(ArtMethod* m, bool value) override REQUIRES_SHARED(Locks::mutator_lock_) { + dont_compiles_.push_back({ m, value }); + } + void SetMustCountLocks(ArtMethod* m, bool value) override + REQUIRES_SHARED(Locks::mutator_lock_) { + count_locks_.push_back({ m, value }); + } + void UpdateFlags(bool skip_access_checks) REQUIRES(Locks::mutator_lock_) { + for (auto it : count_locks_) { + VLOG(verifier_debug) << "Setting " << it.first->PrettyMethod() << " count locks to " + << it.second; + if (it.second) { + it.first->SetMustCountLocks(); + } else { + it.first->ClearMustCountLocks(); + } + if (skip_access_checks && it.first->IsInvokable() && !it.first->IsNative()) { + it.first->SetSkipAccessChecks(); + } + } + for (auto it : dont_compiles_) { + VLOG(verifier_debug) << "Setting " << it.first->PrettyMethod() << " dont-compile to " + << it.second; + if (it.second) { + it.first->SetDontCompile(); + } else { + it.first->ClearDontCompile(); + } + } + } + + private: + std::vector<std::pair<ArtMethod*, bool>> dont_compiles_; + std::vector<std::pair<ArtMethod*, bool>> count_locks_; + }; + DelayedVerifyCallback dvc; FailureKind res = CommonVerifyClass(self, h_klass.Get(), /*callbacks=*/nullptr, + &dvc, /*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); - } + DCHECK_NE(res, FailureKind::kHardFailure); + ScopedThreadSuspension sts(Thread::Current(), ThreadState::kSuspended); + ScopedSuspendAll ssa("Update method flags for reverify"); + dvc.UpdateFlags(res == FailureKind::kNoFailure); return res; } @@ -80,22 +137,24 @@ FailureKind ClassVerifier::VerifyClass(Thread* self, if (klass->IsVerified()) { return FailureKind::kNoFailure; } + StandardVerifyCallback svc; return CommonVerifyClass(self, klass, callbacks, + &svc, allow_soft_failures, log_level, api_level, - /*can_allocate=*/ true, error); } + FailureKind ClassVerifier::CommonVerifyClass(Thread* self, ObjPtr<mirror::Class> klass, CompilerCallbacks* callbacks, + VerifierCallback* verifier_callback, 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; @@ -130,13 +189,14 @@ FailureKind ClassVerifier::CommonVerifyClass(Thread* self, class_loader, *class_def, callbacks, + verifier_callback, allow_soft_failures, log_level, api_level, - can_allocate, error); } + FailureKind ClassVerifier::VerifyClass(Thread* self, const DexFile* dex_file, Handle<mirror::DexCache> dex_cache, @@ -147,16 +207,17 @@ FailureKind ClassVerifier::VerifyClass(Thread* self, HardFailLogMode log_level, uint32_t api_level, std::string* error) { + StandardVerifyCallback svc; return VerifyClass(self, dex_file, dex_cache, class_loader, class_def, callbacks, + &svc, allow_soft_failures, log_level, api_level, - /*can_allocate=*/ true, error); } @@ -166,10 +227,10 @@ FailureKind ClassVerifier::VerifyClass(Thread* self, Handle<mirror::ClassLoader> class_loader, const dex::ClassDef& class_def, CompilerCallbacks* callbacks, + VerifierCallback* verifier_callback, 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)) { @@ -220,12 +281,12 @@ FailureKind ClassVerifier::VerifyClass(Thread* self, resolved_method, method.GetAccessFlags(), callbacks, + verifier_callback, allow_soft_failures, log_level, /*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 c97ea24799..0b229662e8 100644 --- a/runtime/verifier/class_verifier.h +++ b/runtime/verifier/class_verifier.h @@ -25,6 +25,8 @@ #include "base/locks.h" #include "handle.h" #include "obj_ptr.h" +#include "verifier/method_verifier.h" +#include "verifier/reg_type_cache.h" #include "verifier_enums.h" namespace art { @@ -50,14 +52,16 @@ 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. + // Redo verification on a loaded class. This is for use by class redefinition. This must be called + // with all methods already having all of kAccDontCompile and kAccCountLocks and not having + // kAccSkipAccessChecks. This will remove some of these flags from the method. The caller must + // ensure this cannot race with other changes to the verification class flags. static FailureKind ReverifyClass(Thread* self, ObjPtr<mirror::Class> klass, HardFailLogMode log_level, uint32_t api_level, std::string* error) - REQUIRES(Locks::mutator_lock_); + REQUIRES_SHARED(Locks::mutator_lock_); // Verify a class. Returns "kNoFailure" on success. static FailureKind VerifyClass(Thread* self, ObjPtr<mirror::Class> klass, @@ -78,18 +82,6 @@ class ClassVerifier { uint32_t api_level, std::string* error) REQUIRES_SHARED(Locks::mutator_lock_); - static FailureKind VerifyClass(Thread* self, - const DexFile* dex_file, - Handle<mirror::DexCache> dex_cache, - Handle<mirror::ClassLoader> 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(); @@ -101,13 +93,25 @@ class ClassVerifier { static FailureKind CommonVerifyClass(Thread* self, ObjPtr<mirror::Class> klass, CompilerCallbacks* callbacks, + VerifierCallback* verifier_callback, bool allow_soft_failures, HardFailLogMode log_level, uint32_t api_level, - bool can_allocate, std::string* error) REQUIRES_SHARED(Locks::mutator_lock_); + static FailureKind VerifyClass(Thread* self, + const DexFile* dex_file, + Handle<mirror::DexCache> dex_cache, + Handle<mirror::ClassLoader> class_loader, + const dex::ClassDef& class_def, + CompilerCallbacks* callbacks, + VerifierCallback* verifier_callback, + bool allow_soft_failures, + HardFailLogMode log_level, + uint32_t api_level, + 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 29bc40c30c..742f50dec8 100644 --- a/runtime/verifier/method_verifier.cc +++ b/runtime/verifier/method_verifier.cc @@ -5107,12 +5107,12 @@ MethodVerifier::FailureData MethodVerifier::VerifyMethod(Thread* self, ArtMethod* method, uint32_t method_access_flags, CompilerCallbacks* callbacks, + VerifierCallback* verifier_callback, bool allow_soft_failures, HardFailLogMode log_level, 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<true>(self, @@ -5127,12 +5127,12 @@ MethodVerifier::FailureData MethodVerifier::VerifyMethod(Thread* self, method, method_access_flags, callbacks, + verifier_callback, allow_soft_failures, log_level, need_precise_constants, api_level, aot_mode, - allow_suspension, hard_failure_msg); } else { return VerifyMethod<false>(self, @@ -5147,12 +5147,12 @@ MethodVerifier::FailureData MethodVerifier::VerifyMethod(Thread* self, method, method_access_flags, callbacks, + verifier_callback, allow_soft_failures, log_level, need_precise_constants, api_level, aot_mode, - allow_suspension, hard_failure_msg); } } @@ -5170,12 +5170,12 @@ MethodVerifier::FailureData MethodVerifier::VerifyMethod(Thread* self, ArtMethod* method, uint32_t method_access_flags, CompilerCallbacks* callbacks, + VerifierCallback* verifier_callback, bool allow_soft_failures, HardFailLogMode log_level, 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; @@ -5186,8 +5186,8 @@ MethodVerifier::FailureData MethodVerifier::VerifyMethod(Thread* self, dex_file, code_item, method_idx, - /* can_load_classes= */ allow_suspension, - /* allow_thread_suspension= */ allow_suspension, + /* can_load_classes= */ true, + /* allow_thread_suspension= */ true, allow_soft_failures, aot_mode, dex_cache, @@ -5209,6 +5209,7 @@ MethodVerifier::FailureData MethodVerifier::VerifyMethod(Thread* self, callbacks->MethodVerified(&verifier); } + bool set_dont_compile = false; if (verifier.failures_.size() != 0) { if (VLOG_IS_ON(verifier)) { verifier.DumpFailures(VLOG_STREAM(verifier) << "Soft verification failures in " @@ -5221,12 +5222,12 @@ MethodVerifier::FailureData MethodVerifier::VerifyMethod(Thread* self, result.kind = FailureKind::kSoftFailure; if (method != nullptr && !CanCompilerHandleVerificationFailure(verifier.encountered_failure_types_)) { - method->SetDontCompile(); + set_dont_compile = true; } } if (method != nullptr) { if (verifier.HasInstructionThatWillThrow()) { - method->SetDontCompile(); + set_dont_compile = true; if (aot_mode && (callbacks != nullptr) && !callbacks->IsBootImage()) { // When compiling apps, make HasInstructionThatWillThrow a soft error to trigger // re-verification at runtime. @@ -5240,9 +5241,12 @@ MethodVerifier::FailureData MethodVerifier::VerifyMethod(Thread* self, result.kind = FailureKind::kSoftFailure; } } + bool must_count_locks = false; if ((verifier.encountered_failure_types_ & VerifyError::VERIFY_ERROR_LOCKING) != 0) { - method->SetMustCountLocks(); + must_count_locks = true; } + verifier_callback->SetDontCompile(method, set_dont_compile); + verifier_callback->SetMustCountLocks(method, must_count_locks); } } else { // Bad method data. diff --git a/runtime/verifier/method_verifier.h b/runtime/verifier/method_verifier.h index 09d384a069..83dafd3975 100644 --- a/runtime/verifier/method_verifier.h +++ b/runtime/verifier/method_verifier.h @@ -72,6 +72,16 @@ enum RegisterTrackingMode { kTrackRegsAll, }; +// A class used by the verifier to tell users about what options need to be set for given methods. +class VerifierCallback { + public: + virtual ~VerifierCallback() {} + virtual void SetDontCompile(ArtMethod* method, bool value) + REQUIRES_SHARED(Locks::mutator_lock_) = 0; + virtual void SetMustCountLocks(ArtMethod* method, bool value) + REQUIRES_SHARED(Locks::mutator_lock_) = 0; +}; + // A mapping from a dex pc to the register line statuses as they are immediately prior to the // execution of that instruction. class PcToRegisterLineTable { @@ -248,12 +258,12 @@ class MethodVerifier { ArtMethod* method, uint32_t method_access_flags, CompilerCallbacks* callbacks, + VerifierCallback* verifier_callback, bool allow_soft_failures, HardFailLogMode log_level, bool need_precise_constants, uint32_t api_level, bool aot_mode, - bool allow_suspension, std::string* hard_failure_msg) REQUIRES_SHARED(Locks::mutator_lock_); @@ -270,12 +280,12 @@ class MethodVerifier { ArtMethod* method, uint32_t method_access_flags, CompilerCallbacks* callbacks, + VerifierCallback* verifier_callback, bool allow_soft_failures, HardFailLogMode log_level, bool need_precise_constants, uint32_t api_level, bool aot_mode, - bool allow_suspension, std::string* hard_failure_msg) REQUIRES_SHARED(Locks::mutator_lock_); diff --git a/test/1990-structural-bad-verify/expected.txt b/test/1990-structural-bad-verify/expected.txt new file mode 100644 index 0000000000..7478dda33a --- /dev/null +++ b/test/1990-structural-bad-verify/expected.txt @@ -0,0 +1,2 @@ +hello +I say hello and you say goodbye! diff --git a/test/1990-structural-bad-verify/info.txt b/test/1990-structural-bad-verify/info.txt new file mode 100644 index 0000000000..f2ecd68701 --- /dev/null +++ b/test/1990-structural-bad-verify/info.txt @@ -0,0 +1,6 @@ +Tests basic functions in the jvmti plugin. + +b/142876078 + +This tests a crash that could occur which was caused by the dex-cache being in an unexpected +state. diff --git a/test/1990-structural-bad-verify/run b/test/1990-structural-bad-verify/run new file mode 100755 index 0000000000..03e41a58e7 --- /dev/null +++ b/test/1990-structural-bad-verify/run @@ -0,0 +1,17 @@ +#!/bin/bash +# +# Copyright 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. + +./default-run "$@" --jvmti --runtime-option -Xopaque-jni-ids:true diff --git a/test/1990-structural-bad-verify/src/Main.java b/test/1990-structural-bad-verify/src/Main.java new file mode 100644 index 0000000000..5622925df1 --- /dev/null +++ b/test/1990-structural-bad-verify/src/Main.java @@ -0,0 +1,21 @@ +/* + * Copyright (C) 2017 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 { + art.Test1990.run(); + } +} diff --git a/test/1990-structural-bad-verify/src/art/Redefinition.java b/test/1990-structural-bad-verify/src/art/Redefinition.java new file mode 120000 index 0000000000..81eaf31bbb --- /dev/null +++ b/test/1990-structural-bad-verify/src/art/Redefinition.java @@ -0,0 +1 @@ +../../../jvmti-common/Redefinition.java
\ No newline at end of file diff --git a/test/1990-structural-bad-verify/src/art/Test1990.java b/test/1990-structural-bad-verify/src/art/Test1990.java new file mode 100644 index 0000000000..90034fca55 --- /dev/null +++ b/test/1990-structural-bad-verify/src/art/Test1990.java @@ -0,0 +1,123 @@ +/* + * 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. + */ + +package art; + +import java.util.Base64; +public class Test1990 { + + static class Transform { + public static void saySomething() { + System.out.println("hello"); + } + } + + /** + * base64 encoded class/dex file for + * static class Transform { + * public static void saySomething() { + * System.out.println("I say hello and " + sayGoodbye()); + * } + * public static String sayGoodbye() { + * return "you say goodbye!"; + * } + * } + */ + // NB The actual dex codes are as follows. This is an explanation of the error this test checks. + // + // The exact order of instructions is important. Notice the 'invoke-static sayGoodbye' + // (instruction 0002) dominates the rest of the block. During the first (runnable) verification + // step the verifier will first check and verify there are no hard-failures in this class. Next it + // will realize it cannot find the sayGoodbye method on the loaded & resolved Transform class. + // This is (correctly) recognized as a soft-verification failure but then the verifier decides the + // rest of the method is dead-code. This means the verifier will not perform any of the + // soft-failure checks on the rest of the method (since control would never reach there). + // + // Later after performing the redefinition we do a reverify. At this time we held an exclusive + // mutator-lock though so it cannot resolve classes and will not add anything to the dex-cache. + // Here we can get past instruction 0002 and successfully determine the rest of the function is + // fine. In the process we filled in the methods into the dex-cache but not the classes. This + // caused this test to crash when run through the interpreter. + // + // #2 : (in Lart/Test1990$Transform;) + // name : 'saySomething' + // type : '()V' + // access : 0x0009 (PUBLIC STATIC) + // code - + // registers : 4 + // ins : 0 + // outs : 2 + // insns size : 27 16-bit code units + // 0001d0: |[0001d0] art.Test1990$Transform.saySomething:()V + // 0001e0: 6200 0000 |0000: sget-object v0, Ljava/lang/System;.out:Ljava/io/PrintStream; // field@0000 + // 0001e4: 7100 0100 0000 |0002: invoke-static {}, Lart/Test1990$Transform;.sayGoodbye:()Ljava/lang/String; // method@0001 + // 0001ea: 0c01 |0005: move-result-object v1 + // 0001ec: 2202 0700 |0006: new-instance v2, Ljava/lang/StringBuilder; // type@0007 + // 0001f0: 7010 0500 0200 |0008: invoke-direct {v2}, Ljava/lang/StringBuilder;.<init>:()V // method@0005 + // 0001f6: 1a03 0100 |000b: const-string v3, "I say hello and " // string@0001 + // 0001fa: 6e20 0600 3200 |000d: invoke-virtual {v2, v3}, Ljava/lang/StringBuilder;.append:(Ljava/lang/String;)Ljava/lang/StringBuilder; // method@0006 + // 000200: 6e20 0600 1200 |0010: invoke-virtual {v2, v1}, Ljava/lang/StringBuilder;.append:(Ljava/lang/String;)Ljava/lang/StringBuilder; // method@0006 + // 000206: 6e10 0700 0200 |0013: invoke-virtual {v2}, Ljava/lang/StringBuilder;.toString:()Ljava/lang/String; // method@0007 + // 00020c: 0c01 |0016: move-result-object v1 + // 00020e: 6e20 0300 1000 |0017: invoke-virtual {v0, v1}, Ljava/io/PrintStream;.println:(Ljava/lang/String;)V // method@0003 + // 000214: 0e00 |001a: return-void + // catches : (none) + // positions : + // 0x0000 line=5 + // 0x001a line=6 + // locals : + + // Virtual methods - + // source_file_idx : 13 (Test1990.java) + private static final byte[] DEX_BYTES = Base64.getDecoder().decode( +"ZGV4CjAzNQCV0LekDslEGFglxYgCw7HSyxVegIDjERswBQAAcAAAAHhWNBIAAAAAAAAAAGwEAAAc" + +"AAAAcAAAAAoAAADgAAAABAAAAAgBAAABAAAAOAEAAAgAAABAAQAAAQAAAIABAACQAwAAoAEAAC4C" + +"AAA2AgAASAIAAEsCAABPAgAAaQIAAHkCAACdAgAAvQIAANQCAADoAgAA/AIAABcDAAArAwAAOgMA" + +"AEUDAABIAwAATAMAAFkDAABhAwAAZwMAAGwDAAB1AwAAgQMAAI8DAACZAwAAoAMAALIDAAAEAAAA" + +"BQAAAAYAAAAHAAAACAAAAAkAAAAKAAAACwAAAAwAAAAPAAAAAgAAAAYAAAAAAAAAAwAAAAcAAAAo" + +"AgAADwAAAAkAAAAAAAAAEAAAAAkAAAAoAgAACAAEABQAAAAAAAIAAAAAAAAAAAAWAAAAAAACABcA" + +"AAAEAAMAFQAAAAUAAgAAAAAABwACAAAAAAAHAAEAEgAAAAcAAAAYAAAAAAAAAAAAAAAFAAAAAAAA" + +"AA0AAABcBAAAOQQAAAAAAAABAAAAAAAAABoCAAADAAAAGgAaABEAAAABAAEAAQAAABYCAAAEAAAA" + +"cBAEAAAADgAEAAAAAgAAAB4CAAAbAAAAYgAAAHEAAQAAAAwBIgIHAHAQBQACABoDAQBuIAYAMgBu" + +"IAYAEgBuEAcAAgAMAW4gAwAQAA4AAwAOAAgADgAFAA4BGg8AAAAAAQAAAAYABjxpbml0PgAQSSBz" + +"YXkgaGVsbG8gYW5kIAABTAACTEwAGExhcnQvVGVzdDE5OTAkVHJhbnNmb3JtOwAOTGFydC9UZXN0" + +"MTk5MDsAIkxkYWx2aWsvYW5ub3RhdGlvbi9FbmNsb3NpbmdDbGFzczsAHkxkYWx2aWsvYW5ub3Rh" + +"dGlvbi9Jbm5lckNsYXNzOwAVTGphdmEvaW8vUHJpbnRTdHJlYW07ABJMamF2YS9sYW5nL09iamVj" + +"dDsAEkxqYXZhL2xhbmcvU3RyaW5nOwAZTGphdmEvbGFuZy9TdHJpbmdCdWlsZGVyOwASTGphdmEv" + +"bGFuZy9TeXN0ZW07AA1UZXN0MTk5MC5qYXZhAAlUcmFuc2Zvcm0AAVYAAlZMAAthY2Nlc3NGbGFn" + +"cwAGYXBwZW5kAARuYW1lAANvdXQAB3ByaW50bG4ACnNheUdvb2RieWUADHNheVNvbWV0aGluZwAI" + +"dG9TdHJpbmcABXZhbHVlABB5b3Ugc2F5IGdvb2RieWUhAHZ+fkQ4eyJjb21waWxhdGlvbi1tb2Rl" + +"IjoiZGVidWciLCJtaW4tYXBpIjoxLCJzaGEtMSI6IjYwZGE0ZDY3YjM4MWM0MjQ2Nzc1N2M0OWZi" + +"NmU1NTc1NmQ4OGEyZjMiLCJ2ZXJzaW9uIjoiMS43LjEyLWRldiJ9AAICARkYAQIDAhEECBMXDgAA" + +"AwAAgIAEuAMBCaADAQnQAwAAAAAAAgAAACoEAAAwBAAAUAQAAAAAAAAAAAAAAAAAABAAAAAAAAAA" + +"AQAAAAAAAAABAAAAHAAAAHAAAAACAAAACgAAAOAAAAADAAAABAAAAAgBAAAEAAAAAQAAADgBAAAF" + +"AAAACAAAAEABAAAGAAAAAQAAAIABAAABIAAAAwAAAKABAAADIAAAAwAAABYCAAABEAAAAQAAACgC" + +"AAACIAAAHAAAAC4CAAAEIAAAAgAAACoEAAAAIAAAAQAAADkEAAADEAAAAgAAAEwEAAAGIAAAAQAA" + +"AFwEAAAAEAAAAQAAAGwEAAA="); + + + + public static void run() throws Exception { + Redefinition.setTestConfiguration(Redefinition.Config.COMMON_REDEFINE); + doTest(new Transform()); + } + + public static void doTest(Transform t) throws Exception { + Transform.saySomething(); + Redefinition.doCommonStructuralClassRedefinition(Transform.class, DEX_BYTES); + Transform.saySomething(); + } +} diff --git a/test/1992-retransform-no-such-field/expected.txt b/test/1992-retransform-no-such-field/expected.txt new file mode 100644 index 0000000000..53de32a31a --- /dev/null +++ b/test/1992-retransform-no-such-field/expected.txt @@ -0,0 +1,2 @@ +This file was written in the year 2019! +This new class was written in 2019 diff --git a/test/1992-retransform-no-such-field/info.txt b/test/1992-retransform-no-such-field/info.txt new file mode 100644 index 0000000000..875a5f6ec1 --- /dev/null +++ b/test/1992-retransform-no-such-field/info.txt @@ -0,0 +1 @@ +Tests basic functions in the jvmti plugin. diff --git a/test/1992-retransform-no-such-field/run b/test/1992-retransform-no-such-field/run new file mode 100755 index 0000000000..c6e62ae6cd --- /dev/null +++ b/test/1992-retransform-no-such-field/run @@ -0,0 +1,17 @@ +#!/bin/bash +# +# Copyright 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. + +./default-run "$@" --jvmti diff --git a/test/1992-retransform-no-such-field/src/Main.java b/test/1992-retransform-no-such-field/src/Main.java new file mode 100644 index 0000000000..ccffb88e7a --- /dev/null +++ b/test/1992-retransform-no-such-field/src/Main.java @@ -0,0 +1,21 @@ +/* + * Copyright (C) 2017 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 { + art.Test1992.run(); + } +} diff --git a/test/1992-retransform-no-such-field/src/art/Redefinition.java b/test/1992-retransform-no-such-field/src/art/Redefinition.java new file mode 120000 index 0000000000..81eaf31bbb --- /dev/null +++ b/test/1992-retransform-no-such-field/src/art/Redefinition.java @@ -0,0 +1 @@ +../../../jvmti-common/Redefinition.java
\ No newline at end of file diff --git a/test/1992-retransform-no-such-field/src/art/Test1992.java b/test/1992-retransform-no-such-field/src/art/Test1992.java new file mode 100644 index 0000000000..5e1d5bb349 --- /dev/null +++ b/test/1992-retransform-no-such-field/src/art/Test1992.java @@ -0,0 +1,89 @@ +/* + * Copyright (C) 2019 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. + */ + +package art; + +import java.util.Base64; +public class Test1992 { + public static boolean FAIL_IT = false; + + static class Transform { + public void saySomething() { + System.out.println("This file was written in the year 2019!"); + } + } + + /** + * base64 encoded class/dex file for + * class Transform { + * public void saySomething() { + * if (Test1992.FAIL_IT) { + * // Force verification soft-fail. + * Test1992.NOT_THERE = 1; + * } + * System.out.println("This new class was written in " + year); + * } + * } + */ + private static final byte[] CLASS_BYTES = Base64.getDecoder().decode( +"yv66vgAAADUAKQoACAARCQASABMJABIAFAkAFQAWCAAXCgAYABkHABoHAB0BAAY8aW5pdD4BAAMo" + +"KVYBAARDb2RlAQAPTGluZU51bWJlclRhYmxlAQAMc2F5U29tZXRoaW5nAQANU3RhY2tNYXBUYWJs" + +"ZQEAClNvdXJjZUZpbGUBAA1UZXN0MTk5Mi5qYXZhDAAJAAoHAB4MAB8AIAwAIQAiBwAjDAAkACUB" + +"ACJUaGlzIG5ldyBjbGFzcyB3YXMgd3JpdHRlbiBpbiAyMDE5BwAmDAAnACgBABZhcnQvVGVzdDE5" + +"OTIkVHJhbnNmb3JtAQAJVHJhbnNmb3JtAQAMSW5uZXJDbGFzc2VzAQAQamF2YS9sYW5nL09iamVj" + +"dAEADGFydC9UZXN0MTk5MgEAB0ZBSUxfSVQBAAFaAQAJTk9UX1RIRVJFAQABSQEAEGphdmEvbGFu" + +"Zy9TeXN0ZW0BAANvdXQBABVMamF2YS9pby9QcmludFN0cmVhbTsBABNqYXZhL2lvL1ByaW50U3Ry" + +"ZWFtAQAHcHJpbnRsbgEAFShMamF2YS9sYW5nL1N0cmluZzspVgAgAAcACAAAAAAAAgAAAAkACgAB" + +"AAsAAAAdAAEAAQAAAAUqtwABsQAAAAEADAAAAAYAAQAAAAUAAQANAAoAAQALAAAAQAACAAEAAAAT" + +"sgACmQAHBLMAA7IABBIFtgAGsQAAAAIADAAAABIABAAAAAkABgAKAAoADAASAA0ADgAAAAMAAQoA" + +"AgAPAAAAAgAQABwAAAAKAAEABwASABsACA=="); + + private static final byte[] DEX_BYTES = Base64.getDecoder().decode( +"ZGV4CjAzNQAWyivK2j0yR4u2YH/R8Bs3KIFv7O/Hs0WkBAAAcAAAAHhWNBIAAAAAAAAAAOADAAAZ" + +"AAAAcAAAAAsAAADUAAAAAgAAAAABAAADAAAAGAEAAAQAAAAwAQAAAQAAAFABAAA0AwAAcAEAAMoB" + +"AADSAQAA2wEAAN4BAAD4AQAACAIAACwCAABMAgAAYwIAAHcCAACLAgAAnwIAAKoCAAC5AgAA3QIA" + +"AOgCAADrAgAA7wIAAPICAAD/AgAABQMAAAoDAAATAwAAIQMAACgDAAACAAAAAwAAAAQAAAAFAAAA" + +"BgAAAAcAAAAIAAAACQAAAAoAAAAPAAAAEQAAAA8AAAAJAAAAAAAAABAAAAAJAAAAxAEAAAIACgAB" + +"AAAAAgAAAAsAAAAIAAUAFAAAAAEAAAAAAAAAAQAAABYAAAAFAAEAFQAAAAYAAAAAAAAAAQAAAAAA" + +"AAAGAAAAAAAAAAwAAADQAwAArwMAAAAAAAABAAEAAQAAALYBAAAEAAAAcBADAAAADgADAAEAAgAA" + +"ALoBAAAPAAAAYwAAADgABQASEGcAAQBiAAIAGgENAG4gAgAQAA4ABQAOAAkADks9eAAAAAABAAAA" + +"BwAGPGluaXQ+AAdGQUlMX0lUAAFJABhMYXJ0L1Rlc3QxOTkyJFRyYW5zZm9ybTsADkxhcnQvVGVz" + +"dDE5OTI7ACJMZGFsdmlrL2Fubm90YXRpb24vRW5jbG9zaW5nQ2xhc3M7AB5MZGFsdmlrL2Fubm90" + +"YXRpb24vSW5uZXJDbGFzczsAFUxqYXZhL2lvL1ByaW50U3RyZWFtOwASTGphdmEvbGFuZy9PYmpl" + +"Y3Q7ABJMamF2YS9sYW5nL1N0cmluZzsAEkxqYXZhL2xhbmcvU3lzdGVtOwAJTk9UX1RIRVJFAA1U" + +"ZXN0MTk5Mi5qYXZhACJUaGlzIG5ldyBjbGFzcyB3YXMgd3JpdHRlbiBpbiAyMDE5AAlUcmFuc2Zv" + +"cm0AAVYAAlZMAAFaAAthY2Nlc3NGbGFncwAEbmFtZQADb3V0AAdwcmludGxuAAxzYXlTb21ldGhp" + +"bmcABXZhbHVlAHZ+fkQ4eyJjb21waWxhdGlvbi1tb2RlIjoiZGVidWciLCJtaW4tYXBpIjoxLCJz" + +"aGEtMSI6IjYwZGE0ZDY3YjM4MWM0MjQ2Nzc1N2M0OWZiNmU1NTc1NmQ4OGEyZjMiLCJ2ZXJzaW9u" + +"IjoiMS43LjEyLWRldiJ9AAIDARcYAgIEAhIECBMXDgAAAQEAgIAE8AIBAYgDAAAAAAAAAAIAAACg" + +"AwAApgMAAMQDAAAAAAAAAAAAAAAAAAAQAAAAAAAAAAEAAAAAAAAAAQAAABkAAABwAAAAAgAAAAsA" + +"AADUAAAAAwAAAAIAAAAAAQAABAAAAAMAAAAYAQAABQAAAAQAAAAwAQAABgAAAAEAAABQAQAAASAA" + +"AAIAAABwAQAAAyAAAAIAAAC2AQAAARAAAAEAAADEAQAAAiAAABkAAADKAQAABCAAAAIAAACgAwAA" + +"ACAAAAEAAACvAwAAAxAAAAIAAADAAwAABiAAAAEAAADQAwAAABAAAAEAAADgAwAA"); + + + + public static void run() { + Redefinition.setTestConfiguration(Redefinition.Config.COMMON_REDEFINE); + doTest(new Transform()); + } + + public static void doTest(Transform t) { + t.saySomething(); + Redefinition.doCommonClassRedefinition(Transform.class, CLASS_BYTES, DEX_BYTES); + t.saySomething(); + } +} diff --git a/test/knownfailures.json b/test/knownfailures.json index 1e00c3ff6a..2e34943401 100644 --- a/test/knownfailures.json +++ b/test/knownfailures.json @@ -1139,7 +1139,10 @@ "1984-structural-redefine-field-trace", "1985-structural-redefine-stack-scope", "1986-structural-redefine-multi-thread-stack-scope", - "1987-structural-redefine-recursive-stack-scope" + "1987-structural-redefine-recursive-stack-scope", + "1989-transform-bad-monitor", + "1990-structural-bad-verify", + "1992-retransform-no-such-field" ], "variant": "jvm", "description": ["Doesn't run on RI."] |