[patch, configures] Unify AC_SYS_LARGEFILE exception across dirs

[Jan Kratochvil-3]

Hi,

there was an accepted patch of mine:
        [patch] Include AC_SYS_LARGEFILE for libiberty
        http://gcc.gnu.org/ml/gcc-patches/2008-09/msg01100.html
        [patch] Include AC_SYS_LARGEFILE everywhere
        http://www.sourceware.org/ml/binutils/2008-08/msg00003.html

But it was found it required to have exceptions in bfd/ by Joel Brobecker:
        Re: [RFA] do not try large-files support by default on 32bit sparc-solaris
        http://www.sourceware.org/ml/binutils/2008-11/msg00140.html
        disable largefile support on x86-solaris by default
        http://www.sourceware.org/ml/binutils/2008-11/msg00248.html

It also required to unify the exception also in binutils/ by Nick Clifton:
        http://sourceware.org/bugzilla/show_bug.cgi?id=9992

As the same exception is now required in gdb/:
        [patch] AC_SYS_LARGEFILE
        http://sourceware.org/ml/gdb-patches/2009-11/msg00047.html

It was suggested by Tom Tromey a config/ macro would be appropriate:
        Re: [patch] AC_SYS_LARGEFILE
        http://sourceware.org/ml/gdb-patches/2009-11/msg00058.html

Therefore supplying it here.  I do not have the technical GCC SVN check-in
rights.  Please mirror the changes to config/ and libiberty/ of sourceware CVS.
Ran regression tests on x86_64-fedora11-linux-gnu (x86_64 is a bit pointless).
Did not re-run it on i386 due to some local host problems FAILing the build.

It would be also OK to just check-in the single new file in config/ .

Included for libiberty/ although it probably does not need it as its API does
not expose AC_SYS_LARGEFILE-dependent data structures.  Still a future API
extension could hit such AC_SYS_LARGEFILE-application incompatibility.

Included for lto-plugin/ although it probably does not need it as it just uses
libiberty/ .  It is there more as a unification.

Included for libgfortran/ as it uses libiberty/ to make it compatible.
Currently there is probably no AC_SYS_LARGEFILE-dependent type in its API,
though.


Thanks,
Jan


libgfortran/
2009-11-04  Jan Kratochvil  <jan.kratochvil@...>

        * configure.ac: Call now ACX_LARGEFILE.
        * configure: Regenerate.
        * aclocal.m4: Include ../config/largefile.m4.

libiberty/
2009-11-04  Jan Kratochvil  <jan.kratochvil@...>

        * configure.ac: Call now ACX_LARGEFILE.
        * configure: Regenerate.
        * aclocal.m4: Include ../config/largefile.m4.

lto-plugin/
2009-11-04  Jan Kratochvil  <jan.kratochvil@...>

        * configure.ac: Call now ACX_LARGEFILE.
        * configure: Regenerate.
        * aclocal.m4: Regenerate.

config/
2009-11-04  Jan Kratochvil  <jan.kratochvil@...>

        * largefile.m4: New file.

--- libgfortran/configure.ac (revision 153883)
+++ libgfortran/configure.ac (working copy)
@@ -203,7 +203,7 @@ AC_SUBST(extra_ldflags_libgfortran)
 # error message and bail out.
 LIBGFOR_WORKING_GFORTRAN
 
-AC_SYS_LARGEFILE
+ACX_LARGEFILE
 AC_TYPE_OFF_T
 
 # check header files
--- libiberty/configure.ac (revision 153883)
+++ libiberty/configure.ac (working copy)
@@ -130,7 +130,7 @@ fi
 
 GCC_NO_EXECUTABLES
 AC_PROG_CC
