Unify Solaris procfs and largefile handling

Message ID yddbll0mvov.fsf@CeBiTec.Uni-Bielefeld.DE
State New
Headers show
Series
  • Unify Solaris procfs and largefile handling
Related show

Commit Message

Rainer Orth June 30, 2020, 3:15 p.m.
GDB currently doesn't build on 32-bit Solaris:

* On Solaris 11.4/x86:

In file included from /usr/include/sys/procfs.h:26,
                 from /vol/src/gnu/gdb/hg/master/dist/gdb/i386-sol2-nat.c:24:
/usr/include/sys/old_procfs.h:31:2: error: #error "Cannot use procfs in the large file compilation environment"
 #error "Cannot use procfs in the large file compilation environment"
  ^~~~~

* On Solaris 11.3/x86 there are several more instances of this.

The interaction between procfs and large-file support historically has
been a royal mess on Solaris:

* There are two versions of the procfs interface:

** The old ioctl-based /proc, deprecated and not used any longer in
   either gdb or binutils.

** The `new' (introduced in Solaris 2.6, 1997) structured /proc.

* There are two headers one can possibly include:

** <procfs.h> which only provides the structured /proc, definining
   _STRUCTURED_PROC=1 and then including ...

** <sys/procfs.h> which defaults to _STRUCTURED_PROC=0, the ioctl-based
   /proc, but provides structured /proc if _STRUCTURED_PROC == 1.

* procfs and the large-file environment didn't go well together:

** Until Solaris 11.3, <sys/procfs.h> would always #error in 32-bit
   compilations when the large-file environment was active
   (_FILE_OFFSET_BITS == 64).

** In both Solaris 11.4 and Illumos, this restriction was lifted for
   structured /proc.

So one has to be careful always to define _STRUCTURED_PROC=1 when
testing for or using <sys/procfs.h> on Solaris.  As the errors above
show, this isn't always the case in binutils-gdb right now.

Also one may need to disable large-file support for 32-bit compilations
on Solaris.  config/largefile.m4 meant to do this by wrapping the
AC_SYS_LARGEFILE autoconf macro with appropriate checks, yielding
ACX_LARGEFILE.  Unfortunately the macro doesn't always succeed because
it neglects the _STRUCTURED_PROC part.

To make things even worse, since GCC 9 g++ predefines
_FILE_OFFSET_BITS=64 on Solaris.  So even if largefile.m4 deciced not to
enable large-file support, this has no effect, breaking the gdb build.

This patch addresses all this as follows:

* All tests for the <sys/procfs.h> header are made with
  _STRUCTURED_PROC=1, the definition going into the various config.h
  files instead of having to make them (and sometimes failing) in the
  affected sources.

* To cope with the g++ predefine of _FILE_OFFSET_BITS=64,
  -U_FILE_OFFSET_BITS is added to various *_CPPFLAGS variables.  It had
  been far easier to have just

  #undef _FILE_OFFSET_BITS

  in config.h, but unfortunately such a construct in config.in is
  commented by config.status irrespective of indentation and whitespace
  if large-file support is disabled.  I found no way around this and
  putting the #undef in several global headers for bfd, binutils, ld,
  and gdb seemed way more invasive.

* Last, the applicability check in largefile.m4 was modified only to
  disable largefile support if really needed.  To do so, it checks if
  <sys/procfs.h> compiles with _FILE_OFFSET_BITS=64 defined.  If it
  doesn't, the disabling only happens if gdb exists in-tree and isn't
  disabled, otherwise (building binutils from a tarball), there's no
  conflict.

  What initially confused me was the check for $plugins here, which
  originally caused the disabling not to take place.  Since AC_PLUGINGS
  does enable plugin support if <dlfcn.h> exists (which it does on
  Solaris), the disabling never happened.

  I could find no explanation why the linker plugin needs large-file
  support but thought it would be enough if gld and GCC's lto-plugin
  agreed on the _FILE_OFFSET_BITS value.  Unfortunately, that's not
  enough: lto-plugin uses the simple-object interface from libiberty,
  which includes off_t arguments.  So to fully disable large-file
  support would mean also disabling it in libiberty and its users: gcc
  and libstdc++-v3.  This seems highly undesirable, so I decided to
  disable the linker plugin instead if large-file support won't work.

The patch allows binutils+gdb to build on i386-pc-solaris2.11 (both
Solaris 11.3 and 11.4, using GCC 9.3.0 which is the worst case due to
predefined _FILE_OFFSET_BITS=64).  Also regtested on
amd64-pc-solaris2.11 (again on Solaris 11.3 and 11.4),
x86_64-pc-linux-gnu and i686-pc-linux-gnu.

Ok for master?  While it would be nice to have this in the binutils 2.35
and gdb 10 releases, I'd fully understand if the patch were considered
too risky so close to the branch dates.

	Rainer

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


2020-06-25  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	config:
	* largefile.m4 (ACX_LARGEFILE) <sparc-*-solaris*|i?86-*-solaris*>:
	Check for <sys/procfs.h> incompatilibity with large-file support
	on Solaris.
	Only disable large-file support and perhaps plugins if needed.
	Set, substitute LARGEFILE_CPPFLAGS if so.

	bfd:
	* bfd.m4 (BFD_SYS_PROCFS_H): New macro.
	(BFD_HAVE_SYS_PROCFS_TYPE): Require BFD_SYS_PROCFS_H.
	Don't define _STRUCTURED_PROC.
	(BFD_HAVE_SYS_PROCFS_TYPE_MEMBER): Likewise.
	* elf.c [HAVE_SYS_PROCFS_H] (_STRUCTURED_PROC): Don't define.
	* configure.ac: Use BFD_SYS_PROCFS_H to check for <sys/procfs.h>.
	* configure, config.in: Regenerate.
	* Makefile.am (AM_CPPFLAGS): Add LARGEFILE_CPPFLAGS.
	* Makefile.in, doc/Makefile.in: Regenerate.

	binutils:
	* Makefile.am (AM_CPPFLAGS): Add LARGEFILE_CPPFLAGS.
	* Makefile.in, doc/Makefile.in: Regenerate.
	* configure: Regenerate.

	gas:
	* Makefile.am (AM_CPPFLAGS): Add LARGEFILE_CPPFLAGS.
	* Makefile.in, doc/Makefile.in: Regenerate.
	* configure: Regenerate.

	gdb:
	* proc-api.c (_STRUCTURED_PROC): Don't define.
	* proc-events.c: Likewise.
	* proc-flags.c: Likewise.
	* proc-why.c: Likewise.
	* procfs.c: Likewise.

	* Makefile.in (INTERNAL_CPPFLAGS): Add LARGEFILE_CPPFLAGS.
	* configure, config.in: Regenerate.

	gdbserver:
	* configure, config.in: Regenerate.

	gdbsupport:
	* Makefile.am (AM_CPPFLAGS): Add LARGEFILE_CPPFLAGS.
	* common.m4 (GDB_AC_COMMON): Use BFD_SYS_PROCFS_H to check for
	<sys/procfs.h>.
	* Makefile.in: Regenerate.
	* configure, config.in: Regenerate.

	gnulib:
	* configure.ac: Run ACX_LARGEFILE before gl_EARLY.
	* configure: Regenerate.

	gprof:
	* Makefile.am (AM_CPPFLAGS): Add LARGEFILE_CPPFLAGS.
	* Makefile.in: Regenerate.
	* configure: Regenerate.

	ld:
	* Makefile.am (AM_CPPFLAGS): Add LARGEFILE_CPPFLAGS.
	* Makefile.in: Regenerate.
	* configure: Regenerate.

Comments

Rainer Orth July 20, 2020, 11:28 a.m. | #1
Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:

> Could someone please review the GDB side of this patch?  It's more than

> a week now.

>

> Nick already approved the binutils part, but it needs to go in as a

> whole.  binutils 2.35 has branched already and I suspect he will be

> increasingly wary to allow it into the branch the closer the release

> date gets.


11 days gone since the last reminder...

Could someone please review the gdb parts of this patch?  As I'd
mentioned, the binutils side of things has already been approved, but
parts of config/largefile.m4 primarily affect gdb, so a second pair of
eyes there would be helpful.

Thanks.
	Rainer


>> GDB currently doesn't build on 32-bit Solaris:

>>

>> * On Solaris 11.4/x86:

>>

>> In file included from /usr/include/sys/procfs.h:26,

>>                  from /vol/src/gnu/gdb/hg/master/dist/gdb/i386-sol2-nat.c:24:

>> /usr/include/sys/old_procfs.h:31:2: error: #error "Cannot use procfs in the large file compilation environment"

>>  #error "Cannot use procfs in the large file compilation environment"

>>   ^~~~~

>>

>> * On Solaris 11.3/x86 there are several more instances of this.

>>

>> The interaction between procfs and large-file support historically has

>> been a royal mess on Solaris:

>>

>> * There are two versions of the procfs interface:

>>

>> ** The old ioctl-based /proc, deprecated and not used any longer in

>>    either gdb or binutils.

>>

>> ** The `new' (introduced in Solaris 2.6, 1997) structured /proc.

