From mboxrd@z Thu Jan 1 00:00:00 1970 From: Loren James Rittle To: pme@gcc.gnu.org Cc: gcc-prs@gcc.gnu.org Subject: Re: libstdc++/2071 Date: Tue, 12 Jun 2001 01:56:00 -0000 Message-id: <20010612085603.21791.qmail@sourceware.cygnus.com> X-SW-Source: 2001-06/msg00469.html List-Id: The following reply was made to PR libstdc++/2071; it has been noted by GNATS. From: Loren James Rittle To: bkoz@redhat.com Cc: mark@codesourcery.com, gcc-gnats@gcc.gnu.org, pedwards@disaster.jaj.com Subject: Re: libstdc++/2071 Date: Tue, 12 Jun 2001 03:53:38 -0500 (CDT) Hi Benjamin, (CC'd Mark so he knows what is going on as release manager. If you don't have time to read the background, then feel free to jump to [MARK].) Here is my final proposal to kill the outstanding high-priority libstdc++ bug which is actually the same as my May 10th proposal (no one has raised an objection or a better mechanism but I and Phil may have been the only two people working on it in earnest), but I have now personally confirmed that it fixes the problem for sparc-sun-solaris2.7 as well as *-unknown-freebsd*. I start with some commentary on your last remark about this bug. In article , Benjamin Kosnik writes: [...a lot of self beating up removed...] > these need to have ios_base::openmode translated to > SEEK_SET, SEEK_CUR, SEEK_END ^^^^^^^^ I am assuming this is a typo; and that you meant seekdir. No, you are not an idiot! I can believe that you found another (hopefully, minor) independent bug with the library. However, in both Solaris and FreeBSD (the two platforms known to have the interactive and/or pipe-based cin bug explained in libstdc++/2071), we find: ; grep SEEK_ /usr/include/stdio.h|grep define #define SEEK_SET 0 #define SEEK_CUR 1 #define SEEK_END 2 In include/bits/ios_base.h, we fine: static const seekdir beg =3D seekdir(0); static const seekdir cur =3D seekdir(SEEK_CUR); static const seekdir end =3D seekdir(SEEK_END); For maximal portability, I might recommend that we patch the first line to read as follows (but in practice, I'm not sure it matters at all since I have never seen a libc define SEEK_SET any other way): static const seekdir beg =3D seekdir(SEEK_SET); Thus, the mapping you think you overlooked is being done (for 2 out of 3 of the symbols already). Although I was a bit confused by your exact statements since an openmode never translates to a seekdir (which is why I labeled it as an assumed typo). As near as I could tell from a quick read of the standard, openmode is suppose to allow one to selectively affect the read handle or the write handle exclusively. The library implementation doesn't appear to handle that properly but I don't know how to fix it yet either. OK, on to my proposal (I will try to condense down all the past e-mail traffic on this matter): I think that I have demonstrated conclusively that the fseek() idiom used in the C++ code is non-portable by abstracting out the exact usage into trivial C code. See the plain C test case at: http://gcc.gnu.org/ml/libstdc++/2001-05/msg00041.html (actually use the fixed version which is found here but you still need the commentary from the URL above:) http://gcc.gnu.org/ml/libstdc++/2001-05/msg00046.html The problem is that calling ftell()/fseek() on pipes or interactive IO streams is *not* portable according to the C standard. And it is clearly not portable when we look at common libc implementations. Instead, we need to get characters via the getc() function (or macro?) and put back an unused character with ungetc(). Any approach that uses fseek() will not work portability across libc implementations. Since we are currently only getting one character at a time anyways, I think the "overhead" to use getc/ungetc is about on par of using the fseek idiom currently in use. Actually, I posted an analysis that showed fseek() usage in libstdc++ translates to one or more OS call on BSD whereas the getc()/ungetc() idiom maps to zero OS-level calls in the nominal case. Humm, I know which version I want to use when possible... ;-) =46rom my posting on May 10th, assuming we didn't find a better way to avoid fseek() yet still allow synchronized IO with C stdio, the only thing that was left to do (in my mind) was to tighten up the patch and make sure it actually worked for Solaris... Phil reported ( http://gcc.gnu.org/ml/libstdc++/2001-06/msg00071.html ) that it didn't work for him on Solaris 2.8, but I am not convinced yet that it wasn't something minor (and/or the insidious Makefile problem - I don't know Phil's habits when it comes to applying a patch and attempting a rebuild in an already bootstrapped tree - I am now an idiot-savant when it comes to this inside the libstdc++-v3 subtree ;-/). I now have a bootstrapped compiler on solaris2.7 that works with -static. I have added my proposed patch to my virgin 3.0 branch source tree from my mainline tree where is has been for the last month and I have completely rebuilt libstdc++-v3 for solaris2.7. Hey, what do I know? My patch works for me on solaris2.7... ;-) Is solaris2.8 really that different? Or (addressed to Phil), were you bit by the Makefiles not doing enough work in light of file changes? Attached is the *exact* patch that works for me (if you apply it to test it on solaris, please `rm $objdir/sparc-sun-solaris*/libstdc++-v3' before you remake since rebuilding doesn't work after patching in this case). Yes, this patch is a kludge that only solves the problem when underflow() will grab 1 character at a time from the input source. Since that is how we are going to ship libstdc++-v3 with gcc 3.0 and since we still get buffering from the underlying stdio library, I don't see any real problem here. [MARK] Unless someone comes up with something better in 12 hours, I say we go with my patch. Mark, with his release manager hat on, will like the fact that it offers the feature that it can't easily break any platforms that do not define _GLIBCPP_AVOID_FSEEK (however, I explicitly did not protect the generic sys_getc/sys_ungetc concrete implementation mapping since it might be considered an API change to add them later and I truly can't imagine a port that offers everything else libstdc++-v3 demands yet not the standard getc/ungetc). As a reminder, here is the extended version of the libstdc++/2071 test case (must be run as: `echo 1234|a.out' and/or `a.out', followed by 1234 on the interactive input stream, to see the problem thus no one knows how to put it in the automatic dejagnu testsuite yet): #include using namespace std; int main() { int i; cin >> i; cout << "i =3D=3D " << i << endl; } For the record, rebuilt libstdc++-v3 completely in already bootstrapped 3.0 branch build trees (`make check' shows zero non-explainable regressions. In particular, I see the same 8 libstdc++-v3 failures just posted by Mark's automatic run for Solaris 2.8 plus ``FAIL: 27_io/ios_members.cc execution test'' but since it fails with the identical backtrace in __cxa_bad_cast as the c_strings test case, I assume this is not a problem with my patch): i386-unknown-freebsd4.2 alpha-unknown-freebsd4.2 sparc-sun-solaris2.7 (RUNTESTFLAGS\=3D'--target_board ''unix{-static}''') Were I to install this patch, I would clone the config/os/solaris/solaris2.7/bits/os_defines.h into config/os/solaris/solaris*/bits/os_defines.h (since the original Solaris bug report was for 2.6). Regards, Loren 2001-06-12 Loren J. Rittle libstdc++/2071 * config/basic_file_stdio.h (sys_getc): New method. (sys_ungetc): New method. * include/bits/basic_file.h: (sys_getc): New method signature. (sys_ungetc): New method signature. * include/bits/fstream.tcc (underflow): Add conditional code paths which avoid using short seeks on streams (especially useful when the stream might be interactive or a pipe). At the moment, this alternate path only avoids seeking when the ``buffer size'' of underflow() is 1 since the C standard only guarantees buffer space for one ungetc (this technique could be extended since *-*-solaris* supports buffering for 4 calls to ungetc and *-*-*bsd* supports buffering limited only by memory resources). Also, _GLIBCPP_AVOID_FSEEK must be defined in a port's os_defines.h file for this alternate path to even be considered. As a bonus, the idiom of using getc/ungetc requires no system calls whereas fseek maps to one or two system call(s) on many platforms. * config/os/bsd/freebsd/bits/os_defines.h (_GLIBCPP_AVOID_FSEEK): Define it. * config/os/solaris/solaris2.7/bits/os_defines.h (_GLIBCPP_AVOID_FSEEK): Likewise. Index: libstdc++-v3/config/basic_file_stdio.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvs/gcc/gcc/libstdc++-v3/config/basic_file_stdio.h,v retrieving revision 1.1.2.3 diff -c -r1.1.2.3 basic_file_stdio.h *** basic_file_stdio.h 2001/06/09 06:57:15 1.1.2.3 --- basic_file_stdio.h 2001/06/12 07:20:21 *************** *** 91,96 **** --- 91,110 ---- =20 return __ret; } +=20 + template + _CharT + __basic_file<_CharT>::sys_getc() + { + return getc (_M_cfile); + } +=20 + template + _CharT + __basic_file<_CharT>::sys_ungetc(_CharT __s) + { + return ungetc (__s, _M_cfile); + } =20 template __basic_file<_CharT>*=20 Index: libstdc++-v3/include/bits/basic_file.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvs/gcc/gcc/libstdc++-v3/include/bits/basic_file.h,v retrieving revision 1.3.6.3 diff -c -r1.3.6.3 basic_file.h *** basic_file.h 2001/05/14 19:49:03 1.3.6.3 --- basic_file.h 2001/06/12 07:20:21 *************** *** 147,152 **** --- 147,158 ---- __basic_file* sys_open(__c_file_type* __file, ios_base::openmode __mode); =20 + _CharT + sys_getc(); +=20 + _CharT + sys_ungetc(_CharT); +=20 __basic_file*=20 close();=20 =20 Index: libstdc++-v3/include/bits/fstream.tcc =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvs/gcc/gcc/libstdc++-v3/include/bits/fstream.tcc,v retrieving revision 1.4.4.3 diff -c -r1.4.4.3 fstream.tcc *** fstream.tcc 2001/05/22 18:56:12 1.4.4.3 --- fstream.tcc 2001/06/12 07:20:21 *************** *** 261,266 **** --- 261,270 ---- { if (__testout) _M_really_overflow(); + #if _GLIBCPP_AVOID_FSEEK + else if ((_M_in_cur - _M_in_beg) =3D=3D 1) + _M_file->sys_getc(); + #endif else=20 _M_file->seekoff(_M_in_cur - _M_in_beg,=20 ios_base::cur, ios_base::in); *************** *** 276,287 **** --- 280,300 ---- if (__testout) _M_out_cur =3D _M_in_cur; __ret =3D traits_type::to_int_type(*_M_in_cur); + #if _GLIBCPP_AVOID_FSEEK + if (__size =3D=3D 1) + _M_file->sys_ungetc(*_M_in_cur); + else + { + #endif streamoff __p =3D _M_file->seekoff(0 - __size, ios_base::cur,=20 ios_base::in); if (__p =3D=3D -1) { // XXX Something is wrong, do error checking. } + #if _GLIBCPP_AVOID_FSEEK + } + #endif } =20 } } Index: libstdc++-v3/config/os/bsd/freebsd/bits/os_defines.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvs/gcc/gcc/libstdc++-v3/config/os/bsd/freebsd/bits/os_defines.h= ,v retrieving revision 1.1 diff -c -r1.1 os_defines.h *** os_defines.h 2000/12/05 23:25:08 1.1 --- os_defines.h 2001/06/12 07:20:21 *************** *** 35,40 **** --- 35,41 ---- /* System-specific #define, typedefs, corrections, etc, go here. This file will come before all others. */ =20 + #define _GLIBCPP_AVOID_FSEEK 1 =20 #endif =20 Index: libstdc++-v3/config/os/solaris/solaris2.7/bits/os_defines.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvs/gcc/gcc/libstdc++-v3/config/os/solaris/solaris2.7/bits/os_de= fines.h,v retrieving revision 1.5.6.2 diff -c -r1.5.6.2 os_defines.h *** os_defines.h 2001/05/16 01:01:13 1.5.6.2 --- os_defines.h 2001/06/12 07:20:21 *************** *** 31,36 **** --- 31,38 ---- #ifndef _GLIBCPP_OS_DEFINES # define _GLIBCPP_OS_DEFINES =20 + #define _GLIBCPP_AVOID_FSEEK 1 +=20 // These are typedefs which libio assumes are already in place (because // they really are, under Linux). #define __off_t off_t