From 6a2020ec6cc5ac94d9b3350c1faa063967e5c8b6 Mon Sep 17 00:00:00 2001 From: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> Date: Wed, 13 Oct 2021 14:32:12 -0700 Subject: [PATCH 01/10] fixes some warnings for newer versions of VS2022 Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> --- .../Tests/Asset/AssetManagerLoadingTests.cpp | 3 +++ .../Tests/Debug/LocalFileEventLoggerTests.cpp | 6 ++++++ .../IO/Streamer/StorageDriveTests_Windows.cpp | 18 ++++++++++++++++++ Code/Framework/AzCore/Tests/StringFunc.cpp | 6 ++++++ 4 files changed, 33 insertions(+) diff --git a/Code/Framework/AzCore/Tests/Asset/AssetManagerLoadingTests.cpp b/Code/Framework/AzCore/Tests/Asset/AssetManagerLoadingTests.cpp index 3e6376323c..e33dbce9c1 100644 --- a/Code/Framework/AzCore/Tests/Asset/AssetManagerLoadingTests.cpp +++ b/Code/Framework/AzCore/Tests/Asset/AssetManagerLoadingTests.cpp @@ -736,7 +736,10 @@ namespace UnitTest auto& assetManager = AssetManager::Instance(); AssetBusCallbacks callbacks{}; + AZ_PUSH_DISABLE_WARNING(5233, "-Wunknown-warning-option") // Older versions of MSVC toolchain require to pass constexpr in the + // capture. Newer versions issue unused warning callbacks.SetOnAssetReadyCallback([&, AssetNoRefB](const Asset&, AssetBusCallbacks&) + AZ_POP_DISABLE_WARNING { // This callback should run inside the "main thread" dispatch events loop auto loadAsset = assetManager.GetAsset(AZ::Uuid(AssetNoRefB), AssetLoadBehavior::Default); diff --git a/Code/Framework/AzCore/Tests/Debug/LocalFileEventLoggerTests.cpp b/Code/Framework/AzCore/Tests/Debug/LocalFileEventLoggerTests.cpp index 242ac0e65d..d4b3351dbe 100644 --- a/Code/Framework/AzCore/Tests/Debug/LocalFileEventLoggerTests.cpp +++ b/Code/Framework/AzCore/Tests/Debug/LocalFileEventLoggerTests.cpp @@ -109,7 +109,10 @@ namespace AZ::Debug AZStd::thread threads[totalThreads]; for (size_t threadIndex = 0; threadIndex < totalThreads; ++threadIndex) { + AZ_PUSH_DISABLE_WARNING(5233, "-Wunknown-warning-option") // Older versions of MSVC toolchain require to pass constexpr in the + // capture. Newer versions issue unused warning threads[threadIndex] = AZStd::thread([&startLogging, &messages]() + AZ_POP_DISABLE_WARNING { while (!startLogging) { @@ -226,7 +229,10 @@ namespace AZ::Debug AZStd::thread threads[totalThreads]; for (size_t threadIndex = 0; threadIndex < totalThreads; ++threadIndex) { + AZ_PUSH_DISABLE_WARNING(5233, "-Wunknown-warning-option") // Older versions of MSVC toolchain require to pass constexpr in the + // capture. Newer versions issue unused warning threads[threadIndex] = AZStd::thread([&startLogging, &message, &totalRecordsWritten]() + AZ_POP_DISABLE_WARNING { AZ_UNUSED(message); diff --git a/Code/Framework/AzCore/Tests/Platform/Windows/Tests/IO/Streamer/StorageDriveTests_Windows.cpp b/Code/Framework/AzCore/Tests/Platform/Windows/Tests/IO/Streamer/StorageDriveTests_Windows.cpp index 22fb6379d2..e312e2058d 100644 --- a/Code/Framework/AzCore/Tests/Platform/Windows/Tests/IO/Streamer/StorageDriveTests_Windows.cpp +++ b/Code/Framework/AzCore/Tests/Platform/Windows/Tests/IO/Streamer/StorageDriveTests_Windows.cpp @@ -597,7 +597,10 @@ namespace AZ::IO path.InitFromAbsolutePath(m_dummyFilepath); request->CreateRead(nullptr, buffer.get(), fileSize, path, 0, fileSize); + AZ_PUSH_DISABLE_WARNING(5233, "-Wunknown-warning-option") // Older versions of MSVC toolchain require to pass constexpr in the + // capture. Newer versions issue unused warning auto callback = [&fileSize, this](const FileRequest& request) + AZ_POP_DISABLE_WARNING { EXPECT_EQ(request.GetStatus(), AZ::IO::IStreamerTypes::RequestStatus::Completed); auto& readRequest = AZStd::get(request.GetCommand()); @@ -639,7 +642,10 @@ namespace AZ::IO path.InitFromAbsolutePath(m_dummyFilepath); request->CreateRead(nullptr, buffer, unalignedSize + 4, path, unalignedOffset, unalignedSize); + AZ_PUSH_DISABLE_WARNING(5233, "-Wunknown-warning-option") // Older versions of MSVC toolchain require to pass constexpr in the + // capture. Newer versions issue unused warning auto callback = [unalignedOffset, unalignedSize, this](const FileRequest& request) + AZ_POP_DISABLE_WARNING { EXPECT_EQ(request.GetStatus(), AZ::IO::IStreamerTypes::RequestStatus::Completed); auto& readRequest = AZStd::get(request.GetCommand()); @@ -784,7 +790,10 @@ namespace AZ::IO requests[i] = m_context->GetNewInternalRequest(); requests[i]->CreateRead(nullptr, buffers[i].get(), chunkSize, path, i * chunkSize, chunkSize); + AZ_PUSH_DISABLE_WARNING(5233, "-Wunknown-warning-option") // Older versions of MSVC toolchain require to pass constexpr in the + // capture. Newer versions issue unused warning auto callback = [chunkSize, i](const FileRequest& request) + AZ_POP_DISABLE_WARNING { EXPECT_EQ(request.GetStatus(), AZ::IO::IStreamerTypes::RequestStatus::Completed); auto& readRequest = AZStd::get(request.GetCommand()); @@ -970,7 +979,10 @@ namespace AZ::IO i * chunkSize )); + AZ_PUSH_DISABLE_WARNING(5233, "-Wunknown-warning-option") // Older versions of MSVC toolchain require to pass constexpr in the + // capture. Newer versions issue unused warning auto callback = [numChunks, &numCallbacks, &waitForReads](FileRequestHandle request) + AZ_POP_DISABLE_WARNING { IStreamer* streamer = Interface::Get(); if (streamer) @@ -1038,7 +1050,10 @@ namespace AZ::IO i * chunkSize )); + AZ_PUSH_DISABLE_WARNING(5233, "-Wunknown-warning-option") // Older versions of MSVC toolchain require to pass constexpr in the + // capture. Newer versions issue unused warning auto callback = [numChunks, &waitForReads, &waitForSingleRead, &numReadCallbacks]([[maybe_unused]] FileRequestHandle request) + AZ_POP_DISABLE_WARNING { numReadCallbacks++; if (numReadCallbacks == 1) @@ -1059,7 +1074,10 @@ namespace AZ::IO for (size_t i = 0; i < numChunks; ++i) { cancels.push_back(m_streamer->Cancel(requests[numChunks - i - 1])); + AZ_PUSH_DISABLE_WARNING(5233, "-Wunknown-warning-option") // Older versions of MSVC toolchain require to pass constexpr in the + // capture. Newer versions issue unused warning auto callback = [&numCancelCallbacks, &waitForCancels, numChunks](FileRequestHandle request) + AZ_POP_DISABLE_WARNING { auto result = Interface::Get()->GetRequestStatus(request); EXPECT_EQ(result, IStreamerTypes::RequestStatus::Completed); diff --git a/Code/Framework/AzCore/Tests/StringFunc.cpp b/Code/Framework/AzCore/Tests/StringFunc.cpp index 68821ba3f9..dc4bc40e5b 100644 --- a/Code/Framework/AzCore/Tests/StringFunc.cpp +++ b/Code/Framework/AzCore/Tests/StringFunc.cpp @@ -363,7 +363,10 @@ namespace AZ { constexpr AZStd::array visitTokens = { "Hello", "World", "", "More", "", "", "Tokens" }; size_t visitIndex{}; + AZ_PUSH_DISABLE_WARNING(5233, "-Wunknown-warning-option") // Older versions of MSVC toolchain require to pass constexpr in the + // capture. Newer versions issue unused warning auto visitor = [&visitIndex, &visitTokens](AZStd::string_view token) + AZ_POP_DISABLE_WARNING { if (visitIndex > visitTokens.size()) { @@ -389,7 +392,10 @@ namespace AZ { constexpr AZStd::array visitTokens = { "Hello", "World", "", "More", "", "", "Tokens" }; size_t visitIndex = visitTokens.size() - 1; + AZ_PUSH_DISABLE_WARNING(5233, "-Wunknown-warning-option") // Older versions of MSVC toolchain require to pass constexpr in the + // capture. Newer versions issue unused warning auto visitor = [&visitIndex, &visitTokens](AZStd::string_view token) + AZ_POP_DISABLE_WARNING { if (visitIndex > visitTokens.size()) { From 88cc3c774a731ebc3aae7a5cd3f2777d56bf40cf Mon Sep 17 00:00:00 2001 From: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> Date: Thu, 14 Oct 2021 12:32:15 -0700 Subject: [PATCH 02/10] adds stack trace conversion from native Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> --- .../AzCore/AzCore/Debug/StackTracer.h | 6 ++ Code/Framework/AzCore/AzCore/Debug/Trace.cpp | 10 ++- .../Debug/StackTracer_Unimplemented.cpp | 5 ++ .../AzCore/Debug/StackTracer_UnixLike.cpp | 6 ++ .../AzCore/Debug/StackTracer_Windows.cpp | 75 ++++++++++++++++--- 5 files changed, 89 insertions(+), 13 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Debug/StackTracer.h b/Code/Framework/AzCore/AzCore/Debug/StackTracer.h index 430fd69168..2d09c16e99 100644 --- a/Code/Framework/AzCore/AzCore/Debug/StackTracer.h +++ b/Code/Framework/AzCore/AzCore/Debug/StackTracer.h @@ -40,6 +40,12 @@ namespace AZ static unsigned int Record(StackFrame* frames, unsigned int maxNumOfFrames, unsigned int suppressCount = 0, void* nativeThread = 0); }; + class StackConverter + { + public: + static unsigned int FromNative(StackFrame* frames, unsigned int maxNumOfFrames, void* nativeContext); + }; + class SymbolStorage { public: diff --git a/Code/Framework/AzCore/AzCore/Debug/Trace.cpp b/Code/Framework/AzCore/AzCore/Debug/Trace.cpp index 3edcbaa273..38e31393f5 100644 --- a/Code/Framework/AzCore/AzCore/Debug/Trace.cpp +++ b/Code/Framework/AzCore/AzCore/Debug/Trace.cpp @@ -31,6 +31,8 @@ namespace AZ { namespace Debug { + struct StackFrame; + namespace Platform { #if defined(AZ_ENABLE_DEBUG_TOOLS) @@ -556,12 +558,18 @@ namespace AZ //size_t bla = AZStd::alignment_of::value; //printf("Alignment value %d address 0x%08x : 0x%08x\n",bla,frames); SymbolStorage::StackLine lines[AZ_ARRAY_SIZE(frames)]; + unsigned int numFrames = 0; if (!nativeContext) { suppressCount += 1; /// If we don't provide a context we will capture in the RecordFunction, so skip us (Trace::PrinCallstack). + numFrames = StackRecorder::Record(frames, AZ_ARRAY_SIZE(frames), suppressCount); } - unsigned int numFrames = StackRecorder::Record(frames, AZ_ARRAY_SIZE(frames), suppressCount, nativeContext); + else + { + numFrames = StackConverter::FromNative(frames, AZ_ARRAY_SIZE(frames), nativeContext); + } + if (numFrames) { SymbolStorage::DecodeFrames(frames, numFrames, lines); diff --git a/Code/Framework/AzCore/Platform/Common/Unimplemented/AzCore/Debug/StackTracer_Unimplemented.cpp b/Code/Framework/AzCore/Platform/Common/Unimplemented/AzCore/Debug/StackTracer_Unimplemented.cpp index 0c091a7242..a751eb505c 100644 --- a/Code/Framework/AzCore/Platform/Common/Unimplemented/AzCore/Debug/StackTracer_Unimplemented.cpp +++ b/Code/Framework/AzCore/Platform/Common/Unimplemented/AzCore/Debug/StackTracer_Unimplemented.cpp @@ -17,6 +17,11 @@ namespace AZ return false; } + unsigned int StackConverter::FromNative(StackFrame*, unsigned int, void*) + { + return 0; + } + void SymbolStorage::LoadModuleData(const void*, unsigned int) {} diff --git a/Code/Framework/AzCore/Platform/Common/UnixLike/AzCore/Debug/StackTracer_UnixLike.cpp b/Code/Framework/AzCore/Platform/Common/UnixLike/AzCore/Debug/StackTracer_UnixLike.cpp index f66a9d3b18..1948bbf2fe 100644 --- a/Code/Framework/AzCore/Platform/Common/UnixLike/AzCore/Debug/StackTracer_UnixLike.cpp +++ b/Code/Framework/AzCore/Platform/Common/UnixLike/AzCore/Debug/StackTracer_UnixLike.cpp @@ -78,6 +78,12 @@ StackRecorder::Record(StackFrame* frames, unsigned int maxNumOfFrames, unsigned return count; } +unsigned int StackConverter::FromNative([[maybe_unused]] StackFrame* frames, [[maybe_unused]] unsigned int maxNumOfFrames, [[maybe_unused]] void* nativeContext) +{ + AZ_Assert(false, "StackConverter::FromNative() is not supported for UnixLike platform yet"); + return 0; +} + void SymbolStorage::DecodeFrames(const StackFrame* frames, unsigned int numFrames, StackLine* textLines) { diff --git a/Code/Framework/AzCore/Platform/Windows/AzCore/Debug/StackTracer_Windows.cpp b/Code/Framework/AzCore/Platform/Windows/AzCore/Debug/StackTracer_Windows.cpp index b2a1410bf5..b4ccb07296 100644 --- a/Code/Framework/AzCore/Platform/Windows/AzCore/Debug/StackTracer_Windows.cpp +++ b/Code/Framework/AzCore/Platform/Windows/AzCore/Debug/StackTracer_Windows.cpp @@ -1048,9 +1048,9 @@ cleanup: unsigned int StackRecorder::Record(StackFrame* frames, unsigned int maxNumOfFrames, unsigned int suppressCount, void* nativeThread) { -#if defined(AZ_ENABLE_DEBUG_TOOLS) unsigned int numFrames = 0; +#if defined(AZ_ENABLE_DEBUG_TOOLS) if (nativeThread == NULL) { ++suppressCount; // Skip current call @@ -1079,9 +1079,8 @@ cleanup: STACKFRAME64 sf; memset(&sf, 0, sizeof(STACKFRAME64)); - DWORD imageType; + DWORD imageType = IMAGE_FILE_MACHINE_AMD64; - imageType = IMAGE_FILE_MACHINE_AMD64; sf.AddrPC.Offset = context.Rip; sf.AddrPC.Mode = AddrModeFlat; sf.AddrFrame.Offset = context.Rsp; @@ -1090,8 +1089,7 @@ cleanup: sf.AddrStack.Mode = AddrModeFlat; EnterCriticalSection(&g_csDbgHelpDll); - s32 frame = -(s32)suppressCount; - for (; frame < (s32)maxNumOfFrames; ++frame) + for (s32 frame = -static_cast(suppressCount); frame < static_cast(maxNumOfFrames); ++frame) { if (!g_StackWalk64(imageType, g_currentProcess, hThread, &sf, &context, 0, g_SymFunctionTableAccess64, g_SymGetModuleBase64, 0)) { @@ -1111,15 +1109,68 @@ cleanup: } LeaveCriticalSection(&g_csDbgHelpDll); - } - return numFrames; + } #else - (void)frames; - (void)maxNumOfFrames; - (void)suppressCount; - (void)nativeThread; - return 0; + AZ_UNUSED(frames); + AZ_UNUSED(maxNumOfFrames); + AZ_UNUSED(suppressCount); + AZ_UNUSED(nativeThread); #endif // AZ_ENABLE_DEBUG_TOOLS + + return numFrames; + } + + unsigned int StackConverter::FromNative(StackFrame* frames, unsigned int maxNumOfFrames, void* nativeContext) + { + unsigned int numFrames = 0; + +#if defined(AZ_ENABLE_DEBUG_TOOLS) + if (!g_dbgHelpLoaded) + { + LoadDbgHelp(); + } + + HANDLE hThread; + DuplicateHandle(GetCurrentProcess(), GetCurrentThread(), GetCurrentProcess(), &hThread, 0, false, DUPLICATE_SAME_ACCESS); + + PCONTEXT nativeContextType = reinterpret_cast(nativeContext); + STACKFRAME64 sf; + memset(&sf, 0, sizeof(STACKFRAME64)); + + DWORD imageType = IMAGE_FILE_MACHINE_AMD64; + + sf.AddrPC.Offset = nativeContextType->Rip; + sf.AddrPC.Mode = AddrModeFlat; + sf.AddrFrame.Offset = nativeContextType->Rsp; + sf.AddrFrame.Mode = AddrModeFlat; + sf.AddrStack.Offset = nativeContextType->Rsp; + sf.AddrStack.Mode = AddrModeFlat; + + EnterCriticalSection(&g_csDbgHelpDll); + for (unsigned int frame = 0; frame < maxNumOfFrames; ++frame) + { + if (!g_StackWalk64(imageType, g_currentProcess, hThread, &sf, nativeContext, 0, g_SymFunctionTableAccess64, g_SymGetModuleBase64, 0)) + { + break; + } + + if (sf.AddrPC.Offset == sf.AddrReturn.Offset) + { + // "StackWalk64-Endless-Callstack!" + break; + } + + frames[numFrames++].m_programCounter = sf.AddrPC.Offset; + } + + LeaveCriticalSection(&g_csDbgHelpDll); +#else + AZ_UNUSED(frame); + AZ_UNUSED(maxNumOfFrames); + AZ_UNUSED(nativeContext); +#endif + + return numFrames; } ////////////////////////////////////////////////////////////////////////// From 3e729638b51368dce474e2d68d1b3aad3d573fe3 Mon Sep 17 00:00:00 2001 From: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> Date: Thu, 14 Oct 2021 12:32:39 -0700 Subject: [PATCH 03/10] adds unhandled exception test Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> --- .../Tests/Debug/UnhandledExceptions.cpp | 31 +++++++++++++++++++ .../AzCore/Tests/azcoretests_files.cmake | 1 + 2 files changed, 32 insertions(+) create mode 100644 Code/Framework/AzCore/Tests/Debug/UnhandledExceptions.cpp diff --git a/Code/Framework/AzCore/Tests/Debug/UnhandledExceptions.cpp b/Code/Framework/AzCore/Tests/Debug/UnhandledExceptions.cpp new file mode 100644 index 0000000000..cd9055085f --- /dev/null +++ b/Code/Framework/AzCore/Tests/Debug/UnhandledExceptions.cpp @@ -0,0 +1,31 @@ +/* + * Copyright (c) Contributors to the Open 3D Engine Project. + * For complete copyright and license terms please see the LICENSE at the root of this distribution. + * + * SPDX-License-Identifier: Apache-2.0 OR MIT + * + */ + +#include + +namespace UnitTest +{ + class UnhandledExceptions + : public ScopedAllocatorSetupFixture + { + + public: + void causeAccessViolation() + { + int* someVariable = reinterpret_cast(0); + *someVariable = 0; + } + }; + +#if GTEST_HAS_DEATH_TEST + TEST_F(UnhandledExceptions, Handle) + { + EXPECT_DEATH(causeAccessViolation(), ""); + } +#endif +} diff --git a/Code/Framework/AzCore/Tests/azcoretests_files.cmake b/Code/Framework/AzCore/Tests/azcoretests_files.cmake index 6ba9944f7d..cc6000209f 100644 --- a/Code/Framework/AzCore/Tests/azcoretests_files.cmake +++ b/Code/Framework/AzCore/Tests/azcoretests_files.cmake @@ -72,6 +72,7 @@ set(FILES Debug/AssetTracking.cpp Debug/LocalFileEventLoggerTests.cpp Debug/Trace.cpp + Debug/UnhandledExceptions.cpp Name/NameJsonSerializerTests.cpp Name/NameTests.cpp RTTI/TypeSafeIntegralTests.cpp From e078580f2cce3d00fee44cf74932c2e157948916 Mon Sep 17 00:00:00 2001 From: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> Date: Thu, 14 Oct 2021 12:33:18 -0700 Subject: [PATCH 04/10] Changes GTEST_OS_SUPPORTS_DEATH_TEST to the right define which is GTEST_HAS_DEATH_TEST Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> --- Code/Framework/AzCore/Tests/AZStd/Hashed.cpp | 8 ++++---- Code/Framework/AzCore/Tests/AZStd/Ordered.cpp | 8 ++++---- Code/Framework/AzCore/Tests/AZStd/Parallel.cpp | 4 ++-- Code/Framework/AzCore/Tests/Memory/LeakDetection.cpp | 5 ++--- .../Serialization/Json/JsonRegistrationContextTests.cpp | 5 +++-- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/Code/Framework/AzCore/Tests/AZStd/Hashed.cpp b/Code/Framework/AzCore/Tests/AZStd/Hashed.cpp index b92dee44b6..c982868c4f 100644 --- a/Code/Framework/AzCore/Tests/AZStd/Hashed.cpp +++ b/Code/Framework/AzCore/Tests/AZStd/Hashed.cpp @@ -1413,7 +1413,7 @@ namespace UnitTest >; TYPED_TEST_CASE(HashedSetDifferentAllocatorFixture, SetTemplateConfigs); -#if GTEST_OS_SUPPORTS_DEATH_TEST +#if GTEST_HAS_DEATH_TEST TYPED_TEST(HashedSetDifferentAllocatorFixture, InsertNodeHandleWithDifferentAllocatorsLogsTraceMessages) { using ContainerType = typename TypeParam::ContainerType; @@ -1435,7 +1435,7 @@ namespace UnitTest } }, ".*"); } -#endif // GTEST_OS_SUPPORTS_DEATH_TEST +#endif // GTEST_HAS_DEATH_TEST template class HashedMapContainers @@ -1811,7 +1811,7 @@ namespace UnitTest >; TYPED_TEST_CASE(HashedMapDifferentAllocatorFixture, MapTemplateConfigs); -#if GTEST_OS_SUPPORTS_DEATH_TEST +#if GTEST_HAS_DEATH_TEST TYPED_TEST(HashedMapDifferentAllocatorFixture, InsertNodeHandleWithDifferentAllocatorsLogsTraceMessages) { using ContainerType = typename TypeParam::ContainerType; @@ -1833,7 +1833,7 @@ namespace UnitTest } } , ".*"); } -#endif // GTEST_OS_SUPPORTS_DEATH_TEST +#endif // GTEST_HAS_DEATH_TEST namespace HashedContainerTransparentTestInternal { diff --git a/Code/Framework/AzCore/Tests/AZStd/Ordered.cpp b/Code/Framework/AzCore/Tests/AZStd/Ordered.cpp index ca7d5cba42..26838eeb63 100644 --- a/Code/Framework/AzCore/Tests/AZStd/Ordered.cpp +++ b/Code/Framework/AzCore/Tests/AZStd/Ordered.cpp @@ -1095,7 +1095,7 @@ namespace UnitTest >; TYPED_TEST_CASE(TreeSetDifferentAllocatorFixture, SetTemplateConfigs); -#if GTEST_OS_SUPPORTS_DEATH_TEST +#if GTEST_HAS_DEATH_TEST TYPED_TEST(TreeSetDifferentAllocatorFixture, InsertNodeHandleWithDifferentAllocatorsLogsTraceMessages) { using ContainerType = typename TypeParam::ContainerType; @@ -1117,7 +1117,7 @@ namespace UnitTest } }, ".*"); } -#endif // GTEST_OS_SUPPORTS_DEATH_TEST +#endif // GTEST_HAS_DEATH_TEST TYPED_TEST(TreeSetDifferentAllocatorFixture, SwapMovesElementsWhenAllocatorsDiffer) { @@ -1516,7 +1516,7 @@ namespace UnitTest >; TYPED_TEST_CASE(TreeMapDifferentAllocatorFixture, MapTemplateConfigs); -#if GTEST_OS_SUPPORTS_DEATH_TEST +#if GTEST_HAS_DEATH_TEST TYPED_TEST(TreeMapDifferentAllocatorFixture, InsertNodeHandleWithDifferentAllocatorsLogsTraceMessages) { using ContainerType = typename TypeParam::ContainerType; @@ -1538,7 +1538,7 @@ namespace UnitTest } }, ".*"); } -#endif // GTEST_OS_SUPPORTS_DEATH_TEST +#endif // GTEST_HAS_DEATH_TEST TYPED_TEST(TreeMapDifferentAllocatorFixture, SwapMovesElementsWhenAllocatorsDiffer) { diff --git a/Code/Framework/AzCore/Tests/AZStd/Parallel.cpp b/Code/Framework/AzCore/Tests/AZStd/Parallel.cpp index 407cd3c258..6b9202133c 100644 --- a/Code/Framework/AzCore/Tests/AZStd/Parallel.cpp +++ b/Code/Framework/AzCore/Tests/AZStd/Parallel.cpp @@ -1595,7 +1595,7 @@ namespace UnitTest } }; -#if GTEST_OS_SUPPORTS_DEATH_TEST +#if GTEST_HAS_DEATH_TEST TEST_F(ThreadEventsDeathTest, UsingClientBus_AvoidsDeadlock) { EXPECT_EXIT( @@ -1608,5 +1608,5 @@ namespace UnitTest , ::testing::ExitedWithCode(0),".*"); } -#endif // GTEST_OS_SUPPORTS_DEATH_TEST +#endif // GTEST_HAS_DEATH_TEST } diff --git a/Code/Framework/AzCore/Tests/Memory/LeakDetection.cpp b/Code/Framework/AzCore/Tests/Memory/LeakDetection.cpp index b4f129bb48..09255ce16f 100644 --- a/Code/Framework/AzCore/Tests/Memory/LeakDetection.cpp +++ b/Code/Framework/AzCore/Tests/Memory/LeakDetection.cpp @@ -144,14 +144,13 @@ namespace UnitTest } }; -#if GTEST_OS_SUPPORTS_DEATH_TEST - // SPEC-2669: Disabled since it is causing hangs on Linux +#if GTEST_HAS_DEATH_TEST TEST_F(AllocatorsTestFixtureLeakDetectionDeathTest_SKIPCODECOVERAGE, AllocatorLeak) { // testing that the TraceBusHook will fail on cause the test to die EXPECT_DEATH(TestAllocatorLeak(), ""); } -#endif // GTEST_OS_SUPPORTS_DEATH_TEST +#endif // GTEST_HAS_DEATH_TEST //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // Testing ScopedAllocatorSetupFixture. Testing that detects leaks diff --git a/Code/Framework/AzCore/Tests/Serialization/Json/JsonRegistrationContextTests.cpp b/Code/Framework/AzCore/Tests/Serialization/Json/JsonRegistrationContextTests.cpp index cc4d31eca9..9d4af1def5 100644 --- a/Code/Framework/AzCore/Tests/Serialization/Json/JsonRegistrationContextTests.cpp +++ b/Code/Framework/AzCore/Tests/Serialization/Json/JsonRegistrationContextTests.cpp @@ -327,7 +327,7 @@ namespace JsonSerializationTests SerializerWithOneType::Unreflect(m_jsonRegistrationContext.get()); } -#if GTEST_OS_SUPPORTS_DEATH_TEST +#if GTEST_HAS_DEATH_TEST using JsonSerializationDeathTests = JsonRegistrationContextTests; TEST_F(JsonSerializationDeathTests, DoubleUnregisterSerializer_Asserts) { @@ -338,5 +338,6 @@ namespace JsonSerializationTests }, ".*" ); } -#endif // GTEST_OS_SUPPORTS_DEATH_TEST +#endif // GTEST_HAS_DEATH_TEST + } //namespace JsonSerializationTests From 04a6744765494eecf2a6941e4b9518769dd0c566 Mon Sep 17 00:00:00 2001 From: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> Date: Thu, 14 Oct 2021 12:33:53 -0700 Subject: [PATCH 05/10] enables our exception handling and disables gtest's Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> --- Code/Framework/AzTest/AzTest/AzTest.cpp | 8 ++++++-- Code/Tools/AzTestRunner/src/main.cpp | 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/Code/Framework/AzTest/AzTest/AzTest.cpp b/Code/Framework/AzTest/AzTest/AzTest.cpp index 3b809d2c96..baf80d8f83 100644 --- a/Code/Framework/AzTest/AzTest/AzTest.cpp +++ b/Code/Framework/AzTest/AzTest/AzTest.cpp @@ -99,10 +99,14 @@ namespace AZ void ApplyGlobalParameters(int* argc, char** argv) { - // this is a hook that can be used to apply any other global non-google parameters - // that we use. + // this is a hook that can be used to apply any other global parameters that we use. AZ_UNUSED(argc); AZ_UNUSED(argv); + + // Disable gtest catching unhandled exceptions, instead, AzTestRunner will do it through: + // AZ::Debug::Trace::HandleExceptions(true). This gives us a stack trace when the exception + // is thrown (googletest does not). + testing::FLAGS_gtest_catch_exceptions = false; } //! Print out parameters that are not used by the framework diff --git a/Code/Tools/AzTestRunner/src/main.cpp b/Code/Tools/AzTestRunner/src/main.cpp index 0874efeff2..cdd1c97d04 100644 --- a/Code/Tools/AzTestRunner/src/main.cpp +++ b/Code/Tools/AzTestRunner/src/main.cpp @@ -258,6 +258,8 @@ namespace AzTestRunner int wrapped_main(int argc/*=0*/, char** argv/*=nullptr*/) { + AZ::Debug::Trace::HandleExceptions(true); + if (argc>0 && argv!=nullptr) { return wrapped_command_arg_main(argc, argv); From a6c506c12181d2cf2e0f3e89f94ae8df0d0e4218 Mon Sep 17 00:00:00 2001 From: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> Date: Thu, 14 Oct 2021 13:02:29 -0700 Subject: [PATCH 06/10] some warning fixes Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> --- .../Tests/Spawnable/SpawnableEntitiesManagerTests.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Code/Framework/AzFramework/Tests/Spawnable/SpawnableEntitiesManagerTests.cpp b/Code/Framework/AzFramework/Tests/Spawnable/SpawnableEntitiesManagerTests.cpp index 407ab1d21f..8693198eb9 100644 --- a/Code/Framework/AzFramework/Tests/Spawnable/SpawnableEntitiesManagerTests.cpp +++ b/Code/Framework/AzFramework/Tests/Spawnable/SpawnableEntitiesManagerTests.cpp @@ -569,8 +569,11 @@ namespace UnitTest FillSpawnable(NumEntities); CreateEntityReferences(refScheme); + AZ_PUSH_DISABLE_WARNING(5233, "-Wunknown-warning-option") // Older versions of MSVC toolchain require to pass constexpr in the + // capture. Newer versions issue unused warning auto callback = [this, refScheme, NumEntities](AzFramework::EntitySpawnTicket::Id, AzFramework::SpawnableConstEntityContainerView entities) + AZ_POP_DISABLE_WARNING { AZ_UNUSED(refScheme); AZ_UNUSED(NumEntities); @@ -591,8 +594,11 @@ namespace UnitTest FillSpawnable(NumEntities); CreateEntityReferences(refScheme); + AZ_PUSH_DISABLE_WARNING(5233, "-Wunknown-warning-option") // Older versions of MSVC toolchain require to pass constexpr in the + // capture. Newer versions issue unused warning auto callback = [this, refScheme, NumEntities](AzFramework::EntitySpawnTicket::Id, AzFramework::SpawnableConstEntityContainerView entities) + AZ_POP_DISABLE_WARNING { AZ_UNUSED(refScheme); AZ_UNUSED(NumEntities); @@ -720,8 +726,11 @@ namespace UnitTest FillSpawnable(NumEntities); CreateEntityReferences(refScheme); + AZ_PUSH_DISABLE_WARNING(5233, "-Wunknown-warning-option") // Older versions of MSVC toolchain require to pass constexpr in the + // capture. Newer versions issue unused warning auto callback = [this, refScheme, NumEntities](AzFramework::EntitySpawnTicket::Id, AzFramework::SpawnableConstEntityContainerView entities) + AZ_POP_DISABLE_WARNING { AZ_UNUSED(refScheme); AZ_UNUSED(NumEntities); From 3df2ca341f573c23f7b63193cb45e3775b88ab8f Mon Sep 17 00:00:00 2001 From: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> Date: Thu, 14 Oct 2021 13:28:26 -0700 Subject: [PATCH 07/10] function was converted to return a bool but this code was not updated Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> --- .../Code/Source/Viewport/WhiteBoxManipulatorBounds.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Gems/WhiteBox/Code/Source/Viewport/WhiteBoxManipulatorBounds.cpp b/Gems/WhiteBox/Code/Source/Viewport/WhiteBoxManipulatorBounds.cpp index 878ede5e68..a11f2c9905 100644 --- a/Gems/WhiteBox/Code/Source/Viewport/WhiteBoxManipulatorBounds.cpp +++ b/Gems/WhiteBox/Code/Source/Viewport/WhiteBoxManipulatorBounds.cpp @@ -48,10 +48,10 @@ namespace WhiteBox float time; AZ::Vector3 normal; const float rayLength = 1000.0f; - const int intersected = AZ::Intersect::IntersectSegmentTriangleCCW( + const bool intersected = AZ::Intersect::IntersectSegmentTriangleCCW( rayOrigin, rayOrigin + rayDirection * rayLength, p0, p1, p2, normal, time); - if (intersected != 0) + if (intersected) { rayIntersectionDistance = time * rayLength; intersectedTriangleIndex = triangleIndex / 3; From c5da705b469edeac655010d46a5bbc1101616aba Mon Sep 17 00:00:00 2001 From: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> Date: Thu, 14 Oct 2021 14:39:18 -0700 Subject: [PATCH 08/10] fixes typo Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> --- .../Platform/Windows/AzCore/Debug/StackTracer_Windows.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Code/Framework/AzCore/Platform/Windows/AzCore/Debug/StackTracer_Windows.cpp b/Code/Framework/AzCore/Platform/Windows/AzCore/Debug/StackTracer_Windows.cpp index b4ccb07296..90362a3ce8 100644 --- a/Code/Framework/AzCore/Platform/Windows/AzCore/Debug/StackTracer_Windows.cpp +++ b/Code/Framework/AzCore/Platform/Windows/AzCore/Debug/StackTracer_Windows.cpp @@ -1165,7 +1165,7 @@ cleanup: LeaveCriticalSection(&g_csDbgHelpDll); #else - AZ_UNUSED(frame); + AZ_UNUSED(frames); AZ_UNUSED(maxNumOfFrames); AZ_UNUSED(nativeContext); #endif From 17b5312ce5c3ae2a8ef67e507807968eb8725d17 Mon Sep 17 00:00:00 2001 From: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> Date: Thu, 14 Oct 2021 15:53:21 -0700 Subject: [PATCH 09/10] PR comments Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> --- Code/Framework/AzCore/AzCore/Debug/Trace.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/Code/Framework/AzCore/AzCore/Debug/Trace.cpp b/Code/Framework/AzCore/AzCore/Debug/Trace.cpp index 38e31393f5..cde8c36a4e 100644 --- a/Code/Framework/AzCore/AzCore/Debug/Trace.cpp +++ b/Code/Framework/AzCore/AzCore/Debug/Trace.cpp @@ -553,16 +553,12 @@ namespace AZ { StackFrame frames[25]; - // Without StackFrame explicit alignment frames array is aligned to 4 bytes - // which causes the stack tracing to fail. - //size_t bla = AZStd::alignment_of::value; - //printf("Alignment value %d address 0x%08x : 0x%08x\n",bla,frames); SymbolStorage::StackLine lines[AZ_ARRAY_SIZE(frames)]; unsigned int numFrames = 0; if (!nativeContext) { - suppressCount += 1; /// If we don't provide a context we will capture in the RecordFunction, so skip us (Trace::PrinCallstack). + suppressCount += 1; /// If we don't provide a context we will capture in the RecordFunction, so skip us (Trace::PrintCallstack). numFrames = StackRecorder::Record(frames, AZ_ARRAY_SIZE(frames), suppressCount); } else From 721d92b4f96123adf0023cfca4bb8c9cb66aba9b Mon Sep 17 00:00:00 2001 From: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> Date: Fri, 15 Oct 2021 11:02:12 -0700 Subject: [PATCH 10/10] adding exception handling for tests that are not going through AzTestRunner Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com> --- Code/Tools/AssetBundler/tests/tests_main.cpp | 3 +++ .../AssetProcessor/native/tests/SourceFileRelocatorTests.cpp | 2 +- Code/Tools/AssetProcessor/native/tests/test_main.cpp | 3 +++ Code/Tools/ProjectManager/tests/main.cpp | 3 +++ Code/Tools/PythonBindingsExample/tests/TestMain.cpp | 3 +++ 5 files changed, 13 insertions(+), 1 deletion(-) diff --git a/Code/Tools/AssetBundler/tests/tests_main.cpp b/Code/Tools/AssetBundler/tests/tests_main.cpp index 51dd0ce67c..21a6bf8a6f 100644 --- a/Code/Tools/AssetBundler/tests/tests_main.cpp +++ b/Code/Tools/AssetBundler/tests/tests_main.cpp @@ -291,6 +291,9 @@ namespace AssetBundler int main(int argc, char* argv[]) { + AZ::Debug::Trace::HandleExceptions(true); + AZ::Test::ApplyGlobalParameters(&argc, argv); + INVOKE_AZ_UNIT_TEST_MAIN(); AZ::AllocatorInstance::Create(); diff --git a/Code/Tools/AssetProcessor/native/tests/SourceFileRelocatorTests.cpp b/Code/Tools/AssetProcessor/native/tests/SourceFileRelocatorTests.cpp index 4b70f94120..d7801abb3d 100644 --- a/Code/Tools/AssetProcessor/native/tests/SourceFileRelocatorTests.cpp +++ b/Code/Tools/AssetProcessor/native/tests/SourceFileRelocatorTests.cpp @@ -262,7 +262,7 @@ namespace UnitTests auto result = m_data->m_reporter->ComputeDestination(entryContainer, m_data->m_platformConfig.GetScanFolderByPath(scanFolderEntry.m_scanFolder.c_str()), source, destination, destInfo); - ASSERT_EQ(result.IsSuccess(), expectSuccess) << result.GetError().c_str(); + ASSERT_EQ(result.IsSuccess(), expectSuccess) << (!result.IsSuccess() ? result.GetError().c_str() : ""); if (expectSuccess) { diff --git a/Code/Tools/AssetProcessor/native/tests/test_main.cpp b/Code/Tools/AssetProcessor/native/tests/test_main.cpp index 57f14832aa..e0cdc8cb3c 100644 --- a/Code/Tools/AssetProcessor/native/tests/test_main.cpp +++ b/Code/Tools/AssetProcessor/native/tests/test_main.cpp @@ -29,6 +29,9 @@ int main(int argc, char* argv[]) { qputenv("QT_MAC_DISABLE_FOREGROUND_APPLICATION_TRANSFORM", "1"); + AZ::Debug::Trace::HandleExceptions(true); + AZ::Test::ApplyGlobalParameters(&argc, argv); + // If "--unittest" is present on the command line, run unit testing // and return immediately. Otherwise, continue as normal. AZ::Test::addTestEnvironment(new BaseAssetProcessorTestEnvironment()); diff --git a/Code/Tools/ProjectManager/tests/main.cpp b/Code/Tools/ProjectManager/tests/main.cpp index 3dd2fb75df..c666b7ffd0 100644 --- a/Code/Tools/ProjectManager/tests/main.cpp +++ b/Code/Tools/ProjectManager/tests/main.cpp @@ -18,6 +18,9 @@ int runDefaultRunner(int argc, char* argv[]) int main(int argc, char* argv[]) { + AZ::Debug::Trace::HandleExceptions(true); + AZ::Test::ApplyGlobalParameters(&argc, argv); + if (argc == 1) { // if no parameters are provided, add the --unittests parameter diff --git a/Code/Tools/PythonBindingsExample/tests/TestMain.cpp b/Code/Tools/PythonBindingsExample/tests/TestMain.cpp index 7399055ca5..4c0dddecad 100644 --- a/Code/Tools/PythonBindingsExample/tests/TestMain.cpp +++ b/Code/Tools/PythonBindingsExample/tests/TestMain.cpp @@ -23,6 +23,9 @@ int runDefaultRunner(int argc, char* argv[]) int main(int argc, char* argv[]) { + AZ::Debug::Trace::HandleExceptions(true); + AZ::Test::ApplyGlobalParameters(&argc, argv); + // ran with no parameters? if (argc == 1) {