>>

>> * There are two headers one can possibly include:

>>

>> ** <procfs.h> which only provides the structured /proc, definining

>>    _STRUCTURED_PROC=1 and then including ...

>>

>> ** <sys/procfs.h> which defaults to _STRUCTURED_PROC=0, the ioctl-based

>>    /proc, but provides structured /proc if _STRUCTURED_PROC == 1.

>>

>> * procfs and the large-file environment didn't go well together:

>>

>> ** Until Solaris 11.3, <sys/procfs.h> would always #error in 32-bit

>>    compilations when the large-file environment was active

>>    (_FILE_OFFSET_BITS == 64).

>>

>> ** In both Solaris 11.4 and Illumos, this restriction was lifted for

>>    structured /proc.

>>

>> So one has to be careful always to define _STRUCTURED_PROC=1 when

>> testing for or using <sys/procfs.h> on Solaris.  As the errors above

>> show, this isn't always the case in binutils-gdb right now.

>>

>> Also one may need to disable large-file support for 32-bit compilations

>> on Solaris.  config/largefile.m4 meant to do this by wrapping the

>> AC_SYS_LARGEFILE autoconf macro with appropriate checks, yielding

>> ACX_LARGEFILE.  Unfortunately the macro doesn't always succeed because

>> it neglects the _STRUCTURED_PROC part.