-AC_SYS_LARGEFILE
+ACX_LARGEFILE
 AC_PROG_CPP_WERROR
 
 ACX_PROG_CC_WARNING_OPTS([-W -Wall -Wwrite-strings -Wc++-compat \
--- lto-plugin/configure.ac (revision 153883)
+++ lto-plugin/configure.ac (working copy)
@@ -4,7 +4,7 @@ GCC_TOPLEV_SUBDIRS
 AM_INIT_AUTOMAKE([foreign])
 AM_MAINTAINER_MODE
 AC_PROG_CC
-AC_SYS_LARGEFILE
+ACX_LARGEFILE
 AC_ARG_VAR(LIBELFLIBS,[How to link libelf])
 AC_ARG_VAR(LIBELFINC,[How to find libelf include files])
 AM_PROG_LIBTOOL
--- config/largefile.m4 (revision 0)
+++ config/largefile.m4 (revision 0)
@@ -0,0 +1,27 @@
+# This macro wraps AC_SYS_LARGEFILE with one exception keeping the decision
+# compatible across the sourceware subdirectories.
+
+AC_DEFUN([ACX_LARGEFILE],[dnl
+case "${target}" in
+changequote(,)dnl
+  sparc-*-solaris*|i[3-7]86-*-solaris*)
+changequote([,])dnl
+    # On native 32bit sparc and ia32 solaris, large-file and procfs support
+    # are mutually exclusive; and without procfs support, the elf module
+    # cannot provide certain routines such as elfcore_write_prpsinfo
+    # or elfcore_write_prstatus.  So unless the user explicitly requested
+    # large-file support through the --enable-largefile switch, disable
+    # large-file support in favor of procfs support.
+    # PR 9992/binutils: We have to duplicate the behaviour of bfd's
+    # configure script so that all the directories agree on the size of
+    # structures used to describe files.
+    if test "${target}" = "${host}" -a "$enable_largefile" != 'yes'; then
+      if test "$plugins" = "no"; then
+        enable_largefile="no"
+      fi
+    fi
+    ;;
+esac
+
+AC_SYS_LARGEFILE
+])

[Joseph S. Myers]

On Thu, 5 Nov 2009, Jan Kratochvil wrote:

> Included for libgfortran/ as it uses libiberty/ to make it compatible.
> Currently there is probably no AC_SYS_LARGEFILE-dependent type in its API,
> though.

Target libraries had better not have their ABIs depend on
AC_SYS_LARGEFILE; they should be usable by user programs independent of
whether those programs use the large files ABI themselves.

(And how does it use libiberty?  Target libraries' use of libiberty should
be restricted to those parts with a GPL or LGPL runtime exception, and I
don't actually see any libiberty use in libgfortran.)

--
Joseph S. Myers
joseph@...

[Joel Brobecker]

> I do not have the technical GCC SVN check-in rights.  Please mirror
> the changes to config/ and libiberty/ of sourceware CVS.

I do have the SVN rights and I'll be happy to help committing whather
is approved. Just let me know.


--
Joel

[Jan Kratochvil-3]

On Thu, 05 Nov 2009 00:34:26 +0100, Joseph S. Myers wrote:
> On Thu, 5 Nov 2009, Jan Kratochvil wrote:
>
> > Included for libgfortran/ as it uses libiberty/ to make it compatible.
> > Currently there is probably no AC_SYS_LARGEFILE-dependent type in its API,
> > though.
>
> Target libraries had better not have their ABIs depend on
> AC_SYS_LARGEFILE; they should be usable by user programs independent of
> whether those programs use the large files ABI themselves.

Checked now the libgfortran ABI really uses parameters independent of
AC_SYS_LARGEFILE as you said:
        _gfortran_stat_i4 (gfc_array_i4 *)
        _gfortran_stat_i8 (gfc_array_i8 *)

So AC_SYS_LARGEFILE can be used in libgfortran/ always with no exceptions.
As libgfortran is using the stat() syscalls AC_SYS_LARGEFILE does make sense
there (and it is already present there).  So no change is needed there.


> (And how does it use libiberty?  Target libraries' use of libiberty should
> be restricted to those parts with a GPL or LGPL runtime exception, and I
> don't actually see any libiberty use in libgfortran.)

You are right it does not... I was grepping it but made some mistake, sorry.


Also checked what everything is really being affected by AC_SYS_LARGEFILE:
        http://www.unix.org/version2/whatsnew/lfs20mar.html#3.1.2
so
        egrep -r '\<(blkcnt_t|fsblkcnt_t|fsfilcnt_t|fpos_t|ino_t|off_t|rlim_t|dirent|flock|rlimit|stat|statvfs|F_GETLK|F_SETLK|F_SETLKW|RLIM_INFINITY|RLIM_SAVED_MAX|RLIM_SAVED_CUR)\>' include/
says only
        include/plugin-api.h:  off_t offset;
        include/plugin-api.h:  off_t filesize;
        ->
                struct ld_plugin_input_file
                ld_plugin_claim_file_handler
                ld_plugin_get_input_file
therefore:

libgfortran/ really has no ABI incompatibility possibility.

libiberty/ really has no ABI incompatibility possibility.

lto-plugin/ has the ABI compatibility dependency.

gcc/ contains no AC_SYS_LARGEFILE - also when I needed >2GB .o 32bit file
I had to use x86_64 gcc with -m32.  But this is unrelated to this mail.

As there are currently no real ABI issue for the libiberty/ interface and I do
not understand well now the lto-plugin, gold and gcc dependencies while it is
completely unrelated to a regression in GDB I have reduced my request to:


Please include only the config/ part which is required for bfd/ and all its
ABI-dependent directories.  But none of such directories is in GCC SVN.



Thanks,
Jan


config/
2009-11-04  Jan Kratochvil  <jan.kratochvil@...>

        * largefile.m4: New file.

--- config/largefile.m4 (revision 0)
+++ config/largefile.m4 (revision 0)
@@ -0,0 +1,27 @@
+# This macro wraps AC_SYS_LARGEFILE with one exception keeping the decision
+# compatible across the sourceware subdirectories.
+
+AC_DEFUN([ACX_LARGEFILE],[dnl
+case "${target}" in
+changequote(,)dnl
+  sparc-*-solaris*|i[3-7]86-*-solaris*)
+changequote([,])dnl
+    # On native 32bit sparc and ia32 solaris, large-file and procfs support
+    # are mutually exclusive; and without procfs support, the elf module
+    # cannot provide certain routines such as elfcore_write_prpsinfo
+    # or elfcore_write_prstatus.  So unless the user explicitly requested
+    # large-file support through the --enable-largefile switch, disable
+    # large-file support in favor of procfs support.
+    # PR 9992/binutils: We have to duplicate the behaviour of bfd's
+    # configure script so that all the directories agree on the size of
+    # structures used to describe files.
+    if test "${target}" = "${host}" -a "$enable_largefile" != 'yes'; then
+      if test "$plugins" = "no"; then
+        enable_largefile="no"
+      fi
+    fi
+    ;;
+esac
+
+AC_SYS_LARGEFILE
+])

[Paolo Bonzini-2]

On 11/05/2009 12:20 AM, Jan Kratochvil wrote:
 > +case "${target}" in

This is a host test.  You need

+case "${host}" in

 > +    if test "${target}" = "${host}" -a "$enable_largefile" != 'yes';
then
 > +      if test "$plugins" = "no"; then
 > +        enable_largefile="no"
 > +      fi
 > +    fi

Change these five lines to:

+    test "x$plugins" = xno && : ${enable_largefile="no"}

omitting the libgfortran change, and it's fine for me.

Paolo

[Jan Kratochvil-3]

On Thu, 05 Nov 2009 21:52:14 +0100, Paolo Bonzini wrote:
> On 11/05/2009 12:20 AM, Jan Kratochvil wrote:
> > +case "${target}" in
>
> This is a host test.  You need
>
> +case "${host}" in
>
> > +    if test "${target}" = "${host}" [...]

This was already being discussed in:
        http://www.sourceware.org/ml/binutils/2008-11/msg00095.html

as this exception is required only for this case:

bfd/configure.in contains:
        # If we are configured native, pick a core file support file.
        if test "${target}" = "${host}"; then
          [...]
          # ELF corefile support has several flavors, but all of
          # them use something called <sys/procfs.h>
          AC_CHECK_HEADERS(sys/procfs.h)
          [...]
        fi


> > + [...] -a "$enable_largefile" != 'yes'; then
> > +      if test "$plugins" = "no"; then
> > +        enable_largefile="no"
> > +      fi
> > +    fi
>
> Change these five lines to:
>
> +    test "x$plugins" = xno && : ${enable_largefile="no"}

test "${target}" = "${host}" should remain there as discussed above.

"$enable_largefile" != 'yes' should also remain there if we want to keep the
current IMHO-right behavior existing there and tested for a year in bfd/ of
binutils and gdb releases:

        [RFA] do not try large-files support by default on 32bit sparc-solaris
        http://www.sourceware.org/ml/binutils/2008-11/msg00082.html

        > The proposed patch deactivates support for large-files on 32bit
        > sparc-solaris, unless the user explicitly requested it through
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        > --enable-largefile.
          ^^^^^^^^^^^^^^^^^^


So the acceptable part of the change is:

#  if test "$plugins" = "no"; then
#    enable_largefile="no"
#  fi
->
#  test "x$plugins" = xno && : ${enable_largefile="no"}


Tested artificially (no Solaris box here).



Thanks,
Jan


config/
2009-11-05  Jan Kratochvil  <jan.kratochvil@...>

        * largefile.m4: New file.

--- config/largefile.m4 (revision 0)
+++ config/largefile.m4 (revision 0)
@@ -0,0 +1,25 @@
+# This macro wraps AC_SYS_LARGEFILE with one exception keeping the decision
+# compatible across the sourceware subdirectories.
+
+AC_DEFUN([ACX_LARGEFILE],[dnl
+case "${target}" in
+changequote(,)dnl
+  sparc-*-solaris*|i[3-7]86-*-solaris*)
+changequote([,])dnl
+    # On native 32bit sparc and ia32 solaris, large-file and procfs support
+    # are mutually exclusive; and without procfs support, the elf module
+    # cannot provide certain routines such as elfcore_write_prpsinfo
+    # or elfcore_write_prstatus.  So unless the user explicitly requested
+    # large-file support through the --enable-largefile switch, disable
+    # large-file support in favor of procfs support.
+    # PR 9992/binutils: We have to duplicate the behaviour of bfd's
+    # configure script so that all the directories agree on the size of
+    # structures used to describe files.
+    if test "${target}" = "${host}" -a "$enable_largefile" != 'yes'; then
+      test "x$plugins" = xno && : ${enable_largefile="no"}
+    fi
+    ;;
+esac
+
+AC_SYS_LARGEFILE
+])

[Paolo Bonzini-2]

On 11/05/2009 11:11 PM, Jan Kratochvil wrote:
> On Thu, 05 Nov 2009 21:52:14 +0100, Paolo Bonzini wrote:
>> On 11/05/2009 12:20 AM, Jan Kratochvil wrote:
>>> +case "${target}" in
>>
>> This is a host test.  You need
>>
>> +case "${host}" in

Even if the target = host test shall remain, I would say that the case
statement is clearer if you use the host.

>>> +    if test "${target}" = "${host}" [...]
>
> This was already being discussed in:
> http://www.sourceware.org/ml/binutils/2008-11/msg00095.html
>
> as this exception is required only for this case:

I'm not sure I understand the rationale, but that's fine if it was
discussed on the mailing lists.

 > "$enable_largefile" != 'yes' should also remain there if we want to
 > keep the current IMHO-right behavior existing there and tested for a
 > year in bfd/ of binutils and gdb releases:

This was not changed because I used

   : ${enable_largefile=no}

The reason for this change is that the Autoconf test uses
"$enable_largefile" != 'no', so changing anything the user gives
(including --enable-largefile= i.e. the empty string) to 'no' is
undesirable.

Paolo

[Jan Kratochvil-3]

On Fri, 06 Nov 2009 00:31:14 +0100, Paolo Bonzini wrote:
> Even if the target = host test shall remain, I would say that the
> case statement is clearer if you use the host.

Used `case "${host}"'.

> > "$enable_largefile" != 'yes' should also remain there if we want to
> > keep the current IMHO-right behavior existing there and tested for a
> > year in bfd/ of binutils and gdb releases:
>
> This was not changed because I used
>
>   : ${enable_largefile=no}
>
> The reason for this change is that the Autoconf test uses
> "$enable_largefile" != 'no', so changing anything the user gives
> (including --enable-largefile= i.e. the empty string) to 'no' is
> undesirable.

This proposed way there is no way to undo --enable-largefile
/ --disable-largefile present in earlier configure options.

Probably not a problem, included your variant, OK to check-in - also from Joel?


Thanks,
Jan


config/
2009-11-06  Jan Kratochvil  <jan.kratochvil@...>
            Joel Brobecker  <brobecker@...>
            Paolo Bonzini  <bonzini@...>

        * largefile.m4: New file.

--- config/largefile.m4 (revision 0)
+++ config/largefile.m4 (revision 0)
@@ -0,0 +1,23 @@
+# This macro wraps AC_SYS_LARGEFILE with one exception for Solaris.
+# PR 9992/binutils: We have to duplicate the behaviour of bfd's
+# configure script so that all the directories agree on the size of
+# structures used to describe files.
+
+AC_DEFUN([ACX_LARGEFILE],[dnl
+case "${host}" in
+changequote(,)dnl
+  sparc-*-solaris*|i[3-7]86-*-solaris*)
+changequote([,])dnl
+    # On native 32bit sparc and ia32 solaris, large-file and procfs support
+    # are mutually exclusive; and without procfs support, the bfd/ elf module
+    # cannot provide certain routines such as elfcore_write_prpsinfo
+    # or elfcore_write_prstatus.  So unless the user explicitly requested
+    # large-file support through the --enable-largefile switch, disable
+    # large-file support in favor of procfs support.
+    test "${target}" = "${host}" -a "x$plugins" = xno \
+      && : ${enable_largefile="no"}
+    ;;
+esac
+
+AC_SYS_LARGEFILE
+])

[Joel Brobecker]

> Probably not a problem, included your variant, OK to check-in - also
> from Joel?

Looks OK to me.  Just one tiny comment:

> +# This macro wraps AC_SYS_LARGEFILE with one exception for Solaris.
> +# PR 9992/binutils: We have to duplicate the behaviour of bfd's
> +# configure script so that all the directories agree on the size of
> +# structures used to describe files.

Is the comment about duplication still relevant?

Thanks for taking care of this,
--
Joel

[Jan Kratochvil-3]

On Fri, 06 Nov 2009 16:50:32 +0100, Joel Brobecker wrote:
> > +# This macro wraps AC_SYS_LARGEFILE with one exception for Solaris.
> > +# PR 9992/binutils: We have to duplicate the behaviour of bfd's
> > +# configure script so that all the directories agree on the size of
> > +# structures used to describe files.
>
> Is the comment about duplication still relevant?

It is true a better wording is appropriate (although I found the former text
still in some way applicable).

Paolo probably still should approve the last changes before you would check it
in.


Thanks,
Jan


config/
2009-11-06  Jan Kratochvil  <jan.kratochvil@...>
            Joel Brobecker  <brobecker@...>
            Paolo Bonzini  <bonzini@...>

        * largefile.m4: New file.

--- config/largefile.m4 (revision 0)
+++ config/largefile.m4 (revision 0)
@@ -0,0 +1,23 @@
+# This macro wraps AC_SYS_LARGEFILE with one exception for Solaris.
+# PR 9992/binutils: We have to unify the behaviour for configure scripts
+# of directories ABI-dependent on the bfd/ directory so that all the
+# directories agree on the size of structures used to describe files.
+
+AC_DEFUN([ACX_LARGEFILE],[dnl
+case "${host}" in
+changequote(,)dnl
+  sparc-*-solaris*|i[3-7]86-*-solaris*)
+changequote([,])dnl
+    # On native 32bit sparc and ia32 solaris, large-file and procfs support
+    # are mutually exclusive; and without procfs support, the bfd/ elf module
+    # cannot provide certain routines such as elfcore_write_prpsinfo
+    # or elfcore_write_prstatus.  So unless the user explicitly requested
+    # large-file support through the --enable-largefile switch, disable
+    # large-file support in favor of procfs support.
+    test "${target}" = "${host}" -a "x$plugins" = xno \
+      && : ${enable_largefile="no"}
+    ;;
+esac
+
+AC_SYS_LARGEFILE
+])

[Paolo Bonzini-2]

> This proposed way there is no way to undo --enable-largefile
> / --disable-largefile present in earlier configure options.
>
> Probably not a problem, included your variant, OK to check-in - also from Joel?

Yep.

Paolo

[Paolo Bonzini-2]

>> +# This macro wraps AC_SYS_LARGEFILE with one exception for Solaris.
>> +# PR 9992/binutils: We have to duplicate the behaviour of bfd's
>> +# configure script so that all the directories agree on the size of
>> +# structures used to describe files.
>
> Is the comment about duplication still relevant?

Possibly s/duplicate/replicate everywhere/, but I think it's
understandable as is.

Go ahead.

Paolo

[Joel Brobecker]

> config/
> 2009-11-06  Jan Kratochvil  <jan.kratochvil@...>
>    Joel Brobecker  <brobecker@...>
>    Paolo Bonzini  <bonzini@...>
>
> * largefile.m4: New file.

Now checked in both GCC and GDB, with the wording correction suggested
by Paolo. Woohoo :).

--
Joel