From 4d3a017d9b66d2379fce95afad03f2ada1151dcd Mon Sep 17 00:00:00 2001 From: Mateusz Jablonski Date: Mon, 24 Oct 2022 14:53:33 +0000 Subject: [PATCH] Printf handler: enure that long format uses always 64 bit integers Related-To: NEO-7384 Signed-off-by: Mateusz Jablonski --- shared/source/program/print_formatter.cpp | 11 +++ shared/source/program/print_formatter.h | 19 +++-- .../unit_test/program/printf_helper_tests.cpp | 83 +++++++++++++++++-- 3 files changed, 101 insertions(+), 12 deletions(-) diff --git a/shared/source/program/print_formatter.cpp b/shared/source/program/print_formatter.cpp index 339bb2fd5e..de25e29dde 100644 --- a/shared/source/program/print_formatter.cpp +++ b/shared/source/program/print_formatter.cpp @@ -113,6 +113,17 @@ void PrintFormatter::stripVectorTypeConversion(char *format) { } } +template <> +void PrintFormatter::adjustFormatString(std::string &formatString) { + auto longPosition = formatString.find('l'); + + UNRECOVERABLE_IF(longPosition == std::string::npos || longPosition == formatString.size() - 1); + + if (formatString.at(longPosition + 1) != 'l') { + formatString.insert(longPosition, "l"); + } +} + size_t PrintFormatter::printToken(char *output, size_t size, const char *formatString) { PRINTF_DATA_TYPE type(PRINTF_DATA_TYPE::INVALID); read(&type); diff --git a/shared/source/program/print_formatter.h b/shared/source/program/print_formatter.h index 64947fa972..3288f28a4a 100644 --- a/shared/source/program/print_formatter.h +++ b/shared/source/program/print_formatter.h @@ -81,29 +81,36 @@ class PrintFormatter { } template - size_t typedPrintToken(char *output, size_t size, const char *formatString) { + void adjustFormatString(std::string &formatString) {} + + template + size_t typedPrintToken(char *output, size_t size, const char *inputFormatString) { T value{0}; read(&value); constexpr auto offsetToBeDwordAligned = static_cast(std::max(int64_t(sizeof(int) - sizeof(T)), int64_t(0))); currentOffset += offsetToBeDwordAligned; - return simpleSprintf(output, size, formatString, value); + std::string formatString(inputFormatString); + adjustFormatString(formatString); + return simpleSprintf(output, size, formatString.c_str(), value); } template - size_t typedPrintVectorToken(char *output, size_t size, const char *formatString) { + size_t typedPrintVectorToken(char *output, size_t size, const char *inputFormatString) { T value = {0}; int valueCount = 0; read(&valueCount); size_t charactersPrinted = 0; - char strippedFormat[1024]; + char strippedFormat[1024]{}; - stripVectorFormat(formatString, strippedFormat); + stripVectorFormat(inputFormatString, strippedFormat); stripVectorTypeConversion(strippedFormat); + std::string formatString(strippedFormat); + adjustFormatString(formatString); for (int i = 0; i < valueCount; i++) { read(&value); - charactersPrinted += simpleSprintf(output + charactersPrinted, size - charactersPrinted, strippedFormat, value); + charactersPrinted += simpleSprintf(output + charactersPrinted, size - charactersPrinted, formatString.c_str(), value); if (i < valueCount - 1) { charactersPrinted += simpleSprintf(output + charactersPrinted, size - charactersPrinted, "%c", ','); } diff --git a/shared/test/unit_test/program/printf_helper_tests.cpp b/shared/test/unit_test/program/printf_helper_tests.cpp index 1ff244c99e..993f630190 100644 --- a/shared/test/unit_test/program/printf_helper_tests.cpp +++ b/shared/test/unit_test/program/printf_helper_tests.cpp @@ -135,14 +135,21 @@ struct SingleValueTestParam { T value; }; +template +struct SingleValueTestParamWithHostFormat { + std::string format; + std::string hostFormat; + T value; +}; + typedef SingleValueTestParam Int8Params; typedef SingleValueTestParam Uint8Params; typedef SingleValueTestParam Int16Params; typedef SingleValueTestParam Uint16Params; typedef SingleValueTestParam Int32Params; typedef SingleValueTestParam Uint32Params; -typedef SingleValueTestParam Int64Params; -typedef SingleValueTestParam Uint64Params; +typedef SingleValueTestParamWithHostFormat Int64Params; +typedef SingleValueTestParamWithHostFormat Uint64Params; typedef SingleValueTestParam FloatParams; typedef SingleValueTestParam DoubleParams; typedef SingleValueTestParam StringParams; @@ -271,6 +278,70 @@ INSTANTIATE_TEST_CASE_P(PrintfUint32Test, PrintfUint32Test, ::testing::ValuesIn(uintValues)); +Int64Params longValues[] = { + {"%lld", "%lld", INT64_MAX}, + {"%ld", "%lld", INT64_MAX}, + {"%ld", "%lld", INT64_MIN}, + {"%lld", "%lld", INT64_MIN}, + {"%llx", "%llx", INT64_MAX}, + {"%lx", "%llx", INT64_MAX}, + {"%lx", "%llx", INT64_MIN}, + {"%llx", "%llx", INT64_MIN}}; + +class PrintfInt64Test : public PrintFormatterTest, + public ::testing::WithParamInterface {}; + +TEST_P(PrintfInt64Test, GivenFormatContainingIntWhenPrintingThenValueIsInserted) { + auto input = GetParam(); + + auto stringIndex = injectFormatString(input.format); + storeData(stringIndex); + injectValue(input.value); + + char referenceOutput[maxPrintfOutputLength]; + char actualOutput[maxPrintfOutputLength]; + + printFormatter->printKernelOutput([&actualOutput](char *str) { strncpy_s(actualOutput, maxPrintfOutputLength, str, maxPrintfOutputLength - 1); }); + + snprintf(referenceOutput, sizeof(referenceOutput), input.hostFormat.c_str(), input.value); + + EXPECT_STREQ(referenceOutput, actualOutput); +} + +INSTANTIATE_TEST_CASE_P(PrintfInt64Test, + PrintfInt64Test, + ::testing::ValuesIn(longValues)); + +Uint64Params ulongValues[] = { + {"%llu", "%llu", UINT64_MAX}, + {"%lu", "%llu", UINT64_MAX}, + {"%llux", "%llux", UINT64_MAX}, + {"%lux", "%llux", UINT64_MAX}}; + +class PrintfUint64Test : public PrintFormatterTest, + public ::testing::WithParamInterface {}; + +TEST_P(PrintfUint64Test, GivenFormatContainingIntWhenPrintingThenValueIsInserted) { + auto input = GetParam(); + + auto stringIndex = injectFormatString(input.format); + storeData(stringIndex); + injectValue(input.value); + + char referenceOutput[maxPrintfOutputLength]; + char actualOutput[maxPrintfOutputLength]; + + printFormatter->printKernelOutput([&actualOutput](char *str) { strncpy_s(actualOutput, maxPrintfOutputLength, str, maxPrintfOutputLength - 1); }); + + snprintf(referenceOutput, sizeof(referenceOutput), input.hostFormat.c_str(), input.value); + + EXPECT_STREQ(referenceOutput, actualOutput); +} + +INSTANTIATE_TEST_CASE_P(PrintfUint64Test, + PrintfUint64Test, + ::testing::ValuesIn(ulongValues)); + FloatParams floatValues[] = { {"%f", 10.3456f}, {"%.1f", 10.3456f}, @@ -719,21 +790,21 @@ TEST_F(PrintFormatterTest, GivenSpecialVectorWhenPrintingThenAllValuesAreInserte TEST_F(PrintFormatterTest, GivenVectorOfLongsWhenPrintingThenAllValuesAreInserted) { int channelCount = 2; - auto stringIndex = injectFormatString("%v2lld"); + auto stringIndex = injectFormatString("%v2ld"); storeData(stringIndex); storeData(PRINTF_DATA_TYPE::VECTOR_LONG); // channel count storeData(channelCount); - storeData(1); - storeData(2); + storeData(100000000000000); + storeData(200000000000000); char actualOutput[maxPrintfOutputLength]; printFormatter->printKernelOutput([&actualOutput](char *str) { strncpy_s(actualOutput, maxPrintfOutputLength, str, maxPrintfOutputLength - 1); }); - EXPECT_STREQ("1,2", actualOutput); + EXPECT_STREQ("100000000000000,200000000000000", actualOutput); } TEST_F(PrintFormatterTest, GivenVectorOfFloatsWhenPrintingThenAllValuesAreInserted) {