Bug in memcpy() for m68k

Message ID ffa4261e-e0e7-474c-a497-3808928681e2@ya2.so-net.ne.jp
State New
Headers
Series Bug in memcpy() for m68k |

Commit Message

Yuichi Nakamura March 28, 2026, 1:35 a.m. UTC
  Hi all,

I found the following issues in the m68k version of memcpy(). In 
particular, issue #1 is critical because it causes incorrect behavior on 
the 68000.

1. On CPUs that do not support misaligned access (MISALIGNED_OK=0), such 
as the 68000, memcpy() does not correctly copy regions larger than 64 KB 
when the destination is not long-word aligned.
2. The 68020 supports misaligned access, but this is not currently 
implemented.
3. The 68000 can access long-word data at even addresses, but alignment 
is checked as if the address must be a multiple of 4.
4. Because the loop count is checked using signed comparison, memcpy() 
fails for data sizes larger than 2 GB.

The following patch fixes these issues.

Best regards,
Yuichi Nakamura


  #else
  # define MISALIGNED_OK 0
@@ -49,10 +49,10 @@ SYM(memcpy):
  #if !MISALIGNED_OK
         /* Goto .Lresidue if either dest or src is not 4-byte aligned */
         move.l  a0,d0
-       and.l   #3,d0
+       and.l   #1,d0
         bne     .Lresidue
         move.l  a1,d0
-       and.l   #3,d0
+       and.l   #1,d0
         bne     .Lresidue
  #else /* MISALIGNED_OK */
         /* align dest */
@@ -95,7 +95,7 @@ SYM(memcpy):
  #else
         subq.l  #1,d0
  #endif
-       bpl     1b
+       bcc     1b
         bra     .Lresidue

  1:
@@ -104,9 +104,13 @@ SYM(memcpy):
  .Lresidue:
  #if !defined (__mcoldfire__)
         dbra    d1,1b           | loop until done
+#if !MISALIGNED_OK
+       sub.l   #0x10000,d1
+       bcc     1b
+#endif /* !MISALIGNED_OK */
  #else
         subq.l  #1,d1
-       bpl     1b
+       bcc     1b
  #endif
         move.l  4(sp),d0        | return value
         rts
  

Comments

Corinna Vinschen March 30, 2026, 8:47 a.m. UTC | #1
Hi Yuichi,

unfortunately your patch doesn't apply cleanly due to whitespace issues.

Can you please create a patch with `git format-patch' and send it via
`git send-email' to this list?  Alternatively, append the patch created
with git format-patch as attachment to your mail.  This should avoid 
whitespace issue entirely.


Thanks,
Corinna


On Mar 28 10:35, Yuichi Nakamura via Newlib wrote:
> Hi all,
> 
> I found the following issues in the m68k version of memcpy(). In particular,
> issue #1 is critical because it causes incorrect behavior on the 68000.
> 
> 1. On CPUs that do not support misaligned access (MISALIGNED_OK=0), such as
> the 68000, memcpy() does not correctly copy regions larger than 64 KB when
> the destination is not long-word aligned.
> 2. The 68020 supports misaligned access, but this is not currently
> implemented.
> 3. The 68000 can access long-word data at even addresses, but alignment is
> checked as if the address must be a multiple of 4.
> 4. Because the loop count is checked using signed comparison, memcpy() fails
> for data sizes larger than 2 GB.
> 
> The following patch fixes these issues.
> 
> Best regards,
> Yuichi Nakamura
> 
> 
> diff --git a/newlib/libc/machine/m68k/memcpy.S
> b/newlib/libc/machine/m68k/memcpy.S
> index 464da95ef..ecf1da611 100644
> --- a/newlib/libc/machine/m68k/memcpy.S
> +++ b/newlib/libc/machine/m68k/memcpy.S
> @@ -15,7 +15,7 @@
> 
>  #include "m68kasm.h"
> 
> -#if defined (__mcoldfire__) || defined (__mc68030__) || defined
> (__mc68040__) || defined (__mc68060__)
> +#if defined (__mcoldfire__) || defined (__mc68020__) || defined
> (__mc68030__) || defined (__mc68040__) || defined (__mc68060__)
>  # define MISALIGNED_OK 1
>  #else
>  # define MISALIGNED_OK 0
> @@ -49,10 +49,10 @@ SYM(memcpy):
>  #if !MISALIGNED_OK
>         /* Goto .Lresidue if either dest or src is not 4-byte aligned */
>         move.l  a0,d0
> -       and.l   #3,d0
> +       and.l   #1,d0
>         bne     .Lresidue
>         move.l  a1,d0
> -       and.l   #3,d0
> +       and.l   #1,d0
>         bne     .Lresidue
>  #else /* MISALIGNED_OK */
>         /* align dest */
> @@ -95,7 +95,7 @@ SYM(memcpy):
>  #else
>         subq.l  #1,d0
>  #endif
> -       bpl     1b
> +       bcc     1b
>         bra     .Lresidue
> 
>  1:
> @@ -104,9 +104,13 @@ SYM(memcpy):
>  .Lresidue:
>  #if !defined (__mcoldfire__)
>         dbra    d1,1b           | loop until done
> +#if !MISALIGNED_OK
> +       sub.l   #0x10000,d1
> +       bcc     1b
> +#endif /* !MISALIGNED_OK */
>  #else
>         subq.l  #1,d1
> -       bpl     1b
> +       bcc     1b
>  #endif
>         move.l  4(sp),d0        | return value
>         rts
  
