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.

thanks
-- PMM

Reply via email to