__mempcpy vs mempcpy

__mempcpy vs mempcpy

[Michael Eager]

After updating cygwin recently, gcc doesn't build.  It attempts
to link __mempcpy, which cygwin doesn't supply.  Cygwin does
provide mempcpy.  (Which apparently has been an issue in the 
past, because at one time it was declared in string.h but not 
actually included in the library.)

The oddness is that gcc configure checks for mempcpy, while
gcc code actually references __mempcpy.    

There are refs to both mempcpy and __mempcpy:

gcc/intl/loadmsgcat.c:
# if defined _LIBC || HAVE_MEMPCPY
          *((char *) mempcpy (charset, charsetstr, len)) = '\0';
# else
          memcpy (charset, charsetstr, len);
          charset[len] = '\0';
# endif

which looks almost identical to libiberty/regex.c:
#if defined HAVE_MEMPCPY || defined _LIBC
          *((char *) __mempcpy (errbuf, msg, errbuf_size - 1)) = '\0';
#else
          memcpy (errbuf, msg, errbuf_size - 1);
          errbuf[errbuf_size - 1] = 0;
#endif

In gcc/intl/localealias.c, the code looks like loadmsgcat.c, 
except that there's also the following:

#ifdef _LIBC
. . .
# ifndef mempcpy
#  define mempcpy __mempcpy
# endif
# define HAVE_MEMPCPY   1
#endif

So questions:

1)  Should configure be checking for mempcpy when it uses __mempcpy?
2)  Should all uses be either mempcpy or __mempcpy?
3)  The define for HAVE_MEMPCPY in localealias.c looks hinky. 
    after all, if you don't have mempcpy, then renaming it __mempcpy
    isn't much help.
4)  Why bother with mempcpy when it seems to be such a trivial
    modification of memcpy?

There's a bunch of ways to fix this.  Suggestions?


--
Michael Eager     eager@mvista.com	408-328-8426	
MontaVista Software, Inc. 1237 E. Arques Ave., Sunnyvale, CA  94085

Re: __mempcpy vs mempcpy

[Svein E. Seldal]

Hi,

I presume you are aware of:

http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gcc&pr=10198

and

http://gcc.gnu.org/cgi-bin/gnatsweb.pl?cmd=view%20audit-trail&database=gcc&pr=10338

They identify exactly the same issues as you point out now. (However 
they do not take any action towards a solution, so I'll keep this thread 
under close view.)


Svein

Re: __mempcpy vs mempcpy

[DJ Delorie]

> which looks almost identical to libiberty/regex.c:
> #if defined HAVE_MEMPCPY || defined _LIBC
>           *((char *) __mempcpy (errbuf, msg, errbuf_size - 1)) = '\0';
> #else
>           memcpy (errbuf, msg, errbuf_size - 1);
>           errbuf[errbuf_size - 1] = 0;
> #endif

This looks like a bug in the libiberty sources.  Regex is (was)
mirrored from glibc, which probably didn't have to worry about that
kind of portability.  (glibc now uses a different regex).  Does it
actually fail because of this code?  We don't actually check for
mempcpy() in libiberty (although if you want to add that check to
libiberty, the patch would be welcome ;).

Unfortunately, I suspect cygwin may trip over this "incompatibility"
with glibc in the future, in other sources.

Assuming you mean libiberty for the rest...

> 2)  Should all uses be either mempcpy or __mempcpy?

mempcpy

> 3)  The define for HAVE_MEMPCPY in localealias.c looks hinky. 
>     after all, if you don't have mempcpy, then renaming it __mempcpy
>     isn't much help.

_LIBC means you're compiling glibc itself, or linking with glibc.  I
think that code is an attempt to use __mempcpy if memcpy is not
already a macro, not if it doesn't exist.

> 4)  Why bother with mempcpy when it seems to be such a trivial
>     modification of memcpy?

Sounds like it might be a trivially faster function, since it doesn't
need to keep track of the pointer that memcpy would have to return.

