From 2f8e1091791219c01b66968afad705a9568b9aeb Mon Sep 17 00:00:00 2001 From: Bibek Ghimire Date: Tue, 9 Dec 2025 14:34:09 -0500 Subject: [PATCH 1/2] add clear for last error string --- .../hipdnn/backend/include/hipdnn_backend.h | 20 ++++ projects/hipdnn/backend/src/HipdnnBackend.cpp | 24 +++++ .../hipdnn/backend/src/LastErrorManager.cpp | 5 + .../hipdnn/backend/src/LastErrorManager.hpp | 1 + .../backend/tests/TestLastErrorManager.cpp | 52 +++++++++ .../backend/IntegrationGetLastErrorString.cpp | 100 ++++++++++++++++-- 6 files changed, 196 insertions(+), 6 deletions(-) diff --git a/projects/hipdnn/backend/include/hipdnn_backend.h b/projects/hipdnn/backend/include/hipdnn_backend.h index 3ad74b2fb4f..f11245961e4 100644 --- a/projects/hipdnn/backend/include/hipdnn_backend.h +++ b/projects/hipdnn/backend/include/hipdnn_backend.h @@ -244,6 +244,10 @@ HIPDNN_BACKEND_EXPORT const char* hipdnnGetErrorString(hipdnnStatus_t status); * message buffer, up to max_size bytes (including the null terminator). * Note the max size for an error message is HIPDNN_ERROR_STRING_MAX_LENGTH characters. * + * After retrieving the error message, this function clears the internal error state to match + * cudnn behavior. Use hipdnnPeekLastErrorString_ext() if you need to retrieve the error without + * clearing it. + * * @param[out] message Pointer to a character buffer where the error message will be copied. * @param[in] maxSize Maximum number of bytes to copy, including the null terminator. */ @@ -257,6 +261,22 @@ HIPDNN_BACKEND_EXPORT void hipdnnGetLastErrorString(char* message, size_t maxSiz ************************************************************************************************** */ +/** + * @brief Retrieves the last error message for the calling thread without clearing it. + * + * This function copies the last error message associated with the calling thread into the provided + * message buffer, up to max_size bytes (including the null terminator). + * Note the max size for an error message is HIPDNN_ERROR_STRING_MAX_LENGTH characters. + * + * Unlike hipdnnGetLastErrorString(), this function does NOT clear the internal error state after + * retrieval. This preserves the pre-existing behavior for cases where the error needs to be + * queried multiple times. + * + * @param[out] message Pointer to a character buffer where the error message will be copied. + * @param[in] maxSize Maximum number of bytes to copy, including the null terminator. + */ +HIPDNN_BACKEND_EXPORT void hipdnnPeekLastErrorString_ext(char* message, size_t maxSize); + /*! * @brief Creates and deserializes a graph into a backend descriptor. * diff --git a/projects/hipdnn/backend/src/HipdnnBackend.cpp b/projects/hipdnn/backend/src/HipdnnBackend.cpp index 0226dd05d97..f004652993d 100644 --- a/projects/hipdnn/backend/src/HipdnnBackend.cpp +++ b/projects/hipdnn/backend/src/HipdnnBackend.cpp @@ -269,6 +269,30 @@ HIPDNN_BACKEND_EXPORT void hipdnnGetLastErrorString(char* message, size_t maxSiz throw hipdnn_backend::HipdnnException(HIPDNN_STATUS_BAD_PARAM, "maxSize is 0"); } + hipdnn_sdk::utilities::copyMaxSizeWithNullTerminator( + message, hipdnn_backend::LastErrorManager::getLastError(), maxSize); + + // Clear the error after retrieval to match cudnn behavior + hipdnn_backend::LastErrorManager::clearLastError(); + + LOG_API_SUCCESS(apiName, "set_error_message={:p}", static_cast(message)); + }); +} + +HIPDNN_BACKEND_EXPORT void hipdnnPeekLastErrorString_ext(char* message, size_t maxSize) +{ + LOG_API_ENTRY("message_ptr={:p}, maxSize={}", static_cast(message), maxSize); + // Ignore status since API doesn't return it. + // We still want to catch and log if the user provides incorrect parameters. + auto _ = hipdnn_backend::tryCatch([&, apiName = __func__] { + throwIfNull(message); + + if(maxSize == 0) + { + throw hipdnn_backend::HipdnnException(HIPDNN_STATUS_BAD_PARAM, "maxSize is 0"); + } + + // Get the error without clearing it (preserves pre-existing behavior) hipdnn_sdk::utilities::copyMaxSizeWithNullTerminator( message, hipdnn_backend::LastErrorManager::getLastError(), maxSize); diff --git a/projects/hipdnn/backend/src/LastErrorManager.cpp b/projects/hipdnn/backend/src/LastErrorManager.cpp index 7c3b3af300a..14fdde209b8 100644 --- a/projects/hipdnn/backend/src/LastErrorManager.cpp +++ b/projects/hipdnn/backend/src/LastErrorManager.cpp @@ -37,3 +37,8 @@ const char* hipdnn_backend::LastErrorManager::getLastError() { return s_lastError; } + +void hipdnn_backend::LastErrorManager::clearLastError() +{ + s_lastError[0] = '\0'; +} diff --git a/projects/hipdnn/backend/src/LastErrorManager.hpp b/projects/hipdnn/backend/src/LastErrorManager.hpp index 671da655ba3..b39acf82584 100644 --- a/projects/hipdnn/backend/src/LastErrorManager.hpp +++ b/projects/hipdnn/backend/src/LastErrorManager.hpp @@ -24,6 +24,7 @@ class LastErrorManager static hipdnnStatus_t setLastError(hipdnnStatus_t status, const char* message); static hipdnnStatus_t setLastError(hipdnnStatus_t status, const std::string& message); static const char* getLastError(); + static void clearLastError(); }; } // namespace hipdnn_backend diff --git a/projects/hipdnn/backend/tests/TestLastErrorManager.cpp b/projects/hipdnn/backend/tests/TestLastErrorManager.cpp index 453d4362bbb..36e6d7e9cb3 100644 --- a/projects/hipdnn/backend/tests/TestLastErrorManager.cpp +++ b/projects/hipdnn/backend/tests/TestLastErrorManager.cpp @@ -73,3 +73,55 @@ TEST(TestLastErrorManager, SetSuccessCStringDoesNotSetErrorMessage) EXPECT_NE(LastErrorManager::getLastError(), errorMessage); } + +TEST(TestLastErrorManager, ClearLastErrorEmptiesBuffer) +{ + // Set an error + std::string errorMessage = "Test error"; + LastErrorManager::setLastError(HIPDNN_STATUS_BAD_PARAM, errorMessage); + EXPECT_STREQ(LastErrorManager::getLastError(), errorMessage.c_str()); + + // Clear the error + LastErrorManager::clearLastError(); + + // Error should now be empty + EXPECT_STREQ(LastErrorManager::getLastError(), ""); +} + +TEST(TestLastErrorManager, ClearLastErrorIsThreadLocal) +{ + std::string mainError = "Main thread error"; + std::string workerError = "Worker thread error"; + + // Set error in main thread + LastErrorManager::setLastError(HIPDNN_STATUS_BAD_PARAM, mainError); + + // Set and clear error in worker thread + std::thread t([workerError]() { + LastErrorManager::setLastError(HIPDNN_STATUS_NOT_SUPPORTED, workerError); + EXPECT_STREQ(LastErrorManager::getLastError(), workerError.c_str()); + + // Clear in worker thread + LastErrorManager::clearLastError(); + EXPECT_STREQ(LastErrorManager::getLastError(), ""); + }); + t.join(); + + // Main thread error should still be present (clearing is thread-local) + EXPECT_STREQ(LastErrorManager::getLastError(), mainError.c_str()); +} + +TEST(TestLastErrorManager, MultipleClears) +{ + // Set an error + LastErrorManager::setLastError(HIPDNN_STATUS_BAD_PARAM, "Error message"); + EXPECT_STRNE(LastErrorManager::getLastError(), ""); + + // Clear it + LastErrorManager::clearLastError(); + EXPECT_STREQ(LastErrorManager::getLastError(), ""); + + // Clearing again should be safe + LastErrorManager::clearLastError(); + EXPECT_STREQ(LastErrorManager::getLastError(), ""); +} diff --git a/projects/hipdnn/tests/backend/IntegrationGetLastErrorString.cpp b/projects/hipdnn/tests/backend/IntegrationGetLastErrorString.cpp index baf32701a0f..bbb545dde1a 100644 --- a/projects/hipdnn/tests/backend/IntegrationGetLastErrorString.cpp +++ b/projects/hipdnn/tests/backend/IntegrationGetLastErrorString.cpp @@ -76,19 +76,19 @@ TEST(IntegrationGetLastErrorString, PerThreadErrorIsolation) // Set error in main thread hipdnnDestroy(nullptr); char mainBuf[HIPDNN_ERROR_STRING_MAX_LENGTH]; - hipdnnGetLastErrorString(mainBuf, sizeof(mainBuf)); + hipdnnPeekLastErrorString_ext(mainBuf, sizeof(mainBuf)); std::string threadError; std::thread t([&threadError]() { char buf[HIPDNN_ERROR_STRING_MAX_LENGTH]; - hipdnnGetLastErrorString(buf, sizeof(buf)); + hipdnnPeekLastErrorString_ext(buf, sizeof(buf)); threadError = buf; }); t.join(); - // Main thread error should be unchanged + // Main thread error should be unchanged (using peek doesn't clear) char mainBuf2[HIPDNN_ERROR_STRING_MAX_LENGTH]; - hipdnnGetLastErrorString(mainBuf2, sizeof(mainBuf2)); + hipdnnPeekLastErrorString_ext(mainBuf2, sizeof(mainBuf2)); ASSERT_STREQ(mainBuf, mainBuf2); ASSERT_TRUE(threadError.empty()); } @@ -97,10 +97,98 @@ TEST(IntegrationGetLastErrorString, BufferLargerThanMax) { hipdnnDestroy(nullptr); char mainBuf[1028]; - hipdnnGetLastErrorString(mainBuf, sizeof(mainBuf)); + hipdnnPeekLastErrorString_ext(mainBuf, sizeof(mainBuf)); char mainBuf2[HIPDNN_ERROR_STRING_MAX_LENGTH]; - hipdnnGetLastErrorString(mainBuf2, sizeof(mainBuf2)); + hipdnnPeekLastErrorString_ext(mainBuf2, sizeof(mainBuf2)); ASSERT_STREQ(mainBuf, mainBuf2); } + +TEST(IntegrationGetLastErrorString, GetClearsError) +{ + // Generate an error + hipdnnStatus_t status = hipdnnDestroy(nullptr); + ASSERT_EQ(status, HIPDNN_STATUS_BAD_PARAM_NULL_POINTER); + + // First call should return the error + char buffer1[HIPDNN_ERROR_STRING_MAX_LENGTH]; + hipdnnGetLastErrorString(buffer1, sizeof(buffer1)); + ASSERT_STRNE(buffer1, ""); + + // Second call should return empty (error was cleared) + char buffer2[HIPDNN_ERROR_STRING_MAX_LENGTH]; + hipdnnGetLastErrorString(buffer2, sizeof(buffer2)); + ASSERT_STREQ(buffer2, ""); +} + +TEST(IntegrationGetLastErrorString, PeekDoesNotClearError) +{ + // Generate an error + hipdnnStatus_t status = hipdnnDestroy(nullptr); + ASSERT_EQ(status, HIPDNN_STATUS_BAD_PARAM_NULL_POINTER); + + // First peek should return the error + char buffer1[HIPDNN_ERROR_STRING_MAX_LENGTH]; + hipdnnPeekLastErrorString_ext(buffer1, sizeof(buffer1)); + ASSERT_STRNE(buffer1, ""); + + // Second peek should return the same error (not cleared) + char buffer2[HIPDNN_ERROR_STRING_MAX_LENGTH]; + hipdnnPeekLastErrorString_ext(buffer2, sizeof(buffer2)); + ASSERT_STREQ(buffer1, buffer2); + + // Third peek should still return the same error + char buffer3[HIPDNN_ERROR_STRING_MAX_LENGTH]; + hipdnnPeekLastErrorString_ext(buffer3, sizeof(buffer3)); + ASSERT_STREQ(buffer1, buffer3); +} + +TEST(IntegrationGetLastErrorString, PeekThenGetClears) +{ + // Generate an error + hipdnnDestroy(nullptr); + + // Peek should return the error without clearing + char peekBuffer[HIPDNN_ERROR_STRING_MAX_LENGTH]; + hipdnnPeekLastErrorString_ext(peekBuffer, sizeof(peekBuffer)); + ASSERT_STRNE(peekBuffer, ""); + + // Get should return the same error and clear it + char getBuffer[HIPDNN_ERROR_STRING_MAX_LENGTH]; + hipdnnGetLastErrorString(getBuffer, sizeof(getBuffer)); + ASSERT_STREQ(peekBuffer, getBuffer); + + // Next get should return empty (error was cleared by previous get) + char emptyBuffer[HIPDNN_ERROR_STRING_MAX_LENGTH]; + hipdnnGetLastErrorString(emptyBuffer, sizeof(emptyBuffer)); + ASSERT_STREQ(emptyBuffer, ""); +} + +TEST(IntegrationGetLastErrorString, ClearingIsThreadLocal) +{ + // Set error in main thread + hipdnnDestroy(nullptr); + char mainBuffer[HIPDNN_ERROR_STRING_MAX_LENGTH]; + hipdnnPeekLastErrorString_ext(mainBuffer, sizeof(mainBuffer)); + ASSERT_STRNE(mainBuffer, ""); + + // Worker thread generates and clears its own error + std::thread t([]() { + hipdnnDestroy(nullptr); + char workerBuffer[HIPDNN_ERROR_STRING_MAX_LENGTH]; + hipdnnGetLastErrorString(workerBuffer, sizeof(workerBuffer)); + ASSERT_STRNE(workerBuffer, ""); + + // Error should be cleared after get + char emptyBuffer[HIPDNN_ERROR_STRING_MAX_LENGTH]; + hipdnnGetLastErrorString(emptyBuffer, sizeof(emptyBuffer)); + ASSERT_STREQ(emptyBuffer, ""); + }); + t.join(); + + // Main thread error should still be present (clearing is thread-local) + char mainBuffer2[HIPDNN_ERROR_STRING_MAX_LENGTH]; + hipdnnPeekLastErrorString_ext(mainBuffer2, sizeof(mainBuffer2)); + ASSERT_STREQ(mainBuffer, mainBuffer2); +} // NOLINTEND(modernize-avoid-c-arrays) From e5a09a088123cc43ed998c6bc4d4a6b5b8da6bf2 Mon Sep 17 00:00:00 2001 From: Bibek Ghimire Date: Tue, 9 Dec 2025 15:20:52 -0500 Subject: [PATCH 2/2] fix typo --- projects/hipdnn/backend/include/hipdnn_backend.h | 5 ++--- projects/hipdnn/backend/src/HipdnnBackend.cpp | 2 +- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/projects/hipdnn/backend/include/hipdnn_backend.h b/projects/hipdnn/backend/include/hipdnn_backend.h index f11245961e4..7da69e7d0c5 100644 --- a/projects/hipdnn/backend/include/hipdnn_backend.h +++ b/projects/hipdnn/backend/include/hipdnn_backend.h @@ -244,9 +244,8 @@ HIPDNN_BACKEND_EXPORT const char* hipdnnGetErrorString(hipdnnStatus_t status); * message buffer, up to max_size bytes (including the null terminator). * Note the max size for an error message is HIPDNN_ERROR_STRING_MAX_LENGTH characters. * - * After retrieving the error message, this function clears the internal error state to match - * cudnn behavior. Use hipdnnPeekLastErrorString_ext() if you need to retrieve the error without - * clearing it. + * After retrieving the error message, this function clears the internal error state. + * Use hipdnnPeekLastErrorString_ext() if you need to retrieve the error without clearing it. * * @param[out] message Pointer to a character buffer where the error message will be copied. * @param[in] maxSize Maximum number of bytes to copy, including the null terminator. diff --git a/projects/hipdnn/backend/src/HipdnnBackend.cpp b/projects/hipdnn/backend/src/HipdnnBackend.cpp index f004652993d..37e520d27b7 100644 --- a/projects/hipdnn/backend/src/HipdnnBackend.cpp +++ b/projects/hipdnn/backend/src/HipdnnBackend.cpp @@ -272,7 +272,7 @@ HIPDNN_BACKEND_EXPORT void hipdnnGetLastErrorString(char* message, size_t maxSiz hipdnn_sdk::utilities::copyMaxSizeWithNullTerminator( message, hipdnn_backend::LastErrorManager::getLastError(), maxSize); - // Clear the error after retrieval to match cudnn behavior + // Clear the error after retrieval hipdnn_backend::LastErrorManager::clearLastError(); LOG_API_SUCCESS(apiName, "set_error_message={:p}", static_cast(message));