| Message ID | alpine.LSU.2.20.1804061131100.18265@zhemvz.fhfr.qr |
|---|---|
| State | New |
| Headers | show |
| Series | Mitigate PR85222 a bit | expand |
On 6 April 2018 at 10:39, Richard Biener wrote: > > The following allows people configuring the gcc-4 compatible ABI > as the default ABI to retain compatibility with old programs > catching ios_base::failure by matching the abi version thrown > to the configured default ABI. > > This doesn't really fix the PR but it makes behavior between > the dual-ABI with default gcc-4 compatible consistent with that > of the non-dual-ABI which is what I had expected. > > Whether an ABI break is really desired for the case of a c++11 > default ABI is still questionable and any programs that differ > from the default ABI choice will now be broken (compared to > those differing from c++11). > > Bootstrapped on x86_64-unknown-linux-gnu, ok for trunk? Ok for > the GCC 7 branch? Thinking about this further, I seem to recall we decided *not* to do this. Currently the --with-default-libstdcxx-abi flag only affects the default value of a macro in the headers, it doesn't affect the library ABI. The header macro can be overriden per translation unit, but the library ABI is always the same (it always throws the new ios::failure type). With this change the option would affect the library ABI and so --with-default-libstdcxx-abi would cause a new effect on libstdc++.so > I'm not sure if we want to revert r245167 after this? I'm > re-running the testsuite with a gcc4-compatible ABI right now. > At least with a "real" fix we should be able to run the affected > tests twice, once with the new and once with the old ABI. > > Thanks, > Richard. > > 2018-04-06 Richard Biener <rguenther@suse.de> > > PR libstdc++/85222 > * src/c++11/ios.cc: Remove hard define of _GLIBCXX_USE_CXX11_ABI to 1. > Instead use the configured default ABI to decide the ABI version > of ios_base::failure thrown by __throw_ios_failure. > > Index: libstdc++-v3/src/c++11/ios.cc > =================================================================== > --- libstdc++-v3/src/c++11/ios.cc (revision 258812) > +++ libstdc++-v3/src/c++11/ios.cc (working copy) > @@ -26,9 +26,8 @@ > // ISO C++ 14882: 27.4 Iostreams base classes > // > > -// Determines the version of ios_base::failure thrown by __throw_ios_failure. > -// If !_GLIBCXX_USE_DUAL_ABI this will get undefined automatically. > -#define _GLIBCXX_USE_CXX11_ABI 1 > +// The ABI version of ios_base::failure thrown by __throw_ios_failure > +// is determined by the default ABI version choosed at configure time > > #include <ios> > #include <limits>
On Fri, 6 Apr 2018, Jonathan Wakely wrote: > On 6 April 2018 at 10:39, Richard Biener wrote: > > > > The following allows people configuring the gcc-4 compatible ABI > > as the default ABI to retain compatibility with old programs > > catching ios_base::failure by matching the abi version thrown > > to the configured default ABI. > > > > This doesn't really fix the PR but it makes behavior between > > the dual-ABI with default gcc-4 compatible consistent with that > > of the non-dual-ABI which is what I had expected. > > > > Whether an ABI break is really desired for the case of a c++11 > > default ABI is still questionable and any programs that differ > > from the default ABI choice will now be broken (compared to > > those differing from c++11). > > > > Bootstrapped on x86_64-unknown-linux-gnu, ok for trunk? Ok for > > the GCC 7 branch? > > Thinking about this further, I seem to recall we decided *not* to do > this. Currently the --with-default-libstdcxx-abi flag only affects the > default value of a macro in the headers, it doesn't affect the library > ABI. The header macro can be overriden per translation unit, but the > library ABI is always the same (it always throws the new ios::failure > type). With this change the option would affect the library ABI and so > --with-default-libstdcxx-abi would cause a new effect on libstdc++.so But --disable-libstdcxx-dual-abi does so as well. Currently the behavior with that compared to --with-default-libstdcxx-abi=gcc4-compatible is inconsistent with respect to whether the following testcase works or not (note it doesn't explicitely specify the ABI version to use) #include <iostream> #include <fstream> using namespace std; int main () { std::ifstream pstats; pstats.exceptions(ifstream::failbit | ifstream::badbit | ifstream::eofbit); try { printf("\n Opening file : /proc/0/stat "); pstats.open("/proc/0/stat"); } catch(ifstream::failure e) { printf("\n!!Caught ifstream exception!!\n"); if(pstats.is_open()) { pstats.close(); } } return 0; } I call that a bug. Yes, we talked about ways to eventually fix that. But is anybody working on that? I think the current default behavior with --with-default-libstdcxx-abi=gcc4-compatible is undesirable. Thanks, Richard. > > I'm not sure if we want to revert r245167 after this? I'm > > re-running the testsuite with a gcc4-compatible ABI right now. > > At least with a "real" fix we should be able to run the affected > > tests twice, once with the new and once with the old ABI. > > > > Thanks, > > Richard. > > > > 2018-04-06 Richard Biener <rguenther@suse.de> > > > > PR libstdc++/85222 > > * src/c++11/ios.cc: Remove hard define of _GLIBCXX_USE_CXX11_ABI to 1. > > Instead use the configured default ABI to decide the ABI version > > of ios_base::failure thrown by __throw_ios_failure. > > > > Index: libstdc++-v3/src/c++11/ios.cc > > =================================================================== > > --- libstdc++-v3/src/c++11/ios.cc (revision 258812) > > +++ libstdc++-v3/src/c++11/ios.cc (working copy) > > @@ -26,9 +26,8 @@ > > // ISO C++ 14882: 27.4 Iostreams base classes > > // > > > > -// Determines the version of ios_base::failure thrown by __throw_ios_failure. > > -// If !_GLIBCXX_USE_DUAL_ABI this will get undefined automatically. > > -#define _GLIBCXX_USE_CXX11_ABI 1 > > +// The ABI version of ios_base::failure thrown by __throw_ios_failure > > +// is determined by the default ABI version choosed at configure time > > > > #include <ios> > > #include <limits> > >
On 6 April 2018 at 12:22, Richard Biener wrote: > On Fri, 6 Apr 2018, Jonathan Wakely wrote: > >> On 6 April 2018 at 10:39, Richard Biener wrote: >> > >> > The following allows people configuring the gcc-4 compatible ABI >> > as the default ABI to retain compatibility with old programs >> > catching ios_base::failure by matching the abi version thrown >> > to the configured default ABI. >> > >> > This doesn't really fix the PR but it makes behavior between >> > the dual-ABI with default gcc-4 compatible consistent with that >> > of the non-dual-ABI which is what I had expected. >> > >> > Whether an ABI break is really desired for the case of a c++11 >> > default ABI is still questionable and any programs that differ >> > from the default ABI choice will now be broken (compared to >> > those differing from c++11). >> > >> > Bootstrapped on x86_64-unknown-linux-gnu, ok for trunk? Ok for >> > the GCC 7 branch? >> >> Thinking about this further, I seem to recall we decided *not* to do >> this. Currently the --with-default-libstdcxx-abi flag only affects the >> default value of a macro in the headers, it doesn't affect the library >> ABI. The header macro can be overriden per translation unit, but the >> library ABI is always the same (it always throws the new ios::failure >> type). With this change the option would affect the library ABI and so >> --with-default-libstdcxx-abi would cause a new effect on libstdc++.so > > But --disable-libstdcxx-dual-abi does so as well. Right, but that one (obviously) alters the library ABI, by disabling all the duplicated symbols. Currently --with-default-libstdcxx-abi doesn't alter the ABI of the shared lib when the dual-abi is enabled, just whether the initial value of the macro is 0 or 1. All the symbols in the library, and the type of exception thrown, are the same for every dual-abi build of the library. Sure, we _can_ change that, but should we? We could add yet another option, so you can set the initial value of the macro to 0 but still have it throw the new exception type, or vice versa. I don't really like that option either. > Currently the > behavior with that compared to > --with-default-libstdcxx-abi=gcc4-compatible is inconsistent with > respect to whether the following testcase works or not (note it > doesn't explicitely specify the ABI version to use) > > #include <iostream> > #include <fstream> > using namespace std; > int main () > { > std::ifstream pstats; > pstats.exceptions(ifstream::failbit | ifstream::badbit | > ifstream::eofbit); > try { > printf("\n Opening file : /proc/0/stat "); > pstats.open("/proc/0/stat"); > } > catch(ifstream::failure e) > { > printf("\n!!Caught ifstream exception!!\n"); > if(pstats.is_open()) { > pstats.close(); > } > } > return 0; > } > > I call that a bug. Yes, we talked about ways to eventually fix that. > But is anybody working on that? Not for GCC 8.1, but then we already shipped 7.x with the current behaviour anyway.
On 6 April 2018 at 12:48, Jonathan Wakely wrote: > Currently --with-default-libstdcxx-abi doesn't alter the ABI of the > shared lib when the dual-abi is enabled, just whether the initial > value of the macro is 0 or 1. All the symbols in the library, and the > type of exception thrown, are the same for every dual-abi build of the > library. I mean every dual-abi build for a given release, of course. Because the type of exception thrown changed between gcc 6 and gcc 7, so maybe my position that the option doesn't affect the ABI of the .so is untenable, given that it changed independent of that option :-\
On Fri, 6 Apr 2018, Jonathan Wakely wrote: > On 6 April 2018 at 12:48, Jonathan Wakely wrote: > > Currently --with-default-libstdcxx-abi doesn't alter the ABI of the > > shared lib when the dual-abi is enabled, just whether the initial > > value of the macro is 0 or 1. All the symbols in the library, and the > > type of exception thrown, are the same for every dual-abi build of the > > library. > > I mean every dual-abi build for a given release, of course. Because > the type of exception thrown changed between gcc 6 and gcc 7, so maybe > my position that the option doesn't affect the ABI of the .so is > untenable, given that it changed independent of that option :-\ All I can say is that for the SUSE distributions that use the old ABI by default but still provide GCC 7+ as optional choice and thus end up using the newest compiler runtime I'm taking the pragmatic approach of making --with-default-libstdcxx-abi=gcc4-compatible throw the old ABI ios_base::failure class via said patch. Currently there's no way to get the old ABI thrown besides disabling dual-ABI but that removes all the symbols that have been present since those distributions offered GCC 5 so isn't a good option. Which means the ABI break between GCC 5/6 and GCC 7 has to be reverted here. Of course I think the ABI break is even intolerable when the new ABI is the default. Ways to fix that have been discussed in the PR but a fix is of course taking time to implement and verify. Richard.
Index: libstdc++-v3/src/c++11/ios.cc =================================================================== --- libstdc++-v3/src/c++11/ios.cc (revision 258812) +++ libstdc++-v3/src/c++11/ios.cc (working copy) @@ -26,9 +26,8 @@ // ISO C++ 14882: 27.4 Iostreams base classes // -// Determines the version of ios_base::failure thrown by __throw_ios_failure. -// If !_GLIBCXX_USE_DUAL_ABI this will get undefined automatically. -#define _GLIBCXX_USE_CXX11_ABI 1 +// The ABI version of ios_base::failure thrown by __throw_ios_failure +// is determined by the default ABI version choosed at configure time #include <ios> #include <limits>