> There's a bunch of ways to fix this.  Suggestions?

For the libiberty/regex.c bug, please feel free to post a patch for
__mempcpy -> mempcpy.  Consider it pre-approved.

Note that we never check for mempcpy() in libiberty anyway, so that
code won't ever get used (unless _LIBC is defined).

I suspect the non-libiberty sources would want a similar patch.

Re: __mempcpy vs mempcpy

[Svein E. Seldal]

DJ Delorie wrote:
> For the libiberty/regex.c bug, please feel free to post a patch for
> __mempcpy -> mempcpy.  Consider it pre-approved.

gcc/fixinc/gnu-regex.c suffers from the same problem. Maybe a global 
search in the whole gcc source is in order?

I can fix it, but I'm going on easter holiday today. If it time critical 
to fix, maybe someone else should do it instead...

Dj Delorie: How did you intend to fix the bug? By changing this:

     #if defined HAVE_MEMPCPY || defined _LIBC
         *((char *) __mempcpy (errbuf, msg, errbuf_size - 1)) = '\0';
     #else
           memcpy (errbuf, msg, errbuf_size - 1);
           errbuf[errbuf_size - 1] = 0;
     #endif

into this?

     memcpy (errbuf, msg, errbuf_size - 1);
     errbuf[errbuf_size - 1] = 0;


Regards,
Svein

Re: __mempcpy vs mempcpy

[Kaveh R. Ghazi]

 > > 2)  Should all uses be either mempcpy or __mempcpy?
 > 
 > mempcpy


This patch should fix it, but I don't have access to the platforms in
question or any _LIBC boxes to test it.  Can someone regtest it for me?

Assuming that passes, ok for mainline, 3.3 and 3.2?

		Thanks,
		--Kaveh


2003-04-11  Kaveh R. Ghazi  <ghazi@caip.rutgers.edu>

gcc:
	PR target/10338
	PR bootstrap/10198
	PR bootstrap/10140
	* fixinc/gnu-regex.c (regerror): Use mempcpy not __mempcpy.
	
libiberty:
	PR target/10338
	PR bootstrap/10198
	PR bootstrap/10140
	* getopt.c (exchange, _getopt_initialize): Use mempcpy not
	__mempcpy.
	* regex.c (regerror): Likewise.

