From d32b42c538db3da478aa5591830693c8a41c42e9 Mon Sep 17 00:00:00 2001 From: Ilya Matveev Date: Wed, 4 Aug 2021 21:33:01 +0700 Subject: [PATCH] [K/N][New MM] Add runtime asserts to state switch functions --- .../runtime/src/mm/cpp/ThreadState.cpp | 3 ++ .../runtime/src/mm/cpp/ThreadState.hpp | 3 ++ .../runtime/src/mm/cpp/ThreadStateTest.cpp | 38 +++++++++++++++++++ 3 files changed, 44 insertions(+) diff --git a/kotlin-native/runtime/src/mm/cpp/ThreadState.cpp b/kotlin-native/runtime/src/mm/cpp/ThreadState.cpp index bcae03fc76e..3a2c1da22ac 100644 --- a/kotlin-native/runtime/src/mm/cpp/ThreadState.cpp +++ b/kotlin-native/runtime/src/mm/cpp/ThreadState.cpp @@ -29,12 +29,14 @@ std::string kotlin::internal::statesToString(std::initializer_list } ALWAYS_INLINE ThreadState kotlin::SwitchThreadState(MemoryState* thread, ThreadState newState, bool reentrant) noexcept { + RuntimeAssert(thread != nullptr, "thread must not be nullptr"); return SwitchThreadState(thread->GetThreadData(), newState, reentrant); } ALWAYS_INLINE void kotlin::AssertThreadState(MemoryState* thread, ThreadState expected) noexcept { // Avoid redundant read in GetThreadData if runtime asserts are disabled. if (compiler::runtimeAssertsMode() != compiler::RuntimeAssertsMode::kIgnore) { + RuntimeAssert(thread != nullptr, "thread must not be nullptr"); AssertThreadState(thread->GetThreadData(), expected); } } @@ -42,6 +44,7 @@ ALWAYS_INLINE void kotlin::AssertThreadState(MemoryState* thread, ThreadState ex ALWAYS_INLINE void kotlin::AssertThreadState(MemoryState* thread, std::initializer_list expected) noexcept { // Avoid redundant read in GetThreadData if runtime asserts are disabled. if (compiler::runtimeAssertsMode() != compiler::RuntimeAssertsMode::kIgnore) { + RuntimeAssert(thread != nullptr, "thread must not be nullptr"); AssertThreadState(thread->GetThreadData(), expected); } } diff --git a/kotlin-native/runtime/src/mm/cpp/ThreadState.hpp b/kotlin-native/runtime/src/mm/cpp/ThreadState.hpp index 6c0e1904bfe..80aa9a7869e 100644 --- a/kotlin-native/runtime/src/mm/cpp/ThreadState.hpp +++ b/kotlin-native/runtime/src/mm/cpp/ThreadState.hpp @@ -28,6 +28,7 @@ const char* ThreadStateName(ThreadState state) noexcept; // Switches the state of the given thread to `newState` and returns the previous thread state. ALWAYS_INLINE inline ThreadState SwitchThreadState(mm::ThreadData* threadData, ThreadState newState, bool reentrant = false) noexcept { + RuntimeAssert(threadData != nullptr, "threadData must not be nullptr"); auto oldState = threadData->setState(newState); // TODO(perf): Mesaure the impact of this assert in debug and opt modes. RuntimeAssert(internal::isStateSwitchAllowed(oldState, newState, reentrant), @@ -42,6 +43,7 @@ ALWAYS_INLINE inline void AssertThreadState(mm::ThreadData* threadData, ThreadSt // even if its result is unused due to disabled runtime asserts. // So we explicitly avoid the read if asserts are disabled. if (compiler::runtimeAssertsMode() != compiler::RuntimeAssertsMode::kIgnore) { + RuntimeAssert(threadData != nullptr, "threadData must not be nullptr"); auto actual = threadData->state(); RuntimeAssert( actual == expected, "Unexpected thread state. Expected: %s. Actual: %s.", ThreadStateName(expected), @@ -54,6 +56,7 @@ ALWAYS_INLINE inline void AssertThreadState(mm::ThreadData* threadData, std::ini // even if its result is unused due to disabled runtime asserts. // So we explicitly avoid the read if asserts are disabled. if (compiler::runtimeAssertsMode() != compiler::RuntimeAssertsMode::kIgnore) { + RuntimeAssert(threadData != nullptr, "threadData must not be nullptr"); auto actual = threadData->state(); RuntimeAssert( std::any_of(expected.begin(), expected.end(), [actual](ThreadState expected) { return expected == actual; }), diff --git a/kotlin-native/runtime/src/mm/cpp/ThreadStateTest.cpp b/kotlin-native/runtime/src/mm/cpp/ThreadStateTest.cpp index bac4079833b..e6569ae5e1f 100644 --- a/kotlin-native/runtime/src/mm/cpp/ThreadStateTest.cpp +++ b/kotlin-native/runtime/src/mm/cpp/ThreadStateTest.cpp @@ -169,6 +169,23 @@ TEST(ThreadStateDeathTest, StateAsserts) { }); } +TEST(ThreadStateDeathTest, StateAssertsForDetachedThread) { + EXPECT_DEATH(AssertThreadState(static_cast(nullptr), ThreadState::kNative), + "runtime assert: thread must not be nullptr"); + EXPECT_DEATH(AssertThreadState(static_cast(nullptr), ThreadState::kNative), + "runtime assert: threadData must not be nullptr"); + EXPECT_DEATH(AssertThreadState(ThreadState::kNative), + "runtime assert: thread must not be nullptr"); + + EXPECT_DEATH(AssertThreadState(static_cast(nullptr), {ThreadState::kNative}), + "runtime assert: thread must not be nullptr"); + EXPECT_DEATH(AssertThreadState(static_cast(nullptr), {ThreadState::kNative}), + "runtime assert: threadData must not be nullptr"); + EXPECT_DEATH(AssertThreadState({ThreadState::kNative}), + "runtime assert: thread must not be nullptr"); + +} + TEST(ThreadStateDeathTest, IncorrectStateSwitchWithDifferentFunctions) { RunInNewThread([](MemoryState* memoryState) { auto* threadData = memoryState->GetThreadData(); @@ -202,6 +219,14 @@ TEST(ThreadStateDeathTest, StateSwitchCorrectness) { "runtime assert: Illegal thread state switch. Old state: NATIVE. New state: NATIVE"); } +TEST(ThreadStateDeathTest, StateSwitchForDetachedThread) { + EXPECT_DEATH(SwitchThreadState(static_cast(nullptr), ThreadState::kNative), "thread must not be nullptr"); + EXPECT_DEATH(SwitchThreadState(static_cast(nullptr), ThreadState::kNative), "threadData must not be nullptr"); + + EXPECT_DEATH(Kotlin_mm_switchThreadStateNative(), "threadData must not be nullptr"); + EXPECT_DEATH(Kotlin_mm_switchThreadStateRunnable(), "threadData must not be nullptr" ); +} + TEST(ThreadStateDeathTest, ReentrantStateSwitch) { RunInNewThread([](MemoryState* memoryState) { auto* threadData = memoryState->GetThreadData(); @@ -234,3 +259,16 @@ TEST(ThreadStateDeathTest, MovingReentrantGuard) { testing::Not(testing::ContainsRegex("runtime assert: Illegal thread state switch."))); }); } + +TEST(ThreadStateDeathTest, GuardForDetachedThread) { + auto expectedError = "thread must not be nullptr"; + EXPECT_DEATH({ ThreadStateGuard guard(nullptr, ThreadState::kRunnable, false); }, expectedError); + EXPECT_DEATH({ ThreadStateGuard guard(nullptr, ThreadState::kNative, false); }, expectedError); + EXPECT_DEATH({ ThreadStateGuard guard(nullptr, ThreadState::kRunnable, true); }, expectedError); + EXPECT_DEATH({ ThreadStateGuard guard(nullptr, ThreadState::kNative, true); }, expectedError); + + EXPECT_DEATH({ ThreadStateGuard guard(ThreadState::kRunnable, false); }, expectedError); + EXPECT_DEATH({ ThreadStateGuard guard(ThreadState::kNative, false); }, expectedError); + EXPECT_DEATH({ ThreadStateGuard guard(ThreadState::kRunnable, true); }, expectedError); + EXPECT_DEATH({ ThreadStateGuard guard(ThreadState::kNative, true); }, expectedError); +} \ No newline at end of file