On Mon, 2026-07-27 at 13:54 +0100, Peter Maydell wrote: > On Mon, 27 Jul 2026 at 13:13, Nikita Shubin > <[email protected]> wrote: > > > > On Mon, 2026-07-27 at 11:36 +0100, Peter Maydell wrote: > > > On Mon, 27 Jul 2026 at 08:46, Nikita Shubin > > > <[email protected]> wrote: > > > > > > > > The switch from target_ulong to uint64_t broke the mechanism of > > > > passing -1 > > > > (0xffffffff) in arg2 to signal validate_strlen() to compute the > > > > string > > > > length automatically. For 32‑bit semihosting, detect when arg2 > > > > is > > > > 0xffffffff and replace it with 0xffffffffffffffff. This causes > > > > an > > > > overflow > > > > to zero, restoring the original behavior of automatic length > > > > calculation. > > > > > > Why do you think this is a valid thing for the guest to do? The > > > change > > > in this patch is to the SYS_OPEN semihosting call. The spec for > > > that > > > is here: > > > https://github.com/ARM-software/abi-aa/blob/main/semihosting/semihosting.rst#612sys_open-0x01 > > > > > > arg2 is the 3rd word in the argument block, which is documented > > > as: > > > > > > # An integer that gives the length of the string pointed to by > > > field > > > 1. > > > # The length does not include the terminating null character that > > > must > > > be present. > > > > > > There's nothing there about -1 being a valid value. > > > > > > I think this is a bug introduced in commit 5b3f39cb04: it > > > added "allow length parameter to be 0" to a helper function > > > because presumably we need that in some cases where the > > > semihosting > > > ABI passes a pointer to a string without a length. But because it > > > doesn't separate out "length is 0 because ABI doesn't provide > > > one" > > > from "length is provided by guest", that incorrectly allowed the > > > guest to not specify a valid length and QEMU to accept it. > > > > Seems not a bug but intended behavior. Commit 5b3f39cb04 > > deliberately > > added support for a zero length as a signal to invoke strlen(), as > > documented in both the code comments and the commit message. > > > > Citation: > > > > ``` > > Add helpers to validate the length of the filename string. > > Prepare for usage by other semihosting by allowing the > > filename length parameter to be 0, and calling strlen. > > ``` > > I take that "by other semihosting" bit to mean that we have other > semihosting functions where the guest passes a string pointer > with no associated length, and we're going to implement those > by calling this function with length == 0. I don't think it was > intended to make that behaviour visible to the guest. > > > So it seems like a regression bug introduced by 6dfbf9b6cfe. > > > > Before that, open() simply ignored the length argument, so the > > current > > logic is a conscious improvement, not an error. > > Semihosting has to implement the specification; we don't > get to "improve" on it. > > > Still in 64‑bit mode, passing -1 as arg2 still yields the expected > > result, so there is a bug somewhere — either in the 32‑bit handling > > or > > in the common logic. > > The bug is that for both 64-bit and 32-bit passing -1 is an error, > and we shouldn't be making it work on 64-bit.
That's sound reasonable, but still that's breaks behaviour introduced earlier and existed for quite a long period of time. Ok, i'll do the opposite. > > thanks > -- PMM Yours, Shubin.
