| Message ID | 20230601220305.2130121-1-eblake@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 6/1/23 15:02, Eric Blake wrote: > Eric Blake (21): > iotests: Fix test 104 under NBD > qcow2: Explicit mention of padding bytes > test-cutils: Avoid g_assert in unit tests > test-cutils: Use g_assert_cmpuint where appropriate > test-cutils: Test integral qemu_strto* value on failures > test-cutils: Test more integer corner cases > cutils: Fix wraparound parsing in qemu_strtoui > cutils: Document differences between parse_uint and qemu_strtou64 > cutils: Adjust signature of parse_uint[_full] > cutils: Allow NULL endptr in parse_uint() > test-cutils: Add coverage of qemu_strtod > test-cutils: Prepare for upcoming semantic change in qemu_strtosz > test-cutils: Refactor qemu_strtosz tests for less boilerplate > cutils: Allow NULL str in qemu_strtosz > numa: Check for qemu_strtosz_MiB error > test-cutils: Add more coverage to qemu_strtosz > cutils: Set value in all qemu_strtosz* error paths > cutils: Set value in all integral qemu_strto* error paths > cutils: Use parse_uint in qemu_strtosz for negative rejection > cutils: Improve qemu_strtod* error paths > cutils: Improve qemu_strtosz handling of fractions This is failing on Windows (32 and 64-bit): https://gitlab.com/qemu-project/qemu/-/jobs/4399466166#L3524 https://gitlab.com/qemu-project/qemu/-/jobs/4399466165#L3332 | 21/135 /cutils/qemu_strtol/overflow - ERROR:../tests/unit/test-cutils.c:1387:test_qemu_strtol_overflow: assertion failed (res == LONG_MIN): (2147483647 == -2147483648) FAIL It seems to have returned LONG_MAX instead of LONG_MIN. r~
Pattern: First, one of us gets a bright idea on user-friendly interface (here: fractional sizes like 1.5M). Objections, if any, get brushed aside. Then the thing sprouts warts, tentacles, sores, and starts to give off that sickly-sweet smell of bugs feasting on misguided ideas. Until one of us spends a lot more time on containing and reducing the damage than the thing could ever be worth. Cobbler, stick to your last. Thanks, Eric!
On Thu, Jun 01, 2023 at 08:58:46PM -0700, Richard Henderson wrote: > On 6/1/23 15:02, Eric Blake wrote: > > Eric Blake (21): > > iotests: Fix test 104 under NBD > > qcow2: Explicit mention of padding bytes > > test-cutils: Avoid g_assert in unit tests > > test-cutils: Use g_assert_cmpuint where appropriate > > test-cutils: Test integral qemu_strto* value on failures > > test-cutils: Test more integer corner cases > > cutils: Fix wraparound parsing in qemu_strtoui > > cutils: Document differences between parse_uint and qemu_strtou64 > > cutils: Adjust signature of parse_uint[_full] > > cutils: Allow NULL endptr in parse_uint() > > test-cutils: Add coverage of qemu_strtod > > test-cutils: Prepare for upcoming semantic change in qemu_strtosz > > test-cutils: Refactor qemu_strtosz tests for less boilerplate > > cutils: Allow NULL str in qemu_strtosz > > numa: Check for qemu_strtosz_MiB error > > test-cutils: Add more coverage to qemu_strtosz > > cutils: Set value in all qemu_strtosz* error paths > > cutils: Set value in all integral qemu_strto* error paths > > cutils: Use parse_uint in qemu_strtosz for negative rejection > > cutils: Improve qemu_strtod* error paths > > cutils: Improve qemu_strtosz handling of fractions > > This is failing on Windows (32 and 64-bit): > > https://gitlab.com/qemu-project/qemu/-/jobs/4399466166#L3524 > https://gitlab.com/qemu-project/qemu/-/jobs/4399466165#L3332 > > | 21/135 /cutils/qemu_strtol/overflow - > ERROR:../tests/unit/test-cutils.c:1387:test_qemu_strtol_overflow: assertion > failed (res == LONG_MIN): (2147483647 == -2147483648) FAIL > > It seems to have returned LONG_MAX instead of LONG_MIN. Gah. I see the problem; it is a copy/paste typo in part of test-cutils.c guarded by 'if (LONG_MAX == INT_MAX)' and therefore doesn't fire on platforms with 64-bit long. Will respin the pull request to fix it.
On Fri, Jun 02, 2023 at 08:32:34AM +0200, Markus Armbruster wrote: > Pattern: > > First, one of us gets a bright idea on user-friendly interface (here: > fractional sizes like 1.5M). Objections, if any, get brushed aside. > > Then the thing sprouts warts, tentacles, sores, and starts to give off > that sickly-sweet smell of bugs feasting on misguided ideas. > > Until one of us spends a lot more time on containing and reducing the > damage than the thing could ever be worth. > > Cobbler, stick to your last. > > Thanks, Eric! The scary part was that I found several other bugs completely unrelated to the read-out-of-bounds, merely by increasing unit test coverage. When Hanna first pointed me to the problem asking if a 5-line patch would work, I was not expecting it to blow up into a series touching more than 1000 lines (true, most of those lines were in the unit tests).
Eric Blake <eblake@redhat.com> writes: > The scary part was that I found several other bugs completely > unrelated to the read-out-of-bounds, merely by increasing unit test > coverage. When Hanna first pointed me to the problem asking if a > 5-line patch would work, I was not expecting it to blow up into a > series touching more than 1000 lines (true, most of those lines were > in the unit tests). You spot vermin hiding under a rock. What will you likely see when you lift the rock? Thanks again for lifting the rock :)
The following changes since commit 19a720b74fde7e859d19f12c66a72e545947a657: Merge tag 'tracing-pull-request' of https://gitlab.com/stefanha/qemu into staging (2023-06-01 08:30:29 -0700) are available in the Git repository at: https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2023-06-01 for you to fetch changes up to 58516caac47c4bc7ed3ad6a8e2f565404a563dd3: cutils: Improve qemu_strtosz handling of fractions (2023-06-01 16:55:25 -0500) ---------------------------------------------------------------- nbd and misc patches for 2023-06-01 - Eric Blake: Fix iotest 104 for NBD - Eric Blake: Improve qcow2 spec on padding bytes - Eric Blake: Fix read-beyond-bounds bug in qemu_strtosz ---------------------------------------------------------------- Eric Blake (21): iotests: Fix test 104 under NBD qcow2: Explicit mention of padding bytes test-cutils: Avoid g_assert in unit tests test-cutils: Use g_assert_cmpuint where appropriate test-cutils: Test integral qemu_strto* value on failures test-cutils: Test more integer corner cases cutils: Fix wraparound parsing in qemu_strtoui cutils: Document differences between parse_uint and qemu_strtou64 cutils: Adjust signature of parse_uint[_full] cutils: Allow NULL endptr in parse_uint() test-cutils: Add coverage of qemu_strtod test-cutils: Prepare for upcoming semantic change in qemu_strtosz test-cutils: Refactor qemu_strtosz tests for less boilerplate cutils: Allow NULL str in qemu_strtosz numa: Check for qemu_strtosz_MiB error test-cutils: Add more coverage to qemu_strtosz cutils: Set value in all qemu_strtosz* error paths cutils: Set value in all integral qemu_strto* error paths cutils: Use parse_uint in qemu_strtosz for negative rejection cutils: Improve qemu_strtod* error paths cutils: Improve qemu_strtosz handling of fractions docs/interop/qcow2.txt | 1 + include/qemu/cutils.h | 5 +- audio/audio_legacy.c | 4 +- block/gluster.c | 4 +- block/nfs.c | 4 +- blockdev.c | 4 +- contrib/ivshmem-server/main.c | 4 +- hw/core/numa.c | 11 +- qapi/opts-visitor.c | 10 +- tests/unit/test-cutils.c | 2469 ++++++++++++++++++++++++++++---------- ui/vnc.c | 4 +- util/cutils.c | 263 ++-- util/guest-random.c | 4 +- util/qemu-sockets.c | 10 +- tests/qemu-iotests/common.filter | 4 +- tests/qemu-iotests/common.rc | 3 +- tests/qemu-iotests/049.out | 7 +- tests/qemu-iotests/178.out.qcow2 | 3 +- tests/qemu-iotests/178.out.raw | 3 +- 19 files changed, 2035 insertions(+), 782 deletions(-)