Re: [PATCH] Fix regression in memrchr

Looking at the patch from Alexey, he indeed missed that memrchr used a
different UNALIGNED macro definition and replacing it with the common one
needs the change specified.

Patch approved.  Thanks.

-- Jeff J.


On Thu, Mar 19, 2026 at 6:59 AM Corinna Vinschen <[email protected]>
wrote:

> CC the author of c9b74e328, Alexey Lapshin.
>
> Alexey, any input?
>
>
> Thanks,
> Corinna
>
>
>
>
> On Mar 19 08:55, Federico Terraneo wrote:
> > Hi,
> > while upgrading our toolchain for the Miosix kernel to use the lastest
> > Newlib release, we noticed a regression in memrchr. This issue causes an
> > unaligned memory access resulting in a fault in ARM Cortex M0
> > microcontrollers.
> >
> > The bug has been introduced in commit
> > c9b74e32892966c8f66280c752181292bc95f690, where the macro UNALIGNED was
> > replaced with a generic one UNALIGNED_X.
> > However, in memrchr.c the UNALIGNED macro was defined as
> >
> > #define UNALIGNED(X) ((long)(X + 1) & (sizeof (long) - 1))
> >
> > thus checking if X+1 is aligned, not if X is aligned.
> >
> > The replacement macro checks if X is aligned, thus breaking the
> algorithm.
> >
> > The issue was found by my colleague Daniele Cattaneo.
> >
> > Best regards,
> > Federico Terraneo
>
> > From ea9b7d9f09d48b2bf88682d47992f5ee0bd4e57d Mon Sep 17 00:00:00 2001
> > From: Daniele Cattaneo <[email protected]>
> > Date: Wed, 18 Mar 2026 17:33:02 +0100
> > Subject: [PATCH] libc: string: Fix off-by-one alignment bug in memrchr.
> >
> > The loop at line 50 should test the bytes in the buffer, from the end
> towards
> > the beginning, until it reaches a character whose address is aligned,
> leaving
> > `src' pointing to the next character to test. However, the loop
> condition did
> > not test the address of the last character read (`src + 1'), but the
> address of
> > the next character (`src'). As a consequence, a misaligned address was
> computed
> > at line 69, because the (aligned) address in `src' is incremented by 1.
> >
> > This bug was originally introduced in commit c9b74e328 during a
> refactoring of
> > the macros used by string functions. Before the refactoring, the +1
> increment
> > in the loop condition was located in the macros specific to memrchr.
> > ---
> >  newlib/libc/string/memrchr.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/newlib/libc/string/memrchr.c b/newlib/libc/string/memrchr.c
> > index 0a0c80fd9..b56e2b476 100644
> > --- a/newlib/libc/string/memrchr.c
> > +++ b/newlib/libc/string/memrchr.c
> > @@ -47,7 +47,7 @@ memrchr (const void *src_void,
> >    unsigned long  mask;
> >    unsigned int i;
> >
> > -  while (UNALIGNED_X(src))
> > +  while (UNALIGNED_X(src + 1))
> >      {
> >        if (!length--)
> >          return NULL;
> > --
> > 2.53.0
> >
>
>