From bd4829436b9ee3f6b1691dbfc6e37f23ae6968b9 Mon Sep 17 00:00:00 2001 From: Bruce Forstall Date: Thu, 17 Aug 2023 18:01:07 -0700 Subject: [PATCH 1/2] Fix SuperPMI assertion call in `MethodContext::recGetHelperFtn()` We can't use string concatenation in an argument to the `AssertCodeMsg` macro, so construct the string we want to print first. --- src/coreclr/tools/superpmi/superpmi-shared/logging.h | 2 +- .../tools/superpmi/superpmi-shared/methodcontext.cpp | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/logging.h b/src/coreclr/tools/superpmi/superpmi-shared/logging.h index 957da040885eaf..15d7d097fcb5b7 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/logging.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/logging.h @@ -30,7 +30,7 @@ do \ { \ Logger::LogExceptionMessage(__FUNCTION__, __FILE__, __LINE__, exCode, msg, __VA_ARGS__); \ - ThrowSpmiException(exCode, msg, __VA_ARGS__); \ + ThrowSpmiException(exCode, msg, __VA_ARGS__); \ } while (0) // These are specified as flags so subsets of the logging functionality can be enabled/disabled at once diff --git a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp index 5ee9b2bb41dcb2..94f34378c1224a 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp @@ -2364,9 +2364,11 @@ void MethodContext::recGetHelperFtn(CorInfoHelpFunc ftnNum, void** ppIndirection { DLDL oldValue = GetHelperFtn->Get(key); - AssertCodeMsg(oldValue.A == value.A && oldValue.B == oldValue.B, EXCEPTIONCODE_MC, - "collision! old: %016" PRIX64 " %016" PRIX64 ", new: %016" PRIX64 " %016" PRIX64 " \n", oldValue.A, oldValue.B, value.A, - value.B); + // We can't use string concatenation in an argument to the `AssertCodeMsg` macro, so construct the string here. + char assertMsgBuff[200]; + sprintf_s(assertMsgBuff, sizeof(assertMsgBuff), "old: %016" PRIX64 " %016" PRIX64 ", new: %016" PRIX64 " %016" PRIX64, + oldValue.A, oldValue.B, value.A, value.B); + AssertCodeMsg(oldValue.A == value.A && oldValue.B == oldValue.B, EXCEPTIONCODE_MC, "collision! %s", assertMsgBuff); } GetHelperFtn->Add(key, value); From edc03973d62cdfc9138edcf7593a53559ae590f0 Mon Sep 17 00:00:00 2001 From: Bruce Forstall Date: Fri, 18 Aug 2023 08:46:29 -0700 Subject: [PATCH 2/2] Feedback --- .../tools/superpmi/superpmi-shared/errorhandling.h | 2 +- .../tools/superpmi/superpmi-shared/methodcontext.cpp | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/coreclr/tools/superpmi/superpmi-shared/errorhandling.h b/src/coreclr/tools/superpmi/superpmi-shared/errorhandling.h index 3c418b31d2536a..484106fd3878d6 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/errorhandling.h +++ b/src/coreclr/tools/superpmi/superpmi-shared/errorhandling.h @@ -34,7 +34,7 @@ void MSC_ONLY(__declspec(noreturn)) ThrowRecordedException(DWORD innerExceptionC do \ { \ if (!(expr)) \ - LogException(exCode, "SuperPMI assertion '%s' failed (" #msg ")", #expr, ##__VA_ARGS__); \ + LogException(exCode, "SuperPMI assertion '%s' failed (" msg ")", #expr, ##__VA_ARGS__); \ } while (0) #define AssertCode(expr, exCode) \ diff --git a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp index 94f34378c1224a..f103a3f307ba5c 100644 --- a/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp +++ b/src/coreclr/tools/superpmi/superpmi-shared/methodcontext.cpp @@ -2364,11 +2364,9 @@ void MethodContext::recGetHelperFtn(CorInfoHelpFunc ftnNum, void** ppIndirection { DLDL oldValue = GetHelperFtn->Get(key); - // We can't use string concatenation in an argument to the `AssertCodeMsg` macro, so construct the string here. - char assertMsgBuff[200]; - sprintf_s(assertMsgBuff, sizeof(assertMsgBuff), "old: %016" PRIX64 " %016" PRIX64 ", new: %016" PRIX64 " %016" PRIX64, - oldValue.A, oldValue.B, value.A, value.B); - AssertCodeMsg(oldValue.A == value.A && oldValue.B == oldValue.B, EXCEPTIONCODE_MC, "collision! %s", assertMsgBuff); + AssertCodeMsg(oldValue.A == value.A && oldValue.B == oldValue.B, EXCEPTIONCODE_MC, + "collision! old: %016" PRIX64 " %016" PRIX64 ", new: %016" PRIX64 " %016" PRIX64, oldValue.A, oldValue.B, value.A, + value.B); } GetHelperFtn->Add(key, value);