Yuichi Nakamura March 30, 2026, 2:02 p.m. UTC | #2
Hi Corinna,

Thank you for the advice.
I have attached the patch to this email.

Best regards,
Yuichi Nakamura


On 2026/03/30 17:47, Corinna Vinschen wrote:
> Hi Yuichi,
> 
> unfortunately your patch doesn't apply cleanly due to whitespace issues.
> 
> Can you please create a patch with `git format-patch' and send it via
> `git send-email' to this list?  Alternatively, append the patch created
> with git format-patch as attachment to your mail.  This should avoid
> whitespace issue entirely.
> 
> 
> Thanks,
> Corinna
> 
> 
> On Mar 28 10:35, Yuichi Nakamura via Newlib wrote:
>> Hi all,
>>
>> I found the following issues in the m68k version of memcpy(). In particular,
>> issue #1 is critical because it causes incorrect behavior on the 68000.
>>
>> 1. On CPUs that do not support misaligned access (MISALIGNED_OK=0), such as
>> the 68000, memcpy() does not correctly copy regions larger than 64 KB when
>> the destination is not long-word aligned.
>> 2. The 68020 supports misaligned access, but this is not currently
>> implemented.
>> 3. The 68000 can access long-word data at even addresses, but alignment is
>> checked as if the address must be a multiple of 4.
>> 4. Because the loop count is checked using signed comparison, memcpy() fails
>> for data sizes larger than 2 GB.
>>
>> The following patch fixes these issues.
>>
>> Best regards,
>> Yuichi Nakamura
>>
>>
>> diff --git a/newlib/libc/machine/m68k/memcpy.S
>> b/newlib/libc/machine/m68k/memcpy.S
>> index 464da95ef..ecf1da611 100644
>> --- a/newlib/libc/machine/m68k/memcpy.S
>> +++ b/newlib/libc/machine/m68k/memcpy.S
>> @@ -15,7 +15,7 @@
>>
>>   #include "m68kasm.h"
>>
>> -#if defined (__mcoldfire__) || defined (__mc68030__) || defined
>> (__mc68040__) || defined (__mc68060__)
>> +#if defined (__mcoldfire__) || defined (__mc68020__) || defined
>> (__mc68030__) || defined (__mc68040__) || defined (__mc68060__)
>>   # define MISALIGNED_OK 1
>>   #else
>>   # define MISALIGNED_OK 0
>> @@ -49,10 +49,10 @@ SYM(memcpy):
>>   #if !MISALIGNED_OK
>>          /* Goto .Lresidue if either dest or src is not 4-byte aligned */
>>          move.l  a0,d0
>> -       and.l   #3,d0
>> +       and.l   #1,d0
>>          bne     .Lresidue
>>          move.l  a1,d0
>> -       and.l   #3,d0
>> +       and.l   #1,d0
>>          bne     .Lresidue
>>   #else /* MISALIGNED_OK */
>>          /* align dest */
>> @@ -95,7 +95,7 @@ SYM(memcpy):
>>   #else
>>          subq.l  #1,d0
>>   #endif
>> -       bpl     1b
>> +       bcc     1b
>>          bra     .Lresidue
>>
>>   1:
>> @@ -104,9 +104,13 @@ SYM(memcpy):
>>   .Lresidue:
>>   #if !defined (__mcoldfire__)
>>          dbra    d1,1b           | loop until done
>> +#if !MISALIGNED_OK
>> +       sub.l   #0x10000,d1
>> +       bcc     1b
>> +#endif /* !MISALIGNED_OK */
>>   #else
>>          subq.l  #1,d1
>> -       bpl     1b
>> +       bcc     1b
>>   #endif
>>          move.l  4(sp),d0        | return value
>>          rts
From 1bb0e3e1e05fe97974f8e967814c7ff70046dfbf Mon Sep 17 00:00:00 2001
From: Yuichi Nakamura <y.512.nakamura@gmail.com>
Date: Mon, 30 Mar 2026 22:55:33 +0900
Subject: [PATCH] Fix memcpy alignment problems for m68k architecture

---
 newlib/libc/machine/m68k/memcpy.S | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/newlib/libc/machine/m68k/memcpy.S b/newlib/libc/machine/m68k/memcpy.S
index 464da95ef..ecf1da611 100644
--- a/newlib/libc/machine/m68k/memcpy.S
+++ b/newlib/libc/machine/m68k/memcpy.S
@@ -15,7 +15,7 @@
 
 #include "m68kasm.h"
 
-#if defined (__mcoldfire__) || defined (__mc68030__) || defined (__mc68040__) || defined (__mc68060__)
+#if defined (__mcoldfire__) || defined (__mc68020__) || defined (__mc68030__) || defined (__mc68040__) || defined (__mc68060__)
 # define MISALIGNED_OK 1
 #else
 # define MISALIGNED_OK 0
@@ -49,10 +49,10 @@ SYM(memcpy):
 #if !MISALIGNED_OK
 	/* Goto .Lresidue if either dest or src is not 4-byte aligned */
 	move.l	a0,d0
-	and.l	#3,d0
+	and.l	#1,d0
 	bne	.Lresidue
 	move.l	a1,d0
-	and.l	#3,d0
+	and.l	#1,d0
 	bne	.Lresidue
 #else /* MISALIGNED_OK */
 	/* align dest */
@@ -95,7 +95,7 @@ SYM(memcpy):
 #else
 	subq.l	#1,d0
 #endif
-	bpl	1b
+	bcc	1b
 	bra	.Lresidue
 
 1:
@@ -104,9 +104,13 @@ SYM(memcpy):
 .Lresidue:
 #if !defined (__mcoldfire__)
 	dbra	d1,1b		| loop until done
+#if !MISALIGNED_OK
+	sub.l	#0x10000,d1
+	bcc	1b
+#endif /* !MISALIGNED_OK */
 #else
 	subq.l	#1,d1
-	bpl	1b
+	bcc	1b
 #endif
 	move.l	4(sp),d0	| return value
 	rts
  
Corinna Vinschen March 30, 2026, 2:40 p.m. UTC | #3
Hi Yuichi,


Since you didn't add any explainng commit message text, I took the
liberty to add your original description verbatim to your patches
commit message.

Pushed.


Thanks,
Corinna


On Mar 30 23:02, Yuichi Nakamura via Newlib wrote:
> Hi Corinna,
> 
> Thank you for the advice.
> I have attached the patch to this email.
> 
> Best regards,
> Yuichi Nakamura
> 
> 
> On 2026/03/30 17:47, Corinna Vinschen wrote:
> > Hi Yuichi,
> > 
> > unfortunately your patch doesn't apply cleanly due to whitespace issues.
> > 
> > Can you please create a patch with `git format-patch' and send it via
> > `git send-email' to this list?  Alternatively, append the patch created
> > with git format-patch as attachment to your mail.  This should avoid
> > whitespace issue entirely.
> > 
> > 
> > Thanks,
> > Corinna
> > 
> > 
> > On Mar 28 10:35, Yuichi Nakamura via Newlib wrote:
> > > Hi all,
> > > 
> > > I found the following issues in the m68k version of memcpy(). In particular,
> > > issue #1 is critical because it causes incorrect behavior on the 68000.
> > > 
> > > 1. On CPUs that do not support misaligned access (MISALIGNED_OK=0), such as
> > > the 68000, memcpy() does not correctly copy regions larger than 64 KB when
> > > the destination is not long-word aligned.
> > > 2. The 68020 supports misaligned access, but this is not currently
> > > implemented.
> > > 3. The 68000 can access long-word data at even addresses, but alignment is
> > > checked as if the address must be a multiple of 4.
> > > 4. Because the loop count is checked using signed comparison, memcpy() fails
> > > for data sizes larger than 2 GB.
> > > 
> > > The following patch fixes these issues.
> > > 
> > > Best regards,
> > > Yuichi Nakamura
> > > 
> > > 
> > > diff --git a/newlib/libc/machine/m68k/memcpy.S
> > > b/newlib/libc/machine/m68k/memcpy.S
> > > index 464da95ef..ecf1da611 100644
> > > --- a/newlib/libc/machine/m68k/memcpy.S
> > > +++ b/newlib/libc/machine/m68k/memcpy.S
> > > @@ -15,7 +15,7 @@
> > > 
> > >   #include "m68kasm.h"
> > > 
> > > -#if defined (__mcoldfire__) || defined (__mc68030__) || defined
> > > (__mc68040__) || defined (__mc68060__)
> > > +#if defined (__mcoldfire__) || defined (__mc68020__) || defined
> > > (__mc68030__) || defined (__mc68040__) || defined (__mc68060__)
> > >   # define MISALIGNED_OK 1
> > >   #else
> > >   # define MISALIGNED_OK 0
> > > @@ -49,10 +49,10 @@ SYM(memcpy):
> > >   #if !MISALIGNED_OK
> > >          /* Goto .Lresidue if either dest or src is not 4-byte aligned */
> > >          move.l  a0,d0
> > > -       and.l   #3,d0
> > > +       and.l   #1,d0
> > >          bne     .Lresidue
> > >          move.l  a1,d0
> > > -       and.l   #3,d0
> > > +       and.l   #1,d0
> > >          bne     .Lresidue
> > >   #else /* MISALIGNED_OK */
> > >          /* align dest */
> > > @@ -95,7 +95,7 @@ SYM(memcpy):
> > >   #else
> > >          subq.l  #1,d0
> > >   #endif
> > > -       bpl     1b
> > > +       bcc     1b
> > >          bra     .Lresidue
> > > 
> > >   1:
> > > @@ -104,9 +104,13 @@ SYM(memcpy):
> > >   .Lresidue:
> > >   #if !defined (__mcoldfire__)
> > >          dbra    d1,1b           | loop until done
> > > +#if !MISALIGNED_OK
> > > +       sub.l   #0x10000,d1
> > > +       bcc     1b
> > > +#endif /* !MISALIGNED_OK */
> > >   #else
> > >          subq.l  #1,d1
> > > -       bpl     1b
> > > +       bcc     1b
> > >   #endif
> > >          move.l  4(sp),d0        | return value
> > >          rts

