diff options
32 files changed, 506 insertions, 110 deletions
diff --git a/adbconnection/adbconnection.cc b/adbconnection/adbconnection.cc index ee89717043..4c2d4d72d8 100644 --- a/adbconnection/adbconnection.cc +++ b/adbconnection/adbconnection.cc @@ -825,20 +825,10 @@ void AdbConnectionState::PerformHandshake() { void AdbConnectionState::AttachJdwpAgent(art::Thread* self) { art::Runtime* runtime = art::Runtime::Current(); - if (runtime->GetJit() == nullptr && !runtime->GetInstrumentation()->IsForcedInterpretOnly()) { - // If we don't have a JIT we should try to start the jit for performance reasons. - runtime->CreateJit(); - if (runtime->GetJit() == nullptr) { - LOG(WARNING) << "Could not start jit for debugging. This process might be quite slow as it " - << "is running entirely in the interpreter. Try running 'setenforce 0' and " - << "starting the debugging session over."; - } - } self->AssertNoPendingException(); runtime->AttachAgent(/* JNIEnv */ nullptr, MakeAgentArg(), - /* classloader */ nullptr, - /*allow_non_debuggable_tooling*/ true); + /* classloader */ nullptr); if (self->IsExceptionPending()) { LOG(ERROR) << "Failed to load agent " << agent_name_; art::ScopedObjectAccess soa(self); diff --git a/compiler/Android.bp b/compiler/Android.bp index cde64b058c..ec9fef7492 100644 --- a/compiler/Android.bp +++ b/compiler/Android.bp @@ -210,7 +210,10 @@ gensrcs { art_cc_library { name: "libart-compiler", - defaults: ["libart-compiler-defaults"], + defaults: [ + "libart-compiler-defaults", + "dex2oat-pgo-defaults", + ], codegen: { arm: { // VIXL assembly support for ARM targets. @@ -244,11 +247,6 @@ art_cc_library { "libdexfile", ], - pgo: { - instrumentation: true, - profile_file: "art/dex2oat.profdata", - benchmarks: ["dex2oat"], - }, target: { android: { lto: { diff --git a/compiler/optimizing/nodes.cc b/compiler/optimizing/nodes.cc index f784f8f7f3..79bb70b9aa 100644 --- a/compiler/optimizing/nodes.cc +++ b/compiler/optimizing/nodes.cc @@ -1916,15 +1916,6 @@ const HTryBoundary* HBasicBlock::ComputeTryEntryOfSuccessors() const { } } -bool HBasicBlock::HasThrowingInstructions() const { - for (HInstructionIterator it(GetInstructions()); !it.Done(); it.Advance()) { - if (it.Current()->CanThrow()) { - return true; - } - } - return false; -} - static bool HasOnlyOneInstruction(const HBasicBlock& block) { return block.GetPhis().IsEmpty() && !block.GetInstructions().IsEmpty() diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h index 79d733060b..b315c81693 100644 --- a/compiler/optimizing/nodes.h +++ b/compiler/optimizing/nodes.h @@ -1272,8 +1272,6 @@ class HBasicBlock : public ArenaObject<kArenaAllocBasicBlock> { // the appropriate try entry will be returned. const HTryBoundary* ComputeTryEntryOfSuccessors() const; - bool HasThrowingInstructions() const; - // Returns whether this block dominates the blocked passed as parameter. bool Dominates(HBasicBlock* block) const; @@ -2132,6 +2130,7 @@ class HInstruction : public ArenaObject<kArenaAllocInstruction> { !CanThrow() && !IsSuspendCheck() && !IsControlFlow() && + !IsDeoptimize() && !IsNativeDebugInfo() && !IsParameterValue() && // If we added an explicit barrier then we should keep it. @@ -3238,7 +3237,9 @@ class HDeoptimize FINAL : public HVariableInputSizeInstruction { bool NeedsEnvironment() const OVERRIDE { return true; } - bool CanThrow() const OVERRIDE { return true; } + // Even though deoptimize is often used for "exceptional cases" to go back to + // the interpreter, it never throws an exception. + bool CanThrow() const OVERRIDE { return false; } DeoptimizationKind GetDeoptimizationKind() const { return GetPackedField<DeoptimizeKindField>(); } diff --git a/dex2oat/Android.bp b/dex2oat/Android.bp index 49b65fd35e..9353246716 100644 --- a/dex2oat/Android.bp +++ b/dex2oat/Android.bp @@ -149,10 +149,51 @@ cc_defaults { ], } +cc_defaults { + name: "dex2oat-pgo-defaults", + pgo: { + instrumentation: true, + benchmarks: ["dex2oat"], + }, + target: { + android_arm64: { + pgo: { + profile_file: "art/dex2oat_arm_arm64.profdata", + }, + }, + android_arm: { + pgo: { + profile_file: "art/dex2oat_arm_arm64.profdata", + }, + }, + android_x86_64: { + pgo: { + profile_file: "art/dex2oat_x86_x86_64.profdata", + }, + }, + android_x86: { + pgo: { + profile_file: "art/dex2oat_x86_x86_64.profdata", + }, + }, + android_mips64: { + pgo: { + profile_file: "art/dex2oat_mips_mips64.profdata", + }, + }, + android_mips: { + pgo: { + profile_file: "art/dex2oat_mips_mips64.profdata", + }, + }, + }, +} + art_cc_binary { name: "dex2oat", defaults: [ "dex2oat-defaults", + "dex2oat-pgo-defaults", ], shared_libs: [ "libart-compiler", @@ -168,9 +209,7 @@ art_cc_binary { ], pgo: { - instrumentation: true, - profile_file: "art/dex2oat.profdata", - benchmarks: ["dex2oat"], + // Additional cflags just for dex2oat during PGO instrumentation cflags: [ // Ignore frame-size increase resulting from instrumentation. "-Wno-frame-larger-than=", diff --git a/dexlayout/Android.bp b/dexlayout/Android.bp index facda11c60..33ba58f5f7 100644 --- a/dexlayout/Android.bp +++ b/dexlayout/Android.bp @@ -34,17 +34,15 @@ art_cc_defaults { art_cc_library { name: "libart-dexlayout", - defaults: ["libart-dexlayout-defaults"], + defaults: [ + "libart-dexlayout-defaults", + "dex2oat-pgo-defaults", + ], shared_libs: [ "libart", "libdexfile", ], - pgo: { - instrumentation: true, - profile_file: "art/dex2oat.profdata", - benchmarks: ["dex2oat"], - }, target: { android: { lto: { diff --git a/dexoptanalyzer/dexoptanalyzer.cc b/dexoptanalyzer/dexoptanalyzer.cc index febccf1950..871cd081e7 100644 --- a/dexoptanalyzer/dexoptanalyzer.cc +++ b/dexoptanalyzer/dexoptanalyzer.cc @@ -247,11 +247,6 @@ class DexoptAnalyzer FINAL { } int GetDexOptNeeded() { - // If the file does not exist there's nothing to do. - // This is a fast path to avoid creating the runtime (b/34385298). - if (!OS::FileExists(dex_file_.c_str())) { - return kNoDexOptNeeded; - } if (!CreateRuntime()) { return kErrorCannotCreateRuntime; } diff --git a/libdexfile/dex/descriptors_names.cc b/libdexfile/dex/descriptors_names.cc index 8124e7256f..e338b55a9f 100644 --- a/libdexfile/dex/descriptors_names.cc +++ b/libdexfile/dex/descriptors_names.cc @@ -403,22 +403,6 @@ bool IsValidDescriptor(const char* s) { return IsValidClassName<kDescriptor, '/'>(s); } -void Split(const std::string& s, char separator, std::vector<std::string>* result) { - const char* p = s.data(); - const char* end = p + s.size(); - while (p != end) { - if (*p == separator) { - ++p; - } else { - const char* start = p; - while (++p != end && *p != separator) { - // Skip to the next occurrence of the separator. - } - result->push_back(std::string(start, p - start)); - } - } -} - std::string PrettyDescriptor(Primitive::Type type) { return PrettyDescriptor(Primitive::Descriptor(type)); } diff --git a/openjdkjvmti/deopt_manager.cc b/openjdkjvmti/deopt_manager.cc index a6f1207f34..d4e5df1d67 100644 --- a/openjdkjvmti/deopt_manager.cc +++ b/openjdkjvmti/deopt_manager.cc @@ -40,6 +40,7 @@ #include "dex/dex_file_annotations.h" #include "dex/modifiers.h" #include "events-inl.h" +#include "jit/jit.h" #include "jni_internal.h" #include "mirror/class-inl.h" #include "mirror/object_array-inl.h" @@ -127,6 +128,22 @@ void DeoptManager::FinishSetup() { << "loaded too late to change runtime state to DEBUGGABLE. Only kArtTiVersion " << "(0x" << std::hex << kArtTiVersion << ") environments are available. Some " << "functionality might not work properly."; + if (runtime->GetJit() == nullptr && + runtime->GetJITOptions()->UseJitCompilation() && + !runtime->GetInstrumentation()->IsForcedInterpretOnly()) { + // If we don't have a jit we should try to start the jit for performance reasons. We only + // need to do this for late attach on non-debuggable processes because for debuggable + // processes we already rely on jit and we cannot force this jit to start if we are still in + // OnLoad since the runtime hasn't started up sufficiently. This is only expected to happen + // on userdebug/eng builds. + LOG(INFO) << "Attempting to start jit for openjdkjvmti plugin."; + runtime->CreateJit(); + if (runtime->GetJit() == nullptr) { + LOG(WARNING) << "Could not start jit for openjdkjvmti plugin. This process might be " + << "quite slow as it is running entirely in the interpreter. Try running " + << "'setenforce 0' and restarting this process."; + } + } } runtime->DeoptimizeBootImage(); } diff --git a/runtime/check_jni.cc b/runtime/check_jni.cc index c625a9700c..9a43790575 100644 --- a/runtime/check_jni.cc +++ b/runtime/check_jni.cc @@ -275,6 +275,43 @@ class VarArgs { }; }; +// Check whether the current thread is attached. This is usually required +// to be the first check, as ScopedCheck needs a ScopedObjectAccess for +// checking heap values (and that will fail with unattached threads). +bool CheckAttachedThread(const char* function_name) { + Thread* self = Thread::Current(); + if (UNLIKELY(self == nullptr)) { + // Need to attach this thread for a proper abort to work. We prefer this + // to get reasonable stacks and environment, rather than relying on + // tombstoned. + JNIEnv* env; + Runtime::Current()->GetJavaVM()->AttachCurrentThread(&env, /* thread_args */ nullptr); + + std::string tmp = android::base::StringPrintf( + "a thread (tid %" PRId64 " is making JNI calls without being attached", + static_cast<int64_t>(GetTid())); + Runtime::Current()->GetJavaVM()->JniAbort(function_name, tmp.c_str()); + + CHECK_NE(Runtime::Current()->GetJavaVM()->DetachCurrentThread(), JNI_ERR); + return false; + } + return true; +} + +// Macro helpers for the above. +#define CHECK_ATTACHED_THREAD(function_name, fail_val) \ + do { \ + if (!CheckAttachedThread((function_name))) { \ + return fail_val; \ + } \ + } while (false) +#define CHECK_ATTACHED_THREAD_VOID(function_name) \ + do { \ + if (!CheckAttachedThread((function_name))) { \ + return; \ + } \ + } while (false) + class ScopedCheck { public: ScopedCheck(uint16_t flags, const char* functionName, bool has_method = true) @@ -1255,10 +1292,7 @@ class ScopedCheck { bool CheckThread(JNIEnv* env) REQUIRES_SHARED(Locks::mutator_lock_) { Thread* self = Thread::Current(); - if (self == nullptr) { - AbortF("a thread (tid %d) is making JNI calls without being attached", GetTid()); - return false; - } + CHECK(self != nullptr); // Get the current thread's JNIEnv by going through our TLS pointer. JNIEnvExt* threadEnv = self->GetJniEnv(); @@ -1708,6 +1742,7 @@ const char* const GuardedCopy::kCanary = "JNI BUFFER RED ZONE"; class CheckJNI { public: static jint GetVersion(JNIEnv* env) { + CHECK_ATTACHED_THREAD(__FUNCTION__, JNI_ERR); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[1] = {{.E = env }}; @@ -1722,6 +1757,7 @@ class CheckJNI { } static jint GetJavaVM(JNIEnv *env, JavaVM **vm) { + CHECK_ATTACHED_THREAD(__FUNCTION__, JNI_ERR); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[2] = {{.E = env }, {.p = vm}}; @@ -1736,6 +1772,7 @@ class CheckJNI { } static jint RegisterNatives(JNIEnv* env, jclass c, const JNINativeMethod* methods, jint nMethods) { + CHECK_ATTACHED_THREAD(__FUNCTION__, JNI_ERR); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[4] = {{.E = env }, {.c = c}, {.p = methods}, {.I = nMethods}}; @@ -1750,6 +1787,7 @@ class CheckJNI { } static jint UnregisterNatives(JNIEnv* env, jclass c) { + CHECK_ATTACHED_THREAD(__FUNCTION__, JNI_ERR); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[2] = {{.E = env }, {.c = c}}; @@ -1764,6 +1802,7 @@ class CheckJNI { } static jobjectRefType GetObjectRefType(JNIEnv* env, jobject obj) { + CHECK_ATTACHED_THREAD(__FUNCTION__, JNIInvalidRefType); // Note: we use "EL" here but "Ep" has been used in the past on the basis that we'd like to // know the object is invalid. The spec says that passing invalid objects or even ones that // are deleted isn't supported. @@ -1782,6 +1821,7 @@ class CheckJNI { static jclass DefineClass(JNIEnv* env, const char* name, jobject loader, const jbyte* buf, jsize bufLen) { + CHECK_ATTACHED_THREAD(__FUNCTION__, nullptr); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[5] = {{.E = env}, {.u = name}, {.L = loader}, {.p = buf}, {.z = bufLen}}; @@ -1796,6 +1836,7 @@ class CheckJNI { } static jclass FindClass(JNIEnv* env, const char* name) { + CHECK_ATTACHED_THREAD(__FUNCTION__, nullptr); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[2] = {{.E = env}, {.u = name}}; @@ -1810,6 +1851,7 @@ class CheckJNI { } static jclass GetSuperclass(JNIEnv* env, jclass c) { + CHECK_ATTACHED_THREAD(__FUNCTION__, nullptr); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[2] = {{.E = env}, {.c = c}}; @@ -1824,6 +1866,7 @@ class CheckJNI { } static jboolean IsAssignableFrom(JNIEnv* env, jclass c1, jclass c2) { + CHECK_ATTACHED_THREAD(__FUNCTION__, JNI_FALSE); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[3] = {{.E = env}, {.c = c1}, {.c = c2}}; @@ -1838,6 +1881,7 @@ class CheckJNI { } static jmethodID FromReflectedMethod(JNIEnv* env, jobject method) { + CHECK_ATTACHED_THREAD(__FUNCTION__, nullptr); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[2] = {{.E = env}, {.L = method}}; @@ -1852,6 +1896,7 @@ class CheckJNI { } static jfieldID FromReflectedField(JNIEnv* env, jobject field) { + CHECK_ATTACHED_THREAD(__FUNCTION__, nullptr); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[2] = {{.E = env}, {.L = field}}; @@ -1866,6 +1911,7 @@ class CheckJNI { } static jobject ToReflectedMethod(JNIEnv* env, jclass cls, jmethodID mid, jboolean isStatic) { + CHECK_ATTACHED_THREAD(__FUNCTION__, nullptr); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[4] = {{.E = env}, {.c = cls}, {.m = mid}, {.I = isStatic}}; @@ -1881,6 +1927,7 @@ class CheckJNI { } static jobject ToReflectedField(JNIEnv* env, jclass cls, jfieldID fid, jboolean isStatic) { + CHECK_ATTACHED_THREAD(__FUNCTION__, nullptr); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[4] = {{.E = env}, {.c = cls}, {.f = fid}, {.I = isStatic}}; @@ -1896,6 +1943,7 @@ class CheckJNI { } static jint Throw(JNIEnv* env, jthrowable obj) { + CHECK_ATTACHED_THREAD(__FUNCTION__, JNI_ERR); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[2] = {{.E = env}, {.t = obj}}; @@ -1910,6 +1958,7 @@ class CheckJNI { } static jint ThrowNew(JNIEnv* env, jclass c, const char* message) { + CHECK_ATTACHED_THREAD(__FUNCTION__, JNI_ERR); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_NullableUtf, __FUNCTION__); JniValueType args[3] = {{.E = env}, {.c = c}, {.u = message}}; @@ -1924,6 +1973,7 @@ class CheckJNI { } static jthrowable ExceptionOccurred(JNIEnv* env) { + CHECK_ATTACHED_THREAD(__FUNCTION__, nullptr); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_ExcepOkay, __FUNCTION__); JniValueType args[1] = {{.E = env}}; @@ -1938,6 +1988,7 @@ class CheckJNI { } static void ExceptionDescribe(JNIEnv* env) { + CHECK_ATTACHED_THREAD_VOID(__FUNCTION__); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_ExcepOkay, __FUNCTION__); JniValueType args[1] = {{.E = env}}; @@ -1950,6 +2001,7 @@ class CheckJNI { } static void ExceptionClear(JNIEnv* env) { + CHECK_ATTACHED_THREAD_VOID(__FUNCTION__); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_ExcepOkay, __FUNCTION__); JniValueType args[1] = {{.E = env}}; @@ -1962,6 +2014,7 @@ class CheckJNI { } static jboolean ExceptionCheck(JNIEnv* env) { + CHECK_ATTACHED_THREAD(__FUNCTION__, JNI_FALSE); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_CritOkay | kFlag_ExcepOkay, __FUNCTION__); JniValueType args[1] = {{.E = env}}; @@ -1976,6 +2029,7 @@ class CheckJNI { } static void FatalError(JNIEnv* env, const char* msg) { + CHECK_ATTACHED_THREAD_VOID(__FUNCTION__); // The JNI specification doesn't say it's okay to call FatalError with a pending exception, // but you're about to abort anyway, and it's quite likely that you have a pending exception, // and it's not unimaginable that you don't know that you do. So we allow it. @@ -1992,6 +2046,7 @@ class CheckJNI { } static jint PushLocalFrame(JNIEnv* env, jint capacity) { + CHECK_ATTACHED_THREAD(__FUNCTION__, JNI_ERR); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_ExcepOkay, __FUNCTION__); JniValueType args[2] = {{.E = env}, {.I = capacity}}; @@ -2006,6 +2061,7 @@ class CheckJNI { } static jobject PopLocalFrame(JNIEnv* env, jobject res) { + CHECK_ATTACHED_THREAD(__FUNCTION__, nullptr); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_ExcepOkay, __FUNCTION__); JniValueType args[2] = {{.E = env}, {.L = res}}; @@ -2043,6 +2099,7 @@ class CheckJNI { } static jint EnsureLocalCapacity(JNIEnv *env, jint capacity) { + CHECK_ATTACHED_THREAD(__FUNCTION__, JNI_ERR); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[2] = {{.E = env}, {.I = capacity}}; @@ -2057,6 +2114,7 @@ class CheckJNI { } static jboolean IsSameObject(JNIEnv* env, jobject ref1, jobject ref2) { + CHECK_ATTACHED_THREAD(__FUNCTION__, JNI_FALSE); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[3] = {{.E = env}, {.L = ref1}, {.L = ref2}}; @@ -2071,6 +2129,7 @@ class CheckJNI { } static jobject AllocObject(JNIEnv* env, jclass c) { + CHECK_ATTACHED_THREAD(__FUNCTION__, nullptr); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[2] = {{.E = env}, {.c = c}}; @@ -2085,6 +2144,7 @@ class CheckJNI { } static jobject NewObjectV(JNIEnv* env, jclass c, jmethodID mid, va_list vargs) { + CHECK_ATTACHED_THREAD(__FUNCTION__, nullptr); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); VarArgs rest(mid, vargs); @@ -2101,6 +2161,7 @@ class CheckJNI { } static jobject NewObject(JNIEnv* env, jclass c, jmethodID mid, ...) { + CHECK_ATTACHED_THREAD(__FUNCTION__, nullptr); va_list args; va_start(args, mid); jobject result = NewObjectV(env, c, mid, args); @@ -2109,6 +2170,7 @@ class CheckJNI { } static jobject NewObjectA(JNIEnv* env, jclass c, jmethodID mid, jvalue* vargs) { + CHECK_ATTACHED_THREAD(__FUNCTION__, nullptr); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); VarArgs rest(mid, vargs); @@ -2125,6 +2187,7 @@ class CheckJNI { } static jclass GetObjectClass(JNIEnv* env, jobject obj) { + CHECK_ATTACHED_THREAD(__FUNCTION__, nullptr); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[2] = {{.E = env}, {.L = obj}}; @@ -2139,6 +2202,7 @@ class CheckJNI { } static jboolean IsInstanceOf(JNIEnv* env, jobject obj, jclass c) { + CHECK_ATTACHED_THREAD(__FUNCTION__, JNI_FALSE); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[3] = {{.E = env}, {.L = obj}, {.c = c}}; @@ -2314,6 +2378,7 @@ class CheckJNI { #undef CALL static jstring NewString(JNIEnv* env, const jchar* unicode_chars, jsize len) { + CHECK_ATTACHED_THREAD(__FUNCTION__, nullptr); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[3] = {{.E = env}, {.p = unicode_chars}, {.z = len}}; @@ -2328,6 +2393,7 @@ class CheckJNI { } static jstring NewStringUTF(JNIEnv* env, const char* chars) { + CHECK_ATTACHED_THREAD(__FUNCTION__, nullptr); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_NullableUtf, __FUNCTION__); JniValueType args[2] = {{.E = env}, {.u = chars}}; @@ -2343,6 +2409,7 @@ class CheckJNI { } static jsize GetStringLength(JNIEnv* env, jstring string) { + CHECK_ATTACHED_THREAD(__FUNCTION__, JNI_ERR); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_CritOkay, __FUNCTION__); JniValueType args[2] = {{.E = env}, {.s = string}}; @@ -2357,6 +2424,7 @@ class CheckJNI { } static jsize GetStringUTFLength(JNIEnv* env, jstring string) { + CHECK_ATTACHED_THREAD(__FUNCTION__, JNI_ERR); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_CritOkay, __FUNCTION__); JniValueType args[2] = {{.E = env}, {.s = string}}; @@ -2398,6 +2466,7 @@ class CheckJNI { } static void GetStringRegion(JNIEnv* env, jstring string, jsize start, jsize len, jchar* buf) { + CHECK_ATTACHED_THREAD_VOID(__FUNCTION__); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_CritOkay, __FUNCTION__); JniValueType args[5] = {{.E = env}, {.s = string}, {.z = start}, {.z = len}, {.p = buf}}; @@ -2412,6 +2481,7 @@ class CheckJNI { } static void GetStringUTFRegion(JNIEnv* env, jstring string, jsize start, jsize len, char* buf) { + CHECK_ATTACHED_THREAD_VOID(__FUNCTION__); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_CritOkay, __FUNCTION__); JniValueType args[5] = {{.E = env}, {.s = string}, {.z = start}, {.z = len}, {.p = buf}}; @@ -2426,6 +2496,7 @@ class CheckJNI { } static jsize GetArrayLength(JNIEnv* env, jarray array) { + CHECK_ATTACHED_THREAD(__FUNCTION__, JNI_ERR); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_CritOkay, __FUNCTION__); JniValueType args[2] = {{.E = env}, {.a = array}}; @@ -2441,6 +2512,7 @@ class CheckJNI { static jobjectArray NewObjectArray(JNIEnv* env, jsize length, jclass element_class, jobject initial_element) { + CHECK_ATTACHED_THREAD(__FUNCTION__, nullptr); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[4] = @@ -2457,6 +2529,7 @@ class CheckJNI { } static jobject GetObjectArrayElement(JNIEnv* env, jobjectArray array, jsize index) { + CHECK_ATTACHED_THREAD(__FUNCTION__, nullptr); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[3] = {{.E = env}, {.a = array}, {.z = index}}; @@ -2471,6 +2544,7 @@ class CheckJNI { } static void SetObjectArrayElement(JNIEnv* env, jobjectArray array, jsize index, jobject value) { + CHECK_ATTACHED_THREAD_VOID(__FUNCTION__); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[4] = {{.E = env}, {.a = array}, {.z = index}, {.L = value}}; @@ -2557,6 +2631,7 @@ class CheckJNI { #undef PRIMITIVE_ARRAY_FUNCTIONS static jint MonitorEnter(JNIEnv* env, jobject obj) { + CHECK_ATTACHED_THREAD(__FUNCTION__, JNI_ERR); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[2] = {{.E = env}, {.L = obj}}; @@ -2574,6 +2649,7 @@ class CheckJNI { } static jint MonitorExit(JNIEnv* env, jobject obj) { + CHECK_ATTACHED_THREAD(__FUNCTION__, JNI_ERR); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_ExcepOkay, __FUNCTION__); JniValueType args[2] = {{.E = env}, {.L = obj}}; @@ -2591,6 +2667,7 @@ class CheckJNI { } static void* GetPrimitiveArrayCritical(JNIEnv* env, jarray array, jboolean* is_copy) { + CHECK_ATTACHED_THREAD(__FUNCTION__, nullptr); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_CritGet, __FUNCTION__); JniValueType args[3] = {{.E = env}, {.a = array}, {.p = is_copy}}; @@ -2609,6 +2686,7 @@ class CheckJNI { } static void ReleasePrimitiveArrayCritical(JNIEnv* env, jarray array, void* carray, jint mode) { + CHECK_ATTACHED_THREAD_VOID(__FUNCTION__); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_CritRelease | kFlag_ExcepOkay, __FUNCTION__); sc.CheckNonNull(carray); @@ -2625,6 +2703,7 @@ class CheckJNI { } static jobject NewDirectByteBuffer(JNIEnv* env, void* address, jlong capacity) { + CHECK_ATTACHED_THREAD(__FUNCTION__, nullptr); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[3] = {{.E = env}, {.p = address}, {.J = capacity}}; @@ -2640,6 +2719,7 @@ class CheckJNI { } static void* GetDirectBufferAddress(JNIEnv* env, jobject buf) { + CHECK_ATTACHED_THREAD(__FUNCTION__, nullptr); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[2] = {{.E = env}, {.L = buf}}; @@ -2656,6 +2736,7 @@ class CheckJNI { } static jlong GetDirectBufferCapacity(JNIEnv* env, jobject buf) { + CHECK_ATTACHED_THREAD(__FUNCTION__, JNI_ERR); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, __FUNCTION__); JniValueType args[2] = {{.E = env}, {.L = buf}}; @@ -2681,6 +2762,7 @@ class CheckJNI { } static jobject NewRef(const char* function_name, JNIEnv* env, jobject obj, IndirectRefKind kind) { + CHECK_ATTACHED_THREAD(function_name, nullptr); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, function_name); JniValueType args[2] = {{.E = env}, {.L = obj}}; @@ -2709,6 +2791,7 @@ class CheckJNI { } static void DeleteRef(const char* function_name, JNIEnv* env, jobject obj, IndirectRefKind kind) { + CHECK_ATTACHED_THREAD_VOID(function_name); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_ExcepOkay, function_name); JniValueType args[2] = {{.E = env}, {.L = obj}}; @@ -2735,6 +2818,7 @@ class CheckJNI { static jmethodID GetMethodIDInternal(const char* function_name, JNIEnv* env, jclass c, const char* name, const char* sig, bool is_static) { + CHECK_ATTACHED_THREAD(function_name, nullptr); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, function_name); JniValueType args[4] = {{.E = env}, {.c = c}, {.u = name}, {.u = sig}}; @@ -2754,6 +2838,7 @@ class CheckJNI { static jfieldID GetFieldIDInternal(const char* function_name, JNIEnv* env, jclass c, const char* name, const char* sig, bool is_static) { + CHECK_ATTACHED_THREAD(function_name, nullptr); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, function_name); JniValueType args[4] = {{.E = env}, {.c = c}, {.u = name}, {.u = sig}}; @@ -2773,6 +2858,7 @@ class CheckJNI { static JniValueType GetField(const char* function_name, JNIEnv* env, jobject obj, jfieldID fid, bool is_static, Primitive::Type type) { + CHECK_ATTACHED_THREAD(function_name, JniValueType()); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, function_name); JniValueType args[3] = {{.E = env}, {.L = obj}, {.f = fid}}; @@ -2867,6 +2953,7 @@ class CheckJNI { static void SetField(const char* function_name, JNIEnv* env, jobject obj, jfieldID fid, bool is_static, Primitive::Type type, JniValueType value) { + CHECK_ATTACHED_THREAD_VOID(function_name); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, function_name); JniValueType args[4] = {{.E = env}, {.L = obj}, {.f = fid}, value}; @@ -2981,6 +3068,7 @@ class CheckJNI { static JniValueType CallMethodA(const char* function_name, JNIEnv* env, jobject obj, jclass c, jmethodID mid, jvalue* vargs, Primitive::Type type, InvokeType invoke) { + CHECK_ATTACHED_THREAD(function_name, JniValueType()); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, function_name); JniValueType result; @@ -3165,6 +3253,7 @@ class CheckJNI { static JniValueType CallMethodV(const char* function_name, JNIEnv* env, jobject obj, jclass c, jmethodID mid, va_list vargs, Primitive::Type type, InvokeType invoke) { + CHECK_ATTACHED_THREAD(function_name, JniValueType()); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, function_name); JniValueType result; @@ -3348,6 +3437,7 @@ class CheckJNI { static const void* GetStringCharsInternal(const char* function_name, JNIEnv* env, jstring string, jboolean* is_copy, bool utf, bool critical) { + CHECK_ATTACHED_THREAD(function_name, nullptr); ScopedObjectAccess soa(env); int flags = critical ? kFlag_CritGet : kFlag_CritOkay; ScopedCheck sc(flags, function_name); @@ -3388,6 +3478,7 @@ class CheckJNI { static void ReleaseStringCharsInternal(const char* function_name, JNIEnv* env, jstring string, const void* chars, bool utf, bool critical) { + CHECK_ATTACHED_THREAD_VOID(function_name); ScopedObjectAccess soa(env); int flags = kFlag_ExcepOkay | kFlag_Release; if (critical) { @@ -3420,6 +3511,7 @@ class CheckJNI { static jarray NewPrimitiveArray(const char* function_name, JNIEnv* env, jsize length, Primitive::Type type) { + CHECK_ATTACHED_THREAD(function_name, nullptr); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, function_name); JniValueType args[2] = {{.E = env}, {.z = length}}; @@ -3462,6 +3554,7 @@ class CheckJNI { static void* GetPrimitiveArrayElements(const char* function_name, Primitive::Type type, JNIEnv* env, jarray array, jboolean* is_copy) { + CHECK_ATTACHED_THREAD(function_name, nullptr); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, function_name); JniValueType args[3] = {{.E = env}, {.a = array}, {.p = is_copy}}; @@ -3513,6 +3606,7 @@ class CheckJNI { static void ReleasePrimitiveArrayElements(const char* function_name, Primitive::Type type, JNIEnv* env, jarray array, void* elems, jint mode) { + CHECK_ATTACHED_THREAD_VOID(function_name); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_ExcepOkay, function_name); if (sc.CheckNonNull(elems) && sc.CheckPrimitiveArrayType(soa, array, type)) { @@ -3568,6 +3662,7 @@ class CheckJNI { static void GetPrimitiveArrayRegion(const char* function_name, Primitive::Type type, JNIEnv* env, jarray array, jsize start, jsize len, void* buf) { + CHECK_ATTACHED_THREAD_VOID(function_name); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, function_name); JniValueType args[5] = {{.E = env}, {.a = array}, {.z = start}, {.z = len}, {.p = buf}}; @@ -3618,6 +3713,7 @@ class CheckJNI { static void SetPrimitiveArrayRegion(const char* function_name, Primitive::Type type, JNIEnv* env, jarray array, jsize start, jsize len, const void* buf) { + CHECK_ATTACHED_THREAD_VOID(function_name); ScopedObjectAccess soa(env); ScopedCheck sc(kFlag_Default, function_name); JniValueType args[5] = {{.E = env}, {.a = array}, {.z = start}, {.z = len}, {.p = buf}}; diff --git a/runtime/debug_print.cc b/runtime/debug_print.cc index 60487670ac..7e075ce352 100644 --- a/runtime/debug_print.cc +++ b/runtime/debug_print.cc @@ -97,14 +97,27 @@ std::string DescribeLoaders(ObjPtr<mirror::ClassLoader> loader, const char* clas StackHandleScope<1> hs(soa.Self()); Handle<mirror::ClassLoader> handle(hs.NewHandle(loader)); const char* path_separator = ""; - VisitClassLoaderDexFiles(soa, - handle, - [&](const DexFile* dex_file) { - oss << path_separator << dex_file->GetLocation() - << "/" << static_cast<const void*>(dex_file); - path_separator = ":"; - return true; // Continue with the next DexFile. - }); + const DexFile* base_dex_file = nullptr; + VisitClassLoaderDexFiles( + soa, + handle, + [&](const DexFile* dex_file) { + oss << path_separator; + path_separator = ":"; + if (base_dex_file != nullptr && + dex_file->GetLocation().length() > base_dex_file->GetLocation().length() && + dex_file->GetLocation().compare(0u, + base_dex_file->GetLocation().length(), + base_dex_file->GetLocation()) == 0) { + // Replace the base location with "+" to shorten the output. + oss << "+" << dex_file->GetLocation().substr(base_dex_file->GetLocation().length()); + } else { + oss << dex_file->GetLocation(); + base_dex_file = dex_file; + } + oss << "/" << static_cast<const void*>(dex_file); + return true; // Continue with the next DexFile. + }); oss << ")"; } } @@ -132,13 +145,13 @@ void DumpB77342775DebugData(ObjPtr<mirror::Class> target_class, ObjPtr<mirror::C std::string source_descriptor_storage; const char* source_descriptor = src_class->GetDescriptor(&source_descriptor_storage); + LOG(ERROR) << "Maybe bug 77342775, looking for " << target_descriptor + << " with loader " << DescribeLoaders(target_class->GetClassLoader(), target_descriptor); if (target_class->IsInterface()) { ObjPtr<mirror::IfTable> iftable = src_class->GetIfTable(); CHECK(iftable != nullptr); size_t ifcount = iftable->Count(); - LOG(ERROR) << "Maybe bug 77342775, looking for " << target_descriptor - << " with loader " << DescribeLoaders(target_class->GetClassLoader(), target_descriptor) - << " in interface table for " << source_descriptor << " ifcount=" << ifcount + LOG(ERROR) << " in interface table for " << source_descriptor << " ifcount=" << ifcount << " with loader " << DescribeLoaders(src_class->GetClassLoader(), source_descriptor); for (size_t i = 0; i != ifcount; ++i) { ObjPtr<mirror::Class> iface = iftable->GetInterface(i); @@ -147,9 +160,7 @@ void DumpB77342775DebugData(ObjPtr<mirror::Class> target_class, ObjPtr<mirror::C matcher(iface); } } else { - LOG(ERROR) << "Maybe bug 77342775, looking for " << target_descriptor - << " with loader " << DescribeLoaders(target_class->GetClassLoader(), target_descriptor) - << " in superclass chain for " << source_descriptor + LOG(ERROR) << " in superclass chain for " << source_descriptor << " with loader " << DescribeLoaders(src_class->GetClassLoader(), source_descriptor); for (ObjPtr<mirror::Class> klass = src_class; klass != nullptr; diff --git a/runtime/jni_internal_test.cc b/runtime/jni_internal_test.cc index 293e18a5b5..5d74181cef 100644 --- a/runtime/jni_internal_test.cc +++ b/runtime/jni_internal_test.cc @@ -2517,4 +2517,32 @@ TEST_F(JniInternalTest, JNIEnvExtTableOverride) { env_->DeleteGlobalRef(global2); } +TEST_F(JniInternalTest, NonAttachedThread) { + // This tests leads to warnings and errors in the log. + ScopedLogSeverity sls(LogSeverity::FATAL); + CheckJniAbortCatcher check_jni_abort_catcher; + + auto callee = [](void* env_ptr) -> void* { + JNIEnv* env = reinterpret_cast<JNIEnv*>(env_ptr); + env->NewStringUTF("test"); + return nullptr; + }; + + bool old_check_jni = vm_->SetCheckJniEnabled(false); + vm_->SetCheckJniEnabled(true); + { + pthread_t pthread; + int pthread_create_result = pthread_create(&pthread, + /* pthread_attr */ nullptr, + callee, + reinterpret_cast<void*>(env_)); + CHECK_EQ(pthread_create_result, 0); + int pthread_join_result = pthread_join(pthread, /* thread_return */ nullptr); + CHECK_EQ(pthread_join_result, 0); + } + vm_->SetCheckJniEnabled(old_check_jni); + + check_jni_abort_catcher.Check("is making JNI calls without being attached"); +} + } // namespace art diff --git a/runtime/runtime.cc b/runtime/runtime.cc index c394fefb38..8f5295cec6 100644 --- a/runtime/runtime.cc +++ b/runtime/runtime.cc @@ -1619,7 +1619,6 @@ bool Runtime::Init(RuntimeArgumentMap&& runtime_options_in) { } static bool EnsureJvmtiPlugin(Runtime* runtime, - bool allow_non_debuggable_tooling, std::vector<Plugin>* plugins, std::string* error_msg) { constexpr const char* plugin_name = kIsDebugBuild ? "libopenjdkjvmtid.so" : "libopenjdkjvmti.so"; @@ -1631,10 +1630,13 @@ static bool EnsureJvmtiPlugin(Runtime* runtime, } } + // TODO Rename Dbg::IsJdwpAllowed is IsDebuggingAllowed. + DCHECK(Dbg::IsJdwpAllowed() || !runtime->IsJavaDebuggable()) + << "Being debuggable requires that jdwp (i.e. debugging) is allowed."; // Is the process debuggable? Otherwise, do not attempt to load the plugin unless we are // specifically allowed. - if (!allow_non_debuggable_tooling && !runtime->IsJavaDebuggable()) { - *error_msg = "Process is not debuggable."; + if (!Dbg::IsJdwpAllowed()) { + *error_msg = "Process is not allowed to load openjdkjvmti plugin. Process must be debuggable"; return false; } @@ -1654,12 +1656,9 @@ static bool EnsureJvmtiPlugin(Runtime* runtime, // revisit this and make sure we're doing this on the right thread // (and we synchronize access to any shared data structures like "agents_") // -void Runtime::AttachAgent(JNIEnv* env, - const std::string& agent_arg, - jobject class_loader, - bool allow_non_debuggable_tooling) { +void Runtime::AttachAgent(JNIEnv* env, const std::string& agent_arg, jobject class_loader) { std::string error_msg; - if (!EnsureJvmtiPlugin(this, allow_non_debuggable_tooling, &plugins_, &error_msg)) { + if (!EnsureJvmtiPlugin(this, &plugins_, &error_msg)) { LOG(WARNING) << "Could not load plugin: " << error_msg; ScopedObjectAccess soa(Thread::Current()); ThrowIOException("%s", error_msg.c_str()); diff --git a/runtime/runtime.h b/runtime/runtime.h index 3d4b596349..1b7663cbdf 100644 --- a/runtime/runtime.h +++ b/runtime/runtime.h @@ -710,10 +710,7 @@ class Runtime { void AddSystemWeakHolder(gc::AbstractSystemWeakHolder* holder); void RemoveSystemWeakHolder(gc::AbstractSystemWeakHolder* holder); - void AttachAgent(JNIEnv* env, - const std::string& agent_arg, - jobject class_loader, - bool allow_non_debuggable_tooling = false); + void AttachAgent(JNIEnv* env, const std::string& agent_arg, jobject class_loader); const std::list<std::unique_ptr<ti::Agent>>& GetAgents() const { return agents_; diff --git a/test/683-deopt-regression/expected.txt b/test/683-deopt-regression/expected.txt new file mode 100644 index 0000000000..b0aad4deb5 --- /dev/null +++ b/test/683-deopt-regression/expected.txt @@ -0,0 +1 @@ +passed diff --git a/test/683-deopt-regression/info.txt b/test/683-deopt-regression/info.txt new file mode 100644 index 0000000000..0c2cb81af7 --- /dev/null +++ b/test/683-deopt-regression/info.txt @@ -0,0 +1 @@ +Regression test on deopt from BCE diff --git a/test/683-deopt-regression/smali/Deopt.smali b/test/683-deopt-regression/smali/Deopt.smali new file mode 100644 index 0000000000..3bd9f6cf75 --- /dev/null +++ b/test/683-deopt-regression/smali/Deopt.smali @@ -0,0 +1,60 @@ +# Copyright (C) 2018 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +.class public LDeopt; + +.super Ljava/lang/Object; + +.method public constructor <init>()V +.registers 1 + invoke-direct {v0}, Ljava/lang/Object;-><init>()V + return-void +.end method + +.method public static testCase([I)I + .registers 8 + + const v0, 0x0 # counter + const v1, 0xF # loop max + const v2, 0x0 # result + + :try_start + # Something throwing to start the try block. v6 contains a reference. + move-object v6, p0 + aget v3, p0, v0 + + # Invalidate v6 before entering the loop. + const-wide v5, 0x0 + + :loop_start + # Set v6 to a different reference (creates a catch phi). + const v6, 0x0 + + aget v3, p0, v0 + add-int/2addr v2, v3 + add-int/lit8 v0, v0, 0x1 + if-lt v0, v1, :loop_start + + :try_end + .catchall {:try_start .. :try_end} :catch + + :exit + return v2 + + :catch + invoke-virtual {v6}, Ljava/lang/Object;->hashCode()I # use v6 as a reference + goto :exit + +.end method + diff --git a/test/683-deopt-regression/src/Main.java b/test/683-deopt-regression/src/Main.java new file mode 100644 index 0000000000..326fe47c89 --- /dev/null +++ b/test/683-deopt-regression/src/Main.java @@ -0,0 +1,54 @@ +/* + * Copyright (C) 2018 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. + */ + +import java.lang.reflect.Method; + +public class Main { + public static void main(String[] args) throws Exception { + if (System.getProperty("java.vm.name").equals("Dalvik")) { + Class<?> c = Class.forName("Deopt"); + Method m = c.getMethod("testCase", int[].class); + int[] p = null; + try { + m.invoke(null, p); + System.out.println("should not reach"); + } catch (Exception e) { + // Tried to invoke hashCode on incoming null. + } + int result; + int[] q = { 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15 }; + result = ((Integer) m.invoke(null, q)); + expectEquals(120, result); + int[] r = { }; + result = ((Integer) m.invoke(null, r)); + expectEquals(0, result); + int[] s = { 1 }; + try { + m.invoke(null, s); + System.out.println("should not reach"); + } catch (Exception e) { + // Tried to invoke hashCode on generated null. + } + } + System.out.println("passed"); + } + + private static void expectEquals(int expected, int result) { + if (expected != result) { + throw new Error("Expected: " + expected + ", found: " + result); + } + } +} diff --git a/test/909-attach-agent/attach.cc b/test/909-attach-agent/attach.cc index 3a6788a8e3..50ab26a374 100644 --- a/test/909-attach-agent/attach.cc +++ b/test/909-attach-agent/attach.cc @@ -32,6 +32,8 @@ static void Println(const char* c) { fflush(stdout); } +static constexpr jint kArtTiVersion = JVMTI_VERSION_1_2 | 0x40000000; + jint OnAttach(JavaVM* vm, char* options ATTRIBUTE_UNUSED, void* reserved ATTRIBUTE_UNUSED) { @@ -47,7 +49,18 @@ jint OnAttach(JavaVM* vm, } \ } while (false) - CHECK_CALL_SUCCESS(vm->GetEnv(reinterpret_cast<void**>(&env), JVMTI_VERSION_1_0)); + if (vm->GetEnv(reinterpret_cast<void**>(&env), kArtTiVersion) == JNI_OK) { + Println("Created env for kArtTiVersion"); + CHECK_CALL_SUCCESS(env->DisposeEnvironment()); + env = nullptr; + } else { + Println("Failed to create env for kArtTiVersion"); + return -1; + } + if (vm->GetEnv(reinterpret_cast<void**>(&env), JVMTI_VERSION_1_0) != JNI_OK) { + Println("Unable to create env for JVMTI_VERSION_1_0"); + return 0; + } CHECK_CALL_SUCCESS(vm->GetEnv(reinterpret_cast<void**>(&env2), JVMTI_VERSION_1_0)); if (env == env2) { Println("GetEnv returned same environment twice!"); diff --git a/test/909-attach-agent/disallow_debugging.cc b/test/909-attach-agent/disallow_debugging.cc new file mode 100644 index 0000000000..4c70f62e55 --- /dev/null +++ b/test/909-attach-agent/disallow_debugging.cc @@ -0,0 +1,27 @@ +/* + * Copyright (C) 2018 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. + */ + +#include "debugger.h" + +namespace art { +namespace Test909AttachAgent { + +extern "C" JNIEXPORT void JNICALL Java_Main_setDebuggingAllowed(JNIEnv*, jclass, jboolean val) { + Dbg::SetJdwpAllowed(val); +} + +} // namespace Test909AttachAgent +} // namespace art diff --git a/test/909-attach-agent/expected.txt b/test/909-attach-agent/expected.txt index 4d687f531e..eec767d703 100644 --- a/test/909-attach-agent/expected.txt +++ b/test/909-attach-agent/expected.txt @@ -1,12 +1,28 @@ +JNI_OnLoad called Hello, world! Attached Agent for test 909-attach-agent +Created env for kArtTiVersion Attached Agent for test 909-attach-agent +Created env for kArtTiVersion Goodbye! +JNI_OnLoad called Hello, world! Attached Agent for test 909-attach-agent +Created env for kArtTiVersion Attached Agent for test 909-attach-agent +Created env for kArtTiVersion Goodbye! +JNI_OnLoad called Hello, world! -Process is not debuggable. -Process is not debuggable. +Attached Agent for test 909-attach-agent +Created env for kArtTiVersion +version 0x30010000 is not valid!Unable to create env for JVMTI_VERSION_1_0 +Attached Agent for test 909-attach-agent +Created env for kArtTiVersion +version 0x30010000 is not valid!Unable to create env for JVMTI_VERSION_1_0 +Goodbye! +JNI_OnLoad called +Hello, world! +Can't attach agent, process is not debuggable. +Can't attach agent, process is not debuggable. Goodbye! diff --git a/test/909-attach-agent/interpreter-expected.patch b/test/909-attach-agent/interpreter-expected.patch new file mode 100644 index 0000000000..5035c6a4cc --- /dev/null +++ b/test/909-attach-agent/interpreter-expected.patch @@ -0,0 +1,4 @@ +19d18 +< version 0x30010000 is not valid!Unable to create env for JVMTI_VERSION_1_0 +22d20 +< version 0x30010000 is not valid!Unable to create env for JVMTI_VERSION_1_0 diff --git a/test/909-attach-agent/run b/test/909-attach-agent/run index a556bbaffe..fd45abde6b 100755 --- a/test/909-attach-agent/run +++ b/test/909-attach-agent/run @@ -21,6 +21,14 @@ if [[ "$@" == *"-O"* ]]; then plugin=libopenjdkjvmti.so fi +if [[ "$@" == *"--interpreter"* ]]; then + # On interpreter we are fully capable of providing the full jvmti api so we + # have a slightly different expected output. + # TODO We should really be changing this in the 'check' script. + patch -s expected.txt <interpreter-expected.patch +fi + +export ANDROID_LOG_TAGS='*:f' ./default-run "$@" --android-runtime-option -Xplugin:${plugin} \ --android-runtime-option -Xcompiler-option \ --android-runtime-option --debuggable \ @@ -32,8 +40,16 @@ return_status1=$? --args agent:${agent}=909-attach-agent return_status2=$? -./default-run "$@" --args agent:${agent}=909-attach-agent +./default-run "$@" --args agent:${agent}=909-attach-agent --external-log-tags return_status3=$? +./default-run "$@" --args agent:${agent}=909-attach-agent \ + --args disallow-debugging \ + --external-log-tags +return_status4=$? + # Make sure we don't silently ignore an early failure. -(exit $return_status1) && (exit $return_status2) && (exit $return_status3) +(exit $return_status1) && \ + (exit $return_status2) && \ + (exit $return_status3) && \ + (exit $return_status4) diff --git a/test/909-attach-agent/src-art/Main.java b/test/909-attach-agent/src-art/Main.java index 705e61eb99..6359f7e6ae 100644 --- a/test/909-attach-agent/src-art/Main.java +++ b/test/909-attach-agent/src-art/Main.java @@ -24,23 +24,36 @@ import java.io.File; import java.io.IOException; public class Main { - public static void main(String[] args) { + public static void main(String[] args) throws Exception { + System.loadLibrary(args[0]); System.out.println("Hello, world!"); + String agent = null; + // By default allow debugging + boolean debugging_allowed = true; for(String a : args) { if(a.startsWith("agent:")) { - String agent = a.substring(6); - try { - VMDebug.attachAgent(agent); - } catch(IOException e) { - System.out.println(e.getMessage()); - } + agent = a.substring(6); + } else if (a.equals("disallow-debugging")) { + debugging_allowed = false; } } + if (agent == null) { + throw new Error("Could not find agent: argument!"); + } + setDebuggingAllowed(debugging_allowed); + // Setup is finished. Try to attach agent in 2 ways. + try { + VMDebug.attachAgent(agent); + } catch(SecurityException e) { + System.out.println(e.getMessage()); + } attachWithClassLoader(args); System.out.println("Goodbye!"); } - private static void attachWithClassLoader(String[] args) { + private static native void setDebuggingAllowed(boolean val); + + private static void attachWithClassLoader(String[] args) throws Exception { for(String a : args) { if(a.startsWith("agent:")) { String agentName = a.substring(6, a.indexOf('=')); @@ -56,7 +69,7 @@ public class Main { Main.class.getClassLoader()); try { VMDebug.attachAgent(agent, cl); - } catch(IOException e) { + } catch(SecurityException e) { System.out.println(e.getMessage()); } } catch (Exception e) { diff --git a/test/Android.bp b/test/Android.bp index bd13de2fa9..b9312c8f15 100644 --- a/test/Android.bp +++ b/test/Android.bp @@ -440,6 +440,7 @@ cc_defaults { "667-jit-jni-stub/jit_jni_stub_test.cc", "674-hiddenapi/hiddenapi.cc", "708-jit-cache-churn/jit.cc", + "909-attach-agent/disallow_debugging.cc", "1947-breakpoint-redefine-deopt/check_deopt.cc", "common/runtime_state.cc", "common/stack_inspect.cc", diff --git a/tools/libcore_gcstress_debug_failures.txt b/tools/libcore_gcstress_debug_failures.txt index 0029b0a01a..06f6339d76 100644 --- a/tools/libcore_gcstress_debug_failures.txt +++ b/tools/libcore_gcstress_debug_failures.txt @@ -17,5 +17,22 @@ "libcore.java.util.TimeZoneTest#testSetDefaultDeadlock", "libcore.javax.crypto.CipherBasicsTest#testBasicEncryption", "org.apache.harmony.tests.java.util.TimerTest#testThrowingTaskKillsTimerThread"] +}, +{ + description: "Sometimes times out with gcstress and debug.", + result: EXEC_FAILED, + bug: 78228743, + names: [ + "libcore.libcore.icu.RelativeDateTimeFormatterTest#test_combineDateAndTime_apostrophe", + "libcore.libcore.icu.RelativeDateTimeFormatterTest#test_getRelativeDateTimeString", + "libcore.libcore.icu.RelativeDateTimeFormatterTest#test_getRelativeDateTimeStringCTS", + "libcore.libcore.icu.RelativeDateTimeFormatterTest#test_getRelativeDateTimeStringDST", + "libcore.libcore.icu.RelativeDateTimeFormatterTest#test_getRelativeDateTimeStringItalian", + "libcore.libcore.icu.RelativeDateTimeFormatterTest#test_getRelativeTimeSpanString", + "libcore.libcore.icu.RelativeDateTimeFormatterTest#test_getRelativeTimeSpanStringAbbrev", + "libcore.libcore.icu.RelativeDateTimeFormatterTest#test_getRelativeTimeSpanStringCTS", + "libcore.libcore.icu.RelativeDateTimeFormatterTest#test_getRelativeTimeSpanStringFrench", + "libcore.libcore.icu.RelativeDateTimeFormatterTest#test_getRelativeTimeSpanStringGerman" + ] } ] diff --git a/tools/veridex/flow_analysis.cc b/tools/veridex/flow_analysis.cc index abd0b9b28c..a4553f9613 100644 --- a/tools/veridex/flow_analysis.cc +++ b/tools/veridex/flow_analysis.cc @@ -41,7 +41,11 @@ bool VeriFlowAnalysis::IsBranchTarget(uint32_t dex_pc) { bool VeriFlowAnalysis::MergeRegisterValues(uint32_t dex_pc) { // TODO: Do the merging. Right now, just return that we should continue // the iteration if the instruction has not been visited. - return !instruction_infos_[dex_pc].has_been_visited; + if (!instruction_infos_[dex_pc].has_been_visited) { + dex_registers_[dex_pc]->assign(current_registers_.begin(), current_registers_.end()); + return true; + } + return false; } void VeriFlowAnalysis::SetVisited(uint32_t dex_pc) { @@ -260,6 +264,10 @@ void VeriFlowAnalysis::ProcessDexInstruction(const Instruction& instruction) { RegisterValue obj = GetRegister(args[0]); last_result_ = RegisterValue( obj.GetSource(), obj.GetDexFileReference(), VeriClass::class_); + } else if (method == VeriClass::loadClass_) { + RegisterValue value = GetRegister(args[1]); + last_result_ = RegisterValue( + value.GetSource(), value.GetDexFileReference(), VeriClass::class_); } else { last_result_ = GetReturnType(instruction.VRegB_35c()); } diff --git a/tools/veridex/flow_analysis.h b/tools/veridex/flow_analysis.h index c065fb8c24..80ae5fc9df 100644 --- a/tools/veridex/flow_analysis.h +++ b/tools/veridex/flow_analysis.h @@ -20,6 +20,7 @@ #include "dex/code_item_accessors.h" #include "dex/dex_file_reference.h" #include "dex/method_reference.h" +#include "hidden_api.h" #include "veridex.h" namespace art { @@ -52,10 +53,19 @@ class RegisterValue { DexFileReference GetDexFileReference() const { return reference_; } const VeriClass* GetType() const { return type_; } - const char* ToString() const { + std::string ToString() const { switch (source_) { - case RegisterSource::kString: - return reference_.dex_file->StringDataByIdx(dex::StringIndex(reference_.index)); + case RegisterSource::kString: { + const char* str = reference_.dex_file->StringDataByIdx(dex::StringIndex(reference_.index)); + if (type_ == VeriClass::class_) { + // Class names at the Java level are of the form x.y.z, but the list encodes + // them of the form Lx/y/z;. Inner classes have '$' for both Java level class + // names in strings, and hidden API lists. + return HiddenApi::ToInternalName(str); + } else { + return str; + } + } case RegisterSource::kClass: return reference_.dex_file->StringByTypeIdx(dex::TypeIndex(reference_.index)); default: diff --git a/tools/veridex/hidden_api.h b/tools/veridex/hidden_api.h index 4c67768a00..b1c8559374 100644 --- a/tools/veridex/hidden_api.h +++ b/tools/veridex/hidden_api.h @@ -63,6 +63,12 @@ class HiddenApi { return HiddenApi::GetApiMethodName(*ref.dex_file, ref.index); } + static std::string ToInternalName(const std::string& str) { + std::string val = str; + std::replace(val.begin(), val.end(), '.', '/'); + return "L" + val + ";"; + } + private: static bool IsInList(const std::string& name, const std::set<std::string>& list) { return list.find(name) != list.end(); diff --git a/tools/veridex/hidden_api_finder.cc b/tools/veridex/hidden_api_finder.cc index b9be618ed7..b1ae7dd804 100644 --- a/tools/veridex/hidden_api_finder.cc +++ b/tools/veridex/hidden_api_finder.cc @@ -95,9 +95,7 @@ void HiddenApiFinder::CollectAccesses(VeridexResolver* resolver) { // Class names at the Java level are of the form x.y.z, but the list encodes // them of the form Lx/y/z;. Inner classes have '$' for both Java level class // names in strings, and hidden API lists. - std::string str = name; - std::replace(str.begin(), str.end(), '.', '/'); - str = "L" + str + ";"; + std::string str = HiddenApi::ToInternalName(name); // Note: we can query the lists directly, as HiddenApi added classes that own // private methods and fields in them. // We don't add class names to the `strings_` set as we know method/field names diff --git a/tools/veridex/veridex.cc b/tools/veridex/veridex.cc index 6e72faaf57..dc7ea94032 100644 --- a/tools/veridex/veridex.cc +++ b/tools/veridex/veridex.cc @@ -52,6 +52,7 @@ VeriClass* VeriClass::void_ = &v_; // Will be set after boot classpath has been resolved. VeriClass* VeriClass::object_ = nullptr; VeriClass* VeriClass::class_ = nullptr; +VeriClass* VeriClass::class_loader_ = nullptr; VeriClass* VeriClass::string_ = nullptr; VeriClass* VeriClass::throwable_ = nullptr; VeriMethod VeriClass::forName_ = nullptr; @@ -60,6 +61,7 @@ VeriMethod VeriClass::getDeclaredField_ = nullptr; VeriMethod VeriClass::getMethod_ = nullptr; VeriMethod VeriClass::getDeclaredMethod_ = nullptr; VeriMethod VeriClass::getClass_ = nullptr; +VeriMethod VeriClass::loadClass_ = nullptr; struct VeridexOptions { const char* dex_file = nullptr; @@ -176,6 +178,7 @@ class Veridex { // methods. VeriClass::object_ = type_map["Ljava/lang/Object;"]; VeriClass::class_ = type_map["Ljava/lang/Class;"]; + VeriClass::class_loader_ = type_map["Ljava/lang/ClassLoader;"]; VeriClass::string_ = type_map["Ljava/lang/String;"]; VeriClass::throwable_ = type_map["Ljava/lang/Throwable;"]; VeriClass::forName_ = boot_resolvers[0]->LookupDeclaredMethodIn( @@ -194,6 +197,8 @@ class Veridex { "(Ljava/lang/String;[Ljava/lang/Class;)Ljava/lang/reflect/Method;"); VeriClass::getClass_ = boot_resolvers[0]->LookupDeclaredMethodIn( *VeriClass::object_, "getClass", "()Ljava/lang/Class;"); + VeriClass::loadClass_ = boot_resolvers[0]->LookupDeclaredMethodIn( + *VeriClass::class_loader_, "loadClass", "(Ljava/lang/String;)Ljava/lang/Class;"); std::vector<std::unique_ptr<VeridexResolver>> app_resolvers; Resolve(app_dex_files, resolver_map, type_map, &app_resolvers); diff --git a/tools/veridex/veridex.h b/tools/veridex/veridex.h index 75e4845293..9c0a158174 100644 --- a/tools/veridex/veridex.h +++ b/tools/veridex/veridex.h @@ -65,6 +65,7 @@ class VeriClass { static VeriClass* object_; static VeriClass* class_; + static VeriClass* class_loader_; static VeriClass* string_; static VeriClass* throwable_; static VeriClass* boolean_; @@ -83,6 +84,7 @@ class VeriClass { static VeriMethod getMethod_; static VeriMethod getDeclaredMethod_; static VeriMethod getClass_; + static VeriMethod loadClass_; private: Primitive::Type kind_; |