Have g++ define _FILE_OFFSET_BITS=64 on Solaris

Message ID yddy3f88br3.fsf@CeBiTec.Uni-Bielefeld.DE
State New
Headers show
Series
  • Have g++ define _FILE_OFFSET_BITS=64 on Solaris
Related show

Commit Message

Rainer Orth June 21, 2018, 2:17 p.m.
I recently found two libstdc++ testcases failing on some Solaris hosts
for 32-bit only:

FAIL: 27_io/filesystem/operations/space.cc execution test
FAIL: experimental/filesystem/operations/space.cc execution test

Both file in the same way:

terminate called after throwing an instance of 'std::filesystem::__cxx11::filesystem_error'
  what():  filesystem error: cannot get free space: Value too large for defined data type [.]

However, the test PASSes just fine on other systems.

It turns out that the tests FAIL with

statvfs(".", 0xFEFFDB64)                        Err#79 EOVERFLOW

On the failing system, the build filesystem is 3.4 TB, thus the
EOVERFLOW.

It seems g++ on Solaris doesn't fully enable largefile support: it has
-D_LARGEFILE_SOURCE=1 in gcc/config/sol2.h (TARGET_OS_CPP_BUILTINS), but
lacks -D_FILE_OFFSET_BITS=64 which is required to get the
largefile-aware functions (statvfs64 in this case).

The following patch adds that, fixing the two failures.

Bootstrapped without regressions on i386-pc-solaris2.1[01] and
sparc-sun-solaris2.1[01].

Unless someone has an idea why this might cause problems, I'll install
the patch on mainline and backport to the gcc-7 and gcc-8 branches.

	Rainer

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


2018-06-18  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	* config/sol2.h (TARGET_OS_CPP_BUILTINS): Define
	_FILE_OFFSET_BITS=64 for C++.

Comments

Jonathan Wakely June 21, 2018, 2:32 p.m. | #1
On 21/06/18 16:17 +0200, Rainer Orth wrote:
>I recently found two libstdc++ testcases failing on some Solaris hosts

>for 32-bit only:

>

>FAIL: 27_io/filesystem/operations/space.cc execution test

>FAIL: experimental/filesystem/operations/space.cc execution test

>

>Both file in the same way:

>

>terminate called after throwing an instance of 'std::filesystem::__cxx11::filesystem_error'

>  what():  filesystem error: cannot get free space: Value too large for defined data type [.]

>

>However, the test PASSes just fine on other systems.

>

>It turns out that the tests FAIL with

>

>statvfs(".", 0xFEFFDB64)                        Err#79 EOVERFLOW

>

>On the failing system, the build filesystem is 3.4 TB, thus the

>EOVERFLOW.

>

>It seems g++ on Solaris doesn't fully enable largefile support: it has

>-D_LARGEFILE_SOURCE=1 in gcc/config/sol2.h (TARGET_OS_CPP_BUILTINS), but

>lacks -D_FILE_OFFSET_BITS=64 which is required to get the

>largefile-aware functions (statvfs64 in this case).

>

>The following patch adds that, fixing the two failures.

>

>Bootstrapped without regressions on i386-pc-solaris2.1[01] and

>sparc-sun-solaris2.1[01].

>

>Unless someone has an idea why this might cause problems, I'll install

>the patch on mainline and backport to the gcc-7 and gcc-8 branches.


No objection to this patch, but I'll just note that we have
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81091 suggesting we
should use LFS for libstdc++ unconditionally.
Rainer Orth June 21, 2018, 2:49 p.m. | #2
Hi Jonathan,

> No objection to this patch, but I'll just note that we have

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81091 suggesting we

> should use LFS for libstdc++ unconditionally.


seems like a wise move to me.  The libstdc++.so ABI didn't change on
Solaris either (that possibility had caused concern for me initially);
didn't check libstdc++fs.a though.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University
Jonathan Wakely June 21, 2018, 2:53 p.m. | #3
On 21/06/18 16:49 +0200, Rainer Orth wrote:
>Hi Jonathan,