>>

>> To make things even worse, since GCC 9 g++ predefines

>> _FILE_OFFSET_BITS=64 on Solaris.  So even if largefile.m4 deciced not to

>> enable large-file support, this has no effect, breaking the gdb build.

>>

>> This patch addresses all this as follows:

>>

>> * All tests for the <sys/procfs.h> header are made with

>>   _STRUCTURED_PROC=1, the definition going into the various config.h

>>   files instead of having to make them (and sometimes failing) in the

>>   affected sources.

>>

>> * To cope with the g++ predefine of _FILE_OFFSET_BITS=64,

>>   -U_FILE_OFFSET_BITS is added to various *_CPPFLAGS variables.  It had

>>   been far easier to have just

>>

>>   #undef _FILE_OFFSET_BITS

>>

>>   in config.h, but unfortunately such a construct in config.in is

>>   commented by config.status irrespective of indentation and whitespace

>>   if large-file support is disabled.  I found no way around this and

>>   putting the #undef in several global headers for bfd, binutils, ld,

>>   and gdb seemed way more invasive.

>>

>> * Last, the applicability check in largefile.m4 was modified only to

>>   disable largefile support if really needed.  To do so, it checks if

>>   <sys/procfs.h> compiles with _FILE_OFFSET_BITS=64 defined.  If it

>>   doesn't, the disabling only happens if gdb exists in-tree and isn't

>>   disabled, otherwise (building binutils from a tarball), there's no

>>   conflict.

>>

>>   What initially confused me was the check for $plugins here, which

>>   originally caused the disabling not to take place.  Since AC_PLUGINGS

>>   does enable plugin support if <dlfcn.h> exists (which it does on

>>   Solaris), the disabling never happened.

>>

>>   I could find no explanation why the linker plugin needs large-file

>>   support but thought it would be enough if gld and GCC's lto-plugin

>>   agreed on the _FILE_OFFSET_BITS value.  Unfortunately, that's not

>>   enough: lto-plugin uses the simple-object interface from libiberty,

>>   which includes off_t arguments.  So to fully disable large-file

