This is the mail archive of the
libc-ports@sources.redhat.com
mailing list for the libc-ports project.
Re: [PATCH] am33: bring port up to date with NPTL support
- From: "Joseph S. Myers" <joseph at codesourcery dot com>
- To: Mark Salter <msalter at redhat dot com>
- Cc: libc-ports at sourceware dot org
- Date: Mon, 18 Jun 2012 15:23:24 +0000 (UTC)
- Subject: Re: [PATCH] am33: bring port up to date with NPTL support
- References: <1340030785-32581-1-git-send-email-msalter@redhat.com>
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 (<year>-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
<http://sourceware.org/ml/libc-ports/2009-12/msg00013.html>)?
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