diff options
29 files changed, 386 insertions, 248 deletions
diff --git a/cmdline/cmdline_types.h b/cmdline/cmdline_types.h index 529fe2bcdb..d0d6bfd3ce 100644 --- a/cmdline/cmdline_types.h +++ b/cmdline/cmdline_types.h @@ -328,19 +328,19 @@ struct CmdlineType<std::vector<Plugin>> : CmdlineTypeParser<std::vector<Plugin>> }; template <> -struct CmdlineType<std::list<ti::Agent>> : CmdlineTypeParser<std::list<ti::Agent>> { +struct CmdlineType<std::list<ti::AgentSpec>> : CmdlineTypeParser<std::list<ti::AgentSpec>> { Result Parse(const std::string& args) { assert(false && "Use AppendValues() for an Agent list type"); return Result::Failure("Unconditional failure: Agent list must be appended: " + args); } Result ParseAndAppend(const std::string& args, - std::list<ti::Agent>& existing_value) { + std::list<ti::AgentSpec>& existing_value) { existing_value.emplace_back(args); return Result::SuccessNoValue(); } - static const char* Name() { return "std::list<ti::Agent>"; } + static const char* Name() { return "std::list<ti::AgentSpec>"; } }; template <> diff --git a/compiler/optimizing/code_generator_arm_vixl.cc b/compiler/optimizing/code_generator_arm_vixl.cc index 017598d484..c6e1b042a7 100644 --- a/compiler/optimizing/code_generator_arm_vixl.cc +++ b/compiler/optimizing/code_generator_arm_vixl.cc @@ -1128,7 +1128,7 @@ class LoadReferenceWithBakerReadBarrierAndUpdateFieldSlowPathARMVIXL // // Note that this field could also hold a different object, if // another thread had concurrently changed it. In that case, the - // LDREX/SUBS/ITNE sequence of instructions in the compare-and-set + // LDREX/CMP/BNE sequence of instructions in the compare-and-set // (CAS) operation below would abort the CAS, leaving the field // as-is. __ Cmp(temp1_, ref_reg); @@ -1168,28 +1168,16 @@ class LoadReferenceWithBakerReadBarrierAndUpdateFieldSlowPathARMVIXL // tmp = [r_ptr] - expected; // } while (tmp == 0 && failure([r_ptr] <- r_new_value)); - vixl32::Label loop_head, exit_loop; + vixl32::Label loop_head, comparison_failed, exit_loop; __ Bind(&loop_head); - __ Ldrex(tmp, MemOperand(tmp_ptr)); - - __ Subs(tmp, tmp, expected); - - { - ExactAssemblyScope aas(arm_codegen->GetVIXLAssembler(), - 2 * kMaxInstructionSizeInBytes, - CodeBufferCheckScope::kMaximumSize); - - __ it(ne); - __ clrex(ne); - } - - __ B(ne, &exit_loop, /* far_target */ false); - + __ Cmp(tmp, expected); + __ B(ne, &comparison_failed, /* far_target */ false); __ Strex(tmp, value, MemOperand(tmp_ptr)); - __ Cmp(tmp, 1); - __ B(eq, &loop_head, /* far_target */ false); - + __ CompareAndBranchIfZero(tmp, &exit_loop, /* far_target */ false); + __ B(&loop_head); + __ Bind(&comparison_failed); + __ Clrex(); __ Bind(&exit_loop); if (kPoisonHeapReferences) { diff --git a/dexlayout/compact_dex_writer.cc b/dexlayout/compact_dex_writer.cc index f2d46199a2..f44eaefb6e 100644 --- a/dexlayout/compact_dex_writer.cc +++ b/dexlayout/compact_dex_writer.cc @@ -27,7 +27,9 @@ void CompactDexWriter::WriteHeader() { header.checksum_ = header_->Checksum(); std::copy_n(header_->Signature(), DexFile::kSha1DigestSize, header.signature_); header.file_size_ = header_->FileSize(); - header.header_size_ = header_->GetSize(); + // Since we are not necessarily outputting the same format as the input, avoid using the stored + // header size. + header.header_size_ = GetHeaderSize(); header.endian_tag_ = header_->EndianTag(); header.link_size_ = header_->LinkSize(); header.link_off_ = header_->LinkOffset(); @@ -47,7 +49,17 @@ void CompactDexWriter::WriteHeader() { header.class_defs_off_ = collections.ClassDefsOffset(); header.data_size_ = header_->DataSize(); header.data_off_ = header_->DataOffset(); + header.feature_flags_ = 0u; + // In cases where apps are converted to cdex during install, maintain feature flags so that + // the verifier correctly verifies apps that aren't targetting default methods. + if (header_->SupportDefaultMethods()) { + header.feature_flags_ |= static_cast<uint32_t>(CompactDexFile::FeatureFlags::kDefaultMethods); + } UNUSED(Write(reinterpret_cast<uint8_t*>(&header), sizeof(header), 0u)); } +size_t CompactDexWriter::GetHeaderSize() const { + return sizeof(CompactDexFile::Header); +} + } // namespace art diff --git a/dexlayout/compact_dex_writer.h b/dexlayout/compact_dex_writer.h index e28efab5c1..d13333bb18 100644 --- a/dexlayout/compact_dex_writer.h +++ b/dexlayout/compact_dex_writer.h @@ -35,6 +35,8 @@ class CompactDexWriter : public DexWriter { protected: void WriteHeader() OVERRIDE; + size_t GetHeaderSize() const OVERRIDE; + const CompactDexLevel compact_dex_level_; private: diff --git a/dexlayout/dex_ir.h b/dexlayout/dex_ir.h index b25e1645dd..a54c46fdc1 100644 --- a/dexlayout/dex_ir.h +++ b/dexlayout/dex_ir.h @@ -524,7 +524,8 @@ class Header : public Item { uint32_t link_size, uint32_t link_offset, uint32_t data_size, - uint32_t data_offset) + uint32_t data_offset, + bool support_default_methods) : Item(0, kHeaderItemSize), checksum_(checksum), endian_tag_(endian_tag), @@ -533,7 +534,8 @@ class Header : public Item { link_size_(link_size), link_offset_(link_offset), data_size_(data_size), - data_offset_(data_offset) { + data_offset_(data_offset), + support_default_methods_(support_default_methods) { memcpy(magic_, magic, sizeof(magic_)); memcpy(signature_, signature, sizeof(signature_)); } @@ -567,6 +569,10 @@ class Header : public Item { void Accept(AbstractDispatcher* dispatch) { dispatch->Dispatch(this); } + bool SupportDefaultMethods() const { + return support_default_methods_; + } + private: uint8_t magic_[8]; uint32_t checksum_; @@ -578,6 +584,7 @@ class Header : public Item { uint32_t link_offset_; uint32_t data_size_; uint32_t data_offset_; + const bool support_default_methods_; Collections collections_; diff --git a/dexlayout/dex_ir_builder.cc b/dexlayout/dex_ir_builder.cc index 1fd963fe22..231826b7a8 100644 --- a/dexlayout/dex_ir_builder.cc +++ b/dexlayout/dex_ir_builder.cc @@ -37,7 +37,8 @@ Header* DexIrBuilder(const DexFile& dex_file, bool eagerly_assign_offsets) { disk_header.link_size_, disk_header.link_off_, disk_header.data_size_, - disk_header.data_off_); + disk_header.data_off_, + dex_file.SupportsDefaultMethods()); Collections& collections = header->GetCollections(); collections.SetEagerlyAssignOffsets(eagerly_assign_offsets); // Walk the rest of the header fields. diff --git a/dexlayout/dex_writer.cc b/dexlayout/dex_writer.cc index 4fd4157391..41fdcbd8fb 100644 --- a/dexlayout/dex_writer.cc +++ b/dexlayout/dex_writer.cc @@ -780,7 +780,7 @@ void DexWriter::WriteHeader() { header.checksum_ = header_->Checksum(); std::copy_n(header_->Signature(), DexFile::kSha1DigestSize, header.signature_); header.file_size_ = header_->FileSize(); - header.header_size_ = header_->GetSize(); + header.header_size_ = GetHeaderSize(); header.endian_tag_ = header_->EndianTag(); header.link_size_ = header_->LinkSize(); header.link_off_ = header_->LinkOffset(); @@ -801,13 +801,18 @@ void DexWriter::WriteHeader() { header.data_size_ = header_->DataSize(); header.data_off_ = header_->DataOffset(); + CHECK_EQ(sizeof(header), GetHeaderSize()); static_assert(sizeof(header) == 0x70, "Size doesn't match dex spec"); UNUSED(Write(reinterpret_cast<uint8_t*>(&header), sizeof(header), 0u)); } +size_t DexWriter::GetHeaderSize() const { + return sizeof(StandardDexFile::Header); +} + void DexWriter::WriteMemMap() { // Starting offset is right after the header. - uint32_t offset = sizeof(StandardDexFile::Header); + uint32_t offset = GetHeaderSize(); dex_ir::Collections& collection = header_->GetCollections(); diff --git a/dexlayout/dex_writer.h b/dexlayout/dex_writer.h index c47898e533..39295263f5 100644 --- a/dexlayout/dex_writer.h +++ b/dexlayout/dex_writer.h @@ -91,6 +91,7 @@ class DexWriter { // Header and id section virtual void WriteHeader(); + virtual size_t GetHeaderSize() const; // reserve_only means don't write, only reserve space. This is required since the string data // offsets must be assigned. uint32_t WriteStringIds(uint32_t offset, bool reserve_only); diff --git a/runtime/cdex/compact_dex_file.cc b/runtime/cdex/compact_dex_file.cc index 82ffdb0adb..a92ab28a7a 100644 --- a/runtime/cdex/compact_dex_file.cc +++ b/runtime/cdex/compact_dex_file.cc @@ -46,4 +46,9 @@ bool CompactDexFile::IsVersionValid() const { return IsVersionValid(header_->magic_); } +bool CompactDexFile::SupportsDefaultMethods() const { + return (GetHeader().GetFeatureFlags() & + static_cast<uint32_t>(FeatureFlags::kDefaultMethods)) != 0; +} + } // namespace art diff --git a/runtime/cdex/compact_dex_file.h b/runtime/cdex/compact_dex_file.h index f17f8cf68a..e47f97e804 100644 --- a/runtime/cdex/compact_dex_file.h +++ b/runtime/cdex/compact_dex_file.h @@ -17,6 +17,7 @@ #ifndef ART_RUNTIME_CDEX_COMPACT_DEX_FILE_H_ #define ART_RUNTIME_CDEX_COMPACT_DEX_FILE_H_ +#include "base/casts.h" #include "dex_file.h" namespace art { @@ -27,8 +28,20 @@ class CompactDexFile : public DexFile { static constexpr uint8_t kDexMagic[kDexMagicSize] = { 'c', 'd', 'e', 'x' }; static constexpr uint8_t kDexMagicVersion[] = {'0', '0', '1', '\0'}; + enum class FeatureFlags : uint32_t { + kDefaultMethods = 0x1, + }; + class Header : public DexFile::Header { - // Same for now. + public: + uint32_t GetFeatureFlags() const { + return feature_flags_; + } + + private: + uint32_t feature_flags_ = 0u; + + friend class CompactDexWriter; }; struct CodeItem : public DexFile::CodeItem { @@ -51,6 +64,12 @@ class CompactDexFile : public DexFile { static bool IsVersionValid(const uint8_t* magic); virtual bool IsVersionValid() const OVERRIDE; + const Header& GetHeader() const { + return down_cast<const Header&>(DexFile::GetHeader()); + } + + virtual bool SupportsDefaultMethods() const OVERRIDE; + private: // Not supported yet. CompactDexFile(const uint8_t* base, diff --git a/runtime/class_linker-inl.h b/runtime/class_linker-inl.h index 4b317f886f..cd6e8d59e8 100644 --- a/runtime/class_linker-inl.h +++ b/runtime/class_linker-inl.h @@ -174,7 +174,7 @@ inline bool ClassLinker::CheckInvokeClassMismatch(ObjPtr<mirror::DexCache> dex_c break; } case kDirect: - if (dex_cache->GetDexFile()->GetVersion() >= DexFile::kDefaultMethodsVersion) { + if (dex_cache->GetDexFile()->SupportsDefaultMethods()) { break; } FALLTHROUGH_INTENDED; diff --git a/runtime/dex_file.h b/runtime/dex_file.h index e80a13c00c..6c6ae2d54f 100644 --- a/runtime/dex_file.h +++ b/runtime/dex_file.h @@ -69,8 +69,6 @@ class DexFile { static constexpr size_t kDexMagicSize = 4; static constexpr size_t kDexVersionLen = 4; - // First Dex format version supporting default methods. - static const uint32_t kDefaultMethodsVersion = 37; // First Dex format version enforcing class definition ordering rules. static const uint32_t kClassDefinitionOrderEnforcedVersion = 37; @@ -481,7 +479,7 @@ class DexFile { } // Decode the dex magic version - uint32_t GetVersion() const { + uint32_t GetDexVersion() const { return GetHeader().GetVersion(); } @@ -491,6 +489,9 @@ class DexFile { // Returns true if the byte string after the magic is the correct value. virtual bool IsVersionValid() const = 0; + // Returns true if the dex file supports default methods. + virtual bool SupportsDefaultMethods() const = 0; + // Returns the number of string identifiers in the .dex file. size_t NumStringIds() const { DCHECK(header_ != nullptr) << GetLocation(); @@ -1044,6 +1045,9 @@ class DexFile { ALWAYS_INLINE const CompactDexFile* AsCompactDexFile() const; protected: + // First Dex format version supporting default methods. + static const uint32_t kDefaultMethodsVersion = 37; + DexFile(const uint8_t* base, size_t size, const std::string& location, diff --git a/runtime/dex_file_verifier.cc b/runtime/dex_file_verifier.cc index 8656bccfdc..d6f685a595 100644 --- a/runtime/dex_file_verifier.cc +++ b/runtime/dex_file_verifier.cc @@ -386,7 +386,14 @@ bool DexFileVerifier::CheckHeader() { return false; } - if (header_->header_size_ != sizeof(DexFile::Header)) { + bool size_matches = false; + if (dex_file_->IsCompactDexFile()) { + size_matches = header_->header_size_ == sizeof(CompactDexFile::Header); + } else { + size_matches = header_->header_size_ == sizeof(StandardDexFile::Header); + } + + if (!size_matches) { ErrorStringPrintf("Bad header size: %ud", header_->header_size_); return false; } @@ -3004,7 +3011,7 @@ bool DexFileVerifier::CheckFieldAccessFlags(uint32_t idx, GetFieldDescriptionOrError(begin_, header_, idx).c_str(), field_access_flags, PrettyJavaAccessFlags(field_access_flags).c_str()); - if (header_->GetVersion() >= DexFile::kDefaultMethodsVersion) { + if (dex_file_->SupportsDefaultMethods()) { return false; } else { // Allow in older versions, but warn. @@ -3019,7 +3026,7 @@ bool DexFileVerifier::CheckFieldAccessFlags(uint32_t idx, GetFieldDescriptionOrError(begin_, header_, idx).c_str(), field_access_flags, PrettyJavaAccessFlags(field_access_flags).c_str()); - if (header_->GetVersion() >= DexFile::kDefaultMethodsVersion) { + if (dex_file_->SupportsDefaultMethods()) { return false; } else { // Allow in older versions, but warn. @@ -3102,7 +3109,7 @@ bool DexFileVerifier::CheckMethodAccessFlags(uint32_t method_index, *error_msg = StringPrintf("Constructor %" PRIu32 "(%s) is not flagged correctly wrt/ static.", method_index, GetMethodDescriptionOrError(begin_, header_, method_index).c_str()); - if (header_->GetVersion() >= DexFile::kDefaultMethodsVersion) { + if (dex_file_->SupportsDefaultMethods()) { return false; } else { // Allow in older versions, but warn. @@ -3131,14 +3138,14 @@ bool DexFileVerifier::CheckMethodAccessFlags(uint32_t method_index, if ((class_access_flags & kAccInterface) != 0) { // Non-static interface methods must be public or private. uint32_t desired_flags = (kAccPublic | kAccStatic); - if (dex_file_->GetVersion() >= DexFile::kDefaultMethodsVersion) { + if (dex_file_->SupportsDefaultMethods()) { desired_flags |= kAccPrivate; } if ((method_access_flags & desired_flags) == 0) { *error_msg = StringPrintf("Interface virtual method %" PRIu32 "(%s) is not public", method_index, GetMethodDescriptionOrError(begin_, header_, method_index).c_str()); - if (header_->GetVersion() >= DexFile::kDefaultMethodsVersion) { + if (dex_file_->SupportsDefaultMethods()) { return false; } else { // Allow in older versions, but warn. @@ -3163,7 +3170,7 @@ bool DexFileVerifier::CheckMethodAccessFlags(uint32_t method_index, *error_msg = StringPrintf("Constructor %u(%s) must not be abstract or native", method_index, GetMethodDescriptionOrError(begin_, header_, method_index).c_str()); - if (header_->GetVersion() >= DexFile::kDefaultMethodsVersion) { + if (dex_file_->SupportsDefaultMethods()) { return false; } else { // Allow in older versions, but warn. @@ -3197,7 +3204,7 @@ bool DexFileVerifier::CheckMethodAccessFlags(uint32_t method_index, *error_msg = StringPrintf("Interface method %" PRIu32 "(%s) is not public and abstract", method_index, GetMethodDescriptionOrError(begin_, header_, method_index).c_str()); - if (header_->GetVersion() >= DexFile::kDefaultMethodsVersion) { + if (dex_file_->SupportsDefaultMethods()) { return false; } else { // Allow in older versions, but warn. diff --git a/runtime/java_vm_ext.cc b/runtime/java_vm_ext.cc index 104fb66ed8..e15943646d 100644 --- a/runtime/java_vm_ext.cc +++ b/runtime/java_vm_ext.cc @@ -1052,17 +1052,17 @@ bool JavaVMExt::LoadNativeLibrary(JNIEnv* env, static void* FindCodeForNativeMethodInAgents(ArtMethod* m) REQUIRES_SHARED(Locks::mutator_lock_) { std::string jni_short_name(m->JniShortName()); std::string jni_long_name(m->JniLongName()); - for (const ti::Agent& agent : Runtime::Current()->GetAgents()) { - void* fn = agent.FindSymbol(jni_short_name); + for (const std::unique_ptr<ti::Agent>& agent : Runtime::Current()->GetAgents()) { + void* fn = agent->FindSymbol(jni_short_name); if (fn != nullptr) { VLOG(jni) << "Found implementation for " << m->PrettyMethod() - << " (symbol: " << jni_short_name << ") in " << agent; + << " (symbol: " << jni_short_name << ") in " << *agent; return fn; } - fn = agent.FindSymbol(jni_long_name); + fn = agent->FindSymbol(jni_long_name); if (fn != nullptr) { VLOG(jni) << "Found implementation for " << m->PrettyMethod() - << " (symbol: " << jni_long_name << ") in " << agent; + << " (symbol: " << jni_long_name << ") in " << *agent; return fn; } } diff --git a/runtime/native/dalvik_system_VMStack.cc b/runtime/native/dalvik_system_VMStack.cc index a88563da1f..3e8040bfa5 100644 --- a/runtime/native/dalvik_system_VMStack.cc +++ b/runtime/native/dalvik_system_VMStack.cc @@ -16,6 +16,8 @@ #include "dalvik_system_VMStack.h" +#include <type_traits> + #include "nativehelper/jni_macros.h" #include "art_method-inl.h" @@ -32,12 +34,17 @@ namespace art { -static jobject GetThreadStack(const ScopedFastNativeObjectAccess& soa, jobject peer) +template <typename T, + typename ResultT = + typename std::result_of<T(Thread*, const ScopedFastNativeObjectAccess&)>::type> +static ResultT GetThreadStack(const ScopedFastNativeObjectAccess& soa, + jobject peer, + T fn) REQUIRES_SHARED(Locks::mutator_lock_) { - jobject trace = nullptr; + ResultT trace = nullptr; ObjPtr<mirror::Object> decoded_peer = soa.Decode<mirror::Object>(peer); if (decoded_peer == soa.Self()->GetPeer()) { - trace = soa.Self()->CreateInternalStackTrace<false>(soa); + trace = fn(soa.Self(), soa); } else { // Never allow suspending the heap task thread since it may deadlock if allocations are // required for the stack trace. @@ -59,7 +66,7 @@ static jobject GetThreadStack(const ScopedFastNativeObjectAccess& soa, jobject p // Must be runnable to create returned array. { ScopedObjectAccess soa2(soa.Self()); - trace = thread->CreateInternalStackTrace<false>(soa); + trace = fn(thread, soa); } // Restart suspended thread. bool resumed = thread_list->Resume(thread, SuspendReason::kInternal); @@ -75,7 +82,11 @@ static jobject GetThreadStack(const ScopedFastNativeObjectAccess& soa, jobject p static jint VMStack_fillStackTraceElements(JNIEnv* env, jclass, jobject javaThread, jobjectArray javaSteArray) { ScopedFastNativeObjectAccess soa(env); - jobject trace = GetThreadStack(soa, javaThread); + auto fn = [](Thread* thread, const ScopedFastNativeObjectAccess& soaa) + REQUIRES_SHARED(Locks::mutator_lock_) -> jobject { + return thread->CreateInternalStackTrace<false>(soaa); + }; + jobject trace = GetThreadStack(soa, javaThread, fn); if (trace == nullptr) { return 0; } @@ -138,7 +149,11 @@ static jclass VMStack_getStackClass2(JNIEnv* env, jclass) { static jobjectArray VMStack_getThreadStackTrace(JNIEnv* env, jclass, jobject javaThread) { ScopedFastNativeObjectAccess soa(env); - jobject trace = GetThreadStack(soa, javaThread); + auto fn = [](Thread* thread, const ScopedFastNativeObjectAccess& soaa) + REQUIRES_SHARED(Locks::mutator_lock_) -> jobject { + return thread->CreateInternalStackTrace<false>(soaa); + }; + jobject trace = GetThreadStack(soa, javaThread, fn); if (trace == nullptr) { return nullptr; } diff --git a/runtime/parsed_options.cc b/runtime/parsed_options.cc index 47309edfd7..3ac3d03e90 100644 --- a/runtime/parsed_options.cc +++ b/runtime/parsed_options.cc @@ -103,7 +103,7 @@ std::unique_ptr<RuntimeParser> ParsedOptions::MakeParser(bool ignore_unrecognize // .WithType<std::vector<ti::Agent>>().AppendValues() // .IntoKey(M::AgentLib) .Define("-agentpath:_") - .WithType<std::list<ti::Agent>>().AppendValues() + .WithType<std::list<ti::AgentSpec>>().AppendValues() .IntoKey(M::AgentPath) .Define("-Xms_") .WithType<MemoryKiB>() diff --git a/runtime/runtime.cc b/runtime/runtime.cc index 54aa9e57e6..37ba9e473d 100644 --- a/runtime/runtime.cc +++ b/runtime/runtime.cc @@ -366,7 +366,7 @@ Runtime::~Runtime() { // TODO Maybe do some locking. for (auto& agent : agents_) { - agent.Unload(); + agent->Unload(); } // TODO Maybe do some locking @@ -1166,7 +1166,7 @@ bool Runtime::Init(RuntimeArgumentMap&& runtime_options_in) { madvise_random_access_ = runtime_options.GetOrDefault(Opt::MadviseRandomAccess); plugins_ = runtime_options.ReleaseOrDefault(Opt::Plugins); - agents_ = runtime_options.ReleaseOrDefault(Opt::AgentPath); + agent_specs_ = runtime_options.ReleaseOrDefault(Opt::AgentPath); // TODO Add back in -agentlib // for (auto lib : runtime_options.ReleaseOrDefault(Opt::AgentLib)) { // agents_.push_back(lib); @@ -1498,16 +1498,32 @@ bool Runtime::Init(RuntimeArgumentMap&& runtime_options_in) { // Startup agents // TODO Maybe we should start a new thread to run these on. Investigate RI behavior more. - for (auto& agent : agents_) { + for (auto& agent_spec : agent_specs_) { // TODO Check err int res = 0; std::string err = ""; - ti::Agent::LoadError result = agent.Load(&res, &err); - if (result == ti::Agent::kInitializationError) { - LOG(FATAL) << "Unable to initialize agent!"; - } else if (result != ti::Agent::kNoError) { - LOG(ERROR) << "Unable to load an agent: " << err; + ti::LoadError error; + std::unique_ptr<ti::Agent> agent = agent_spec.Load(&res, &error, &err); + + if (agent != nullptr) { + agents_.push_back(std::move(agent)); + continue; + } + + switch (error) { + case ti::LoadError::kInitializationError: + LOG(FATAL) << "Unable to initialize agent!"; + UNREACHABLE(); + + case ti::LoadError::kLoadingError: + LOG(ERROR) << "Unable to load an agent: " << err; + continue; + + case ti::LoadError::kNoError: + break; } + LOG(FATAL) << "Unreachable"; + UNREACHABLE(); } { ScopedObjectAccess soa(self); @@ -1563,15 +1579,16 @@ void Runtime::AttachAgent(const std::string& agent_arg) { return; } - ti::Agent agent(agent_arg); + ti::AgentSpec agent_spec(agent_arg); int res = 0; - ti::Agent::LoadError result = agent.Attach(&res, &error_msg); + ti::LoadError error; + std::unique_ptr<ti::Agent> agent = agent_spec.Attach(&res, &error, &error_msg); - if (result == ti::Agent::kNoError) { + if (agent != nullptr) { agents_.push_back(std::move(agent)); } else { - LOG(WARNING) << "Agent attach failed (result=" << result << ") : " << error_msg; + LOG(WARNING) << "Agent attach failed (result=" << error << ") : " << error_msg; ScopedObjectAccess soa(Thread::Current()); ThrowIOException("%s", error_msg.c_str()); } diff --git a/runtime/runtime.h b/runtime/runtime.h index 89caac4f3c..ac29ed42c3 100644 --- a/runtime/runtime.h +++ b/runtime/runtime.h @@ -66,6 +66,7 @@ class Throwable; } // namespace mirror namespace ti { class Agent; +class AgentSpec; } // namespace ti namespace verifier { class MethodVerifier; @@ -662,7 +663,7 @@ class Runtime { void AttachAgent(const std::string& agent_arg); - const std::list<ti::Agent>& GetAgents() const { + const std::list<std::unique_ptr<ti::Agent>>& GetAgents() const { return agents_; } @@ -779,7 +780,8 @@ class Runtime { std::string class_path_string_; std::vector<std::string> properties_; - std::list<ti::Agent> agents_; + std::list<ti::AgentSpec> agent_specs_; + std::list<std::unique_ptr<ti::Agent>> agents_; std::vector<Plugin> plugins_; // The default stack size for managed threads created by the runtime. diff --git a/runtime/runtime_options.def b/runtime/runtime_options.def index 4bc824570a..1dd3de5039 100644 --- a/runtime/runtime_options.def +++ b/runtime/runtime_options.def @@ -124,8 +124,8 @@ RUNTIME_OPTIONS_KEY (Unit, NoDexFileFallback) RUNTIME_OPTIONS_KEY (std::string, CpuAbiList) RUNTIME_OPTIONS_KEY (std::string, Fingerprint) RUNTIME_OPTIONS_KEY (ExperimentalFlags, Experimental, ExperimentalFlags::kNone) // -Xexperimental:{...} -RUNTIME_OPTIONS_KEY (std::list<ti::Agent>, AgentLib) // -agentlib:<libname>=<options> -RUNTIME_OPTIONS_KEY (std::list<ti::Agent>, AgentPath) // -agentpath:<libname>=<options> +RUNTIME_OPTIONS_KEY (std::list<ti::AgentSpec>, AgentLib) // -agentlib:<libname>=<options> +RUNTIME_OPTIONS_KEY (std::list<ti::AgentSpec>, AgentPath) // -agentpath:<libname>=<options> RUNTIME_OPTIONS_KEY (std::vector<Plugin>, Plugins) // -Xplugin:<library> // Not parse-able from command line, but can be provided explicitly. diff --git a/runtime/standard_dex_file.cc b/runtime/standard_dex_file.cc index 4c1d3081d8..b608341a92 100644 --- a/runtime/standard_dex_file.cc +++ b/runtime/standard_dex_file.cc @@ -63,4 +63,8 @@ bool StandardDexFile::IsVersionValid() const { return IsVersionValid(header_->magic_); } +bool StandardDexFile::SupportsDefaultMethods() const { + return GetDexVersion() >= DexFile::kDefaultMethodsVersion; +} + } // namespace art diff --git a/runtime/standard_dex_file.h b/runtime/standard_dex_file.h index 5d5359776d..80c406ebc7 100644 --- a/runtime/standard_dex_file.h +++ b/runtime/standard_dex_file.h @@ -56,6 +56,8 @@ class StandardDexFile : public DexFile { static bool IsVersionValid(const uint8_t* magic); virtual bool IsVersionValid() const OVERRIDE; + virtual bool SupportsDefaultMethods() const OVERRIDE; + private: StandardDexFile(const uint8_t* base, size_t size, diff --git a/runtime/ti/agent.cc b/runtime/ti/agent.cc index 548752e980..aa09d84d74 100644 --- a/runtime/ti/agent.cc +++ b/runtime/ti/agent.cc @@ -33,31 +33,54 @@ const char* AGENT_ON_LOAD_FUNCTION_NAME = "Agent_OnLoad"; const char* AGENT_ON_ATTACH_FUNCTION_NAME = "Agent_OnAttach"; const char* AGENT_ON_UNLOAD_FUNCTION_NAME = "Agent_OnUnload"; +AgentSpec::AgentSpec(const std::string& arg) { + size_t eq = arg.find_first_of('='); + if (eq == std::string::npos) { + name_ = arg; + } else { + name_ = arg.substr(0, eq); + args_ = arg.substr(eq + 1, arg.length()); + } +} + +std::unique_ptr<Agent> AgentSpec::Load(/*out*/jint* call_res, + /*out*/LoadError* error, + /*out*/std::string* error_msg) { + VLOG(agents) << "Loading agent: " << name_ << " " << args_; + return DoLoadHelper(false, call_res, error, error_msg); +} + +// Tries to attach the agent using its OnAttach method. Returns true on success. +std::unique_ptr<Agent> AgentSpec::Attach(/*out*/jint* call_res, + /*out*/LoadError* error, + /*out*/std::string* error_msg) { + VLOG(agents) << "Attaching agent: " << name_ << " " << args_; + return DoLoadHelper(true, call_res, error, error_msg); +} + + // TODO We need to acquire some locks probably. -Agent::LoadError Agent::DoLoadHelper(bool attaching, - /*out*/jint* call_res, - /*out*/std::string* error_msg) { +std::unique_ptr<Agent> AgentSpec::DoLoadHelper(bool attaching, + /*out*/jint* call_res, + /*out*/LoadError* error, + /*out*/std::string* error_msg) { ScopedThreadStateChange stsc(Thread::Current(), ThreadState::kNative); DCHECK(call_res != nullptr); DCHECK(error_msg != nullptr); - if (IsStarted()) { - *error_msg = StringPrintf("the agent at %s has already been started!", name_.c_str()); - VLOG(agents) << "err: " << *error_msg; - return kAlreadyStarted; - } - LoadError err = DoDlOpen(error_msg); - if (err != kNoError) { + std::unique_ptr<Agent> agent = DoDlOpen(error, error_msg); + if (agent == nullptr) { VLOG(agents) << "err: " << *error_msg; - return err; + return nullptr; } - AgentOnLoadFunction callback = attaching ? onattach_ : onload_; + AgentOnLoadFunction callback = attaching ? agent->onattach_ : agent->onload_; if (callback == nullptr) { *error_msg = StringPrintf("Unable to start agent %s: No %s callback found", (attaching ? "attach" : "load"), name_.c_str()); VLOG(agents) << "err: " << *error_msg; - return kLoadingError; + *error = kLoadingError; + return nullptr; } // Need to let the function fiddle with the array. std::unique_ptr<char[]> copied_args(new char[args_.size() + 1]); @@ -70,44 +93,36 @@ Agent::LoadError Agent::DoLoadHelper(bool attaching, *error_msg = StringPrintf("Initialization of %s returned non-zero value of %d", name_.c_str(), *call_res); VLOG(agents) << "err: " << *error_msg; - return kInitializationError; - } else { - return kNoError; + *error = kInitializationError; + return nullptr; } + return agent; } -void* Agent::FindSymbol(const std::string& name) const { - CHECK(IsStarted()) << "Cannot find symbols in an unloaded agent library " << this; - return dlsym(dlopen_handle_, name.c_str()); -} - -Agent::LoadError Agent::DoDlOpen(/*out*/std::string* error_msg) { +std::unique_ptr<Agent> AgentSpec::DoDlOpen(/*out*/LoadError* error, /*out*/std::string* error_msg) { DCHECK(error_msg != nullptr); - DCHECK(dlopen_handle_ == nullptr); - DCHECK(onload_ == nullptr); - DCHECK(onattach_ == nullptr); - DCHECK(onunload_ == nullptr); - - dlopen_handle_ = dlopen(name_.c_str(), RTLD_LAZY); - if (dlopen_handle_ == nullptr) { + void* dlopen_handle = dlopen(name_.c_str(), RTLD_LAZY); + if (dlopen_handle == nullptr) { *error_msg = StringPrintf("Unable to dlopen %s: %s", name_.c_str(), dlerror()); - return kLoadingError; + *error = kLoadingError; + return nullptr; } - onload_ = reinterpret_cast<AgentOnLoadFunction>(FindSymbol(AGENT_ON_LOAD_FUNCTION_NAME)); - if (onload_ == nullptr) { - VLOG(agents) << "Unable to find 'Agent_OnLoad' symbol in " << this; - } - onattach_ = reinterpret_cast<AgentOnLoadFunction>(FindSymbol(AGENT_ON_ATTACH_FUNCTION_NAME)); - if (onattach_ == nullptr) { - VLOG(agents) << "Unable to find 'Agent_OnAttach' symbol in " << this; - } - onunload_ = reinterpret_cast<AgentOnUnloadFunction>(FindSymbol(AGENT_ON_UNLOAD_FUNCTION_NAME)); - if (onunload_ == nullptr) { - VLOG(agents) << "Unable to find 'Agent_OnUnload' symbol in " << this; - } - return kNoError; + std::unique_ptr<Agent> agent(new Agent(name_, dlopen_handle)); + agent->PopulateFunctions(); + *error = kNoError; + return agent; +} + +std::ostream& operator<<(std::ostream &os, AgentSpec const& m) { + return os << "AgentSpec { name=\"" << m.name_ << "\", args=\"" << m.args_ << "\" }"; +} + + +void* Agent::FindSymbol(const std::string& name) const { + CHECK(dlopen_handle_ != nullptr) << "Cannot find symbols in an unloaded agent library " << this; + return dlsym(dlopen_handle_, name.c_str()); } // TODO Lock some stuff probably. @@ -127,58 +142,6 @@ void Agent::Unload() { } } -Agent::Agent(const std::string& arg) - : dlopen_handle_(nullptr), - onload_(nullptr), - onattach_(nullptr), - onunload_(nullptr) { - size_t eq = arg.find_first_of('='); - if (eq == std::string::npos) { - name_ = arg; - } else { - name_ = arg.substr(0, eq); - args_ = arg.substr(eq + 1, arg.length()); - } -} - -Agent::Agent(const Agent& other) - : dlopen_handle_(nullptr), - onload_(nullptr), - onattach_(nullptr), - onunload_(nullptr) { - *this = other; -} - -// Attempting to copy to/from loaded/started agents is a fatal error -Agent& Agent::operator=(const Agent& other) { - if (this != &other) { - if (other.dlopen_handle_ != nullptr) { - LOG(FATAL) << "Attempting to copy a loaded agent!"; - } - - if (dlopen_handle_ != nullptr) { - LOG(FATAL) << "Attempting to assign into a loaded agent!"; - } - - DCHECK(other.onload_ == nullptr); - DCHECK(other.onattach_ == nullptr); - DCHECK(other.onunload_ == nullptr); - - DCHECK(onload_ == nullptr); - DCHECK(onattach_ == nullptr); - DCHECK(onunload_ == nullptr); - - name_ = other.name_; - args_ = other.args_; - - dlopen_handle_ = nullptr; - onload_ = nullptr; - onattach_ = nullptr; - onunload_ = nullptr; - } - return *this; -} - Agent::Agent(Agent&& other) : dlopen_handle_(nullptr), onload_(nullptr), @@ -190,10 +153,9 @@ Agent::Agent(Agent&& other) Agent& Agent::operator=(Agent&& other) { if (this != &other) { if (dlopen_handle_ != nullptr) { - dlclose(dlopen_handle_); + Unload(); } name_ = std::move(other.name_); - args_ = std::move(other.args_); dlopen_handle_ = other.dlopen_handle_; onload_ = other.onload_; onattach_ = other.onattach_; @@ -206,9 +168,24 @@ Agent& Agent::operator=(Agent&& other) { return *this; } +void Agent::PopulateFunctions() { + onload_ = reinterpret_cast<AgentOnLoadFunction>(FindSymbol(AGENT_ON_LOAD_FUNCTION_NAME)); + if (onload_ == nullptr) { + VLOG(agents) << "Unable to find 'Agent_OnLoad' symbol in " << this; + } + onattach_ = reinterpret_cast<AgentOnLoadFunction>(FindSymbol(AGENT_ON_ATTACH_FUNCTION_NAME)); + if (onattach_ == nullptr) { + VLOG(agents) << "Unable to find 'Agent_OnAttach' symbol in " << this; + } + onunload_ = reinterpret_cast<AgentOnUnloadFunction>(FindSymbol(AGENT_ON_UNLOAD_FUNCTION_NAME)); + if (onunload_ == nullptr) { + VLOG(agents) << "Unable to find 'Agent_OnUnload' symbol in " << this; + } +} + Agent::~Agent() { if (dlopen_handle_ != nullptr) { - dlclose(dlopen_handle_); + Unload(); } } @@ -217,8 +194,7 @@ std::ostream& operator<<(std::ostream &os, const Agent* m) { } std::ostream& operator<<(std::ostream &os, Agent const& m) { - return os << "Agent { name=\"" << m.name_ << "\", args=\"" << m.args_ << "\", handle=" - << m.dlopen_handle_ << " }"; + return os << "Agent { name=\"" << m.name_ << "\", handle=" << m.dlopen_handle_ << " }"; } } // namespace ti diff --git a/runtime/ti/agent.h b/runtime/ti/agent.h index d6f1f2ef06..68b49485be 100644 --- a/runtime/ti/agent.h +++ b/runtime/ti/agent.h @@ -20,34 +20,24 @@ #include <dlfcn.h> #include <jni.h> // for jint, JavaVM* etc declarations +#include <memory> + #include "base/logging.h" namespace art { namespace ti { -using AgentOnLoadFunction = jint (*)(JavaVM*, const char*, void*); -using AgentOnUnloadFunction = void (*)(JavaVM*); +class Agent; -// Agents are native libraries that will be loaded by the runtime for the purpose of -// instrumentation. They will be entered by Agent_OnLoad or Agent_OnAttach depending on whether the -// agent is being attached during runtime startup or later. -// -// The agent's Agent_OnUnload function will be called during runtime shutdown. -// -// TODO: consider splitting ti::Agent into command line, agent and shared library handler classes -// TODO Support native-bridge. Currently agents can only be the actual runtime ISA of the device. -class Agent { +enum LoadError { + kNoError, // No error occurred.. + kLoadingError, // dlopen or dlsym returned an error. + kInitializationError, // The entrypoint did not return 0. This might require an abort. +}; + +class AgentSpec { public: - enum LoadError { - kNoError, // No error occurred.. - kAlreadyStarted, // The agent has already been loaded. - kLoadingError, // dlopen or dlsym returned an error. - kInitializationError, // The entrypoint did not return 0. This might require an abort. - }; - - bool IsStarted() const { - return dlopen_handle_ != nullptr; - } + explicit AgentSpec(const std::string& arg); const std::string& GetName() const { return name_; @@ -61,26 +51,52 @@ class Agent { return !GetArgs().empty(); } - void* FindSymbol(const std::string& name) const; + std::unique_ptr<Agent> Load(/*out*/jint* call_res, + /*out*/LoadError* error, + /*out*/std::string* error_msg); - LoadError Load(/*out*/jint* call_res, /*out*/std::string* error_msg) { - VLOG(agents) << "Loading agent: " << name_ << " " << args_; - return DoLoadHelper(false, call_res, error_msg); - } + // Tries to attach the agent using its OnAttach method. Returns true on success. + std::unique_ptr<Agent> Attach(/*out*/jint* call_res, + /*out*/LoadError* error, + /*out*/std::string* error_msg); - // TODO We need to acquire some locks probably. - void Unload(); + private: + std::unique_ptr<Agent> DoDlOpen(/*out*/LoadError* error, /*out*/std::string* error_msg); - // Tries to attach the agent using its OnAttach method. Returns true on success. - LoadError Attach(/*out*/jint* call_res, /*out*/std::string* error_msg) { - VLOG(agents) << "Attaching agent: " << name_ << " " << args_; - return DoLoadHelper(true, call_res, error_msg); + std::unique_ptr<Agent> DoLoadHelper(bool attaching, + /*out*/jint* call_res, + /*out*/LoadError* error, + /*out*/std::string* error_msg); + + std::string name_; + std::string args_; + + friend std::ostream& operator<<(std::ostream &os, AgentSpec const& m); +}; + +std::ostream& operator<<(std::ostream &os, AgentSpec const& m); + +using AgentOnLoadFunction = jint (*)(JavaVM*, const char*, void*); +using AgentOnUnloadFunction = void (*)(JavaVM*); + +// Agents are native libraries that will be loaded by the runtime for the purpose of +// instrumentation. They will be entered by Agent_OnLoad or Agent_OnAttach depending on whether the +// agent is being attached during runtime startup or later. +// +// The agent's Agent_OnUnload function will be called during runtime shutdown. +// +// TODO: consider splitting ti::Agent into command line, agent and shared library handler classes +// TODO Support native-bridge. Currently agents can only be the actual runtime ISA of the device. +class Agent { + public: + const std::string& GetName() const { + return name_; } - explicit Agent(const std::string& arg); + void* FindSymbol(const std::string& name) const; - Agent(const Agent& other); - Agent& operator=(const Agent& other); + // TODO We need to acquire some locks probably. + void Unload(); Agent(Agent&& other); Agent& operator=(Agent&& other); @@ -88,14 +104,17 @@ class Agent { ~Agent(); private: - LoadError DoDlOpen(/*out*/std::string* error_msg); + Agent(const std::string& name, void* dlopen_handle) : name_(name), + dlopen_handle_(dlopen_handle), + onload_(nullptr), + onattach_(nullptr), + onunload_(nullptr) { + DCHECK(dlopen_handle != nullptr); + } - LoadError DoLoadHelper(bool attaching, - /*out*/jint* call_res, - /*out*/std::string* error_msg); + void PopulateFunctions(); std::string name_; - std::string args_; void* dlopen_handle_; // The entrypoints. @@ -103,7 +122,10 @@ class Agent { AgentOnLoadFunction onattach_; AgentOnUnloadFunction onunload_; + friend class AgentSpec; friend std::ostream& operator<<(std::ostream &os, Agent const& m); + + DISALLOW_COPY_AND_ASSIGN(Agent); }; std::ostream& operator<<(std::ostream &os, Agent const& m); diff --git a/runtime/verifier/method_verifier.cc b/runtime/verifier/method_verifier.cc index 63f21f8013..5e5e96bc7f 100644 --- a/runtime/verifier/method_verifier.cc +++ b/runtime/verifier/method_verifier.cc @@ -836,7 +836,7 @@ bool MethodVerifier::Verify() { return false; } else { uint32_t access_flag_options = kAccPublic; - if (dex_file_->GetVersion() >= DexFile::kDefaultMethodsVersion) { + if (dex_file_->SupportsDefaultMethods()) { access_flag_options |= kAccPrivate; } if (!(method_access_flags_ & access_flag_options)) { @@ -3965,10 +3965,10 @@ ArtMethod* MethodVerifier::ResolveMethodAndCheckAccess( // while this check implies an IncompatibleClassChangeError. if (klass->IsInterface()) { // methods called on interfaces should be invoke-interface, invoke-super, invoke-direct (if - // dex file version is 37 or greater), or invoke-static. + // default methods are supported for the dex file), or invoke-static. if (method_type != METHOD_INTERFACE && method_type != METHOD_STATIC && - ((dex_file_->GetVersion() < DexFile::kDefaultMethodsVersion) || + (!dex_file_->SupportsDefaultMethods() || method_type != METHOD_DIRECT) && method_type != METHOD_SUPER) { Fail(VERIFY_ERROR_CLASS_CHANGE) diff --git a/runtime/verifier/method_verifier.h b/runtime/verifier/method_verifier.h index f26f3e2655..b66433c370 100644 --- a/runtime/verifier/method_verifier.h +++ b/runtime/verifier/method_verifier.h @@ -127,10 +127,6 @@ class MethodVerifier { return *dex_file_; } - uint32_t DexFileVersion() const { - return dex_file_->GetVersion(); - } - RegTypeCache* GetRegTypeCache() { return ®_types_; } diff --git a/test/1941-dispose-stress/dispose_stress.cc b/test/1941-dispose-stress/dispose_stress.cc index e8fcc775e9..f973abe28b 100644 --- a/test/1941-dispose-stress/dispose_stress.cc +++ b/test/1941-dispose-stress/dispose_stress.cc @@ -30,6 +30,17 @@ namespace art { namespace Test1941DisposeStress { +extern "C" JNIEXPORT void JNICALL Java_art_Test1941_setTracingOn(JNIEnv* env, + jclass, + jthread thr, + jboolean enable) { + JvmtiErrorToException(env, + jvmti_env, + jvmti_env->SetEventNotificationMode(enable ? JVMTI_ENABLE : JVMTI_DISABLE, + JVMTI_EVENT_SINGLE_STEP, + thr)); +} + extern "C" JNIEXPORT jlong JNICALL Java_art_Test1941_AllocEnv(JNIEnv* env, jclass) { JavaVM* vm = nullptr; if (env->GetJavaVM(&vm) != 0) { diff --git a/test/1941-dispose-stress/src/art/Test1941.java b/test/1941-dispose-stress/src/art/Test1941.java index d5a9de6cab..29cea6bdb8 100644 --- a/test/1941-dispose-stress/src/art/Test1941.java +++ b/test/1941-dispose-stress/src/art/Test1941.java @@ -19,6 +19,7 @@ package art; import java.util.Arrays; import java.lang.reflect.Executable; import java.lang.reflect.Method; +import java.util.concurrent.Semaphore; public class Test1941 { public static final boolean PRINT_CNT = false; @@ -41,7 +42,8 @@ public class Test1941 { // Don't bother actually doing anything. } - public static void LoopAllocFreeEnv() { + public static void LoopAllocFreeEnv(Semaphore sem) { + sem.release(); while (!Thread.interrupted()) { CNT++; long env = AllocEnv(); @@ -52,18 +54,25 @@ public class Test1941 { public static native long AllocEnv(); public static native void FreeEnv(long env); + public static native void setTracingOn(Thread thr, boolean enable); + public static void run() throws Exception { - Thread thr = new Thread(Test1941::LoopAllocFreeEnv, "LoopNative"); + final Semaphore sem = new Semaphore(0); + Thread thr = new Thread(() -> { LoopAllocFreeEnv(sem); }, "LoopNative"); thr.start(); + // Make sure the other thread is actually started. + sem.acquire(); Trace.enableSingleStepTracing(Test1941.class, Test1941.class.getDeclaredMethod( "notifySingleStep", Thread.class, Executable.class, Long.TYPE), - null); + thr); + setTracingOn(Thread.currentThread(), true); System.out.println("fib(20) is " + fib(20)); thr.interrupt(); thr.join(); + setTracingOn(Thread.currentThread(), false); Trace.disableTracing(null); if (PRINT_CNT) { System.out.println("Number of envs created/destroyed: " + CNT); diff --git a/test/knownfailures.json b/test/knownfailures.json index dc051d9515..a12510c9dc 100644 --- a/test/knownfailures.json +++ b/test/knownfailures.json @@ -649,11 +649,6 @@ "tests": "661-oat-writer-layout", "variant": "interp-ac | interpreter | jit | no-dex2oat | no-prebuild | no-image | trace", "description": ["Test is designed to only check --compiler-filter=speed"] - }, - { - "tests": ["975-iface-private"], - "variant": "cdex-fast", - "description": ["CompactDex doesn't yet work with 975-iface private"] } ] diff --git a/test/ti-agent/trace_helper.cc b/test/ti-agent/trace_helper.cc index bbc7754a45..b590175d77 100644 --- a/test/ti-agent/trace_helper.cc +++ b/test/ti-agent/trace_helper.cc @@ -27,6 +27,49 @@ namespace art { namespace common_trace { +static bool IsInCallback(JNIEnv* env, jvmtiEnv *jvmti, jthread thr) { + void* data; + ScopedLocalRef<jthrowable> exc(env, env->ExceptionOccurred()); + env->ExceptionClear(); + jvmti->GetThreadLocalStorage(thr, &data); + if (exc.get() != nullptr) { + env->Throw(exc.get()); + } + if (data == nullptr) { + return false; + } else { + return true; + } +} + +static void SetInCallback(JNIEnv* env, jvmtiEnv *jvmti, jthread thr, bool val) { + ScopedLocalRef<jthrowable> exc(env, env->ExceptionOccurred()); + env->ExceptionClear(); + jvmti->SetThreadLocalStorage(thr, (val ? reinterpret_cast<void*>(0x1) + : reinterpret_cast<void*>(0x0))); + if (exc.get() != nullptr) { + env->Throw(exc.get()); + } +} + +class ScopedCallbackState { + public: + ScopedCallbackState(JNIEnv* jnienv, jvmtiEnv* env, jthread thr) + : jnienv_(jnienv), env_(env), thr_(thr) { + CHECK(!IsInCallback(jnienv_, env_, thr_)); + SetInCallback(jnienv_, env_, thr_, true); + } + ~ScopedCallbackState() { + CHECK(IsInCallback(jnienv_, env_, thr_)); + SetInCallback(jnienv_, env_, thr_, false); + } + + private: + JNIEnv* jnienv_; + jvmtiEnv* env_; + jthread thr_; +}; + struct TraceData { jclass test_klass; jmethodID enter_method; @@ -36,7 +79,6 @@ struct TraceData { jmethodID single_step; jmethodID thread_start; jmethodID thread_end; - bool in_callback; bool access_watch_on_load; bool modify_watch_on_load; jrawMonitorID trace_mon; @@ -94,7 +136,7 @@ static void singleStepCB(jvmtiEnv* jvmti, jvmti->GetEnvironmentLocalStorage(reinterpret_cast<void**>(&data)))) { return; } - if (data->in_callback) { + if (IsInCallback(jnienv, jvmti, thread)) { return; } ScopedLocalRef<jclass> klass(jnienv, data->GetTestClass(jvmti, jnienv)); @@ -102,7 +144,7 @@ static void singleStepCB(jvmtiEnv* jvmti, return; } CHECK(data->single_step != nullptr); - data->in_callback = true; + ScopedCallbackState st(jnienv, jvmti, thread); jobject method_arg = GetJavaMethod(jvmti, jnienv, method); jnienv->CallStaticVoidMethod(klass.get(), data->single_step, @@ -110,12 +152,11 @@ static void singleStepCB(jvmtiEnv* jvmti, method_arg, static_cast<jlong>(location)); jnienv->DeleteLocalRef(method_arg); - data->in_callback = false; } static void fieldAccessCB(jvmtiEnv* jvmti, JNIEnv* jnienv, - jthread thr ATTRIBUTE_UNUSED, + jthread thr, jmethodID method, jlocation location, jclass field_klass, @@ -126,7 +167,7 @@ static void fieldAccessCB(jvmtiEnv* jvmti, jvmti->GetEnvironmentLocalStorage(reinterpret_cast<void**>(&data)))) { return; } - if (data->in_callback) { + if (IsInCallback(jnienv, jvmti, thr)) { // Don't do callback for either of these to prevent an infinite loop. return; } @@ -135,7 +176,7 @@ static void fieldAccessCB(jvmtiEnv* jvmti, return; } CHECK(data->field_access != nullptr); - data->in_callback = true; + ScopedCallbackState st(jnienv, jvmti, thr); jobject method_arg = GetJavaMethod(jvmti, jnienv, method); jobject field_arg = GetJavaField(jvmti, jnienv, field_klass, field); jnienv->CallStaticVoidMethod(klass.get(), @@ -147,12 +188,11 @@ static void fieldAccessCB(jvmtiEnv* jvmti, field_arg); jnienv->DeleteLocalRef(method_arg); jnienv->DeleteLocalRef(field_arg); - data->in_callback = false; } static void fieldModificationCB(jvmtiEnv* jvmti, JNIEnv* jnienv, - jthread thr ATTRIBUTE_UNUSED, + jthread thr, jmethodID method, jlocation location, jclass field_klass, @@ -165,7 +205,7 @@ static void fieldModificationCB(jvmtiEnv* jvmti, jvmti->GetEnvironmentLocalStorage(reinterpret_cast<void**>(&data)))) { return; } - if (data->in_callback) { + if (IsInCallback(jnienv, jvmti, thr)) { // Don't do callback recursively to prevent an infinite loop. return; } @@ -174,12 +214,11 @@ static void fieldModificationCB(jvmtiEnv* jvmti, return; } CHECK(data->field_modify != nullptr); - data->in_callback = true; + ScopedCallbackState st(jnienv, jvmti, thr); jobject method_arg = GetJavaMethod(jvmti, jnienv, method); jobject field_arg = GetJavaField(jvmti, jnienv, field_klass, field); jobject value = GetJavaValueByType(jnienv, type_char, new_value); if (jnienv->ExceptionCheck()) { - data->in_callback = false; jnienv->DeleteLocalRef(method_arg); jnienv->DeleteLocalRef(field_arg); return; @@ -194,12 +233,11 @@ static void fieldModificationCB(jvmtiEnv* jvmti, value); jnienv->DeleteLocalRef(method_arg); jnienv->DeleteLocalRef(field_arg); - data->in_callback = false; } static void methodExitCB(jvmtiEnv* jvmti, JNIEnv* jnienv, - jthread thr ATTRIBUTE_UNUSED, + jthread thr, jmethodID method, jboolean was_popped_by_exception, jvalue return_value) { @@ -208,7 +246,9 @@ static void methodExitCB(jvmtiEnv* jvmti, jvmti->GetEnvironmentLocalStorage(reinterpret_cast<void**>(&data)))) { return; } - if (method == data->exit_method || method == data->enter_method || data->in_callback) { + if (method == data->exit_method || + method == data->enter_method || + IsInCallback(jnienv, jvmti, thr)) { // Don't do callback for either of these to prevent an infinite loop. return; } @@ -217,12 +257,11 @@ static void methodExitCB(jvmtiEnv* jvmti, return; } CHECK(data->exit_method != nullptr); - data->in_callback = true; + ScopedCallbackState st(jnienv, jvmti, thr); jobject method_arg = GetJavaMethod(jvmti, jnienv, method); jobject result = was_popped_by_exception ? nullptr : GetJavaValue(jvmti, jnienv, method, return_value); if (jnienv->ExceptionCheck()) { - data->in_callback = false; return; } jnienv->CallStaticVoidMethod(klass.get(), @@ -231,12 +270,11 @@ static void methodExitCB(jvmtiEnv* jvmti, was_popped_by_exception, result); jnienv->DeleteLocalRef(method_arg); - data->in_callback = false; } static void methodEntryCB(jvmtiEnv* jvmti, JNIEnv* jnienv, - jthread thr ATTRIBUTE_UNUSED, + jthread thr, jmethodID method) { TraceData* data = nullptr; if (JvmtiErrorToException(jnienv, jvmti, @@ -244,7 +282,9 @@ static void methodEntryCB(jvmtiEnv* jvmti, return; } CHECK(data->enter_method != nullptr); - if (method == data->exit_method || method == data->enter_method || data->in_callback) { + if (method == data->exit_method || + method == data->enter_method || + IsInCallback(jnienv, jvmti, thr)) { // Don't do callback for either of these to prevent an infinite loop. return; } @@ -252,14 +292,13 @@ static void methodEntryCB(jvmtiEnv* jvmti, if (klass.get() == nullptr) { return; } - data->in_callback = true; + ScopedCallbackState st(jnienv, jvmti, thr); jobject method_arg = GetJavaMethod(jvmti, jnienv, method); if (jnienv->ExceptionCheck()) { return; } jnienv->CallStaticVoidMethod(klass.get(), data->enter_method, method_arg); jnienv->DeleteLocalRef(method_arg); - data->in_callback = false; } static void classPrepareCB(jvmtiEnv* jvmti, @@ -459,7 +498,6 @@ extern "C" JNIEXPORT void JNICALL Java_art_Trace_enableTracing2( data->single_step = single_step != nullptr ? env->FromReflectedMethod(single_step) : nullptr; data->thread_start = thread_start != nullptr ? env->FromReflectedMethod(thread_start) : nullptr; data->thread_end = thread_end != nullptr ? env->FromReflectedMethod(thread_end) : nullptr; - data->in_callback = false; TraceData* old_data = nullptr; if (JvmtiErrorToException(env, jvmti_env, |