>>   support would mean also disabling it in libiberty and its users: gcc

>>   and libstdc++-v3.  This seems highly undesirable, so I decided to

>>   disable the linker plugin instead if large-file support won't work.

>>

>> The patch allows binutils+gdb to build on i386-pc-solaris2.11 (both

>> Solaris 11.3 and 11.4, using GCC 9.3.0 which is the worst case due to

>> predefined _FILE_OFFSET_BITS=64).  Also regtested on

>> amd64-pc-solaris2.11 (again on Solaris 11.3 and 11.4),

>> x86_64-pc-linux-gnu and i686-pc-linux-gnu.

>>

>> Ok for master?  While it would be nice to have this in the binutils 2.35

>> and gdb 10 releases, I'd fully understand if the patch were considered

>> too risky so close to the branch dates.

>>

>> 	Rainer


-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University
Simon Marchi July 28, 2020, 1:47 p.m. | #2
On 2020-06-30 11:15 a.m., Rainer Orth wrote:
> GDB currently doesn't build on 32-bit Solaris:

> 

> * On Solaris 11.4/x86:

> 

> In file included from /usr/include/sys/procfs.h:26,

>                  from /vol/src/gnu/gdb/hg/master/dist/gdb/i386-sol2-nat.c:24:

> /usr/include/sys/old_procfs.h:31:2: error: #error "Cannot use procfs in the large file compilation environment"

>  #error "Cannot use procfs in the large file compilation environment"

>   ^~~~~

> 

> * On Solaris 11.3/x86 there are several more instances of this.

> 

> The interaction between procfs and large-file support historically has

> been a royal mess on Solaris:

> 

> * There are two versions of the procfs interface:

> 

> ** The old ioctl-based /proc, deprecated and not used any longer in

>    either gdb or binutils.

> 

