casaroli opened a new pull request, #21474: URL: https://github.com/apache/nuttx/pull/21474
## Summary `printf` can crash or write past a buffer sized for the width, for a valid format with correct arguments. This PR fixes these cases, one commit per defect, with no new Kconfig option. It is the first step of #21467. The measurement is the suite at https://github.com/casaroli/printf-report. ## What is fixed - Each argument is read at its right size: `%a` and `%A`, `%n` of any length, `%L` as a `long double`, the C23 `wN` and `wfN` lengths for 8, 16, 32 and 64 bits, and all numbered arguments, also `%x`, `%X`, `%o`, `%S` and `%n`. - `%a` and `%A`, the `wN` and `wfN` integer conversions, and `%e`, `%f`, `%g` with `CONFIG_LIBC_FLOATINGPOINT` off, read their argument and print `?` padded to the width. The old text with float off was `*float*`. - `%n` reads its pointer, stores nothing and prints nothing. - A width above 255 gives the right length: `%300d` wrote 556 bytes. A three-digit exponent counts in the width: `%12.1e` of 1e100 wrote 13 bytes. - `%.767e` and `%.0g` no longer print a NUL byte, and `%#.0x` of 0 no longer reads `buf[-1]`. - Width and precision stop at `INT_MAX`, also for `*` with `INT_MIN` and for a numbered `*`. Output above `INT_MAX` returns -1 with `EOVERFLOW`. - `lib_bsprintf()` no longer writes past its 64-byte conversion buffer. `%pV` and `%pB` count in the return value. ## What is not fixed - Wrong usage: an unknown conversion, a `wN` of another size, and a format that ends after `%`, a flag, a width or a length. These behave as on master. An unknown conversion takes an unsigned int and the arguments after it can be wrong, and a format that ends early reads past its end. The check costs flash, so a later step of #21467 adds it behind an option. - A wrong argument from the caller, such as a bad pointer for `%s` or an argument of the wrong type: `printf` cannot detect it. - `scanf` with `%L`. - A legal `%2147483647d` prints in full, as the standard requires. A caller with a smaller buffer must use `snprintf`. - Output that is not correct but is safe: `-0.0`, a real `%a` and `wN`, more than 15 digits, `%b`. These are later steps of #21467. ## Decisions - A numbered argument that the format does not use is wrong usage, but it stays safe here: its size is not known, so the place of the next arguments is not known. `printf` reads only the arguments before it, and a conversion with a later number ends the output, as a number above `NL_ARGMAX` does. - A width or precision with too many digits stops at `INT_MAX`, so `%99999999999d` writes `INT_MAX` bytes and returns `INT_MAX`. If the total then passes `INT_MAX`, the result is -1 with `EOVERFLOW`, and the output written before that stays. One conversion longer than `INT_MAX` fails before its output. - An integer conversion with `L` reads a `long long`, as in GNU libc. ## Testing The suite on sim arm64, master next to this PR: | | C99 required | Correctly rounded | C23 | POSIX | XSI | Bit-exact | Crash | Hang | |---|---|---|---|---|---|---|---|---| | master | 61.73% | 57.31% | 1.08% | 78.05% | 36.36% | 55.68% | 27 | 0 | | this PR | 67.81% | 63.11% | 1.08% | 92.68% | 36.36% | 61.46% | 0 | 0 | No test goes from pass to fail. Six tests go from "not correctly rounded" to "fail": `%.18Le`, `%.20Le`, `%.21Le` and the same with `E`, of `LDBL_EPSILON`. Master printed 0 there, and this PR prints the value with 15 digits. The suite has no case where `%x` is the only use of a numbered argument, such as `%2$s %1$x`, and it always builds with float on, so it has no case for float off with numbered on. A host harness covers both: the real sources under ASan and UBSan in 11 configurations of float, numbered arguments, locale and `CONFIG_ARCH_ROMGETC`, up to 202,000 valid formats per configuration, with huge widths. On gcc Linux arm64 this PR has 0 reports for correct usage in all 11 configurations. On clang macOS: 8 configurations done with 0 reports, the others not finished. Master has hundreds of reports per configuration in 4,000 cases. The same harness has generated formats for wrong usage, which still fail here as on master: the end of the format, unknown conversions followed by `%s`, and `%w` at the end. The test in nuttx-apps also covers `%2$s %1$x`, in ostest. The conversion of a binary128 `long double` against the cast, on the target CPU, run on the same function before the last rebase: 4,126,084 values on arm64, rv32 and rv64, with 0 differences in the result bits. A NaN stays a NaN. Runtime, a test program for these cases and ostest: qemu-armv8a:knsh (kernel) and rv-virt:knsh64 (kernel) pass. ostest also passes on qemu-armv8a:nsh (flat). Not finished: mps2-an500:knsh (protected), qemu-armv8a:nsh and rv-virt:nsh with float off and on, the rv-virt:pnsh build, and the test in nuttx-apps on this revision. ## Size Master -> this PR: | CPU, config | Float | Numbered | Image text | `vsprintf_internal` stack | `lib_vsprintf` stack | |---|---|---|---|---|---| | Cortex-M0, nucleo-f072rb:nsh | off | off | 46668 -> 46792 (+124) | 80 -> 80 | 8 -> 8 | | Cortex-M0, nucleo-f072rb:nsh | off | on | 47288 -> 47480 (+192) | 96 -> 96 | 64 -> 64 | | Cortex-M0, nucleo-f072rb:nsh | on | off | 56784 -> 56984 (+200) | 96 -> 96 | 8 -> 8 | | Cortex-M0, nucleo-f072rb:nsh | on | on | 57440 -> 57672 (+232) | 112 -> 112 | 64 -> 64 | | Cortex-M3, nucleo-f103rb:nsh | off | off | not finished | not finished | not finished | | Cortex-M3, nucleo-f103rb:nsh | off | on | not finished | not finished | not finished | | Cortex-M3, nucleo-f103rb:nsh | on | off | not finished | not finished | not finished | | Cortex-M3, nucleo-f103rb:nsh | on | on | not finished | not finished | not finished | | arm64, qemu-armv8a:nsh | off | off | 357200 -> 357448 (+248) | 160 -> 144 | 48 -> 48 | | arm64, qemu-armv8a:nsh | off | on | 358228 -> 358660 (+432) | 160 -> 160 | 160 -> 160 | | arm64, qemu-armv8a:nsh | on | off | 359436 -> 359900 (+464) | 176 -> 160 | 48 -> 48 | | arm64, qemu-armv8a:nsh | on | on | 360568 -> 361208 (+640) | 176 -> 192 | 160 -> 192 | | RISC-V 32, rv-virt:nsh | off | off | 228214 -> 228460 (+246) | 80 -> 80 | 0 -> 0 | | RISC-V 32, rv-virt:nsh | off | on | not finished | not finished | not finished | | RISC-V 32, rv-virt:nsh | on | off | 230006 -> 230602 (+596) | 112 -> 144 | 0 -> 0 | | RISC-V 32, rv-virt:nsh | on | on | 230696 -> 231514 (+818) | 112 -> 144 | 48 -> 96 | | RISC-V 64, rv-virt:nsh64 | off | off | not finished | not finished | not finished | | RISC-V 64, rv-virt:nsh64 | off | on | not finished | not finished | not finished | | RISC-V 64, rv-virt:nsh64 | on | off | not finished | not finished | not finished | | RISC-V 64, rv-virt:nsh64 | on | on | not finished | not finished | not finished | The growth is the new checks. No CPU pulls in the soft-float code of libgcc: the conversion of a binary128 `long double` is integer code of 154 to 274 bytes. A cast would add 854 to 1268 bytes more. The `wN` commit adds 68 to 76 bytes on Cortex-M0, 32 to 44 on Cortex-M3 and 108 to 148 on arm64 and RISC-V, in the printf object, measured with -Os and test headers. The conversion by hand applies when `LDBL_MANT_DIG` is 113: measured on arm64, rv32 and rv64. Cortex-M0, Cortex-M3, Cortex-A9 and Xtensa have 53, so `long double` is `double` there, and x86_64 has 64: a cast stays, with no cost on the first ones. On rv32 the stack of `vsprintf_internal` goes from 112 to 144 with float on, in the two `long double` commits only. The growth of `lib_vsprintf` with numbered arguments on rv32 comes with the `%L` commit. Both were measured per commit on rv32. The reason is read from the code and the ABI, not measured: a `long double` has 16 bytes there and it is passed by reference, so the function holds copies of it on its stack. `lib_vsprintf` also grows on arm64 with float and numbered arguments on; its cause was not measured there. ## Impact Only the output of formats that were wrong or undefined changes. No code in nuttx or nuttx-apps depends on `*float*`. This PR updates two sample outputs in the board documentation and the help text of `LIBC_FLOATINGPOINT`. No API and no Kconfig change. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
