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

Reply via email to