diff -rup orig/egcc-3.2-CVS20030410/gcc/fixinc/gnu-regex.c egcc-3.2-CVS20030410/gcc/fixinc/gnu-regex.c
--- orig/egcc-3.2-CVS20030410/gcc/fixinc/gnu-regex.c	Tue Feb 26 19:23:56 2002
+++ egcc-3.2-CVS20030410/gcc/fixinc/gnu-regex.c	Fri Apr 11 10:13:19 2003
@@ -5720,7 +5720,7 @@ regerror (errcode, preg, errbuf, errbuf_
       if (msg_size > errbuf_size)
         {
 #if defined HAVE_MEMPCPY || defined _LIBC
-	  *((char *) __mempcpy (errbuf, msg, errbuf_size - 1)) = '\0';
+	  *((char *) mempcpy (errbuf, msg, errbuf_size - 1)) = '\0';
 #else
           memcpy (errbuf, msg, errbuf_size - 1);
           errbuf[errbuf_size - 1] = 0;
diff -rup orig/egcc-3.2-CVS20030410/libiberty/getopt.c egcc-3.2-CVS20030410/libiberty/getopt.c
--- orig/egcc-3.2-CVS20030410/libiberty/getopt.c	Mon Nov 12 22:46:00 2001
+++ egcc-3.2-CVS20030410/libiberty/getopt.c	Fri Apr 11 10:16:41 2003
@@ -333,8 +333,8 @@ exchange (argv)
 	nonoption_flags_len = nonoption_flags_max_len = 0;
       else
 	{
-	  memset (__mempcpy (new_str, __getopt_nonoption_flags,
-			     nonoption_flags_max_len),
+	  memset (mempcpy (new_str, __getopt_nonoption_flags,
+			   nonoption_flags_max_len),
 		  '\0', top + 1 - nonoption_flags_max_len);
 	  nonoption_flags_max_len = top + 1;
 	  __getopt_nonoption_flags = new_str;
@@ -444,7 +444,7 @@ _getopt_initialize (argc, argv, optstrin
 	      if (__getopt_nonoption_flags == NULL)
 		nonoption_flags_max_len = -1;
 	      else
-		memset (__mempcpy (__getopt_nonoption_flags, orig_str, len),
+		memset (mempcpy (__getopt_nonoption_flags, orig_str, len),
 			'\0', nonoption_flags_max_len - len);
 	    }
 	}
diff -rup orig/egcc-3.2-CVS20030410/libiberty/regex.c egcc-3.2-CVS20030410/libiberty/regex.c
--- orig/egcc-3.2-CVS20030410/libiberty/regex.c	Fri Nov 16 16:54:17 2001
+++ egcc-3.2-CVS20030410/libiberty/regex.c	Fri Apr 11 10:15:03 2003
@@ -8256,7 +8256,7 @@ regerror (errcode, preg, errbuf, errbuf_
       if (msg_size > errbuf_size)
         {
 #if defined HAVE_MEMPCPY || defined _LIBC
-	  *((char *) __mempcpy (errbuf, msg, errbuf_size - 1)) = '\0';
+	  *((char *) mempcpy (errbuf, msg, errbuf_size - 1)) = '\0';
 #else
           memcpy (errbuf, msg, errbuf_size - 1);
           errbuf[errbuf_size - 1] = 0;

Re: __mempcpy vs mempcpy

[Andreas Schwab]

"Kaveh R. Ghazi" <ghazi@caip.rutgers.edu> writes:

|>  > > 2)  Should all uses be either mempcpy or __mempcpy?
|>  > 
|>  > mempcpy
|> 
|> 
|> This patch should fix it, but I don't have access to the platforms in
|> question or any _LIBC boxes to test it.

_LIBC is only defined when building glibc itself.  Since glibc does not
use that implementation any more it probably doesn't matter.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux AG, Deutschherrnstr. 15-19, D-90429 Nürnberg
Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: __mempcpy vs mempcpy

[Kaveh R. Ghazi]

 > From: Andreas Schwab <schwab@suse.de>
 > 
 > "Kaveh R. Ghazi" <ghazi@caip.rutgers.edu> writes:
 > 
 > |>  > > 2)  Should all uses be either mempcpy or __mempcpy?
 > |>  > 
 > |>  > mempcpy
 > |> 
 > |> 
 > |> This patch should fix it, but I don't have access to the platforms in
 > |> question or any _LIBC boxes to test it.
 > 
 > _LIBC is only defined when building glibc itself.  Since glibc does not
 > use that implementation any more it probably doesn't matter.
 > Andreas.

It seems to matter to the people who filed these PRs.  Perhaps someone
with cygwin can test for me?

		--Kaveh
--
Kaveh R. Ghazi			ghazi@caip.rutgers.edu

Re: __mempcpy vs mempcpy

[DJ Delorie]

> Dj Delorie: How did you intend to fix the bug? By changing this:

It's DJ

>      #if defined HAVE_MEMPCPY || defined _LIBC
>          *((char *) __mempcpy (errbuf, msg, errbuf_size - 1)) = '\0';
>      #else
>            memcpy (errbuf, msg, errbuf_size - 1);
>            errbuf[errbuf_size - 1] = 0;
>      #endif
> 
> into this?
> 
>      memcpy (errbuf, msg, errbuf_size - 1);
>      errbuf[errbuf_size - 1] = 0;

No, my recommendation was to just change __mempcpy to mempcpy.

Re: __mempcpy vs mempcpy

[DJ Delorie]

> Assuming that passes, ok for mainline, 3.3 and 3.2?

The libiberty bits are OK with me for all, assuming the branch allows
me to say so ;-)