>

>> No objection to this patch, but I'll just note that we have

>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81091 suggesting we

>> should use LFS for libstdc++ unconditionally.

>

>seems like a wise move to me.  The libstdc++.so ABI didn't change on

>Solaris either (that possibility had caused concern for me initially);

>didn't check libstdc++fs.a though.


Well the main reason that's only a static library for now is to allow
us to make ABI incompatible changes before we declare it stable and
add those symbols to libstdc++.so forever.
Franz Sirl June 21, 2018, 3:17 p.m. | #4
Am 2018-06-21 um 16:17 schrieb Rainer Orth:
> I recently found two libstdc++ testcases failing on some Solaris hosts

> for 32-bit only:

> 

> FAIL: 27_io/filesystem/operations/space.cc execution test

> FAIL: experimental/filesystem/operations/space.cc execution test

> 

> Both file in the same way:

> 

> terminate called after throwing an instance of 'std::filesystem::__cxx11::filesystem_error'

>    what():  filesystem error: cannot get free space: Value too large for defined data type [.]

> 

> However, the test PASSes just fine on other systems.

> 

> It turns out that the tests FAIL with

> 

> statvfs(".", 0xFEFFDB64)                        Err#79 EOVERFLOW

> 

> On the failing system, the build filesystem is 3.4 TB, thus the

> EOVERFLOW.

> 

> It seems g++ on Solaris doesn't fully enable largefile support: it has

> -D_LARGEFILE_SOURCE=1 in gcc/config/sol2.h (TARGET_OS_CPP_BUILTINS), but

> lacks -D_FILE_OFFSET_BITS=64 which is required to get the

> largefile-aware functions (statvfs64 in this case).

> 

> The following patch adds that, fixing the two failures.

> 

> Bootstrapped without regressions on i386-pc-solaris2.1[01] and

> sparc-sun-solaris2.1[01].

> 

> Unless someone has an idea why this might cause problems, I'll install

> the patch on mainline and backport to the gcc-7 and gcc-8 branches.


No idea about possible problems, but isn't it usually recommended to use 
either _FILE_OFFSET_BITS=64 or _LARGEFILE{64}_SOURCE=1, not both at the 
same time?

Franz
Rainer Orth June 22, 2018, 7:43 a.m. | #5
Hi Jonathan,

> On 21/06/18 16:49 +0200, Rainer Orth wrote:

>>Hi Jonathan,

>>

>>> No objection to this patch, but I'll just note that we have

>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81091 suggesting we

>>> should use LFS for libstdc++ unconditionally.

>>

>>seems like a wise move to me.  The libstdc++.so ABI didn't change on

>>Solaris either (that possibility had caused concern for me initially);

>>didn't check libstdc++fs.a though.

>

> Well the main reason that's only a static library for now is to allow

> us to make ABI incompatible changes before we declare it stable and

> add those symbols to libstdc++.so forever.


I suspected that much.  However, once you're reasonable confident the
interface is stable, usability would be much improved by moving into
libstdc++.so: e.g. before Solaris 11.4 sendfile lives in a separate
libsendfile.  This can easily be dealt with as a shared library
dependency, but is harder (or puts the burden on the user) for static
libstdc++fs.a.  One might thing about introducing libstdc++.spec which
could better deal with issues like this, just like libgfortran.spec.
I'd started to work on this long ago do get rid of the hardcoded -lrt in
g++, but never completed it.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University
Rainer Orth June 22, 2018, 7:51 a.m. | #6
Hi Franz,

> No idea about possible problems, but isn't it usually recommended to use

> either _FILE_OFFSET_BITS=64 or _LARGEFILE{64}_SOURCE=1, not both at the

> same time?


quite the contrary: for regular largefile support, you're supposed to
use `getconf LFS_CFLAGS', i.e. -D_LARGEFILE_SOURCE
-D_FILE_OFFSET_BITS=64, while `getconf LFS64_CFLAGS'
(-D_LARGEFILE64_SOURCE) enables the transitional largefile interfaces
(e.g. explicit stat64 calls and struct stat64 instead of making stat and
struct stat largefile-aware).

