From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26618 invoked by alias); 18 Jun 2012 15:23:46 -0000 Received: (qmail 26605 invoked by uid 22791); 18 Jun 2012 15:23:44 -0000 X-SWARE-Spam-Status: No, hits=-4.3 required=5.0 tests=AWL,BAYES_00,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_HOSTKARMA_W,RCVD_IN_HOSTKARMA_WL,TW_GJ X-Spam-Check-By: sourceware.org Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 18 Jun 2012 15:23:29 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=EU1-MAIL.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1Sgdnc-00003Z-91 from joseph_myers@mentor.com ; Mon, 18 Jun 2012 08:23:28 -0700 Received: from digraph.polyomino.org.uk ([172.16.63.104]) by EU1-MAIL.mgc.mentorg.com with Microsoft SMTPSVC(6.0.3790.1830); Mon, 18 Jun 2012 16:23:26 +0100 Received: from jsm28 (helo=localhost) by digraph.polyomino.org.uk with local-esmtp (Exim 4.74) (envelope-from ) id 1SgdnY-0001oT-W7; Mon, 18 Jun 2012 15:23:25 +0000 Date: Mon, 18 Jun 2012 15:23:00 -0000 From: "Joseph S. Myers" To: Mark Salter cc: libc-ports@sourceware.org Subject: Re: [PATCH] am33: bring port up to date with NPTL support In-Reply-To: <1340030785-32581-1-git-send-email-msalter@redhat.com> Message-ID: References: <1340030785-32581-1-git-send-email-msalter@redhat.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Mailing-List: contact libc-ports-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: libc-ports-owner@sourceware.org X-SW-Source: 2012-06/txt/msg00067.txt.bz2 On Mon, 18 Jun 2012, Mark Salter wrote: > This is a first cut at bringing the AM33 port up to date. The basis of > the NPTL support comes from an older working implementation based on > glibc 2.9. Additional changes have been made to deal with glibc changes > and a newer gcc. Currently, this is untested pending some gcc tls support > fixes. General observations: * New files should use a URL instead of an FSF postal address; that change was made globally in libc and ports. * There should be an ABI test baseline in sysdeps/unix/sysv/linux/am33/nptl/. * There should be a C++ types baseline in data/ (and a localplt one if the generic one doesn't suffice). * sysdeps/unix/sysv/linux/am33/configure.in should change arch_minimum_kernel to 2.6.25, the version where am33 support went into kernel.org kernels, and you should have a kernel-features.h unless the libc version is fully correct about when features were supported on am33. * Use copyright ranges (-2012) in all new or changed files with more than one copyright year. Some files appear to be missing copyright date updates. Specific comments: > diff --git a/sysdeps/am33/atomicity.h b/sysdeps/am33/atomicity.h > index b0ba43d..b3ba5a4 100644 > --- a/sysdeps/am33/atomicity.h > +++ b/sysdeps/am33/atomicity.h You're patching a file that nothing (in libc, ports or this patch) uses. Remove it instead. It's probably a good idea to check all am33 port files (after this patch) to make sure they are actually used. > diff --git a/sysdeps/am33/bits/setjmp.h b/sysdeps/am33/bits/setjmp.h > index 6dd87cb..8d0c11b 100644 > --- a/sysdeps/am33/bits/setjmp.h > +++ b/sysdeps/am33/bits/setjmp.h > +/* Test if longjmp to JMPBUF would unwind the frame > + containing a local variable at ADDRESS. */ > +#define _JMPBUF_UNWINDS(jmpbuf, address) \ > + ((void *) (address) < (void *) (jmpbuf[__JMP_BUF_SP])) This belongs in jmpbuf-unwind.h, not bits/setjmp.h. The other definitions you add belong in jmpbuf-offsets.h, not bits/setjmp.h. > +#if !defined RTLD_BOOTSTRAP || USE___THREAD USE___THREAD is an obsolete macro. > +#if (!defined RTLD_BOOTSTRAP || USE___THREAD) \ Likewise. > diff --git a/sysdeps/am33/elf/configure.in b/sysdeps/am33/elf/configure.in /elf/ sysdeps directories are no longer used, so this belongs directly in sysdeps/am33/. > +if test "$usetls" != no; then Obsolete conditional. > +if test $libc_cv_am33_tls = yes; then > + AC_DEFINE(HAVE_TLS_SUPPORT) > +fi Obsolete macro. Instead, use AC_MSG_ERROR if TLS support is missing. > diff --git a/sysdeps/am33/fpu/feholdexcpt.c b/sysdeps/am33/fpu/feholdexcpt.c > +libm_hidden_def (feholdexcept) > diff --git a/sysdeps/am33/fpu/fesetround.c b/sysdeps/am33/fpu/fesetround.c > +libm_hidden_def (fesetround) I think you need such changes to fegetenv.c, fesetenv.c, feupdateenv.c, fraiseexcpt.c and ftestexcept.c as well. > diff --git a/sysdeps/am33/fpu/libm-test-ulps b/sysdeps/am33/fpu/libm-test-ulps > new file mode 100644 > index 0000000..ad50fcb > --- /dev/null > +++ b/sysdeps/am33/fpu/libm-test-ulps > @@ -0,0 +1,605 @@ That looks too short to have been recently generated ... once you have testing working you should regenerate this for current glibc, starting with an empty file. > diff --git a/sysdeps/am33/init-misc.c b/sysdeps/am33/init-misc.c I do wonder whether overriding this file is really the best way to initialize something architecture-specific ... maybe someone else has a better way to do it? > +#ifdef HAVE_ELF > +#define L(name) .L##name > +#else HAVE_ELF is obsolete. > diff --git a/sysdeps/arm/dl-machine.h b/sysdeps/arm/dl-machine.h You should not be modifying files for another machine when submitting a port. If you wish to submit this patch, you need to do so separately, with its own rationale. > diff --git a/sysdeps/unix/sysv/linux/am33/bits/fcntl.h b/sysdeps/unix/sysv/linux/am33/bits/fcntl.h > index 33b8bcd..588e5fa 100644 > --- a/sysdeps/unix/sysv/linux/am33/bits/fcntl.h > +++ b/sysdeps/unix/sysv/linux/am33/bits/fcntl.h Do you need changes to O_SYNC and O_DSYNC to match 2.6.33+ kernels (see )? You'll want to compare with other architectures - it looks like there are various other ways this patch does not update bits/fcntl.h to be fully in sync regarding what macros are defined under what conditions, what functions are declared, etc. > diff --git a/sysdeps/unix/sysv/linux/am33/bits/mman.h b/sysdeps/unix/sysv/linux/am33/bits/mman.h > index 763b060..ff759b5 100644 > --- a/sysdeps/unix/sysv/linux/am33/bits/mman.h > +++ b/sysdeps/unix/sysv/linux/am33/bits/mman.h Again, does not appear to be a complete update. > diff --git a/sysdeps/unix/sysv/linux/am33/nptl/Versions b/sysdeps/unix/sysv/linux/am33/nptl/Versions > new file mode 100644 > index 0000000..435c921 > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/am33/nptl/Versions > @@ -0,0 +1,8 @@ > +libc { > + GLIBC_PRIVATE { > + # A copy of sigaction lives in NPTL, and needs these. > + __default_sa_restorer; __default_rt_sa_restorer; > + __default_sa_restorer_v1; __default_rt_sa_restorer_v1; > + __default_sa_restorer_v2; __default_rt_sa_restorer_v2; > + } > +} Too much copying from other ports - these are ARM-specific and don't exist for am33. > +strong_alias (__pthread_once, __pthread_once_internal) We removed those aliases, replacing them by hidden_def. There may be more issues with the port not being up to date with global changes, but they would probably be easier to see once the above issues are fixed and the fixed patch checked in, rather than trying to compare the combination of the old port and this patch against other ports. -- Joseph S. Myers joseph@codesourcery.com