lib/os: Rework/shrink printk conversions, add 64 bit support

Add support for 64 bit conversions in a uniformly expressable way by
printing values backwards into a buffer on the stack first.  This
allows all operations to work on the low bits of the value and so the
code doesn't need to care (beyond the size of that buffer) about the
word size.  This trick also doesn't care about the specifics of the
base value, so in the process this unifies the decimal and hex printk
conversion code to a single function.

This comes at a mild cost in CPU cycles to the decimal converter and
somewhat higher cost to hex (because it's now doing a full div/mod
operation instead of shifting and masking).  And stack usage has grown
by a few words to hold the temporary.  But the benefits in code size
are substantial (e.g. ~250 bytes of .text on arm32).

Note that this also contains a change to tests/kernel/common to
address what appears to have been a bug in the original converters.
The printk test uses a format string that looks like "%-4x%-2p" and
feeds it the literal arguments "0xABCDEF" and "(char *)42".
Now... clearly both those results are going to overflow the 4 and
2-byte field sizes, so there shouldn't be any whitespace between these
fields.  But the test was written to expect two spaces, inexplicably
(yes, I checked: POSIX-compatible printf implementations don't have
those spaces either).

The new code is definitely doing the right thing, so fix the test
instead.

Signed-off-by: Andy Ross <[email protected]>

Author: Andy Ross

lib/os/Kconfig

@@ -56,4 +56,21 @@ config SYS_HEAP_ALIGNED_ALLOC
 	  increase heap memory overhead on 32 bit platforms when using
 	  small (<256kb) heaps.
 
+config PRINTK64
+	bool
+	prompt "Enable 64 bit printk conversions" if !64BIT
+	default y
+	help
+	  When true, 64 bit values will be printable on 32 bit systems
+	  using the "ll" flag to the %d, %i, %u, %x and %X specifiers.
+	  When false, these values will be correctly parsed from the
+	  function arguments, but will print "ERR" if their value is
+	  unrepresentable in 32 bits.  This setting is always true on
+	  64 bit systems.  Note that setting this =n does not produce
+	  significant code savings within the printk library code
+	  itself.  Instead, it suppresses the use of the
+	  toolchain-provided 64 bit division implementation, which may
+	  reduce code size if it is unused elsewhere in the
+	  application.  Most apps should leave this set to y.
+
 endmenu

lib/os/printk.c

@@ -30,12 +30,17 @@ enum pad_type {
 	PAD_SPACE_AFTER,
 };
 
-static void _printk_dec_ulong(out_func_t out, void *ctx,
-			      const unsigned long num, enum pad_type padding,
-			      int min_width);
-static void _printk_hex_ulong(out_func_t out, void *ctx,
-			      const unsigned long long num, enum pad_type padding,
-			      int min_width);
+#ifdef CONFIG_PRINTK64
+typedef uint64_t printk_val_t;
+#else
+typedef uint32_t printk_val_t;
+#endif
+
+/* Maximum number of digits in a printed decimal value (hex is always
+ * less, obviously).  Funny formula produces 10 max digits for 32 bit,
+ * 21 for 64.
+ */
+#define DIGITS_BUFLEN (11 * (sizeof(printk_val_t) / 4) - 1)
 
 #ifdef CONFIG_PRINTK
 /**
@@ -87,11 +92,67 @@ void *__printk_get_hook(void)
 }
 #endif /* CONFIG_PRINTK */
 