For all the gory details, see the lfcompile(7), lfcompile64(7), and
lf64(7) man pages:

	https://docs.oracle.com/cd/E88353_01/html/E37853/index.html

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University
Franz Sirl June 22, 2018, 10:08 a.m. | #7
Am 2018-06-22 um 09:51 schrieb Rainer Orth:
> Hi Franz,

> 

>> No idea about possible problems, but isn't it usually recommended to use

>> either _FILE_OFFSET_BITS=64 or _LARGEFILE{64}_SOURCE=1, not both at the

>> same time?

> 

> quite the contrary: for regular largefile support, you're supposed to

> use `getconf LFS_CFLAGS', i.e. -D_LARGEFILE_SOURCE

> -D_FILE_OFFSET_BITS=64, while `getconf LFS64_CFLAGS'

> (-D_LARGEFILE64_SOURCE) enables the transitional largefile interfaces

> (e.g. explicit stat64 calls and struct stat64 instead of making stat and

> struct stat largefile-aware).

> 

> For all the gory details, see the lfcompile(7), lfcompile64(7), and

> lf64(7) man pages:

> 

> 	https://docs.oracle.com/cd/E88353_01/html/E37853/index.html

> 

> 	Rainer

> 


Hi Rainer,

so you are supposed to use "-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64", 
but at least a quick glance at the Sol10 headers shows that the 
additional -D_LARGEFILE_SOURCE only makes a difference for 
fseeko/ftello. That still doesn't explain -D_LARGEFILE64_SOURCE, does 
libstdc++ really need to use _LARGEFILE64_SOURCE functions?

Re-reading lfcompile(7) again shows that you can use either 
"-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64" (for portable applications) 
or only "-D_FILE_OFFSET_BITS=64". But in the GCC case we only need it 
for C++/libstdc++ so it seems "-D_FILE_OFFSET_BITS=64" should be enough. 
The rest is up to the users application, or?
My guess is that without defining _LARGEFILE_SOURCE and 
_LARGEFILE64_SOURCE the configure check in libstdc++-v3/acinclude.m4 
just won't define _GLIBCXX_USE_LFS and everything will fall in place. 
This would leave HPUX as the last user of _GLIBCXX_USE_LFS.

Franz
Rainer Orth June 25, 2018, 1:57 p.m. | #8
Hi Franz,

> so you are supposed to use "-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64",

> but at least a quick glance at the Sol10 headers shows that the additional

> -D_LARGEFILE_SOURCE only makes a difference for fseeko/ftello. That still


right, that's also explained in lfcompile(7).

> doesn't explain -D_LARGEFILE64_SOURCE, does libstdc++ really need to use

> _LARGEFILE64_SOURCE functions?


Honestly, I hadn't checked, but wondered the same thing.  However, I'm a
bit wary to simply remove them after years for fear of breaking user
existing user code.

> Re-reading lfcompile(7) again shows that you can use either

> "-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64" (for portable applications) or

> only "-D_FILE_OFFSET_BITS=64". But in the GCC case we only need it for

> C++/libstdc++ so it seems "-D_FILE_OFFSET_BITS=64" should be enough. The

> rest is up to the users application, or?


One might argue that way, but again it's a bit late to change this now
for no compelling reason.

> My guess is that without defining _LARGEFILE_SOURCE and _LARGEFILE64_SOURCE

> the configure check in libstdc++-v3/acinclude.m4 just won't define

> _GLIBCXX_USE_LFS and everything will fall in place. This would leave HPUX

> as the last user of _GLIBCXX_USE_LFS.


