Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>
To: Simon Marchi <simark@simark.ca>
Cc: binutils@sourceware.org,  gdb-patches@sourceware.org
Subject: Re: [PATCH] Unify Solaris procfs and largefile handling
Date: Wed, 29 Jul 2020 13:19:27 +0200	[thread overview]
Message-ID: <ydd5za64lhc.fsf@CeBiTec.Uni-Bielefeld.DE> (raw)
In-Reply-To: <5aae580e-ab2e-f008-91c6-f3b1c1f757b7@simark.ca> (Simon Marchi's message of "Tue, 28 Jul 2020 10:17:22 -0400")

[-- Attachment #1: Type: text/plain, Size: 2087 bytes --]

Hi Simon,

> On 2020-07-28 9:51 a.m., Rainer Orth wrote:
>> Unfortunately not: <sys/procfs.h> is sometimes used in code shared with
>> non-Solaris systems, none of which have <procfs.h>.  So we'd have to
>> conditionalize on HAVE_PROCFS_H vs. HAVE_SYS_PROCFS_H.
>> 
>> And on older Solaris 11.3, even when using the new procfs interface,
>> <sys/procfs.h> errors out when largefile support is enabled.
>> 
>> As I said: it's a royal mess ;-(
>> 
>> 	Rainer
>
> Ok, I see.
>
> The only part I'm not sure about is the part that adds --enable-gdb to all
> configure
> files.  For example we now have this in bfd's configure:
>
> $ ./binutils/configure --help | grep gdb
>   --enable-gdb[=ARG]     build gdb [ARG={yes,no}]
>
> I understand that you want to catch whether the user enabled or disabled
> building GDB
> with --enable-gdb or --disable-gdb, but the result is a bit weird.  Is
> there a way not
> to include it in the --help?
>
> Ideally, the top-level configure system would be able to tell which modules
> are enabled.
> I don't know much about it, maybe there's a way.

It's even simpler: every configure script has code to parse
--enable-foo/--disable-foo and turn the result into enable_foo=[yes|no].  

AC_ARG_ENABLE primarily adds default and additional values and properly
aligned help text, none of which is needed in this case.  So just
removing the macro invocation still works.

> In your patch, can
>
>   : ${enable_largefile="no"}
>
> become just
>
>   enable_largefile="no"
>
> ?

No: the code has been (and should remain) like this.  It allows the user
to override the automatic largefile detection with explicit
--enable-largefile/--disable-largefile options without having to change
the code.

I've now removed AC_ARG_ENABLE from largefile.m4.  Retested on
i386-pc-solaris2.11 without and with --disable-gdb, checking that
_FILE_OFFSET_BITS are set as expected, and amd64-pc-solaris2.11.

Ok for master now?

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: sol2-procfs-lfs.patch --]
[-- Type: text/x-patch, Size: 8909 bytes --]

# HG changeset patch
# Parent  a6e459650cf69716409ebec7fae0049adaa6abab
Unify Solaris procfs and largefile handling

diff --git a/bfd/Makefile.am b/bfd/Makefile.am
--- a/bfd/Makefile.am
+++ b/bfd/Makefile.am
@@ -53,7 +53,7 @@ ZLIBINC = @zlibinc@
 WARN_CFLAGS = @WARN_CFLAGS@
 NO_WERROR = @NO_WERROR@
 AM_CFLAGS = $(WARN_CFLAGS) $(ZLIBINC)
-AM_CPPFLAGS = -DBINDIR='"$(bindir)"' -DLIBDIR='"$(libdir)"'
+AM_CPPFLAGS = -DBINDIR='"$(bindir)"' -DLIBDIR='"$(libdir)"' @LARGEFILE_CPPFLAGS@
 if PLUGINS
 bfdinclude_HEADERS += $(INCDIR)/plugin-api.h
 LIBDL = @lt_cv_dlopen_libs@
diff --git a/bfd/configure.ac b/bfd/configure.ac
--- a/bfd/configure.ac
+++ b/bfd/configure.ac
@@ -1039,7 +1039,7 @@ changequote([,])dnl
 
   # ELF corefile support has several flavors, but all of
   # them use something called <sys/procfs.h>
-  AC_CHECK_HEADERS(sys/procfs.h)
+  BFD_SYS_PROCFS_H
   if test "$ac_cv_header_sys_procfs_h" = yes; then
     BFD_HAVE_SYS_PROCFS_TYPE(prstatus_t)
     BFD_HAVE_SYS_PROCFS_TYPE(prstatus32_t)
diff --git a/bfd/elf.c b/bfd/elf.c
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -9453,8 +9453,6 @@ bfd_reloc_status_type
    out details about the corefile.  */
 
 #ifdef HAVE_SYS_PROCFS_H
-/* Needed for new procfs interface on sparc-solaris.  */
-# define _STRUCTURED_PROC 1
 # include <sys/procfs.h>
 #endif
 
diff --git a/binutils/Makefile.am b/binutils/Makefile.am
--- a/binutils/Makefile.am
+++ b/binutils/Makefile.am
@@ -116,6 +116,7 @@ INCDIR	= $(BASEDIR)/include
 AM_CPPFLAGS = -I. -I$(srcdir) -I../bfd -I$(BFDDIR) -I$(INCDIR) \
 	 @HDEFINES@ \
 	 @INCINTL@ \
+	 @LARGEFILE_CPPFLAGS@ \
 	 -DLOCALEDIR="\"$(datadir)/locale\"" \
 	 -Dbin_dummy_emulation=$(EMULATION_VECTOR)
 
diff --git a/config/largefile.m4 b/config/largefile.m4
--- a/config/largefile.m4
+++ b/config/largefile.m4
@@ -1,5 +1,5 @@
 # This macro wraps AC_SYS_LARGEFILE with one exception for Solaris.
-# PR 9992/binutils: We have to replicate everywhere the behaviour of
+# PR binutils/9992: We have to replicate everywhere the behaviour of
 # bfd's configure script so that all the directories agree on the size
 # of structures used to describe files.
 
@@ -16,17 +16,38 @@ AC_REQUIRE([AC_CANONICAL_TARGET])
 AC_PLUGINS
 
 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"}
+  sparc-*-solaris*|i?86-*-solaris*)
+    # On native 32-bit Solaris/SPARC and x86, large-file and procfs support
+    # were mutually exclusive until Solaris 11.3.  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.
+    #
+    # Check if <sys/procfs.h> is incompatible with large-file support.
+    AC_TRY_COMPILE([#define _FILE_OFFSET_BITS 64
+#define _STRUCTURED_PROC 1
+#include <sys/procfs.h>], , acx_cv_procfs_lfs=yes, acx_cv_procfs_lfs=no)
+    #
+    # Forcefully disable large-file support only if necessary, gdb is in
+    # tree and enabled.
+    if test "${target}" = "${host}" -a "$acx_cv_procfs_lfs" = no \
+         -a -d $srcdir/../gdb -a "$enable_gdb" != no; then
+      : ${enable_largefile="no"}
+      if test "$plugins" = yes; then
+	AC_MSG_WARN([
+plugin support disabled; require large-file support which is incompatible with GDB.])
+	plugins=no
+      fi
+    fi
+    #
+    # Explicitly undef _FILE_OFFSET_BITS if enable_largefile=no for the
+    # benefit of g++ 9+ which predefines it on Solaris.
+    if test "$enable_largefile" = no; then
+      LARGEFILE_CPPFLAGS="-U_FILE_OFFSET_BITS"
+      AC_SUBST(LARGEFILE_CPPFLAGS)
+    fi
     ;;
 esac
 
diff --git a/gas/Makefile.am b/gas/Makefile.am
--- a/gas/Makefile.am
+++ b/gas/Makefile.am
@@ -389,7 +389,7 @@ INCDIR = $(BASEDIR)/include
 # so that tm.h and config.h will be found in the compilation
 # subdirectory rather than in the source directory.
 AM_CPPFLAGS = -I. -I$(srcdir) -I../bfd -I$(srcdir)/config \
-	-I$(INCDIR) -I$(srcdir)/.. -I$(BFDDIR) @INCINTL@ \
+	-I$(INCDIR) -I$(srcdir)/.. -I$(BFDDIR) @INCINTL@ @LARGEFILE_CPPFLAGS@ \
 	-DLOCALEDIR="\"$(datadir)/locale\""
 
 # How to link with both our special library facilities
diff --git a/gdb/Makefile.in b/gdb/Makefile.in
--- a/gdb/Makefile.in
+++ b/gdb/Makefile.in
@@ -589,7 +589,8 @@ CPPFLAGS = @CPPFLAGS@
 # are sometimes a little generic, we think that the risk of collision
 # with other header files is high.  If that happens, we try to mitigate
 # a bit the consequences by putting the Python includes last in the list.
-INTERNAL_CPPFLAGS = $(CPPFLAGS) @GUILE_CPPFLAGS@ @PYTHON_CPPFLAGS@
+INTERNAL_CPPFLAGS = $(CPPFLAGS) @GUILE_CPPFLAGS@ @PYTHON_CPPFLAGS@ \
+	@LARGEFILE_CPPFLAGS@
 
 # INTERNAL_CFLAGS is the aggregate of all other *CFLAGS macros.
 INTERNAL_CFLAGS_BASE = \
diff --git a/gdb/proc-api.c b/gdb/proc-api.c
--- a/gdb/proc-api.c
+++ b/gdb/proc-api.c
@@ -28,8 +28,6 @@
 #include "gdbcmd.h"
 #include "completer.h"
 
-#define _STRUCTURED_PROC 1
-
 #include <sys/types.h>
 #include <sys/procfs.h>
 #include <sys/proc.h>	/* for struct proc */
diff --git a/gdb/proc-events.c b/gdb/proc-events.c
--- a/gdb/proc-events.c
+++ b/gdb/proc-events.c
@@ -30,8 +30,6 @@
 
 #include "defs.h"
 
-#define _STRUCTURED_PROC 1
-
 #include <sys/types.h>
 #include <sys/procfs.h>
 #include <sys/syscall.h>
diff --git a/gdb/proc-flags.c b/gdb/proc-flags.c
--- a/gdb/proc-flags.c
+++ b/gdb/proc-flags.c
@@ -27,8 +27,6 @@
 
 #include "defs.h"
 
-#define _STRUCTURED_PROC 1
-
 #include <sys/types.h>
 #include <sys/procfs.h>
 
diff --git a/gdb/proc-why.c b/gdb/proc-why.c
--- a/gdb/proc-why.c
+++ b/gdb/proc-why.c
@@ -20,8 +20,6 @@
 
 #include "defs.h"
 
-#define _STRUCTURED_PROC 1
-
 #include <sys/types.h>
 #include <sys/procfs.h>
 
diff --git a/gdb/procfs.c b/gdb/procfs.c
--- a/gdb/procfs.c
+++ b/gdb/procfs.c
@@ -33,8 +33,6 @@
 #include "nat/fork-inferior.h"
 #include "gdbarch.h"
 
-#define _STRUCTURED_PROC 1	/* Should be done by configure script.  */
-
 #include <sys/procfs.h>
 #include <sys/fault.h>
 #include <sys/syscall.h>
diff --git a/gdbsupport/Makefile.am b/gdbsupport/Makefile.am
--- a/gdbsupport/Makefile.am
+++ b/gdbsupport/Makefile.am
@@ -22,7 +22,8 @@ ACLOCAL_AMFLAGS = -I . -I ../config
 
 AM_CPPFLAGS = -I$(srcdir)/../include -I$(srcdir)/../gdb \
     -I../gnulib/import -I$(srcdir)/../gnulib/import \
-    -I.. -I$(srcdir)/.. $(INCINTL) -I../bfd -I$(srcdir)/../bfd
+    -I.. -I$(srcdir)/.. $(INCINTL) -I../bfd -I$(srcdir)/../bfd \
+    @LARGEFILE_CPPFLAGS@
 
 override CXX += $(CXX_DIALECT)
 
diff --git a/gdbsupport/common.m4 b/gdbsupport/common.m4
--- a/gdbsupport/common.m4
+++ b/gdbsupport/common.m4
@@ -46,7 +46,7 @@ AC_DEFUN([GDB_AC_COMMON], [
 		   thread_db.h wait.h dnl
 		   termios.h dnl
 		   dlfcn.h dnl
-		   linux/elf.h sys/procfs.h proc_service.h dnl
+		   linux/elf.h proc_service.h dnl
 		   poll.h sys/poll.h sys/select.h)
 
   AC_FUNC_MMAP
@@ -173,6 +173,7 @@ AC_DEFUN([GDB_AC_COMMON], [
     fi
   fi
 
+  BFD_SYS_PROCFS_H
   if test "$ac_cv_header_sys_procfs_h" = yes; then
     BFD_HAVE_SYS_PROCFS_TYPE(gregset_t)
     BFD_HAVE_SYS_PROCFS_TYPE(fpregset_t)
diff --git a/gnulib/configure.ac b/gnulib/configure.ac
--- a/gnulib/configure.ac
+++ b/gnulib/configure.ac
@@ -27,11 +27,12 @@ AM_MAINTAINER_MODE
 
 AC_PROG_CC
 AC_USE_SYSTEM_EXTENSIONS
+# Needs to run before gl_EARLY so it can override AC_SYS_LARGEFILE included
+# there.
+ACX_LARGEFILE
 gl_EARLY
 AM_PROG_CC_STDC
 
-ACX_LARGEFILE
-
 AC_CONFIG_AUX_DIR(..)
 AC_CANONICAL_SYSTEM
 
diff --git a/gprof/Makefile.am b/gprof/Makefile.am
--- a/gprof/Makefile.am
+++ b/gprof/Makefile.am
@@ -34,7 +34,7 @@ NO_WERROR = @NO_WERROR@
 AM_CFLAGS = $(WARN_CFLAGS)
 
 AM_CPPFLAGS = -DDEBUG -I../bfd -I$(srcdir)/../include \
-	-I$(srcdir)/../bfd @INCINTL@ -I. \
+	-I$(srcdir)/../bfd @INCINTL@ @LARGEFILE_CPPFLAGS@ -I. \
 	-DLOCALEDIR="\"$(datadir)/locale\""
 
 bin_PROGRAMS = gprof
diff --git a/ld/Makefile.am b/ld/Makefile.am
--- a/ld/Makefile.am
+++ b/ld/Makefile.am
@@ -139,7 +139,7 @@ TEXI2DVI = texi2dvi -I $(srcdir) -I $(BF
 		    -I $(top_srcdir)/../libiberty
 
 AM_CPPFLAGS = -I. -I$(srcdir) -I../bfd -I$(BFDDIR) -I$(INCDIR) @zlibinc@ \
-	@INCINTL@ $(HDEFINES) $(CFLAGS) \
+	@INCINTL@ $(HDEFINES) $(CFLAGS) @LARGEFILE_CPPFLAGS@ \
 	-DLOCALEDIR="\"$(datadir)/locale\""
 
 BFDLIB = ../bfd/libbfd.la

  reply	other threads:[~2020-07-29 11:19 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-30 15:15 Rainer Orth
2020-07-01 12:46 ` Nick Clifton
2020-07-09 10:50 ` Rainer Orth
2020-07-20 11:28   ` Rainer Orth
2020-07-28 13:03     ` Rainer Orth
2020-07-28 13:47 ` Simon Marchi
2020-07-28 13:51   ` Rainer Orth
2020-07-28 14:17     ` Simon Marchi
2020-07-29 11:19       ` Rainer Orth [this message]
2020-07-29 19:55         ` Simon Marchi
2020-07-30  9:17           ` Rainer Orth
2020-07-30 12:43             ` Simon Marchi
2020-07-30 13:49               ` Rainer Orth
2020-08-07 15:12               ` Joel Brobecker

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ydd5za64lhc.fsf@CeBiTec.Uni-Bielefeld.DE \
    --to=ro@cebitec.uni-bielefeld.de \
    --cc=binutils@sourceware.org \
    --cc=gdb-patches@sourceware.org \
    --cc=simark@simark.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox