Date: Mon, 06 Apr 2015 16:00:30 -0600 From: Ian Lepore <ian@freebsd.org> To: John-Mark Gurney <jmg@funkthat.com> Cc: freebsd-arm@freebsd.org, Rui Paulo <rpaulo@me.com> Subject: Re: remove broken lib/libc/arm/string/memcpy_xscale.S Message-ID: <1428357630.82583.157.camel@freebsd.org> In-Reply-To: <20150406215156.GZ51048@funkthat.com> References: <20150405015245.GO51048@funkthat.com> <20150406171248.GV51048@funkthat.com> <20150406174130.GA63423@ci0.org> <3427080.JV46itjP9L@akita> <20150406194007.GA64433@ci0.org> <20150406215156.GZ51048@funkthat.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 2015-04-06 at 14:51 -0700, John-Mark Gurney wrote:
> Olivier Houchard wrote this message on Mon, Apr 06, 2015 at 21:40 +0200:
> > On Mon, Apr 06, 2015 at 11:32:25AM -0700, Rui Paulo wrote:
> > > On Monday 06 April 2015 19:41:30 Olivier Houchard wrote:
> > > > On Mon, Apr 06, 2015 at 10:12:48AM -0700, John-Mark Gurney wrote:
> > > > > Warner Losh wrote this message on Mon, Apr 06, 2015 at 09:58 -0600:
> > > > > > > On Apr 4, 2015, at 7:52 PM, John-Mark Gurney <jmg@funkthat.com> wrote:
> > > > > > >
> > > > > > > I would like to remove this file as it does not implement our defined
> > > > > > > memcpy. Per POSIX, overlapping regions passed to memcpy is undefined
> > > > > > > behavior. We have defined it to have the same symatics as memmove.
> > > > > > >
> > > > > > > Sample test program:
> > > > > > > #include <stdio.h>
> > > > > > > #include <string.h>
> > > > > > >
> > > > > > > char bufa[512] = "this is a test buffer that should be copied fine.";
> > > > > > > int
> > > > > > > main()
> > > > > > > {
> > > > > > >
> > > > > > > memcpy(&bufa[10], &bufa[0], strlen(&bufa[10]));
> > > > > > > printf("%s\n", bufa);
> > > > > > >
> > > > > > > return 0;
> > > > > > >
> > > > > > > }
> > > > > > >
> > > > > > > Output on amd64 HEAD:
> > > > > > > this is a this is a test buffer that should be co
> > > > > > >
> > > > > > > Output on old armv4 from 9.x:
> > > > > > > this is a this is a thst buffethst bufhould beufh
> > > > > > >
> > > > > > > If you just look at the file, it is clear that the implementation does
> > > > > > > not adjust the copy direction based upon pointers. We imported the
> > > > > > > code from NetBSD, and NetBSD does apparently require memcpy's
> > > > > > > arguments
> > > > > > > to be non-overlapping.
> > > > > > >
> > > > > > > I'll remove the file shortly unless someone can prove to me that all
> > > > > > > uses of memcpy in our tree do not depend upon our defined behavior
> > > > > > > per memcpy(3)'s man page.
> > > > > >
> > > > > > Any chance you can fix this implementation instead?
> > > > >
> > > > > I don't know arm assembly well enough, nor do I have the time to fix
> > > > > it.. I am willing to test any implementations as I have access to
> > > > > hardware...
> > > > >
> > > > > I guess I should add a test to verify that memcpy behavese like memmove
> > > > > to our test suite...
> > > >
> > > > I think the bug is in the manpage, not the code, and we should fix it the
> > > > way NetBSD did.
> > >
> > > Our implementation of memcpy() allows strings to overlap, so we really
> > > shouldn't be special-casing armv4.
> >
> > Any use of memcpy() with overlapping strings is a bug, I fail to see why we
> > should make any effort to make it work.
>
> Are you willing to do the work to make amd64/i386 and other platforms
> behave incorrectly (one way or the other) when they overlap? If you
> aren't willing to do that (and fix the resulting bugs), how do you
> expect me to deal w/ unfound bugs in armv4 that depend upon this
> "buggy" behavior?
>
As Olivier pointed out on irc, this isn't actually xscale code, it's
used for all armv5+ systems. To me that says that there isn't a whole
lot of existing code relying on the non-standard behavior because arm
systems actually work. For non-arm systems, the potential breakage
exists primarily in drivers and MD code that aren't used on arm, and
that's a much smaller universe of potential breakage.
-- Ian
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1428357630.82583.157.camel>