From 5a384b9ea8c289e8ce52384261c764534ea77efa Mon Sep 17 00:00:00 2001 From: Nicolas Pitre Date: Tue, 31 Aug 2021 16:05:18 -0400 Subject: [PATCH] lib/os/cbprintf_nano.c: avoid sign extension on unsigned formats There might be a sign extension when a long is promoted to int_value_type and the former type is smaller than the later. This produces the wrong output if the specified format is unsigned. Let's avoid this problem by handling signed and unsigned cases explicitly. When the type already matches int_value_type then the compiler is smart enough to recognize the redundancy and removes unneeded duplications automatically, meaning that the code will stay small when code size matters. A similar issue also existed in the restricted %llu case. The fix is the same as above. Those fixes exposed wrong results in the printk.c test with %llx so fix that as well. Signed-off-by: Nicolas Pitre --- lib/os/cbprintf_nano.c | 57 ++++++++++++++++++++++++-------- tests/kernel/common/src/printk.c | 2 +- tests/unit/cbprintf/main.c | 15 +++++---- 3 files changed, 52 insertions(+), 22 deletions(-) diff --git a/lib/os/cbprintf_nano.c b/lib/os/cbprintf_nano.c index bfe44925c91..1ccd8047d68 100644 --- a/lib/os/cbprintf_nano.c +++ b/lib/os/cbprintf_nano.c @@ -174,20 +174,40 @@ start: uint_value_type d; if (length_mod == 'z') { - d = va_arg(ap, ssize_t); - } else if (length_mod == 'l') { - d = va_arg(ap, long); - } else if (length_mod == 'L') { - long long lld = va_arg(ap, long long); - - if (sizeof(int_value_type) < 8U && - lld != (int_value_type) lld) { - data = "ERR"; - data_len = 3; - precision = 0; - break; + if (*fmt == 'u') { + d = va_arg(ap, size_t); + } else { + d = va_arg(ap, ssize_t); + } + } else if (length_mod == 'l') { + if (*fmt == 'u') { + d = va_arg(ap, unsigned long); + } else { + d = va_arg(ap, long); + } + } else if (length_mod == 'L') { + if (*fmt == 'u') { + unsigned long long llu = + va_arg(ap, unsigned long long); + + if (llu != (uint_value_type) llu) { + data = "ERR"; + data_len = 3; + precision = 0; + break; + } + d = (uint_value_type) llu; + } else { + long long lld = va_arg(ap, long long); + + if (lld != (int_value_type) lld) { + data = "ERR"; + data_len = 3; + precision = 0; + break; + } + d = (int_value_type) lld; } - d = (uint_value_type) lld; } else if (*fmt == 'u') { d = va_arg(ap, unsigned int); } else { @@ -229,7 +249,16 @@ start: } else if (length_mod == 'l') { x = va_arg(ap, unsigned long); } else if (length_mod == 'L') { - x = va_arg(ap, unsigned long long); + unsigned long long llx = + va_arg(ap, unsigned long long); + + if (llx != (uint_value_type) llx) { + data = "ERR"; + data_len = 3; + precision = 0; + break; + } + x = (uint_value_type) llx; } else { x = va_arg(ap, unsigned int); } diff --git a/tests/kernel/common/src/printk.c b/tests/kernel/common/src/printk.c index a29ca9e0468..4e60686078c 100644 --- a/tests/kernel/common/src/printk.c +++ b/tests/kernel/common/src/printk.c @@ -51,7 +51,7 @@ char *expected = "22 113 10000 32768 40000 22\n" "42 42 42 42\n" "42 42 0042 00000042\n" "255 42 abcdef 42\n" - "ERR -1 4294967295 ffffffff\n" + "ERR -1 ERR ERR\n" "0xcafebabe 0xbeef 0x2a\n" ; #endif diff --git a/tests/unit/cbprintf/main.c b/tests/unit/cbprintf/main.c index 19e23f8af9f..b4c40d7f820 100644 --- a/tests/unit/cbprintf/main.c +++ b/tests/unit/cbprintf/main.c @@ -510,6 +510,7 @@ static void test_d_length(void) int max = 1876543210; long long svll = 123LL << 48; long long svll2 = -2LL; + unsigned long long uvll = 4000000000LLU; int rc; TEST_PRF(&rc, "%d/%d", min, max); @@ -526,22 +527,22 @@ static void test_d_length(void) PRF_CHECK("46/-22", rc); } - TEST_PRF(&rc, "%ld/%ld", (long)min, (long)max); + TEST_PRF(&rc, "%ld/%ld/%lu/", (long)min, (long)max, 4000000000UL); if (IS_ENABLED(CONFIG_CBPRINTF_FULL_INTEGRAL) || (sizeof(long) <= 4) || IS_ENABLED(CONFIG_CBPRINTF_NANO)) { - PRF_CHECK("-1234567890/1876543210", rc); + PRF_CHECK("-1234567890/1876543210/4000000000/", rc); } else { - PRF_CHECK("%ld/%ld", rc); + PRF_CHECK("%ld/%ld/%lu/", rc); } - TEST_PRF(&rc, "/%lld/%lld/%lld/", svll, -svll, svll2); + TEST_PRF(&rc, "/%lld/%lld/%lld/%llu/", svll, -svll, svll2, uvll); if (IS_ENABLED(CONFIG_CBPRINTF_FULL_INTEGRAL)) { - PRF_CHECK("/34621422135410688/-34621422135410688/-2/", rc); + PRF_CHECK("/34621422135410688/-34621422135410688/-2/4000000000/", rc); } else if (IS_ENABLED(CONFIG_CBPRINTF_COMPLETE)) { - PRF_CHECK("/%lld/%lld/%lld/", rc); + PRF_CHECK("/%lld/%lld/%lld/%llu/", rc); } else if (IS_ENABLED(CONFIG_CBPRINTF_NANO)) { - PRF_CHECK("/ERR/ERR/-2/", rc); + PRF_CHECK("/ERR/ERR/-2/4000000000/", rc); } else { zassert_true(false, "Missed case!"); }