This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: PATCH: for Serious v3 shared library bug
See all this commentary for background on and analysis of this patch:
http://gcc.gnu.org/ml/libstdc++/2001-01/msg00011.html
http://gcc.gnu.org/ml/libstdc++/2001-01/msg00015.html
http://gcc.gnu.org/ml/libstdc++/2000-12/msg00264.html
http://gcc.gnu.org/ml/libstdc++/2001-01/msg00017.html
Fully bootstrapped and regression tested on i386-*-freebsd4.2 and
alpha-*-freebsd4.2. I suspect that this patch should be tested
elsewhere before being applied to the tree. There is also an
outstanding concern that this patch is just papering over a deeper
issue. I concede the point since, although I think I understand the
POSIX stdio model quite well, I may be confused. ;-) Please report
success or failure on other platforms here. I suppose if AIX, Linux
and Solaris all like it as well and no one finds a logical fault in
the reasoning, then this is better than nothing.
There is another line that could be taken but I reject: FILE* handles
could continue to be found with fdopen() and without dup() but never
have fclose() called on them in certain cases. This would appear to
be a resource leak whereas my approach has no such leak that I can
determine. Also, it is unclear if sync_with_stdio(true) could ever
work as expected when multiple FILE* handles have been obtained
against the file descriptors, each with there own buffering schemes.
But let me also be clear: this patch does not yet make
sync_with_stdio(true) --- which is the default of libstdc++-v3 ---
suddenly work! I now have it so the same FILE* handle is used by both
the C stdio library for stdout and the C++ template IO library for
cout, but there is still internal buffering enabled in the C++ library
that I don't know how to properly disable [i.e. unless one calls
sync_with_stdio(false), I think we should use stdio library buffering
for the file descriptors 0, 1 and 2 and disable any provided by
basic_streambuf<>]. However, this patch does appear to correctly fix
the 27_io/filebuf_members.cc test case and Jason's test case as well.
This new case should be checked:
#include <cstdio>
#include <iostream>
int main ()
{
std::ios_base::sync_with_stdio (true);
std::printf ("1");
std::cout << "2";
std::printf ("3");
std::cout << "4" << std::endl;
}
It must produce 1234\n on stdout. It currently produces 1324\n on my
platforms.
For the record, I also had this uncommitted, believed to be unrelated,
FreeBSD configuration patch in my libstdc++-v3 tree:
http://gcc.gnu.org/ml/libstdc++/2000-12/msg00294.html
and many completely unrelated port configuration patches (that don't
affect libstdc++-v3 testsuite results when added) in my gcc tree.
I can now report that the only failures I see in libstdc++-v3 are:
23_containers/map_operators.cc and 23_containers/set_operators.cc !
pass/fail results: 98/2 shared + 98/2 static = 196/4 total (i386-*-freebsd*)
pass/fail results: 0/100 shared + 98/2 static = 98/102 total (alpha-*-freebsd*)
Regards,
Loren
2001-01-02 Loren J. Rittle <ljrittle@acm.org>
* config/c_io_stdio.cc (__basic_file<_CharT>::sys_open()): On
systems that support it, call dup() before fdopen(). On all
systems, provide special handling for the three stdio file
descriptors since we know how to obtain the primary FILE*
handle constructed around them in a portable manner without
using fdopen().
(__basic_file<_CharT>::~__basic_file()): Never close a file
related to C stdio since that will prematurely close the
underlying file descriptor (according to POSIX). Otherwise,
all other existing FILE* handles related to those file
descriptors, such as those used by C stdio, would be unusable
after that point.
* src/ios.cc (ios_base::sync_with_stdio()): On systems that
support it, build completely new file descriptors for the file
descriptors used for unsynchronized cout, cin and cerr. Note:
The code path for systems that do not support dup() is still
buggy in that it creates new files in the current directory.
I also consider it a bug that wcout and cout do not share a
file descriptor but I have no idea how to best solve that
problem without more structural rework (and the ability to
test such changes).
Index: config/c_io_stdio.cc
===================================================================
RCS file: /cvs/gcc/egcs/libstdc++-v3/config/c_io_stdio.cc,v
retrieving revision 1.2
diff -c -r1.2 c_io_stdio.cc
*** c_io_stdio.cc 2000/11/22 06:37:34 1.2
--- c_io_stdio.cc 2001/01/03 02:23:15
***************
*** 32,37 ****
--- 32,40 ----
//
#include <bits/basic_file.h>
+ #if _GLIBCPP_HAVE_UNISTD_H
+ #include <unistd.h>
+ #endif
namespace std {
***************
*** 51,57 ****
if (this->is_open())
{
fflush(_M_cfile);
! this->close();
}
}
--- 54,62 ----
if (this->is_open())
{
fflush(_M_cfile);
! if (!((_M_cfile == stdin) || (_M_cfile == stdout) ||
! (_M_cfile == stderr)))
! this->close();
}
}
***************
*** 96,102 ****
if (!this->is_open())
{
! if ((_M_cfile = fdopen(__fd, __c_mode)))
{
_M_fileno = __fd;
__ret = this;
--- 101,123 ----
if (!this->is_open())
{
! if (__fd == 0)
! _M_cfile = stdin;
! else if (__fd == 1)
! _M_cfile = stdout;
! else if (__fd == 2)
! _M_cfile = stderr;
! else
! {
! #if _GLIBCPP_HAVE_UNISTD_H
! __fd = dup (__fd);
! if (__fd == -1)
! return __ret;
! #endif
! _M_cfile = fdopen(__fd, __c_mode);
! }
!
! if (_M_cfile)
{
_M_fileno = __fd;
__ret = this;
Index: src/ios.cc
===================================================================
RCS file: /cvs/gcc/egcs/libstdc++-v3/src/ios.cc,v
retrieving revision 1.8
diff -c -r1.8 ios.cc
*** ios.cc 2000/12/20 08:04:41 1.8
--- ios.cc 2001/01/03 02:23:15
***************
*** 34,39 ****
--- 34,42 ----
#include <bits/std_ios.h>
#include <bits/std_iostream.h>
#include <bits/std_fstream.h>
+ #if _GLIBCPP_HAVE_UNISTD_H
+ #include <unistd.h>
+ #endif
namespace std {
***************
*** 322,333 ****
--- 325,342 ----
__ioinit._M_cout->~filebuf();
__ioinit._M_cin->~filebuf();
__ioinit._M_cerr->~filebuf();
+ #if _GLIBCPP_HAVE_UNISTD_H
+ __ioinit._M_cout = new filebuf(dup (1), "stdout", ios_base::out);
+ __ioinit._M_cin = new filebuf(dup (0), "stdin", ios_base::in);
+ __ioinit._M_cerr = new filebuf(dup (2), "stderr", ios_base::out);
+ #else
__ioinit._M_cout = new filebuf();
__ioinit._M_cin = new filebuf();
__ioinit._M_cerr = new filebuf();
__ioinit._M_cout->open("stdout", ios_base::out);
__ioinit._M_cin->open("stdin", ios_base::in);
__ioinit._M_cerr->open("stderr", ios_base::out);
+ #endif
cout.rdbuf(__ioinit._M_cout);
cin.rdbuf(__ioinit._M_cin);
cerr.rdbuf(__ioinit._M_cerr);
***************
*** 337,348 ****
--- 346,363 ----
__ioinit._M_wcout->~wfilebuf();
__ioinit._M_wcin->~wfilebuf();
__ioinit._M_wcerr->~wfilebuf();
+ #if _GLIBCPP_HAVE_UNISTD_H
+ __ioinit._M_wcout = new wfilebuf(dup (1), "stdout", ios_base::out);
+ __ioinit._M_wcin = new wfilebuf(dup (0), "stdin", ios_base::in);
+ __ioinit._M_wcerr = new wfilebuf(dup (2), "stderr", ios_base::out);
+ #else
__ioinit._M_wcout = new wfilebuf();
__ioinit._M_wcin = new wfilebuf();
__ioinit._M_wcerr = new wfilebuf();
__ioinit._M_wcout->open("wstdout", ios_base::out);
__ioinit._M_wcin->open("wstdin", ios_base::in);
__ioinit._M_wcerr->open("wstderr", ios_base::out);
+ #endif
wcout.rdbuf(__ioinit._M_wcout);
wcin.rdbuf(__ioinit._M_wcin);
wcerr.rdbuf(__ioinit._M_wcerr);