> ** The `new' (introduced in Solaris 2.6, 1997) structured /proc.

> 

> * There are two headers one can possibly include:

> 

> ** <procfs.h> which only provides the structured /proc, definining

>    _STRUCTURED_PROC=1 and then including ...

> 

> ** <sys/procfs.h> which defaults to _STRUCTURED_PROC=0, the ioctl-based

>    /proc, but provides structured /proc if _STRUCTURED_PROC == 1.

> 

> * procfs and the large-file environment didn't go well together:

> 

> ** Until Solaris 11.3, <sys/procfs.h> would always #error in 32-bit

>    compilations when the large-file environment was active

>    (_FILE_OFFSET_BITS == 64).

> 

> ** In both Solaris 11.4 and Illumos, this restriction was lifted for

>    structured /proc.

> 

> So one has to be careful always to define _STRUCTURED_PROC=1 when

> testing for or using <sys/procfs.h> on Solaris.  As the errors above

> show, this isn't always the case in binutils-gdb right now.

> 

> Also one may need to disable large-file support for 32-bit compilations

> on Solaris.  config/largefile.m4 meant to do this by wrapping the

> AC_SYS_LARGEFILE autoconf macro with appropriate checks, yielding

> ACX_LARGEFILE.  Unfortunately the macro doesn't always succeed because

> it neglects the _STRUCTURED_PROC part.

> 

> To make things even worse, since GCC 9 g++ predefines

> _FILE_OFFSET_BITS=64 on Solaris.  So even if largefile.m4 deciced not to

> enable large-file support, this has no effect, breaking the gdb build.

> 

> This patch addresses all this as follows:

> 

> * All tests for the <sys/procfs.h> header are made with

>   _STRUCTURED_PROC=1, the definition going into the various config.h

>   files instead of having to make them (and sometimes failing) in the

>   affected sources.

> 

> * To cope with the g++ predefine of _FILE_OFFSET_BITS=64,

>   -U_FILE_OFFSET_BITS is added to various *_CPPFLAGS variables.  It had

>   been far easier to have just

> 

>   #undef _FILE_OFFSET_BITS

> 

>   in config.h, but unfortunately such a construct in config.in is

>   commented by config.status irrespective of indentation and whitespace

>   if large-file support is disabled.  I found no way around this and

>   putting the #undef in several global headers for bfd, binutils, ld,

>   and gdb seemed way more invasive.

> 

> * Last, the applicability check in largefile.m4 was modified only to

>   disable largefile support if really needed.  To do so, it checks if

>   <sys/procfs.h> compiles with _FILE_OFFSET_BITS=64 defined.  If it

>   doesn't, the disabling only happens if gdb exists in-tree and isn't

>   disabled, otherwise (building binutils from a tarball), there's no

>   conflict.

> 

>   What initially confused me was the check for $plugins here, which

>   originally caused the disabling not to take place.  Since AC_PLUGINGS

>   does enable plugin support if <dlfcn.h> exists (which it does on

>   Solaris), the disabling never happened.

> 

>   I could find no explanation why the linker plugin needs large-file

>   support but thought it would be enough if gld and GCC's lto-plugin

>   agreed on the _FILE_OFFSET_BITS value.  Unfortunately, that's not

>   enough: lto-plugin uses the simple-object interface from libiberty,

>   which includes off_t arguments.  So to fully disable large-file

>   support would mean also disabling it in libiberty and its users: gcc

>   and libstdc++-v3.  This seems highly undesirable, so I decided to

>   disable the linker plugin instead if large-file support won't work.

> 

> The patch allows binutils+gdb to build on i386-pc-solaris2.11 (both

> Solaris 11.3 and 11.4, using GCC 9.3.0 which is the worst case due to

> predefined _FILE_OFFSET_BITS=64).  Also regtested on

> amd64-pc-solaris2.11 (again on Solaris 11.3 and 11.4),

> x86_64-pc-linux-gnu and i686-pc-linux-gnu.

> 

> Ok for master?  While it would be nice to have this in the binutils 2.35

> and gdb 10 releases, I'd fully understand if the patch were considered

> too risky so close to the branch dates.

> 

> 	Rainer


I understood some (not all of this).

Regarding the "old" and "new" procfs interfaces, can't you just always include
procfs.h and never sys/procfs.h?  Using sys/procfs.h seems like it would only
be useful for pre-1997 Solaris, which doesn't seem useful.  And then you could
have large file support always on?

Simon
Rainer Orth July 28, 2020, 1:51 p.m. | #3
Hi Simon,

> On 2020-06-30 11:15 a.m., Rainer Orth wrote:

>> GDB currently doesn't build on 32-bit Solaris:

>> 

>> * On Solaris 11.4/x86:

>> 

>> In file included from /usr/include/sys/procfs.h:26,

>>                  from /vol/src/gnu/gdb/hg/master/dist/gdb/i386-sol2-nat.c:24:

>> /usr/include/sys/old_procfs.h:31:2: error: #error "Cannot use procfs in the large file compilation environment"

>>  #error "Cannot use procfs in the large file compilation environment"

>>   ^~~~~

>> 

>> * On Solaris 11.3/x86 there are several more instances of this.

>> 

>> The interaction between procfs and large-file support historically has

>> been a royal mess on Solaris:

>> 

>> * There are two versions of the procfs interface:

>> 

>> ** The old ioctl-based /proc, deprecated and not used any longer in

>>    either gdb or binutils.

>> 

>> ** The `new' (introduced in Solaris 2.6, 1997) structured /proc.

>> 

>> * There are two headers one can possibly include:

>> 

>> ** <procfs.h> which only provides the structured /proc, definining

>>    _STRUCTURED_PROC=1 and then including ...

>> 

>> ** <sys/procfs.h> which defaults to _STRUCTURED_PROC=0, the ioctl-based

>>    /proc, but provides structured /proc if _STRUCTURED_PROC == 1.

>> 

>> * procfs and the large-file environment didn't go well together:

>> 

>> ** Until Solaris 11.3, <sys/procfs.h> would always #error in 32-bit

>>    compilations when the large-file environment was active

>>    (_FILE_OFFSET_BITS == 64).

>> 

>> ** In both Solaris 11.4 and Illumos, this restriction was lifted for

>>    structured /proc.

>> 

>> So one has to be careful always to define _STRUCTURED_PROC=1 when

>> testing for or using <sys/procfs.h> on Solaris.  As the errors above

>> show, this isn't always the case in binutils-gdb right now.

>> 

>> Also one may need to disable large-file support for 32-bit compilations