-static void print_err(out_func_t out, void *ctx)
+static void print_digits(out_func_t out, void *ctx, printk_val_t num, int base,
+			 bool pad_before, char pad_char, int min_width)
+{
+	char buf[DIGITS_BUFLEN];
+	int i;
+
+	/* Print it backwards into the end of the buffer, low digits first */
+	for (i = DIGITS_BUFLEN - 1; num != 0; i--) {
+		buf[i] = "0123456789abcdef"[num % base];
+		num /= base;
+	}
+
+	if (i == DIGITS_BUFLEN - 1) {
+		buf[i] = '0';
+	} else {
+		i++;
+	}
+
+	int pad = MAX(min_width - (DIGITS_BUFLEN - i), 0);
+
+	for (/**/; pad > 0 && pad_before; pad--) {
+		out(pad_char, ctx);
+	}
+	for (/**/; i < DIGITS_BUFLEN; i++) {
+		out(buf[i], ctx);
+	}
+	for (/**/; pad > 0; pad--) {
+		out(pad_char, ctx);
+	}
+}
+
+static void print_hex(out_func_t out, void *ctx, printk_val_t num,
+		      enum pad_type padding, int min_width)
+{
+	print_digits(out, ctx, num, 16, padding != PAD_SPACE_AFTER,
+		     padding == PAD_ZERO_BEFORE ? '0' : ' ', min_width);
+}
+
+static void print_dec(out_func_t out, void *ctx, printk_val_t num,
+		      enum pad_type padding, int min_width)
 {
-	out('E', ctx);
-	out('R', ctx);
-	out('R', ctx);
+	print_digits(out, ctx, num, 10, padding != PAD_SPACE_AFTER,
+		     padding == PAD_ZERO_BEFORE ? '0' : ' ', min_width);
+}
+
+static bool ok64(out_func_t out, void *ctx, long long val)
+{
+	if (sizeof(printk_val_t) < 8 && val != (long) val) {
+		out('E', ctx);
+		out('R', ctx);
+		out('R', ctx);
+		return false;
+	}
+	return true;
+}
+
+static bool negative(printk_val_t val)
+{
+	const printk_val_t hibit = ~(((printk_val_t) ~1) >> 1);
+
+	return (val & hibit) != 0;
 }
 
 /**
@@ -169,71 +230,46 @@ void z_vprintk(out_func_t out, void *ctx, const char *fmt, va_list ap)
 				}
 				goto still_might_format;
 			case 'd':
-			case 'i': {
-				long d;
+			case 'i':
+			case 'u': {
+				printk_val_t 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 (lld > __LONG_MAX__ ||
-					    lld < ~__LONG_MAX__) {
-						print_err(out, ctx);
+					if (!ok64(out, ctx, lld)) {
 						break;
 					}
-					d = lld;
+					d = (printk_val_t) lld;
 				} else {
 					d = va_arg(ap, int);
 				}
 
-				if (d < 0) {
+				if (*fmt != 'u' && negative(d)) {
 					out((int)'-', ctx);
 					d = -d;
 					min_width--;
 				}
-				_printk_dec_ulong(out, ctx, d, padding,
-						  min_width);
-				break;
-			}
-			case 'u': {
-				unsigned long u;
-
-				if (length_mod == 'z') {
-					u = va_arg(ap, size_t);
-				} else if (length_mod == 'l') {
-					u = va_arg(ap, unsigned long);
-				} else if (length_mod == 'L') {
-					unsigned long long llu =
-						va_arg(ap, unsigned long long);
-					if (llu > ~0UL) {
-						print_err(out, ctx);
-						break;
-					}
-					u = llu;
-				} else {
-					u = va_arg(ap, unsigned int);
-				}
-
-				_printk_dec_ulong(out, ctx, u, padding,
-						  min_width);
+				print_dec(out, ctx, d, padding, min_width);
 				break;
 			}
 			case 'p':
 				out('0', ctx);
 				out('x', ctx);
 				/* left-pad pointers with zeros */
 				padding = PAD_ZERO_BEFORE;
-				if (IS_ENABLED(CONFIG_64BIT)) {
+				if (sizeof(printk_val_t) > 4) {
 					min_width = 16;
 				} else {
 					min_width = 8;
 				}
 				/* Fall through */
 			case 'x':
 			case 'X': {
-				unsigned long long x;
+				printk_val_t x;
 
 				if (*fmt == 'p') {
 					x = (uintptr_t)va_arg(ap, void *);
@@ -245,8 +281,7 @@ void z_vprintk(out_func_t out, void *ctx, const char *fmt, va_list ap)
 					x = va_arg(ap, unsigned int);
 				}
 
-				_printk_hex_ulong(out, ctx, x, padding,
-						  min_width);
+				print_hex(out, ctx, x, padding, min_width);
 				break;
 			}
 			case 's': {
@@ -408,107 +443,6 @@ void printk(const char *fmt, ...)
 }
 #endif /* CONFIG_PRINTK */
 
-/**
- * @brief Output an unsigned long long in hex format
- *
- * Output an unsigned long long on output installed by platform at init time.
- * Able to print full 64-bit values.
- * @param num Number to output
- *
- * @return N/A
- */
-static void _printk_hex_ulong(out_func_t out, void *ctx,
-			      const unsigned long long num,
-			      enum pad_type padding,
-			      int min_width)
-{
-	int shift = sizeof(num) * 8;
-	int found_largest_digit = 0;
-	int remaining = 16; /* 16 digits max */
-	int digits = 0;
-	char nibble;
-
-	while (shift >= 4) {
-		shift -= 4;
-		nibble = (num >> shift) & 0xf;
-
-		if (nibble != 0 || found_largest_digit != 0 || shift == 0) {
-			found_largest_digit = 1;
-			nibble += nibble > 9 ? 87 : 48;
-			out((int)nibble, ctx);
-			digits++;
-			continue;
-		}
-
-		if (remaining-- <= min_width) {
-			if (padding == PAD_ZERO_BEFORE) {
-				out('0', ctx);
-			} else if (padding == PAD_SPACE_BEFORE) {
-				out(' ', ctx);
-			}
-		}
-	}
-
-	if (padding == PAD_SPACE_AFTER) {
-		remaining = min_width * 2 - digits;
-		while (remaining-- > 0) {
-			out(' ', ctx);
-		}
-	}
-}
-
-/**
- * @brief Output an unsigned long in decimal format
- *
- * Output an unsigned long on output installed by platform at init time.
- *
- * @param num Number to output
- *
- * @return N/A
- */
-static void _printk_dec_ulong(out_func_t out, void *ctx,
-			      const unsigned long num, enum pad_type padding,
-			      int min_width)
-{
-	unsigned long pos = 1000000000;
-	unsigned long remainder = num;
-	int found_largest_digit = 0;
-	int remaining = sizeof(long) * 5 / 2;
-	int digits = 1;
-
-	if (sizeof(long) == 8) {
-		pos *= 10000000000;
-	}
-
-	/* make sure we don't skip if value is zero */
-	if (min_width <= 0) {
-		min_width = 1;
-	}
-
-	while (pos >= 10) {
-		if (found_largest_digit != 0 || remainder >= pos) {
-			found_largest_digit = 1;
-			out((int)(remainder / pos + 48), ctx);
-			digits++;
-		} else if (remaining <= min_width
-				&& padding < PAD_SPACE_AFTER) {
-			out((int)(padding == PAD_ZERO_BEFORE ? '0' : ' '), ctx);
-			digits++;
-		}
-		remaining--;
-		remainder %= pos;
-		pos /= 10;
-	}
-	out((int)(remainder + 48), ctx);
-
-	if (padding == PAD_SPACE_AFTER) {
-		remaining = min_width - digits;
-		while (remaining-- > 0) {
-			out(' ', ctx);
-		}
-	}
-}
-
 struct str_context {
 	char *str;
 	int max;

tests/kernel/common/src/printk.c

@@ -15,7 +15,7 @@ void __printk_hook_install(int (*fn)(int));
 void *__printk_get_hook(void);
 int (*_old_char_out)(int);
 
-#ifndef CONFIG_64BIT
+#ifndef CONFIG_PRINTK64
 
 char *expected = "22 113 10000 32768 40000 22\n"
 		 "p 112 -10000 -32768 -40000 -22\n"
@@ -101,7 +101,7 @@ void test_printk(void)
 	printk("%d %02d %04d %08d\n", -42, -42, -42, -42);
 	printk("%u %2u %4u %8u\n", 42, 42, 42, 42);
 	printk("%u %02u %04u %08u\n", 42, 42, 42, 42);
-	printk("%-8u%-6d%-4x%-2p%8d\n", 0xFF, 42, 0xABCDEF, (char *)42, 42);
+	printk("%-8u%-6d%-4x  %-2p%8d\n", 0xFF, 42, 0xABCDEF, (char *)42, 42);
 	printk("%lld %lld %llu %llx\n", 0xFFFFFFFFFULL, -1LL, -1ULL, -1ULL);
 
 	pk_console[pos] = '\0';
@@ -130,7 +130,7 @@ void test_printk(void)
 	count += snprintk(pk_console + count, sizeof(pk_console) - count,
 			  "%u %02u %04u %08u\n", 42, 42, 42, 42);
 	count += snprintk(pk_console + count, sizeof(pk_console) - count,
-			  "%-8u%-6d%-4x%-2p%8d\n",
+			  "%-8u%-6d%-4x  %-2p%8d\n",
 			  0xFF, 42, 0xABCDEF, (char *)42, 42);
 	count += snprintk(pk_console + count, sizeof(pk_console) - count,
 			  "%lld %lld %llu %llx\n",