diff options
-rw-r--r-- | baseboard/nucleo-f412zg/base-board.h | 2 | ||||
-rw-r--r-- | baseboard/nucleo-h743zi/base-board.h | 2 | ||||
-rw-r--r-- | board/hatch_fp/board.h | 2 | ||||
-rw-r--r-- | board/nocturne_fp/board.h | 2 | ||||
-rw-r--r-- | builtin/stdint.h | 6 | ||||
-rw-r--r-- | common/printf.c | 26 | ||||
-rw-r--r-- | include/config.h | 8 | ||||
-rw-r--r-- | include/printf.h | 2 | ||||
-rw-r--r-- | test/printf.c | 120 | ||||
-rw-r--r-- | util/config_allowed.txt | 2 |
10 files changed, 140 insertions, 32 deletions
diff --git a/baseboard/nucleo-f412zg/base-board.h b/baseboard/nucleo-f412zg/base-board.h index b104621ec7..bd5710e247 100644 --- a/baseboard/nucleo-f412zg/base-board.h +++ b/baseboard/nucleo-f412zg/base-board.h @@ -164,7 +164,7 @@ #define CONFIG_HOST_COMMAND_STATUS #define CONFIG_MKBP_EVENT #define CONFIG_MKBP_USE_GPIO -#define CONFIG_PRINTF_LEGACY_LI_FORMAT +#define CONFIG_PRINTF_LONG_IS_32BITS #define CONFIG_RNG #define CONFIG_SHA256 #define CONFIG_SHA256_UNROLLED diff --git a/baseboard/nucleo-h743zi/base-board.h b/baseboard/nucleo-h743zi/base-board.h index 10f780505d..f96ba41bcc 100644 --- a/baseboard/nucleo-h743zi/base-board.h +++ b/baseboard/nucleo-h743zi/base-board.h @@ -92,7 +92,7 @@ #define CONFIG_LOW_POWER_IDLE #define CONFIG_MKBP_EVENT #define CONFIG_MKBP_USE_GPIO -#define CONFIG_PRINTF_LEGACY_LI_FORMAT +#define CONFIG_PRINTF_LONG_IS_32BITS #define CONFIG_RNG #define CONFIG_RWSIG_TYPE_RWSIG #define CONFIG_SHA256 diff --git a/board/hatch_fp/board.h b/board/hatch_fp/board.h index 2702152576..a4c4e2923b 100644 --- a/board/hatch_fp/board.h +++ b/board/hatch_fp/board.h @@ -230,7 +230,7 @@ #define CONFIG_HOST_COMMAND_STATUS #define CONFIG_MKBP_EVENT #define CONFIG_MKBP_USE_GPIO -#define CONFIG_PRINTF_LEGACY_LI_FORMAT +#define CONFIG_PRINTF_LONG_IS_32BITS #define CONFIG_RNG #define CONFIG_SHA256 #define CONFIG_SHA256_UNROLLED diff --git a/board/nocturne_fp/board.h b/board/nocturne_fp/board.h index 5b12d05321..2d0f5586cf 100644 --- a/board/nocturne_fp/board.h +++ b/board/nocturne_fp/board.h @@ -106,7 +106,7 @@ #undef CONFIG_LID_SWITCH #define CONFIG_MKBP_EVENT #define CONFIG_MKBP_USE_GPIO -#define CONFIG_PRINTF_LEGACY_LI_FORMAT +#define CONFIG_PRINTF_LONG_IS_32BITS #define CONFIG_SHA256 #define CONFIG_SHA256_UNROLLED #define CONFIG_SPI diff --git a/builtin/stdint.h b/builtin/stdint.h index 87993d8e2d..f47c9e6422 100644 --- a/builtin/stdint.h +++ b/builtin/stdint.h @@ -67,6 +67,9 @@ typedef int64_t int_fast64_t; #ifndef INT32_MAX #define INT32_MAX (2147483647U) #endif +#ifndef INT32_MIN +#define INT32_MIN (-2147483648) +#endif #ifndef UINT64_C #define UINT64_C(c) c##ULL @@ -81,5 +84,8 @@ typedef int64_t int_fast64_t; #ifndef INT64_MAX #define INT64_MAX INT64_C(9223372036854775807) #endif +#ifndef INT64_MIN +#define INT64_MIN (INT64_C(-9223372036854775807) - 1) +#endif #endif /* __CROS_EC_STDINT_H__ */ diff --git a/common/printf.c b/common/printf.c index 9fbac19e20..5102dd8f20 100644 --- a/common/printf.c +++ b/common/printf.c @@ -224,7 +224,9 @@ int vfnprintf(int (*addchar)(void *context, int c), void *context, /* * Handle length: - * %l - DEPRECATED (see below) + * %l - supports 64-bit longs, 32-bit longs are + * supported with a config flag, see comment + * below for more details * %ll - long long * %z - size_t */ @@ -239,18 +241,23 @@ int vfnprintf(int (*addchar)(void *context, int c), void *context, } /* - * %l on 32-bit systems is deliberately - * deprecated. It was originally used as - * shorthand for 64-bit values. When + * The CONFIG_PRINTF_LONG_IS_32BITS flag is + * required to enable the %l flag on systems + * where it would signify a 32-bit value. + * Otherwise, %l on 32-bit systems is + * deliberately deprecated. %l was originally + * used as shorthand for 64-bit values. When * compile-time printf format checking was * enabled, it had to be cleaned up to be * sizeof(long), which is 32 bits on today's * ECs. This presents a mismatch which can be * dangerous if a new-style printf call is * cherry-picked into an old firmware branch. - * See crbug.com/984041 for more context. + * For more context, see + * https://issuetracker.google.com/issues/172210614 */ - if (!(flags & PF_64BIT)) { + if (!IS_ENABLED(CONFIG_PRINTF_LONG_IS_32BITS) + && !(flags & PF_64BIT)) { format = error_str; continue; } @@ -336,12 +343,9 @@ int vfnprintf(int (*addchar)(void *context, int c), void *context, } switch (c) { -#ifdef CONFIG_PRINTF_LEGACY_LI_FORMAT +#ifdef CONFIG_PRINTF_LONG_IS_32BITS case 'i': - /* force 32-bit for compatibility */ - flags &= ~PF_64BIT; - /* fall-through */ -#endif /* CONFIG_PRINTF_LEGACY_LI_FORMAT */ +#endif /* CONFIG_PRINTF_LONG_IS_32BITS */ case 'd': if (flags & PF_64BIT) { if ((int64_t)v < 0) { diff --git a/include/config.h b/include/config.h index bcf070f82f..70dc284a8f 100644 --- a/include/config.h +++ b/include/config.h @@ -3639,10 +3639,12 @@ #undef CONFIG_POWER_TRACK_HOST_SLEEP_STATE /* - * Implement the '%li' printf format as a *32-bit* integer format, - * as it might be expected by non-EC code. + * Allow the use of the "long" printf length modifier ('l') to be in 32-bit + * systems along with any supported conversion specifiers. Note that this also + * reenables support for the 'i' printf format. This config will only take + * effect if sizeof(long) == sizeof(uint32_t). */ -#undef CONFIG_PRINTF_LEGACY_LI_FORMAT +#undef CONFIG_PRINTF_LONG_IS_32BITS /* * On x86 systems, define this option if the CPU_PROCHOT signal is active low. diff --git a/include/printf.h b/include/printf.h index 8eed1618f8..333e622b7b 100644 --- a/include/printf.h +++ b/include/printf.h @@ -51,7 +51,7 @@ * - 'c' - character * - 's' - null-terminated ASCII string * - 'd' - signed integer - * - 'i' - signed integer if CONFIG_PRINTF_LEGACY_LI_FORMAT is set (ignore l) + * - 'i' - signed integer (if CONFIG_PRINTF_LONG_IS_32BITS is enabled) * - 'u' - unsigned integer * - 'x' - unsigned integer, print as lower-case hexadecimal * - 'X' - unsigned integer, print as upper-case hexadecimal diff --git a/test/printf.c b/test/printf.c index 83d03c210f..388e0c7e80 100644 --- a/test/printf.c +++ b/test/printf.c @@ -171,23 +171,118 @@ test_static int test_vsnprintf_int(void) T(expect_success("5e", "%x", 0X5E)); T(expect_success("5E", "%X", 0X5E)); + return EC_SUCCESS; +} + +test_static int test_printf_long32_enabled(void) +{ + bool use_l32 = IS_ENABLED(CONFIG_PRINTF_LONG_IS_32BITS); + + if (IS_ENABLED(BOARD_BLOONCHIPPER) || IS_ENABLED(BOARD_DARTMONKEY)) + TEST_ASSERT(use_l32); + else + TEST_ASSERT(!use_l32); + return EC_SUCCESS; +} + +test_static int test_vsnprintf_32bit_long_supported(void) +{ + long long_min = INT32_MIN; + long long_max = INT32_MAX; + unsigned long ulong_max = UINT32_MAX; + char const *long_min_str = "-2147483648"; + char const *long_max_str = "2147483647"; + char const *ulong_max_str = "4294967295"; + char const *long_min_hexstr = "80000000"; + char const *long_max_hexstr = "7fffffff"; + char const *ulong_max_hexstr = "ffffffff"; + + T(expect_success(long_min_str, "%ld", long_min)); + T(expect_success(long_min_hexstr, "%lx", long_min)); + T(expect_success(long_max_str, "%ld", long_max)); + T(expect_success(long_max_hexstr, "%lx", long_max)); + T(expect_success(ulong_max_str, "%lu", ulong_max)); + T(expect_success(ulong_max_hexstr, "%lx", ulong_max)); + T(expect_success(long_max_str, "%ld", long_max)); + + T(expect_success(" +123", "%+*ld", 5, 123)); + T(expect_success("00000123", "%08lu", 123)); + T(expect_success("131415", "%d%lu%d", 13, 14L, 15)); + /* - * %l is deprecated on 32-bit systems (see crbug.com/984041), but is - * is still functional on 64-bit systems. + * %i and %li are only supported via the CONFIG_PRINTF_LONG_IS_32BITS + * configuration (see https://issuetracker.google.com/issues/172210614). */ - if (sizeof(long) == sizeof(uint32_t)) { - T(expect_success(err_str, "%lx", 0x7b)); - T(expect_success(err_str, "%08lu", 0x7b)); - T(expect_success("13ERROR", "%d%lu", 13, 14)); - } else { - T(expect_success("7b", "%lx", 0x7b)); - T(expect_success("00000123", "%08lu", 123)); - T(expect_success("131415", "%d%lu%d", 13, 14L, 15)); - } + T(expect_success("123", "%i", 123)); + T(expect_success("123", "%li", 123)); + + return EC_SUCCESS; +} + +test_static int test_vsnprintf_64bit_long_supported(void) +{ + /* These lines are only executed when sizeof(long) is 64-bits but are + * still compiled by systems with 32-bit longs, so the casts are needed + * to avoid compilation errors. + */ + long long_min = (long)INT64_MIN; + long long_max = (long)INT64_MAX; + unsigned long ulong_max = (unsigned long)UINT64_MAX; + char const *long_min_str = "-9223372036854775808"; + char const *long_max_str = "9223372036854775807"; + char const *ulong_max_str = "18446744073709551615"; + char const *long_min_hexstr = "8000000000000000"; + char const *long_max_hexstr = "7fffffffffffffff"; + char const *ulong_max_hexstr = "ffffffffffffffff"; + + T(expect_success(long_min_str, "%ld", long_min)); + T(expect_success(long_min_hexstr, "%lx", long_min)); + T(expect_success(long_max_str, "%ld", long_max)); + T(expect_success(long_max_hexstr, "%lx", long_max)); + T(expect_success(ulong_max_str, "%lu", ulong_max)); + T(expect_success(ulong_max_hexstr, "%lx", ulong_max)); + T(expect_success(long_max_str, "%ld", long_max)); + + T(expect_success(" +123", "%+*ld", 5, 123)); + T(expect_success("00000123", "%08lu", 123)); + T(expect_success("131415", "%d%lu%d", 13, 14L, 15)); + + T(expect_success(err_str, "%i", 123)); + T(expect_success(err_str, "%li", 123)); return EC_SUCCESS; } +test_static int test_vsnprintf_long_not_supported(void) +{ + T(expect_success(err_str, "%ld", 0x7b)); + T(expect_success(err_str, "%li", 0x7b)); + T(expect_success(err_str, "%lu", 0x7b)); + T(expect_success(err_str, "%lx", 0x7b)); + T(expect_success(err_str, "%08lu", 123)); + T(expect_success("13ERROR", "%d%lu%d", 13, 14L, 15)); + + T(expect_success(err_str, "%i", 123)); + T(expect_success(err_str, "%li", 123)); + + return EC_SUCCESS; +} + +test_static int test_vsnprintf_long(void) +{ + /* + * %l is functional on 64-bit systems but is not supported on 32-bit + * systems (see https://issuetracker.google.com/issues/172210614) unless + * explicitly enabled via configuration. + */ + if (IS_ENABLED(CONFIG_PRINTF_LONG_IS_32BITS)) + return test_vsnprintf_32bit_long_supported(); + else if (sizeof(long) == sizeof(uint64_t)) + return test_vsnprintf_64bit_long_supported(); + else + return test_vsnprintf_long_not_supported(); +} + test_static int test_vsnprintf_pointers(void) { void *ptr = (void *)0x55005E00; @@ -288,12 +383,13 @@ void run_test(int argc, char **argv) RUN_TEST(test_vsnprintf_args); RUN_TEST(test_vsnprintf_int); + RUN_TEST(test_printf_long32_enabled); + RUN_TEST(test_vsnprintf_long); RUN_TEST(test_vsnprintf_pointers); RUN_TEST(test_vsnprintf_chars); RUN_TEST(test_vsnprintf_strings); RUN_TEST(test_vsnprintf_timestamps); RUN_TEST(test_vsnprintf_hexdump); RUN_TEST(test_vsnprintf_combined); - test_print_result(); } diff --git a/util/config_allowed.txt b/util/config_allowed.txt index 58317b8384..0e352e7602 100644 --- a/util/config_allowed.txt +++ b/util/config_allowed.txt @@ -716,7 +716,7 @@ CONFIG_POWER_SIGNAL_INTERRUPT_STORM_DETECT_THRESHOLD CONFIG_POWER_SIGNAL_RUNTIME_CONFIG CONFIG_POWER_TRACK_HOST_SLEEP_STATE CONFIG_PRESERVE_LOGS -CONFIG_PRINTF_LEGACY_LI_FORMAT +CONFIG_PRINTF_LONG_IS_32BITS CONFIG_PRINT_IN_INT CONFIG_PROGRAM_MEMORY_BASE CONFIG_PROGRAM_MEMORY_BASE_LOAD |