forwarded message from Chen, Kenneth W

forwarded message from Chen, Kenneth W

[David Mosberger]

[-- Attachment #1: message body text --]
[-- Type: text/plain, Size: 185 bytes --]

I'm forwarding this mail because I don't see it in the archive at
http://sources.redhat.com/ml/bug-glibc/.  (bug-glibc@gnu.org is the
right address for this archive, right?)

	--david


[-- Attachment #2: forwarded message --]
[-- Type: message/rfc822, Size: 11860 bytes --]

[-- Attachment #2.1.1: Type: text/plain, Size: 2045 bytes --]

We found more bugs in the code, and here is an updated patch to fix them.
Note this patch supercedes the one I sent yesterday.

Also attached is the test program we used to verify the fix.

- Ken Chen


-----Original Message-----
From: Chen, Kenneth W 
Sent: Wednesday, November 06, 2002 6:57 PM
To: Chen, Kenneth W; Reese Faucette; bug-glibc@gnu.org;
linux-ia64@linuxia64.org
Cc: Akyil, Levent; susan@myri.com; Mallick, Asit K
Subject: RE: [Linux-ia64] IA64 strncpy in 2.2.4-30 - bug and patch


Here is a patch that fixes bugs in strncpy function included in glibc-2.2.4.  We found there are more bugs during the investigation.  Please pound on this patch and let us know any issues.

- Ken Chen


-----Original Message-----
From: Chen, Kenneth W 
Sent: Tuesday, November 05, 2002 4:44 PM
To: Reese Faucette; bug-glibc@gnu.org; linux-ia64@linuxia64.org
Cc: Akyil, Levent; susan@myri.com
Subject: RE: [Linux-ia64] IA64 strncpy in 2.2.4-30 - bug and patch


I'm not able to reproduce the bug with your test case ...

-----Original Message-----
From: Reese Faucette [mailto:reese@myri.com]
Sent: Tuesday, November 05, 2002 4:39 PM
To: bug-glibc@gnu.org; linux-ia64@linuxia64.org
Cc: Akyil, Levent; susan@myri.com
Subject: [Linux-ia64] IA64 strncpy in 2.2.4-30 - bug and patch


Hi,
I opened a bug with RedHat about a problem with strncpy() in glibc-2.2.4-30 on IA64, see https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=76952, but I've been told you folks are really the ones to talk to.
 
The bug report contains a testcase illustrating the problem, and also a patch.  In short, the recovery code is busted, even after the patch from David Mosberger.  Attached is a copy of the patch from the bug report.
 
I have a testcase which exercizes both the recovery3 and recovery4 paths if you're interested.  
Regards,
-reese faucette
 
 


_______________________________________________
Linux-IA64 mailing list
Linux-IA64@linuxia64.org
http://lists.linuxia64.org/lists/listinfo/linux-ia64

[-- Attachment #2.1.2: strncpy2.fix.patch --]
[-- Type: application/octet-stream, Size: 1122 bytes --]

--- strncpy.S.orig	Wed Nov  6 17:16:31 2002
+++ strncpy.S	Wed Nov  6 18:47:25 2002
@@ -48,6 +48,7 @@
 #define	sh1		r29
 #define loopcnt		r30
 #define	value		r31
+#define tmp2		r14
 
 ENTRY(strncpy)
 	.prologue
@@ -62,6 +63,7 @@
 	mov	saved_pr = pr           // save the predicate registers
 	.save ar.lc, saved_lc
 	mov 	saved_lc = ar.lc	// save the loop counter
+	mov	ar.ec = 0
 	.body
 	cmp.geu p6, p5 = 24, in2
 (p6)	br.cond.spnt .short_len
@@ -209,15 +211,21 @@
 	mov	pr = saved_pr, -1	// restore the predicate registers
 	br.ret.sptk.many b0
 .recovery2:
+	sub	tmp2=len,thresh
+	;;
+	cmp.ge	p8,p9=-8, tmp2
 	add	tmp = -8, asrc ;;
-	ld8	r[0] = [tmp]
+(p8)	mov	r[0] = r0
+(p9)	ld8	r[0] = [tmp]
 	br.cond.sptk .back2
 .recovery3:
-	add	tmp = -MEMLAT * 8, src ;;
+	add	tmp = -(MEMLAT+1) * 8, src ;;
 	ld8	r[MEMLAT] = [tmp]
 	br.cond.sptk .back3
 .recovery4:
-	add	tmp = -(MEMLAT - 1) * 8, src ;;
-	ld8	r[MEMLAT - 1] = [tmp]
+	cmp.eq	p8,p9=0, len
+	add	tmp = -(MEMLAT) * 8, src ;;
+(p8)	mov	r[MEMLAT - 1] = r0
+(p9)	ld8	r[MEMLAT - 1] = [tmp]
 	br.cond.sptk .back4
 END(strncpy)

[-- Attachment #2.1.3: bug.c --]
[-- Type: application/octet-stream, Size: 1341 bytes --]

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <sys/shm.h>

#define MCPYSIZE 2048

extern char *my_strncpy(char *dest, const char *src, size_t n);

int main()
{
  long buffer[MCPYSIZE];

  int i, j, k, x;
  char *src, *dst;
  size_t len;
  int shmid;
  char* shmaddr;


  shmid = shmget(1, 16384, IPC_CREAT|SHM_R|SHM_W);
  shmaddr = (char*) shmat(shmid, NULL, SHM_RND);

  for (i=0; i<16384; i++)
    shmaddr[i] = i;

  dst = (char*) buffer;

  for (k=1; k<1000; k+=1)
    for (i=0; i<8; i++)
      for (j=0; j<8; j++) {

	src = (char*) shmaddr + 16384 - k - i;
	dst = (char*) buffer + j;
	len  = k;

//	printf("dst %p src %p n %4d\n", dst, src, len);
	my_strncpy(dst, src, len);

	for (x=0; x<len; x++) {
		if (src[x] == 0) break;
		if ( dst[x] != src[x] ) {
			unsigned long * l_src = (unsigned long*) ((unsigned long) &src[x] & -8);
			unsigned long * l_dst = (unsigned long*) ((unsigned long) &dst[x] & -8);

			printf("error\t");
			printf("dst %p src %p n %4d\n", dst, src, len);

			printf("%d: %x %x\n", x, dst[x], src[x]);

			printf("%d: %16x %16x\n", x, *l_dst, *l_src);
			exit(0);
//			break;
		}
	}

//	if (strcmp(dst, src, len) != 0)
//	  printf("bug found\n");

      } // for (j=0 ...

  shmdt(shmaddr);
  shmctl(shmid, IPC_RMID, NULL);
}

[-- Attachment #2.1.4: bug2.c --]
[-- Type: application/octet-stream, Size: 1468 bytes --]

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <sys/shm.h>
#include <sys/mman.h>
#include <unistd.h>
#include <fcntl.h>

#define MCPYSIZE 2048

extern char *my_strncpy(char *dest, const char *src, size_t n);

int main()
{
  long buffer[MCPYSIZE];

  int i, j, k, l, x;
  char *src, *dst;
  size_t len;
  int shmid;
  char* shmaddr;
  int fd;

/*
  shmid = shmget(1, 16384, IPC_CREAT|SHM_R|SHM_W);
  shmaddr = (char*) shmat(shmid, NULL, SHM_RND);
*/
  fd = open("data", O_RDONLY);

  dst = (char*) buffer;

  for (k=0; k<10000; k+=1)
   for (l=0; l<10; l++)
    for (i=0; i<8; i++)
      for (j=0; j<8; j++) {

  shmaddr = (char*) mmap(NULL, 16384*2, PROT_READ, MAP_SHARED, fd, 0);
  if (shmaddr == NULL) perror("zero address");

	src = (char*) shmaddr + 16384 - k - i;
	dst = (char*) buffer + j;
	len  = k+l;

//	printf("dst %p src %p n %4d\n", dst, src, len);
	my_strncpy(dst, src, len);

	for (x=0; x<len; x++) {
		if (src[x] == 0) break;
		if ( dst[x] != src[x] ) {
			unsigned long * l_src = (unsigned long*) ((unsigned long) &src[x] & -8);
			unsigned long * l_dst = (unsigned long*) ((unsigned long) &dst[x] & -8);

			printf("error\t");
			printf("dst %p src %p n %4d\n", dst, src, len);

			printf("%d: %x %x\n", x, dst[x], src[x]);

			printf("%d: %16x %16x\n", x, *l_dst, *l_src);
//			exit(0);
//			break;
		}
	}

  munmap(shmaddr, 16384*2);

      } // for (j=0 ...
}

Re: forwarded message from Chen, Kenneth W

[Roland McGrath]

If the new string/test-strncpy.c test program (in glibc CVS) does not
already test the cases that you found problems with, could you please send
us a patch to that program to make it test these cases?

Jakub Jelinek made some additional fixes to ia64/strncpy.S, that overlap
with some of your changes.  I don't understand ia64 enough to know whether
your other changes are warranted as well.


Thanks,
Roland