public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>
To: gdb-patches@sourceware.org
Cc: binutils@sourceware.org
Subject: Re: [PATCH] Unify Solaris procfs and largefile handling
Date: Mon, 20 Jul 2020 13:28:19 +0200	[thread overview]
Message-ID: <ydd365mbf3g.fsf@CeBiTec.Uni-Bielefeld.DE> (raw)
In-Reply-To: <yddwo3dgdxf.fsf@CeBiTec.Uni-Bielefeld.DE> (Rainer Orth's message of "Thu, 09 Jul 2020 12:50:36 +0200")

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

  reply	other threads:[~2020-07-20 11:28 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 [this message]
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
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=ydd365mbf3g.fsf@CeBiTec.Uni-Bielefeld.DE \
    --to=ro@cebitec.uni-bielefeld.de \
    --cc=binutils@sourceware.org \
    --cc=gdb-patches@sourceware.org \
    /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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).