All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Edgar E. Iglesias" <edgar.iglesias@amd.com>
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: xen-devel@lists.xenproject.org, sstabellini@kernel.org,
	julien@xen.org, bertrand.marquis@arm.com, michal.orzel@amd.com,
	Volodymyr_Babchuk@epam.com
Subject: Re: [PATCH v1 1/1] arm64: Fix strrchr() matching of null terminator
Date: Tue, 19 May 2026 18:27:23 +0200	[thread overview]
Message-ID: <agyPa90M2iQKTQGj@zapote> (raw)
In-Reply-To: <b9088d84-c92d-4b71-9d02-177282221a90@citrix.com>

On Tue, May 19, 2026 at 01:45:37PM +0100, Andrew Cooper wrote:
> On 19/05/2026 12:43 am, Edgar E. Iglesias wrote:
> > The generic Xen strrchr() implementation returns a pointer to the string
> > terminator when searching for '\0', matching the standard C semantics.
> >
> > The ARM64 assembly version stopped as soon as it loaded the terminator and
> > returned the previous match pointer instead.  This made strrchr("", '\0')
> > return NULL.
> >
> > Compare the loaded byte against the requested character before deciding
> > whether to stop at the terminator, so the terminator itself can be returned
> > when it is the requested character.
> >
> > Fixes: 42c4eb6a83 ("xen: arm64: assembly optimised mem* and str*")
> > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@amd.com>
> > ---
> >  xen/arch/arm/arm64/lib/strrchr.S | 7 +++----
> >  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> A couple of tangential things.
> 
> This file was inherited from Linux.  Does the same bug need fixing
> there?  What about the arm32 side?

Yes, I'll have a look at Linux. Arm32 looks fine to me.

> 
> Looking at your example, it surely wasn't actually as simple as
> strrchr("", '\0') ?  I'd expect the optimiser to be able to turn that
> into a constant and not call out to the library implementation.
> 

We ran into this while exploring a new QEMU based test framework, we had
it do some fuzzing and it tripped over this.


> Elsewhere, I've created xen/common/bitops.c to be CONFIG_SELF_TESTS for
> the bit operations including the arch-optimised variations, because
> they're subtle and easy to get wrong.  This looks like it's worth doing
> the same for the bits of libc we implement.
>

Yes, that looks like a good fit, thanks.

Cheers,
Edgar


      reply	other threads:[~2026-05-19 16:27 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-18 23:43 [PATCH v1 0/1] arm64: Fix strrchr() matching of null terminator Edgar E. Iglesias
2026-05-18 23:43 ` [PATCH v1 1/1] " Edgar E. Iglesias
2026-05-19  6:40   ` Jan Beulich
2026-05-19  7:47     ` Julien Grall
2026-05-19 15:28       ` Edgar E. Iglesias
2026-05-19 15:24     ` Edgar E. Iglesias
2026-05-19 12:45   ` Andrew Cooper
2026-05-19 16:27     ` Edgar E. Iglesias [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=agyPa90M2iQKTQGj@zapote \
    --to=edgar.iglesias@amd.com \
    --cc=Volodymyr_Babchuk@epam.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=bertrand.marquis@arm.com \
    --cc=julien@xen.org \
    --cc=michal.orzel@amd.com \
    --cc=sstabellini@kernel.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.