Frank, this patch has remained unreviewed for a week. Could you please have a look?
Thanks. Rainer Rainer Orth <r...@cebitec.uni-bielefeld.de> writes: > This is the first of two patches to get mudflap fully working on > Solaris 11, both with Sun ld and GNU ld. > > It addresses a couple of testsuite failures: > > * Several tests fail with 3 unexpected register violations: > > ******* > mudflap violation 1 (register): time=1309356076.070433 ptr=21680 size=16 > pc=7fa07a64 > > /var/gcc/regression/trunk/11-gcc/build/sparc-sun-solaris2.11/libmudflap/.libs/libmudflap.so.0.0.0'__mf_register+0x2c > [0x7fa07a64] > > /var/gcc/regression/trunk/11-gcc/build/sparc-sun-solaris2.11/libmudflap/.libs/libmudflap.so.0.0.0'__wrap_main+0x194 > [0x7fa07c18] > > /var/gcc/regression/trunk/11-gcc/build/sparc-sun-solaris2.11/libmudflap/testsuite/heap-scalestress.exe'_start+0x5c > [0x10afc] > Nearby object 1: checked region begins 0B into and ends 15B into > mudflap object a2aa8: name=`/usr/include/iso/stdio_iso.h:163:15 __iob' > bounds=[21680,217bf] size=320 area=static check=0r/0w liveness=0 > alloc time=1309356076.069900 pc=7fa07a64 > number of nearby objects: 1 > > All 3 are 0, 16, or 32 bytes __iob[]. The error goes away with > -no-heur-stdlib. > > If running the test with -trace-calls, I find: > > mf: register ptr=21680 size=16 type=4 name='stdin' > mf: violation pc=7fa07b94 location= type=3 ptr=21680 size=16 > ******* > mudflap violation 1 (register): time=1309365780.121411 ptr=21680 size=16 > pc=7fa07b94 > > /var/gcc/regression/trunk/11-gcc/build/sparc-sun-solaris2.11/libmudflap/.libs/libmudflap.so.0.0.0'__mf_register+0x2c > [0x7fa07b94] > > /var/gcc/regression/trunk/11-gcc/build/sparc-sun-solaris2.11/libmudflap/.l > ibs/libmudflap.so.0.0.0'__wrap_main+0x194 [0x7fa07d48] > > /var/gcc/regression/trunk/11-gcc/build/sparc-sun-solaris2.11/libmudflap/te > stsuite/heap-scalestress.exe'_start+0x5c [0x10afc] > Nearby object 1: checked region begins 0B into and ends 15B into > mudflap object a2aa8: name=`/usr/include/iso/stdio_iso.h:163:15 __iob' > bounds=[21680,217bf] size=320 area=static check=0r/0w liveness=0 > alloc time=1309365780.077107 pc=7fa07b94 > number of nearby objects: 1 > > The conflict is between > > mf: register ptr=21680 size=320 type=4 > name='/usr/include/iso/stdio_iso.h:163:15 __iob' > > and > > mf: register ptr=21680 size=16 type=4 name='stdin' > mf: violation pc=7fa07260 location= type=3 ptr=21680 size=16 > > where the registration of __iob has been done automatically by the > compiler. I avoid this problem by not registering stdin, stdout, and > stderr separately on Solaris. > > * Some tests were failing while calling unregister in munmap. It turned > out that there had been no corresponding mmap registration before. > This occurs because Solaris has mmap64 for largefile-aware programs > instead. Fixed by wrapping mmap64, too. What I don't know is if > mmap64 needs to be added to MFWRAP_SPEC in gcc.c? If so, I'd rather > do it by adding some MFWRAP_OS_SPEC to avoid having to duplicate the > whole spec in the Solaris config headers. > > * As noted in the last patch, the getmntent signature differs in > Solaris. This patch implements a wrapper for the Solaris version. > > * libmudflap.cth/pass37-frag.c would fail like this: > > ******* > mudflap violation 1 (unregister): time=1309444614.922185 ptr=7f9e90a4 size=4 > pc=7fa07e78 > > /var/gcc/regression/trunk/11-gcc/build/sparc-sun-solaris2.11/libmudflap/.libs/libmudflapth.so.0.0.0'__mf_unregister+0xec > [0x7fa07e78] > > /var/gcc/regression/trunk/11-gcc/build/sparc-sun-solaris2.11/libmudflap/.libs/libmudflapth.so.0.0.0'__mf_pthread_cleanup+0x4c > [0x7fa277dc] > > /var/gcc/regression/trunk/11-gcc/build/sparc-sun-solaris2.11/libmudflap/.libs/libmudflapth.so.0.0.0'__mf_pthread_spawner+0x14c > [0x7fa27938] > Nearby object 1: checked region begins 0B into and ends 3B into > mudflap object a43b8: name=`errno area' > bounds=[7f9e90a4,7f9e90a7] size=4 area=static check=0r/0w liveness=0 > alloc time=1309444614.909941 pc=7fa077e8 thread=1 > number of nearby objects: 1 > FAIL: libmudflap.cth/pass37-frag.c execution test > > Investigating with -trace-calls reveals that all registrations and > unregistrations of errno are for the same address, which is wrong for > multithreaded programs which access errno via an accessor function. > To enable that, <errno.h> needs to be included with _REENTRANT > defined. It turned out that it suffices to do this in mf-hooks3.c. > > * libmudflap.c/heap-scalestress.c always timed out on my SPARC test > system: on a 1.2 GHz UltraSPARC-T2, it takes > > real 8:47.06 > user 43.12 > sys 8:03.77 > > which is way over the limit. On my laptop (1.6 GHz Core i7), it takes > > real 37.35 > user 5.06 > sys 32.23 > > I've divided SCALE by 10 to account for this. > > * I've replaced all the __FreeBSD__ && ... tests in > libmudflap.c/pass-stratcliff.c with appropriate autoconf macros, and > also define MIN which can be missing. > > * libmudflap.c/pass47-frag.c originally failed like this: > > FAIL: libmudflap.c/pass47-frag.c (test for excess errors) > Excess errors: > /vol/gcc/src/hg/trunk/local/libmudflap/testsuite/libmudflap.c/pass47-frag.c:10:1: > warning: mudflap cannot track unknown size extern '__ctype' [-Wmudflap] > > warning is benign, cf. <iso/ctype_iso.h>: > > extern unsigned char __ctype[]; > > I expect that warning on Solaris to handle this. Besides, there were > several violations that turned out to be related to __ctype[] and > __ctype_mask. Unfortunately, I have to calculate or hardcode their > sizes in clumsy ways, but those sizes are unchanged since Solaris 8 > and are identical between 32 and 64-bit libc. > > With this patch (and the next), I get almost clean testsuite results on > sparc-sun-solaris2.11 (both multilibs): > > === libmudflap Summary === > > # of expected passes 2865 > # of unexpected failures 1 > > The only failures is > > FAIL: libmudflap.c++/pass55-frag.cxx ( -O) execution test > > for 64-bit only which also occurs on x86_64-unknown-linux-gnu. > > Ok for mainline? > > Thanks. > Rainer > > > 2011-06-29 Rainer Orth <r...@cebitec.uni-bielefeld.de> > > libmudflap/49550 > * mf-runtime.c (__wrap_main) [__sun__ && __svr4__]: Don't register > stdin, stdout, stderr. > Register __ctype, __ctype_mask. > > * configure.ac: Check for mmap64. > Check for rawmemchr, stpcpy, mempcpy. > * configure: Regenerate. > * config.h.in: Regenerate. > * mf-hooks1.c [HAVE_MMAP64] (__mf_0fn_mmap64): New function. > (mmap64): New wrapper function. > * mf-impl.h (__mf_dynamic_index) [HAVE_MMAP64]: Add dyn_mmap64. > * mf-runtime.c (__mf_dynamic) [HAVE_MMAP64]: Handle mmap64. > > * mf-hooks2.c [HAVE_GETMNTENT && HAVE_SYS_MNTTAB_H]: Implement > getmntent wrapper. > > * mf-hooks3.c (_REENTRANT): Define. > > * testsuite/libmudflap.c/heap-scalestress.c (SCALE): Reduce to 10000. > > * testsuite/libmudflap.c/pass-stratcliff.c: Include ../config.h. > (MIN): Define. > Use HAVE_RAWMEMCHR, HAVE_STPCPY, HAVE_MEMPCPY as guards. > > * testsuite/libmudflap.c/pass47-frag.c: Expect __ctype warning on > *-*-solaris2.*. > > diff --git a/libmudflap/configure.ac b/libmudflap/configure.ac > --- a/libmudflap/configure.ac > +++ b/libmudflap/configure.ac > @@ -75,7 +75,9 @@ AC_CHECK_FUNCS(getservent getservbyname > AC_CHECK_FUNCS(getprotoent getprotobyname getprotobynumber) > AC_CHECK_FUNCS(getmntent setmntent addmntent) > AC_CHECK_FUNCS(inet_ntoa mmap munmap) > +AC_CHECK_FUNCS(mmap64) > AC_CHECK_FUNCS(__libc_freeres) > +AC_CHECK_FUNCS(rawmemchr stpcpy mempcpy) > > AC_TRY_COMPILE([#include <sys/types.h> > #include <sys/ipc.h> > diff --git a/libmudflap/mf-hooks1.c b/libmudflap/mf-hooks1.c > --- a/libmudflap/mf-hooks1.c > +++ b/libmudflap/mf-hooks1.c > @@ -1,5 +1,5 @@ > /* Mudflap: narrow-pointer bounds-checking by tree rewriting. > - Copyright (C) 2002, 2003, 2004, 2009 Free Software Foundation, Inc. > + Copyright (C) 2002, 2003, 2004, 2009, 2011 Free Software Foundation, Inc. > Contributed by Frank Ch. Eigler <f...@redhat.com> > and Graydon Hoare <gray...@redhat.com> > > @@ -414,6 +414,61 @@ WRAPPER(int , munmap, void *start, size_ > #endif /* HAVE_MMAP */ > > > +#ifdef HAVE_MMAP64 > +#if PIC > +/* A special bootstrap variant. */ > +void * > +__mf_0fn_mmap64 (void *start, size_t l, int prot, int f, int fd, off64_t off) > +{ > + return (void *) -1; > +} > +#endif > + > + > +#undef mmap > +WRAPPER(void *, mmap64, > + void *start, size_t length, int prot, > + int flags, int fd, off64_t offset) > +{ > + DECLARE(void *, mmap64, void *, size_t, int, > + int, int, off64_t); > + void *result; > + BEGIN_PROTECT (mmap64, start, length, prot, flags, fd, offset); > + > + result = CALL_REAL (mmap64, start, length, prot, > + flags, fd, offset); > + > + /* > + VERBOSE_TRACE ("mmap64 (%08lx, %08lx, ...) => %08lx\n", > + (uintptr_t) start, (uintptr_t) length, > + (uintptr_t) result); > + */ > + > + if (result != (void *)-1) > + { > + /* Register each page as a heap object. Why not register it all > + as a single segment? That's so that a later munmap() call > + can unmap individual pages. XXX: would __MF_TYPE_GUESS make > + this more automatic? */ > + size_t ps = getpagesize (); > + uintptr_t base = (uintptr_t) result; > + uintptr_t offset; > + > + for (offset=0; offset<length; offset+=ps) > + { > + /* XXX: We could map PROT_NONE to __MF_TYPE_NOACCESS. */ > + /* XXX: Unaccessed HEAP pages are reported as leaks. Is this > + appropriate for unaccessed mmap pages? */ > + __mf_register ((void *) CLAMPADD (base, offset), ps, > + __MF_TYPE_HEAP_I, "mmap64 page"); > + } > + } > + > + return result; > +} > +#endif /* HAVE_MMAP64 */ > + > + > /* This wrapper is a little different, as it's called indirectly from > __mf_fini also to clean up pending allocations. */ > void * > diff --git a/libmudflap/mf-hooks2.c b/libmudflap/mf-hooks2.c > --- a/libmudflap/mf-hooks2.c > +++ b/libmudflap/mf-hooks2.c > @@ -2102,7 +2102,42 @@ WRAPPER2(struct mntent *, getmntent, FIL > return m; > } > #elif defined HAVE_SYS_MNTTAB_H > -/* FIXME: Implement. */ > +WRAPPER2(int, getmntent, FILE *filep, struct mnttab *mp) > +{ > + static struct mnttab *last = NULL; > + int res; > + > + MF_VALIDATE_EXTENT (filep, sizeof (*filep), __MF_CHECK_WRITE, > + "getmntent stream"); > +#define UR(field) __mf_unregister(last->field, strlen (last->field)+1, > __MF_TYPE_STATIC) > + if (last) > + { > + UR (mnt_special); > + UR (mnt_mountp); > + UR (mnt_fstype); > + UR (mnt_mntopts); > + UR (mnt_time); > + __mf_unregister (last, sizeof (*last), __MF_TYPE_STATIC); > + } > +#undef UR > + > + res = getmntent (filep, mp); > + last = mp; > + > +#define R(field) __mf_register(last->field, strlen (last->field)+1, > __MF_TYPE_STATIC, "mntent " #field) > + if (mp) > + { > + R (mnt_special); > + R (mnt_mountp); > + R (mnt_fstype); > + R (mnt_mntopts); > + R (mnt_time); > + __mf_register (last, sizeof (*last), __MF_TYPE_STATIC, "getmntent > result"); > + } > +#undef R > + > + return res; > +} > #endif > #endif > > diff --git a/libmudflap/mf-hooks3.c b/libmudflap/mf-hooks3.c > --- a/libmudflap/mf-hooks3.c > +++ b/libmudflap/mf-hooks3.c > @@ -1,5 +1,5 @@ > /* Mudflap: narrow-pointer bounds-checking by tree rewriting. > - Copyright (C) 2002, 2003, 2004, 2005, 2009 > + Copyright (C) 2002, 2003, 2004, 2005, 2009, 2011 > Free Software Foundation, Inc. > Contributed by Frank Ch. Eigler <f...@redhat.com> > and Graydon Hoare <gray...@redhat.com> > @@ -44,6 +44,7 @@ see the files COPYING3 and COPYING.RUNTI > #define _ALL_SOURCE > #define _LARGE_FILE_API > #define _XOPEN_SOURCE_EXTENDED 1 > +#define _REENTRANT > > #include <string.h> > #include <stdio.h> > diff --git a/libmudflap/mf-impl.h b/libmudflap/mf-impl.h > --- a/libmudflap/mf-impl.h > +++ b/libmudflap/mf-impl.h > @@ -1,6 +1,6 @@ > /* Implementation header for mudflap runtime library. > Mudflap: narrow-pointer bounds-checking by tree rewriting. > - Copyright (C) 2002, 2003, 2004, 2009 Free Software Foundation, Inc. > + Copyright (C) 2002, 2003, 2004, 2009, 2011 Free Software Foundation, Inc. > Contributed by Frank Ch. Eigler <f...@redhat.com> > and Graydon Hoare <gray...@redhat.com> > > @@ -212,6 +212,9 @@ extern struct __mf_dynamic_entry __mf_dy > enum __mf_dynamic_index > { > dyn_calloc, dyn_free, dyn_malloc, dyn_mmap, > +#ifdef HAVE_MMAP64 > + dyn_mmap64, > +#endif > dyn_munmap, dyn_realloc, > dyn_INITRESOLVE, /* Marker for last init-time resolution. */ > #ifdef LIBMUDFLAPTH > diff --git a/libmudflap/mf-runtime.c b/libmudflap/mf-runtime.c > --- a/libmudflap/mf-runtime.c > +++ b/libmudflap/mf-runtime.c > @@ -666,6 +666,9 @@ struct __mf_dynamic_entry __mf_dynamic [ > {NULL, "free", NULL}, > {NULL, "malloc", NULL}, > {NULL, "mmap", NULL}, > +#ifdef HAVE_MMAP64 > + {NULL, "mmap64", NULL}, > +#endif > {NULL, "munmap", NULL}, > {NULL, "realloc", NULL}, > {NULL, "DUMMY", NULL}, /* dyn_INITRESOLVE */ > @@ -781,12 +784,22 @@ __wrap_main (int argc, char* argv[]) > > __mf_register (& errno, sizeof (errno), __MF_TYPE_STATIC, "errno > area"); > > +#if !(defined(__sun__) && defined(__svr4__)) > + /* Conflicts with the automatic registration of __iob[]. */ > __mf_register (stdin, sizeof (*stdin), __MF_TYPE_STATIC, "stdin"); > __mf_register (stdout, sizeof (*stdout), __MF_TYPE_STATIC, "stdout"); > __mf_register (stderr, sizeof (*stderr), __MF_TYPE_STATIC, "stderr"); > +#endif > > /* Make some effort to register ctype.h static arrays. */ > - /* XXX: e.g., on Solaris, may need to register __ctype, _ctype, > __ctype_mask, __toupper, etc. */ > +#if defined(__sun__) && defined(__svr4__) > + /* __ctype[] is declared without size, but MB_CUR_MAX is the last > + member. There seems to be no proper way to determine the size. */ > + __mf_register (__ctype, &MB_CUR_MAX - &__ctype[0] + 1, > __MF_TYPE_STATIC, "__ctype"); > + /* __ctype_mask points at _C_masks[1]. The size can only determined > + using nm on libc.so.1. */ > + __mf_register (__ctype_mask - 1, 1028, __MF_TYPE_STATIC, "_C_masks"); > +#endif > /* On modern Linux GLIBC, these are thread-specific and changeable, > and are dealt > with in mf-hooks2.c. */ > } > diff --git a/libmudflap/testsuite/libmudflap.c/heap-scalestress.c > b/libmudflap/testsuite/libmudflap.c/heap-scalestress.c > --- a/libmudflap/testsuite/libmudflap.c/heap-scalestress.c > +++ b/libmudflap/testsuite/libmudflap.c/heap-scalestress.c > @@ -8,7 +8,7 @@ > #include <unistd.h> > > #ifndef SCALE > -#define SCALE 100000 > +#define SCALE 10000 > #endif > > > diff --git a/libmudflap/testsuite/libmudflap.c/pass-stratcliff.c > b/libmudflap/testsuite/libmudflap.c/pass-stratcliff.c > --- a/libmudflap/testsuite/libmudflap.c/pass-stratcliff.c > +++ b/libmudflap/testsuite/libmudflap.c/pass-stratcliff.c > @@ -1,5 +1,6 @@ > /* Test for string function add boundaries of usable memory. > - Copyright (C) 1996,1997,1999,2000,2001,2002 Free Software Foundation, Inc. > + Copyright (C) 1996, 1997, 1999, 2000, 2001, 2002, 2011 > + Free Software Foundation, Inc. > This file is part of the GNU C Library. > Contributed by Ulrich Drepper <drep...@cygnus.com>, 1996. > > @@ -25,6 +26,8 @@ > test the real implementation. */ > #undef __USE_STRING_INLINES > > +#include "../config.h" > + > #include <errno.h> > #include <stdio.h> > #include <string.h> > @@ -36,6 +39,10 @@ > #define MAX(a, b) ((a) > (b) ? (a) : (b)) > #endif > > +#ifndef MIN > +#define MIN(a, b) ((a) < (b) ? (a) : (b)) > +#endif > + > int > main (int argc, char *argv[]) > { > @@ -153,7 +160,7 @@ main (int argc, char *argv[]) > } > } > > -#if !defined __FreeBSD__ && !(defined __sun__ && defined __svr4__) > +#ifdef HAVE_RAWMEMCHR > /* rawmemchr test */ > for (outer = size - 1; outer >= MAX (0, size - 128); --outer) > { > @@ -250,7 +257,7 @@ main (int argc, char *argv[]) > } > } > > -#ifndef __FreeBSD__ && !(defined __sun__ && defined __svr4__) > +#ifdef HAVE_STPCPY > /* stpcpy test */ > for (outer = size - 1; outer >= MAX (0, size - 128); --outer) > { > @@ -302,7 +309,7 @@ main (int argc, char *argv[]) > result = 1; > } > > -#if !defined __FreeBSD__ && !(defined __sun__ && defined __svr4__) > +#ifdef HAVE_MEMPCPY > /* mempcpy test */ > for (outer = size - 1; outer >= MAX (0, size - 128); --outer) > for (inner = 0; inner < size - outer; ++inner) > diff --git a/libmudflap/testsuite/libmudflap.c/pass47-frag.c > b/libmudflap/testsuite/libmudflap.c/pass47-frag.c > --- a/libmudflap/testsuite/libmudflap.c/pass47-frag.c > +++ b/libmudflap/testsuite/libmudflap.c/pass47-frag.c > @@ -8,3 +8,5 @@ int main () > tolower (buf[4]) == 'o' && tolower ('X') == 'x' && > isdigit (buf[3])) == 0 && isalnum ('4')); > } > + > +/* { dg-warning "cannot track unknown size extern .__ctype." "Solaris > __ctype declared without size" { target *-*-solaris2.* } 0 } */ -- ----------------------------------------------------------------------------- Rainer Orth, Center for Biotechnology, Bielefeld University