>> on Solaris.  config/largefile.m4 meant to do this by wrapping the

>> AC_SYS_LARGEFILE autoconf macro with appropriate checks, yielding

>> ACX_LARGEFILE.  Unfortunately the macro doesn't always succeed because

>> it neglects the _STRUCTURED_PROC part.

>> 

>> To make things even worse, since GCC 9 g++ predefines

>> _FILE_OFFSET_BITS=64 on Solaris.  So even if largefile.m4 deciced not to

>> enable large-file support, this has no effect, breaking the gdb build.

>> 

>> This patch addresses all this as follows:

>> 

>> * All tests for the <sys/procfs.h> header are made with

>>   _STRUCTURED_PROC=1, the definition going into the various config.h

>>   files instead of having to make them (and sometimes failing) in the

>>   affected sources.

>> 

>> * To cope with the g++ predefine of _FILE_OFFSET_BITS=64,

>>   -U_FILE_OFFSET_BITS is added to various *_CPPFLAGS variables.  It had

>>   been far easier to have just

>> 

>>   #undef _FILE_OFFSET_BITS

>> 

>>   in config.h, but unfortunately such a construct in config.in is

>>   commented by config.status irrespective of indentation and whitespace

>>   if large-file support is disabled.  I found no way around this and

>>   putting the #undef in several global headers for bfd, binutils, ld,

>>   and gdb seemed way more invasive.

>> 

>> * Last, the applicability check in largefile.m4 was modified only to

>>   disable largefile support if really needed.  To do so, it checks if

>>   <sys/procfs.h> compiles with _FILE_OFFSET_BITS=64 defined.  If it

>>   doesn't, the disabling only happens if gdb exists in-tree and isn't

>>   disabled, otherwise (building binutils from a tarball), there's no

>>   conflict.

>> 

>>   What initially confused me was the check for $plugins here, which

>>   originally caused the disabling not to take place.  Since AC_PLUGINGS

>>   does enable plugin support if <dlfcn.h> exists (which it does on

>>   Solaris), the disabling never happened.

>> 

>>   I could find no explanation why the linker plugin needs large-file

>>   support but thought it would be enough if gld and GCC's lto-plugin

>>   agreed on the _FILE_OFFSET_BITS value.  Unfortunately, that's not

>>   enough: lto-plugin uses the simple-object interface from libiberty,

>>   which includes off_t arguments.  So to fully disable large-file

>>   support would mean also disabling it in libiberty and its users: gcc

>>   and libstdc++-v3.  This seems highly undesirable, so I decided to

>>   disable the linker plugin instead if large-file support won't work.

>> 

>> The patch allows binutils+gdb to build on i386-pc-solaris2.11 (both

>> Solaris 11.3 and 11.4, using GCC 9.3.0 which is the worst case due to

>> predefined _FILE_OFFSET_BITS=64).  Also regtested on

>> amd64-pc-solaris2.11 (again on Solaris 11.3 and 11.4),

>> x86_64-pc-linux-gnu and i686-pc-linux-gnu.

>> 

>> Ok for master?  While it would be nice to have this in the binutils 2.35

>> and gdb 10 releases, I'd fully understand if the patch were considered

>> too risky so close to the branch dates.

>> 

>> 	Rainer

>

> I understood some (not all of this).

>

> Regarding the "old" and "new" procfs interfaces, can't you just always include

> procfs.h and never sys/procfs.h?  Using sys/procfs.h seems like it would only

> be useful for pre-1997 Solaris, which doesn't seem useful.  And then you could

> have large file support always on?


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

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University
Simon Marchi July 28, 2020, 2:17 p.m. | #4
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.

In your patch, can

  : ${enable_largefile="no"}

become just

  enable_largefile="no"

?

Simon
Simon Marchi July 29, 2020, 7:55 p.m. | #5
On 2020-07-29 7:19 a.m., Rainer Orth wrote:
> It's even simpler: every configure script has code to parse

> --enable-foo/--disable-foo and turn the result into enable_foo=[yes|no].  


Ok, nice!

> 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.


Ack.

> 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?


When I run `autoreconf -vf`, I get a lot of changes.  Make sure to run
it in any directory you touch that has a configure.ac and add the resulting
changes.

