Re: libstdc++/2071

[Loren James Rittle <rittle@latour.rsch.comm.mot.com>]

The following reply was made to PR libstdc++/2071; it has been noted by GNATS.

From: Loren James Rittle <rittle@latour.rsch.comm.mot.com>
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 <Pine.SOL.3.91.1010611185357.4913A-100000@taarna.cygnus.com>,
 Benjamin Kosnik <bkoz@redhat.com> 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 <iostream>
 
 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  <ljrittle@acm.org>
 
 	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<typename _CharT>
 +     _CharT
 +     __basic_file<_CharT>::sys_getc()
 +     {
 +       return getc (_M_cfile);
 +     }
 +=20
 +   template<typename _CharT>
 +     _CharT
 +     __basic_file<_CharT>::sys_ungetc(_CharT __s)
 +     {
 +       return ungetc (__s, _M_cfile);
 +     }
    =20
     template<typename _CharT>
       __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