diff options
| -rw-r--r-- | compiler/optimizing/ssa_liveness_analysis.h | 2 | ||||
| -rw-r--r-- | runtime/interpreter/unstarted_runtime.cc | 195 | ||||
| -rw-r--r-- | runtime/interpreter/unstarted_runtime_list.h | 3 | ||||
| -rw-r--r-- | runtime/interpreter/unstarted_runtime_test.cc | 113 | ||||
| -rw-r--r-- | runtime/monitor.cc | 186 | ||||
| -rw-r--r-- | runtime/monitor.h | 17 | ||||
| -rw-r--r-- | runtime/thread.h | 1 | ||||
| -rw-r--r-- | runtime/thread_list.cc | 7 | ||||
| -rw-r--r-- | runtime/thread_list.h | 4 | ||||
| -rw-r--r-- | test/594-checker-irreducible-linorder/smali/IrreducibleLoop.smali | 5 |
10 files changed, 459 insertions, 74 deletions
diff --git a/compiler/optimizing/ssa_liveness_analysis.h b/compiler/optimizing/ssa_liveness_analysis.h index 719feec468..40dab74a23 100644 --- a/compiler/optimizing/ssa_liveness_analysis.h +++ b/compiler/optimizing/ssa_liveness_analysis.h @@ -971,7 +971,7 @@ class LiveInterval : public ArenaObject<kArenaAllocSsaLiveness> { bool IsLinearOrderWellFormed(const HGraph& graph) { for (HBasicBlock* header : graph.GetBlocks()) { - if (!header->IsLoopHeader()) { + if (header == nullptr || !header->IsLoopHeader()) { continue; } diff --git a/runtime/interpreter/unstarted_runtime.cc b/runtime/interpreter/unstarted_runtime.cc index fe388cd16d..a2a190e588 100644 --- a/runtime/interpreter/unstarted_runtime.cc +++ b/runtime/interpreter/unstarted_runtime.cc @@ -16,11 +16,13 @@ #include "unstarted_runtime.h" +#include <ctype.h> #include <errno.h> #include <stdlib.h> #include <cmath> #include <limits> +#include <locale> #include <unordered_map> #include "ScopedLocalRef.h" @@ -70,6 +72,43 @@ static void AbortTransactionOrFail(Thread* self, const char* fmt, ...) { } } +// Restricted support for character upper case / lower case. Only support ASCII, where +// it's easy. Abort the transaction otherwise. +static void CharacterLowerUpper(Thread* self, + ShadowFrame* shadow_frame, + JValue* result, + size_t arg_offset, + bool to_lower_case) SHARED_REQUIRES(Locks::mutator_lock_) { + uint32_t int_value = static_cast<uint32_t>(shadow_frame->GetVReg(arg_offset)); + + // Only ASCII (7-bit). + if (!isascii(int_value)) { + AbortTransactionOrFail(self, + "Only support ASCII characters for toLowerCase/toUpperCase: %u", + int_value); + return; + } + + std::locale c_locale("C"); + char char_value = static_cast<char>(int_value); + + if (to_lower_case) { + result->SetI(std::tolower(char_value, c_locale)); + } else { + result->SetI(std::toupper(char_value, c_locale)); + } +} + +void UnstartedRuntime::UnstartedCharacterToLowerCase( + Thread* self, ShadowFrame* shadow_frame, JValue* result, size_t arg_offset) { + CharacterLowerUpper(self, shadow_frame, result, arg_offset, true); +} + +void UnstartedRuntime::UnstartedCharacterToUpperCase( + Thread* self, ShadowFrame* shadow_frame, JValue* result, size_t arg_offset) { + CharacterLowerUpper(self, shadow_frame, result, arg_offset, false); +} + // Helper function to deal with class loading in an unstarted runtime. static void UnstartedRuntimeFindClass(Thread* self, Handle<mirror::String> className, Handle<mirror::ClassLoader> class_loader, JValue* result, @@ -313,6 +352,162 @@ void UnstartedRuntime::UnstartedClassGetEnclosingClass( result->SetL(klass->GetDexFile().GetEnclosingClass(klass)); } +static std::unique_ptr<MemMap> FindAndExtractEntry(const std::string& jar_file, + const char* entry_name, + size_t* size, + std::string* error_msg) { + CHECK(size != nullptr); + + std::unique_ptr<ZipArchive> zip_archive(ZipArchive::Open(jar_file.c_str(), error_msg)); + if (zip_archive == nullptr) { + return nullptr;; + } + std::unique_ptr<ZipEntry> zip_entry(zip_archive->Find(entry_name, error_msg)); + if (zip_entry == nullptr) { + return nullptr; + } + std::unique_ptr<MemMap> tmp_map( + zip_entry->ExtractToMemMap(jar_file.c_str(), entry_name, error_msg)); + if (tmp_map == nullptr) { + return nullptr; + } + + // OK, from here everything seems fine. + *size = zip_entry->GetUncompressedLength(); + return tmp_map; +} + +static void GetResourceAsStream(Thread* self, + ShadowFrame* shadow_frame, + JValue* result, + size_t arg_offset) SHARED_REQUIRES(Locks::mutator_lock_) { + mirror::Object* resource_obj = shadow_frame->GetVRegReference(arg_offset + 1); + if (resource_obj == nullptr) { + AbortTransactionOrFail(self, "null name for getResourceAsStream"); + return; + } + CHECK(resource_obj->IsString()); + mirror::String* resource_name = resource_obj->AsString(); + + std::string resource_name_str = resource_name->ToModifiedUtf8(); + if (resource_name_str.empty() || resource_name_str == "/") { + AbortTransactionOrFail(self, + "Unsupported name %s for getResourceAsStream", + resource_name_str.c_str()); + return; + } + const char* resource_cstr = resource_name_str.c_str(); + if (resource_cstr[0] == '/') { + resource_cstr++; + } + + Runtime* runtime = Runtime::Current(); + + std::vector<std::string> split; + Split(runtime->GetBootClassPathString(), ':', &split); + if (split.empty()) { + AbortTransactionOrFail(self, + "Boot classpath not set or split error:: %s", + runtime->GetBootClassPathString().c_str()); + return; + } + + std::unique_ptr<MemMap> mem_map; + size_t map_size; + std::string last_error_msg; // Only store the last message (we could concatenate). + + for (const std::string& jar_file : split) { + mem_map = FindAndExtractEntry(jar_file, resource_cstr, &map_size, &last_error_msg); + if (mem_map != nullptr) { + break; + } + } + + if (mem_map == nullptr) { + // Didn't find it. There's a good chance this will be the same at runtime, but still + // conservatively abort the transaction here. + AbortTransactionOrFail(self, + "Could not find resource %s. Last error was %s.", + resource_name_str.c_str(), + last_error_msg.c_str()); + return; + } + + StackHandleScope<3> hs(self); + + // Create byte array for content. + Handle<mirror::ByteArray> h_array(hs.NewHandle(mirror::ByteArray::Alloc(self, map_size))); + if (h_array.Get() == nullptr) { + AbortTransactionOrFail(self, "Could not find/create byte array class"); + return; + } + // Copy in content. + memcpy(h_array->GetData(), mem_map->Begin(), map_size); + // Be proactive releasing memory. + mem_map.release(); + + // Create a ByteArrayInputStream. + Handle<mirror::Class> h_class(hs.NewHandle( + runtime->GetClassLinker()->FindClass(self, + "Ljava/io/ByteArrayInputStream;", + ScopedNullHandle<mirror::ClassLoader>()))); + if (h_class.Get() == nullptr) { + AbortTransactionOrFail(self, "Could not find ByteArrayInputStream class"); + return; + } + if (!runtime->GetClassLinker()->EnsureInitialized(self, h_class, true, true)) { + AbortTransactionOrFail(self, "Could not initialize ByteArrayInputStream class"); + return; + } + + Handle<mirror::Object> h_obj(hs.NewHandle(h_class->AllocObject(self))); + if (h_obj.Get() == nullptr) { + AbortTransactionOrFail(self, "Could not allocate ByteArrayInputStream object"); + return; + } + + auto* cl = Runtime::Current()->GetClassLinker(); + ArtMethod* constructor = h_class->FindDeclaredDirectMethod( + "<init>", "([B)V", cl->GetImagePointerSize()); + if (constructor == nullptr) { + AbortTransactionOrFail(self, "Could not find ByteArrayInputStream constructor"); + return; + } + + uint32_t args[1]; + args[0] = static_cast<uint32_t>(reinterpret_cast<uintptr_t>(h_array.Get())); + EnterInterpreterFromInvoke(self, constructor, h_obj.Get(), args, nullptr); + + if (self->IsExceptionPending()) { + AbortTransactionOrFail(self, "Could not run ByteArrayInputStream constructor"); + return; + } + + result->SetL(h_obj.Get()); +} + +void UnstartedRuntime::UnstartedClassLoaderGetResourceAsStream( + Thread* self, ShadowFrame* shadow_frame, JValue* result, size_t arg_offset) { + { + mirror::Object* this_obj = shadow_frame->GetVRegReference(arg_offset); + CHECK(this_obj != nullptr); + CHECK(this_obj->IsClassLoader()); + + StackHandleScope<1> hs(self); + Handle<mirror::Class> this_classloader_class(hs.NewHandle(this_obj->GetClass())); + + if (self->DecodeJObject(WellKnownClasses::java_lang_BootClassLoader) != + this_classloader_class.Get()) { + AbortTransactionOrFail(self, + "Unsupported classloader type %s for getResourceAsStream", + PrettyClass(this_classloader_class.Get()).c_str()); + return; + } + } + + GetResourceAsStream(self, shadow_frame, result, arg_offset); +} + void UnstartedRuntime::UnstartedVmClassLoaderFindLoadedClass( Thread* self, ShadowFrame* shadow_frame, JValue* result, size_t arg_offset) { mirror::String* class_name = shadow_frame->GetVRegReference(arg_offset + 1)->AsString(); diff --git a/runtime/interpreter/unstarted_runtime_list.h b/runtime/interpreter/unstarted_runtime_list.h index 3b5bee829e..be881cd2c5 100644 --- a/runtime/interpreter/unstarted_runtime_list.h +++ b/runtime/interpreter/unstarted_runtime_list.h @@ -19,6 +19,8 @@ // Methods that intercept available libcore implementations. #define UNSTARTED_RUNTIME_DIRECT_LIST(V) \ + V(CharacterToLowerCase, "int java.lang.Character.toLowerCase(int)") \ + V(CharacterToUpperCase, "int java.lang.Character.toUpperCase(int)") \ V(ClassForName, "java.lang.Class java.lang.Class.forName(java.lang.String)") \ V(ClassForNameLong, "java.lang.Class java.lang.Class.forName(java.lang.String, boolean, java.lang.ClassLoader)") \ V(ClassClassForName, "java.lang.Class java.lang.Class.classForName(java.lang.String, boolean, java.lang.ClassLoader)") \ @@ -27,6 +29,7 @@ V(ClassGetDeclaredMethod, "java.lang.reflect.Method java.lang.Class.getDeclaredMethodInternal(java.lang.String, java.lang.Class[])") \ V(ClassGetDeclaredConstructor, "java.lang.reflect.Constructor java.lang.Class.getDeclaredConstructorInternal(java.lang.Class[])") \ V(ClassGetEnclosingClass, "java.lang.Class java.lang.Class.getEnclosingClass()") \ + V(ClassLoaderGetResourceAsStream, "java.io.InputStream java.lang.ClassLoader.getResourceAsStream(java.lang.String)") \ V(VmClassLoaderFindLoadedClass, "java.lang.Class java.lang.VMClassLoader.findLoadedClass(java.lang.ClassLoader, java.lang.String)") \ V(VoidLookupType, "java.lang.Class java.lang.Void.lookupType()") \ V(SystemArraycopy, "void java.lang.System.arraycopy(java.lang.Object, int, java.lang.Object, int, int)") \ diff --git a/runtime/interpreter/unstarted_runtime_test.cc b/runtime/interpreter/unstarted_runtime_test.cc index 1b5b665ed2..100a44626b 100644 --- a/runtime/interpreter/unstarted_runtime_test.cc +++ b/runtime/interpreter/unstarted_runtime_test.cc @@ -17,6 +17,7 @@ #include "unstarted_runtime.h" #include <limits> +#include <locale> #include "base/casts.h" #include "class_linker.h" @@ -30,6 +31,7 @@ #include "runtime.h" #include "scoped_thread_state_change.h" #include "thread.h" +#include "transaction.h" namespace art { namespace interpreter { @@ -182,6 +184,16 @@ class UnstartedRuntimeTest : public CommonRuntimeTest { EXPECT_EQ(expect_int64t, result_int64t) << result.GetD() << " vs " << test_pairs[i][1]; } } + + // Prepare for aborts. Aborts assume that the exception class is already resolved, as the + // loading code doesn't work under transactions. + void PrepareForAborts() SHARED_REQUIRES(Locks::mutator_lock_) { + mirror::Object* result = Runtime::Current()->GetClassLinker()->FindClass( + Thread::Current(), + Transaction::kAbortExceptionSignature, + ScopedNullHandle<mirror::ClassLoader>()); + CHECK(result != nullptr); + } }; TEST_F(UnstartedRuntimeTest, MemoryPeekByte) { @@ -689,5 +701,106 @@ TEST_F(UnstartedRuntimeTest, Floor) { ShadowFrame::DeleteDeoptimizedFrame(tmp); } +TEST_F(UnstartedRuntimeTest, ToLowerUpper) { + Thread* self = Thread::Current(); + ScopedObjectAccess soa(self); + + ShadowFrame* tmp = ShadowFrame::CreateDeoptimizedFrame(10, nullptr, nullptr, 0); + + std::locale c_locale("C"); + + // Check ASCII. + for (uint32_t i = 0; i < 128; ++i) { + bool c_upper = std::isupper(static_cast<char>(i), c_locale); + bool c_lower = std::islower(static_cast<char>(i), c_locale); + EXPECT_FALSE(c_upper && c_lower) << i; + + // Check toLowerCase. + { + JValue result; + tmp->SetVReg(0, static_cast<int32_t>(i)); + UnstartedCharacterToLowerCase(self, tmp, &result, 0); + ASSERT_FALSE(self->IsExceptionPending()); + uint32_t lower_result = static_cast<uint32_t>(result.GetI()); + if (c_lower) { + EXPECT_EQ(i, lower_result); + } else if (c_upper) { + EXPECT_EQ(static_cast<uint32_t>(std::tolower(static_cast<char>(i), c_locale)), + lower_result); + } else { + EXPECT_EQ(i, lower_result); + } + } + + // Check toUpperCase. + { + JValue result2; + tmp->SetVReg(0, static_cast<int32_t>(i)); + UnstartedCharacterToUpperCase(self, tmp, &result2, 0); + ASSERT_FALSE(self->IsExceptionPending()); + uint32_t upper_result = static_cast<uint32_t>(result2.GetI()); + if (c_upper) { + EXPECT_EQ(i, upper_result); + } else if (c_lower) { + EXPECT_EQ(static_cast<uint32_t>(std::toupper(static_cast<char>(i), c_locale)), + upper_result); + } else { + EXPECT_EQ(i, upper_result); + } + } + } + + // Check abort for other things. Can't test all. + + PrepareForAborts(); + + for (uint32_t i = 128; i < 256; ++i) { + { + JValue result; + tmp->SetVReg(0, static_cast<int32_t>(i)); + Transaction transaction; + Runtime::Current()->EnterTransactionMode(&transaction); + UnstartedCharacterToLowerCase(self, tmp, &result, 0); + Runtime::Current()->ExitTransactionMode(); + ASSERT_TRUE(self->IsExceptionPending()); + ASSERT_TRUE(transaction.IsAborted()); + } + { + JValue result; + tmp->SetVReg(0, static_cast<int32_t>(i)); + Transaction transaction; + Runtime::Current()->EnterTransactionMode(&transaction); + UnstartedCharacterToUpperCase(self, tmp, &result, 0); + Runtime::Current()->ExitTransactionMode(); + ASSERT_TRUE(self->IsExceptionPending()); + ASSERT_TRUE(transaction.IsAborted()); + } + } + for (uint64_t i = 256; i <= std::numeric_limits<uint32_t>::max(); i <<= 1) { + { + JValue result; + tmp->SetVReg(0, static_cast<int32_t>(i)); + Transaction transaction; + Runtime::Current()->EnterTransactionMode(&transaction); + UnstartedCharacterToLowerCase(self, tmp, &result, 0); + Runtime::Current()->ExitTransactionMode(); + ASSERT_TRUE(self->IsExceptionPending()); + ASSERT_TRUE(transaction.IsAborted()); + } + { + JValue result; + tmp->SetVReg(0, static_cast<int32_t>(i)); + Transaction transaction; + Runtime::Current()->EnterTransactionMode(&transaction); + UnstartedCharacterToUpperCase(self, tmp, &result, 0); + Runtime::Current()->ExitTransactionMode(); + ASSERT_TRUE(self->IsExceptionPending()); + ASSERT_TRUE(transaction.IsAborted()); + } + } + + ShadowFrame::DeleteDeoptimizedFrame(tmp); +} + } // namespace interpreter } // namespace art diff --git a/runtime/monitor.cc b/runtime/monitor.cc index 6290cb2ff7..ed6a8e471c 100644 --- a/runtime/monitor.cc +++ b/runtime/monitor.cc @@ -215,6 +215,28 @@ void Monitor::SetObject(mirror::Object* object) { obj_ = GcRoot<mirror::Object>(object); } +std::string Monitor::PrettyContentionInfo(Thread* owner, + ArtMethod* owners_method, + uint32_t owners_dex_pc, + size_t num_waiters) { + DCHECK(owner != nullptr); + const char* owners_filename; + int32_t owners_line_number = 0; + std::string name; + owner->GetThreadName(name); + if (owners_method != nullptr) { + TranslateLocation(owners_method, owners_dex_pc, &owners_filename, &owners_line_number); + } + std::ostringstream oss; + oss << "monitor contention with owner " << name << " (" << owner->GetTid() << ")"; + if (owners_method != nullptr) { + oss << " owner method=" << PrettyMethod(owners_method); + oss << " from " << owners_filename << ":" << owners_line_number; + } + oss << " waiters=" << num_waiters; + return oss.str(); +} + void Monitor::Lock(Thread* self) { MutexLock mu(self, monitor_lock_); while (true) { @@ -242,36 +264,66 @@ void Monitor::Lock(Thread* self) { monitor_lock_.Unlock(self); // Let go of locks in order. self->SetMonitorEnterObject(GetObject()); { + uint32_t original_owner_thread_id = 0u; ScopedThreadStateChange tsc(self, kBlocked); // Change to blocked and give up mutator_lock_. - // Reacquire monitor_lock_ without mutator_lock_ for Wait. - MutexLock mu2(self, monitor_lock_); - if (owner_ != nullptr) { // Did the owner_ give the lock up? - if (ATRACE_ENABLED()) { - std::string name; - owner_->GetThreadName(name); - ATRACE_BEGIN(("Contended on monitor with owner " + name).c_str()); + { + // Reacquire monitor_lock_ without mutator_lock_ for Wait. + MutexLock mu2(self, monitor_lock_); + if (owner_ != nullptr) { // Did the owner_ give the lock up? + original_owner_thread_id = owner_->GetThreadId(); + if (ATRACE_ENABLED()) { + std::ostringstream oss; + oss << PrettyContentionInfo(owner_, owners_method, owners_dex_pc, num_waiters); + // Add info for contending thread. + uint32_t pc; + ArtMethod* m = self->GetCurrentMethod(&pc); + const char* filename; + int32_t line_number; + TranslateLocation(m, pc, &filename, &line_number); + oss << " blocking from " << (filename != nullptr ? filename : "null") + << ":" << line_number; + ATRACE_BEGIN(oss.str().c_str()); + } + monitor_contenders_.Wait(self); // Still contended so wait. } - monitor_contenders_.Wait(self); // Still contended so wait. + } + if (original_owner_thread_id != 0u) { // Woken from contention. if (log_contention) { - uint64_t wait_ms = MilliTime() - wait_start_ms; - uint32_t sample_percent; - if (wait_ms >= lock_profiling_threshold_) { - sample_percent = 100; - } else { - sample_percent = 100 * wait_ms / lock_profiling_threshold_; - } - if (sample_percent != 0 && (static_cast<uint32_t>(rand() % 100) < sample_percent)) { - const char* owners_filename; - int32_t owners_line_number; - TranslateLocation(owners_method, owners_dex_pc, &owners_filename, &owners_line_number); - if (wait_ms > kLongWaitMs && owners_method != nullptr) { - LOG(WARNING) << "Long monitor contention event with owner method=" - << PrettyMethod(owners_method) << " from " << owners_filename << ":" - << owners_line_number << " waiters=" << num_waiters << " for " - << PrettyDuration(MsToNs(wait_ms)); + MutexLock mu2(Thread::Current(), *Locks::thread_list_lock_); + // Re-find the owner in case the thread got killed. + Thread* original_owner = Runtime::Current()->GetThreadList()->FindThreadByThreadId( + original_owner_thread_id); + if (original_owner != nullptr) { + uint64_t wait_ms = MilliTime() - wait_start_ms; + uint32_t sample_percent; + if (wait_ms >= lock_profiling_threshold_) { + sample_percent = 100; + } else { + sample_percent = 100 * wait_ms / lock_profiling_threshold_; + } + if (sample_percent != 0 && (static_cast<uint32_t>(rand() % 100) < sample_percent)) { + if (wait_ms > kLongWaitMs && owners_method != nullptr) { + // TODO: We should maybe check that original_owner is still a live thread. + LOG(WARNING) << "Long " + << PrettyContentionInfo(original_owner, + owners_method, + owners_dex_pc, + num_waiters) + << " for " << PrettyDuration(MsToNs(wait_ms)); + } + const char* owners_filename; + int32_t owners_line_number; + TranslateLocation(owners_method, + owners_dex_pc, + &owners_filename, + &owners_line_number); + LogContentionEvent(self, + wait_ms, + sample_percent, + owners_filename, + owners_line_number); } - LogContentionEvent(self, wait_ms, sample_percent, owners_filename, owners_line_number); } } ATRACE_END(); @@ -311,25 +363,34 @@ static std::string ThreadToString(Thread* thread) { return oss.str(); } -void Monitor::FailedUnlock(mirror::Object* o, Thread* expected_owner, Thread* found_owner, +void Monitor::FailedUnlock(mirror::Object* o, + uint32_t expected_owner_thread_id, + uint32_t found_owner_thread_id, Monitor* monitor) { - Thread* current_owner = nullptr; + // Acquire thread list lock so threads won't disappear from under us. std::string current_owner_string; std::string expected_owner_string; std::string found_owner_string; + uint32_t current_owner_thread_id = 0u; { - // TODO: isn't this too late to prevent threads from disappearing? - // Acquire thread list lock so threads won't disappear from under us. MutexLock mu(Thread::Current(), *Locks::thread_list_lock_); + ThreadList* const thread_list = Runtime::Current()->GetThreadList(); + Thread* expected_owner = thread_list->FindThreadByThreadId(expected_owner_thread_id); + Thread* found_owner = thread_list->FindThreadByThreadId(found_owner_thread_id); + // Re-read owner now that we hold lock. - current_owner = (monitor != nullptr) ? monitor->GetOwner() : nullptr; + Thread* current_owner = (monitor != nullptr) ? monitor->GetOwner() : nullptr; + if (current_owner != nullptr) { + current_owner_thread_id = current_owner->GetThreadId(); + } // Get short descriptions of the threads involved. current_owner_string = ThreadToString(current_owner); - expected_owner_string = ThreadToString(expected_owner); - found_owner_string = ThreadToString(found_owner); + expected_owner_string = expected_owner != nullptr ? ThreadToString(expected_owner) : "unnamed"; + found_owner_string = found_owner != nullptr ? ThreadToString(found_owner) : "unnamed"; } - if (current_owner == nullptr) { - if (found_owner == nullptr) { + + if (current_owner_thread_id == 0u) { + if (found_owner_thread_id == 0u) { ThrowIllegalMonitorStateExceptionF("unlock of unowned monitor on object of type '%s'" " on thread '%s'", PrettyTypeOf(o).c_str(), @@ -343,7 +404,7 @@ void Monitor::FailedUnlock(mirror::Object* o, Thread* expected_owner, Thread* fo expected_owner_string.c_str()); } } else { - if (found_owner == nullptr) { + if (found_owner_thread_id == 0u) { // Race: originally there was no owner, there is now ThrowIllegalMonitorStateExceptionF("unlock of monitor owned by '%s' on object of type '%s'" " (originally believed to be unowned) on thread '%s'", @@ -351,7 +412,7 @@ void Monitor::FailedUnlock(mirror::Object* o, Thread* expected_owner, Thread* fo PrettyTypeOf(o).c_str(), expected_owner_string.c_str()); } else { - if (found_owner != current_owner) { + if (found_owner_thread_id != current_owner_thread_id) { // Race: originally found and current owner have changed ThrowIllegalMonitorStateExceptionF("unlock of monitor originally owned by '%s' (now" " owned by '%s') on object of type '%s' on thread '%s'", @@ -372,27 +433,29 @@ void Monitor::FailedUnlock(mirror::Object* o, Thread* expected_owner, Thread* fo bool Monitor::Unlock(Thread* self) { DCHECK(self != nullptr); - MutexLock mu(self, monitor_lock_); - Thread* owner = owner_; - if (owner == self) { - // We own the monitor, so nobody else can be in here. - if (lock_count_ == 0) { - owner_ = nullptr; - locking_method_ = nullptr; - locking_dex_pc_ = 0; - // Wake a contender. - monitor_contenders_.Signal(self); - } else { - --lock_count_; + uint32_t owner_thread_id; + { + MutexLock mu(self, monitor_lock_); + Thread* owner = owner_; + owner_thread_id = owner->GetThreadId(); + if (owner == self) { + // We own the monitor, so nobody else can be in here. + if (lock_count_ == 0) { + owner_ = nullptr; + locking_method_ = nullptr; + locking_dex_pc_ = 0; + // Wake a contender. + monitor_contenders_.Signal(self); + } else { + --lock_count_; + } + return true; } - } else { - // We don't own this, so we're not allowed to unlock it. - // The JNI spec says that we should throw IllegalMonitorStateException - // in this case. - FailedUnlock(GetObject(), self, owner, this); - return false; } - return true; + // We don't own this, so we're not allowed to unlock it. + // The JNI spec says that we should throw IllegalMonitorStateException in this case. + FailedUnlock(GetObject(), self->GetThreadId(), owner_thread_id, this); + return false; } void Monitor::Wait(Thread* self, int64_t ms, int32_t ns, @@ -769,16 +832,13 @@ bool Monitor::MonitorExit(Thread* self, mirror::Object* obj) { case LockWord::kHashCode: // Fall-through. case LockWord::kUnlocked: - FailedUnlock(h_obj.Get(), self, nullptr, nullptr); + FailedUnlock(h_obj.Get(), self->GetThreadId(), 0u, nullptr); return false; // Failure. case LockWord::kThinLocked: { uint32_t thread_id = self->GetThreadId(); uint32_t owner_thread_id = lock_word.ThinLockOwner(); if (owner_thread_id != thread_id) { - // TODO: there's a race here with the owner dying while we unlock. - Thread* owner = - Runtime::Current()->GetThreadList()->FindThreadByThreadId(lock_word.ThinLockOwner()); - FailedUnlock(h_obj.Get(), self, owner, nullptr); + FailedUnlock(h_obj.Get(), thread_id, owner_thread_id, nullptr); return false; // Failure. } else { // We own the lock, decrease the recursion count. @@ -1072,8 +1132,10 @@ bool Monitor::IsLocked() SHARED_REQUIRES(Locks::mutator_lock_) { return owner_ != nullptr; } -void Monitor::TranslateLocation(ArtMethod* method, uint32_t dex_pc, - const char** source_file, int32_t* line_number) const { +void Monitor::TranslateLocation(ArtMethod* method, + uint32_t dex_pc, + const char** source_file, + int32_t* line_number) { // If method is null, location is unknown if (method == nullptr) { *source_file = ""; diff --git a/runtime/monitor.h b/runtime/monitor.h index ae9b3cca82..3d9d10a167 100644 --- a/runtime/monitor.h +++ b/runtime/monitor.h @@ -185,9 +185,12 @@ class Monitor { const char* owner_filename, int32_t owner_line_number) SHARED_REQUIRES(Locks::mutator_lock_); - static void FailedUnlock(mirror::Object* obj, Thread* expected_owner, Thread* found_owner, + static void FailedUnlock(mirror::Object* obj, + uint32_t expected_owner_thread_id, + uint32_t found_owner_thread_id, Monitor* mon) - REQUIRES(!Locks::thread_list_lock_) + REQUIRES(!Locks::thread_list_lock_, + !monitor_lock_) SHARED_REQUIRES(Locks::mutator_lock_); void Lock(Thread* self) @@ -208,6 +211,11 @@ class Monitor { REQUIRES(!monitor_lock_) SHARED_REQUIRES(Locks::mutator_lock_); + static std::string PrettyContentionInfo(Thread* owner, + ArtMethod* owners_method, + uint32_t owners_dex_pc, + size_t num_waiters) + SHARED_REQUIRES(Locks::mutator_lock_); // Wait on a monitor until timeout, interrupt, or notification. Used for Object.wait() and // (somewhat indirectly) Thread.sleep() and Thread.join(). @@ -233,8 +241,9 @@ class Monitor { SHARED_REQUIRES(Locks::mutator_lock_); // Translates the provided method and pc into its declaring class' source file and line number. - void TranslateLocation(ArtMethod* method, uint32_t pc, - const char** source_file, int32_t* line_number) const + static void TranslateLocation(ArtMethod* method, uint32_t pc, + const char** source_file, + int32_t* line_number) SHARED_REQUIRES(Locks::mutator_lock_); uint32_t GetOwnerThreadId() REQUIRES(!monitor_lock_); diff --git a/runtime/thread.h b/runtime/thread.h index 2218b5a9d8..ed42e462ae 100644 --- a/runtime/thread.h +++ b/runtime/thread.h @@ -312,6 +312,7 @@ class Thread { */ static int GetNativePriority(); + // Guaranteed to be non-zero. uint32_t GetThreadId() const { return tls32_.thin_lock_thread_id; } diff --git a/runtime/thread_list.cc b/runtime/thread_list.cc index a9ce056fd9..1e5264c9cd 100644 --- a/runtime/thread_list.cc +++ b/runtime/thread_list.cc @@ -923,12 +923,9 @@ Thread* ThreadList::SuspendThreadByThreadId(uint32_t thread_id, } } -Thread* ThreadList::FindThreadByThreadId(uint32_t thin_lock_id) { - Thread* self = Thread::Current(); - MutexLock mu(self, *Locks::thread_list_lock_); +Thread* ThreadList::FindThreadByThreadId(uint32_t thread_id) { for (const auto& thread : list_) { - if (thread->GetThreadId() == thin_lock_id) { - CHECK(thread == self || thread->IsSuspended()); + if (thread->GetThreadId() == thread_id) { return thread; } } diff --git a/runtime/thread_list.h b/runtime/thread_list.h index f97ecd34a5..df81ad1a7b 100644 --- a/runtime/thread_list.h +++ b/runtime/thread_list.h @@ -89,8 +89,8 @@ class ThreadList { !Locks::thread_list_lock_, !Locks::thread_suspend_count_lock_); - // Find an already suspended thread (or self) by its id. - Thread* FindThreadByThreadId(uint32_t thin_lock_id); + // Find an existing thread (or self) by its thread id (not tid). + Thread* FindThreadByThreadId(uint32_t thread_id) REQUIRES(Locks::thread_list_lock_); // Run a checkpoint on threads, running threads are not suspended but run the checkpoint inside // of the suspend check. Returns how many checkpoints that are expected to run, including for diff --git a/test/594-checker-irreducible-linorder/smali/IrreducibleLoop.smali b/test/594-checker-irreducible-linorder/smali/IrreducibleLoop.smali index 8e01084841..366c7b9b68 100644 --- a/test/594-checker-irreducible-linorder/smali/IrreducibleLoop.smali +++ b/test/594-checker-irreducible-linorder/smali/IrreducibleLoop.smali @@ -40,6 +40,11 @@ invoke-static {v0}, Ljava/lang/System;->exit(I)V goto :body1 + # Trivially dead code to ensure linear order verification skips removed blocks (b/28252537). + :dead_code + nop + goto :dead_code + :header mul-int/2addr p3, p3 if-eqz p1, :body2 |