I don't know about HP-UX, but _GLIBC_USE_LFS is defined on Linux/x86_64,
too.  To me, the meaning seems a bit confused: 27_io/fpos/14775.cc
suggests that it denotes all system with largefile support, while
acinclude.m4 (GLIBCXX_CHECK_LFS) only tests for the transitional
functions (enabled by _LARGEFILE64_SOURCE on Solaris) while ignoring the
`transparent' largefile support from _LARGEFILE_SOURCE
_FILE_OFFSET_BITS=64.

I'd rather not mess with this stuff.

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University
Franz Sirl June 25, 2018, 4:15 p.m. | #9
Am 2018-06-25 um 15:57 schrieb Rainer Orth:
> Hi Franz,

> 

>> so you are supposed to use "-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64",

>> but at least a quick glance at the Sol10 headers shows that the additional

>> -D_LARGEFILE_SOURCE only makes a difference for fseeko/ftello. That still

> 

> right, that's also explained in lfcompile(7).

> 

>> doesn't explain -D_LARGEFILE64_SOURCE, does libstdc++ really need to use

>> _LARGEFILE64_SOURCE functions?

> 

> Honestly, I hadn't checked, but wondered the same thing.  However, I'm a

> bit wary to simply remove them after years for fear of breaking user

> existing user code.


But adding _FILE_OFFSET_BITS=64 is the far bigger change for the user. 
Now suddenly (for 32-bit applications) off_t changes size and thus many 
applications with mixed C/C++-code simply might break. The reason is 
that now (if they didn't take care of _LARGEFILE_SOURCE themselves), for 
example fread() really does a fread64() in the C++ parts and a fread() 
(the 32-bit version) in the C parts. This situation was avoided before 
by enabling _LARGEFILE_SOURCE without _FILE_OFFSET_BITS=64.

>> Re-reading lfcompile(7) again shows that you can use either

>> "-D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64" (for portable applications) or

>> only "-D_FILE_OFFSET_BITS=64". But in the GCC case we only need it for

>> C++/libstdc++ so it seems "-D_FILE_OFFSET_BITS=64" should be enough. The

>> rest is up to the users application, or?

> 

> One might argue that way, but again it's a bit late to change this now

> for no compelling reason.


The compelling reason is that with _FILE_OFFSET_BITS=64 all bets are off 
anyway IMO, see above.

>> My guess is that without defining _LARGEFILE_SOURCE and _LARGEFILE64_SOURCE

>> the configure check in libstdc++-v3/acinclude.m4 just won't define

>> _GLIBCXX_USE_LFS and everything will fall in place. This would leave HPUX

>> as the last user of _GLIBCXX_USE_LFS.

> 

> I don't know about HP-UX, but _GLIBC_USE_LFS is defined on Linux/x86_64,

> too.  To me, the meaning seems a bit confused: 27_io/fpos/14775.cc

> suggests that it denotes all system with largefile support, while

> acinclude.m4 (GLIBCXX_CHECK_LFS) only tests for the transitional

> functions (enabled by _LARGEFILE64_SOURCE on Solaris) while ignoring the

> `transparent' largefile support from _LARGEFILE_SOURCE

> _FILE_OFFSET_BITS=64.

> 

> I'd rather not mess with this stuff.


That I can fully understand ;-). Maybe a solution is to define the 
macros also for C, not only for C++. And to conditionalize 
_LARGEFILE_SOURCE on 32-bit compile and _LARGEFILE64_SOURCE on 64-bit 
compile to make it at least less confusing.

Franz

Patch

# HG changeset patch
# Parent  48a63094f075d53e7bbbe0f2de0513c267ef9e96
Have g++ define _FILE_OFFSET_BITS=64 on 32-bit Solaris

diff --git a/gcc/config/sol2.h b/gcc/config/sol2.h
--- a/gcc/config/sol2.h
+++ b/gcc/config/sol2.h
@@ -113,6 +113,7 @@  along with GCC; see the file COPYING3.  
 	builtin_define ("_XOPEN_SOURCE=600");		\
 	builtin_define ("_LARGEFILE_SOURCE=1");		\
 	builtin_define ("_LARGEFILE64_SOURCE=1");	\
+	builtin_define ("_FILE_OFFSET_BITS=64");	\
 	builtin_define ("__EXTENSIONS__");		\
       }							\
     TARGET_SUB_OS_CPP_BUILTINS();			\