Simon
Joel Brobecker Aug. 7, 2020, 3:12 p.m. | #6
> > I don't usually include generated files in patch submissions, though:

> > they are heavily frowned upon at least over in GCC because they make

> > review quite difficult, espcially in a case like this where the

> > largefile.m4 change spreads to lots of configure scripts, obscuring the

> > change proper.

> > 

> > Does GDB handle things differently here?

> 

> I don't think there's a hard rule.  If it makes the patch too big for

> sending on the list, then it's fine for sure to not include them.  If

> you don't want to include them, that's fine with my too, but in either

> case it's important to say that you've omitted them on purpose so we

> know it's not an oversight (otherwise I'll complain about them missing

> :)).


Historically, we have generally avoided to include them, and if memory
serves me right, we've asked people to exclude them from the diff
sent for review, because they tend to be mostly noise. We still knew
that the contributor wasn't forgetting to recreate them thanks to
the ChangeLog entry mentioning the list of files being regenerated.

That being said, since the switch to git, and in particular with
the use of git-send-email which is uber handy, it's been easier to
forget about the stripping. I don't think anyone's made a comment
against small diffs of regenerated files.

One thing about having the being sent as part of the review is that
people can then verify that the files are regenerated using the correct
version of the autotools...

-- 
Joel

Patch

# HG changeset patch
# Parent  1744e2404ea984f5881de57ecb5a23f789ac29f3
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/bfd.m4 b/bfd/bfd.m4
--- a/bfd/bfd.m4
+++ b/bfd/bfd.m4
@@ -17,15 +17,20 @@  dnl along with this program; see the fil
 dnl <http://www.gnu.org/licenses/>.
 dnl
 
+dnl Check for sys/procfs.h, enforcing structured /proc on Solaris.
+
+AC_DEFUN([BFD_SYS_PROCFS_H],
+[AC_DEFINE(_STRUCTURED_PROC, 1, [Use structured /proc on Solaris.])
+ AC_CHECK_HEADERS(sys/procfs.h)])
+
 dnl Check for existence of a type $1 in sys/procfs.h
 
 AC_DEFUN([BFD_HAVE_SYS_PROCFS_TYPE],
-[AC_MSG_CHECKING([for $1 in sys/procfs.h])
+[AC_REQUIRE([BFD_SYS_PROCFS_H])
+ AC_MSG_CHECKING([for $1 in sys/procfs.h])
  AC_CACHE_VAL(bfd_cv_have_sys_procfs_type_$1,
    [AC_TRY_COMPILE([
 #define _SYSCALL32
-/* Needed for new procfs interface on sparc-solaris.  */
-#define _STRUCTURED_PROC 1
 #include <sys/procfs.h>],
       [$1 avar],
       bfd_cv_have_sys_procfs_type_$1=yes,
@@ -41,12 +46,11 @@  AC_DEFUN([BFD_HAVE_SYS_PROCFS_TYPE],
 dnl Check for existence of member $2 in type $1 in sys/procfs.h
 
 AC_DEFUN([BFD_HAVE_SYS_PROCFS_TYPE_MEMBER],
-[AC_MSG_CHECKING([for $1.$2 in sys/procfs.h])
+[AC_REQUIRE([BFD_SYS_PROCFS_H])
+ AC_MSG_CHECKING([for $1.$2 in sys/procfs.h])
  AC_CACHE_VAL(bfd_cv_have_sys_procfs_type_member_$1_$2,
    [AC_TRY_COMPILE([
 #define _SYSCALL32
-/* Needed for new procfs interface on sparc-solaris.  */
-#define _STRUCTURED_PROC 1
 #include <sys/procfs.h>],
       [$1 avar; void* aref = (void*) &avar.$2],
       bfd_cv_have_sys_procfs_type_member_$1_$2=yes,
diff --git a/bfd/configure.ac b/bfd/configure.ac
--- a/bfd/configure.ac
+++ b/bfd/configure.ac
@@ -1046,7 +1046,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
@@ -9471,8 +9471,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,40 @@  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.
+    AC_ARG_ENABLE(gdb,[[  --enable-gdb[=ARG]     build gdb [ARG={yes,no}]]])
+
+    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
@@ -587,7 +587,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