> From 1bb0e3e1e05fe97974f8e967814c7ff70046dfbf Mon Sep 17 00:00:00 2001
> From: Yuichi Nakamura <y.512.nakamura@gmail.com>
> Date: Mon, 30 Mar 2026 22:55:33 +0900
> Subject: [PATCH] Fix memcpy alignment problems for m68k architecture
> 
> ---
>  newlib/libc/machine/m68k/memcpy.S | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/newlib/libc/machine/m68k/memcpy.S b/newlib/libc/machine/m68k/memcpy.S
> index 464da95ef..ecf1da611 100644
> --- a/newlib/libc/machine/m68k/memcpy.S
> +++ b/newlib/libc/machine/m68k/memcpy.S
> @@ -15,7 +15,7 @@
>  
>  #include "m68kasm.h"
>  
> -#if defined (__mcoldfire__) || defined (__mc68030__) || defined (__mc68040__) || defined (__mc68060__)
> +#if defined (__mcoldfire__) || defined (__mc68020__) || defined (__mc68030__) || defined (__mc68040__) || defined (__mc68060__)
>  # define MISALIGNED_OK 1
>  #else
>  # define MISALIGNED_OK 0
> @@ -49,10 +49,10 @@ SYM(memcpy):
>  #if !MISALIGNED_OK
>  	/* Goto .Lresidue if either dest or src is not 4-byte aligned */
>  	move.l	a0,d0
> -	and.l	#3,d0
> +	and.l	#1,d0
>  	bne	.Lresidue
>  	move.l	a1,d0
> -	and.l	#3,d0
> +	and.l	#1,d0
>  	bne	.Lresidue
>  #else /* MISALIGNED_OK */
>  	/* align dest */
> @@ -95,7 +95,7 @@ SYM(memcpy):
>  #else
>  	subq.l	#1,d0
>  #endif
> -	bpl	1b
> +	bcc	1b
>  	bra	.Lresidue
>  
>  1:
> @@ -104,9 +104,13 @@ SYM(memcpy):
>  .Lresidue:
>  #if !defined (__mcoldfire__)
>  	dbra	d1,1b		| loop until done
> +#if !MISALIGNED_OK
> +	sub.l	#0x10000,d1
> +	bcc	1b
> +#endif /* !MISALIGNED_OK */
>  #else
>  	subq.l	#1,d1
> -	bpl	1b
> +	bcc	1b
>  #endif
>  	move.l	4(sp),d0	| return value
>  	rts
> -- 
> 2.43.0
>
  

Patch

diff --git a/newlib/libc/machine/m68k/memcpy.S 
b/newlib/libc/machine/m68k/memcpy.S
index 464da95ef..ecf1da611 100644
--- a/newlib/libc/machine/m68k/memcpy.S
+++ b/newlib/libc/machine/m68k/memcpy.S
@@ -15,7 +15,7 @@ 

  #include "m68kasm.h"

-#if defined (__mcoldfire__) || defined (__mc68030__) || defined 
(__mc68040__) || defined (__mc68060__)
+#if defined (__mcoldfire__) || defined (__mc68020__) || defined 
(__mc68030__) || defined (__mc68040__) || defined (__mc68060__)
  # define MISALIGNED_OK 1