From 27c72eb3cef62585f14e9a3480c4412f049ae563 Mon Sep 17 00:00:00 2001 From: Fabian Schiebel Date: Tue, 27 Feb 2024 18:42:55 +0100 Subject: [PATCH 1/3] fix: Make the assert in install_bad_alloc_error_handler reference the correct variable --- llvm/lib/Support/ErrorHandling.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/llvm/lib/Support/ErrorHandling.cpp b/llvm/lib/Support/ErrorHandling.cpp index b8b3b7424ac6b..d2d3dcc2f478c 100644 --- a/llvm/lib/Support/ErrorHandling.cpp +++ b/llvm/lib/Support/ErrorHandling.cpp @@ -130,7 +130,8 @@ void llvm::install_bad_alloc_error_handler(fatal_error_handler_t handler, #if LLVM_ENABLE_THREADS == 1 std::lock_guard Lock(BadAllocErrorHandlerMutex); #endif - assert(!ErrorHandler && "Bad alloc error handler already registered!\n"); + assert(!BadAllocErrorHandler && + "Bad alloc error handler already registered!\n"); BadAllocErrorHandler = handler; BadAllocErrorHandlerUserData = user_data; } From 00382c109b80362e89daf9ce981cb101c8aeb06d Mon Sep 17 00:00:00 2001 From: Fabian Schiebel Date: Tue, 27 Feb 2024 21:12:49 +0100 Subject: [PATCH 2/3] Add minimal test to catch potential unwanted interaction in fatal/bad_alloc error handler registrations --- llvm/unittests/Support/ErrorTest.cpp | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/llvm/unittests/Support/ErrorTest.cpp b/llvm/unittests/Support/ErrorTest.cpp index 11f93203597bf..94ee04a0296f5 100644 --- a/llvm/unittests/Support/ErrorTest.cpp +++ b/llvm/unittests/Support/ErrorTest.cpp @@ -1132,4 +1132,26 @@ TEST(Error, moveInto) { } } +TEST(Error, FatalBadAllocErrorHandlersInteraction) { + auto ErrorHandler = [](void *Data, const char *, bool) {}; + install_fatal_error_handler(ErrorHandler, nullptr); + // The following call should not crash + install_bad_alloc_error_handler(ErrorHandler, nullptr); + + // Don't interfere with other tests: + remove_fatal_error_handler(); + remove_bad_alloc_error_handler(); +} + +TEST(Error, BadAllocFatalErrorHandlersInteraction) { + auto ErrorHandler = [](void *Data, const char *, bool) {}; + install_bad_alloc_error_handler(ErrorHandler, nullptr); + // The following call should not crash + install_fatal_error_handler(ErrorHandler, nullptr); + + // Don't interfere with other tests: + remove_fatal_error_handler(); + remove_bad_alloc_error_handler(); +} + } // namespace From c6a854579fd179c5980b31f14679ba2138d3a621 Mon Sep 17 00:00:00 2001 From: Fabian Schiebel Date: Wed, 28 Feb 2024 19:01:53 +0100 Subject: [PATCH 3/3] Apply review comments --- llvm/unittests/Support/ErrorTest.cpp | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/llvm/unittests/Support/ErrorTest.cpp b/llvm/unittests/Support/ErrorTest.cpp index 94ee04a0296f5..1229282cf0def 100644 --- a/llvm/unittests/Support/ErrorTest.cpp +++ b/llvm/unittests/Support/ErrorTest.cpp @@ -1135,10 +1135,12 @@ TEST(Error, moveInto) { TEST(Error, FatalBadAllocErrorHandlersInteraction) { auto ErrorHandler = [](void *Data, const char *, bool) {}; install_fatal_error_handler(ErrorHandler, nullptr); - // The following call should not crash + // The following call should not crash; previously, a bug in + // install_bad_alloc_error_handler asserted that no fatal-error handler is + // installed already. install_bad_alloc_error_handler(ErrorHandler, nullptr); - // Don't interfere with other tests: + // Don't interfere with other tests. remove_fatal_error_handler(); remove_bad_alloc_error_handler(); } @@ -1146,10 +1148,12 @@ TEST(Error, FatalBadAllocErrorHandlersInteraction) { TEST(Error, BadAllocFatalErrorHandlersInteraction) { auto ErrorHandler = [](void *Data, const char *, bool) {}; install_bad_alloc_error_handler(ErrorHandler, nullptr); - // The following call should not crash + // The following call should not crash; related to + // FatalBadAllocErrorHandlersInteraction: Ensure that the error does not occur + // in the other direction. install_fatal_error_handler(ErrorHandler, nullptr); - // Don't interfere with other tests: + // Don't interfere with other tests. remove_fatal_error_handler(); remove_bad_alloc_error_handler(); }