Re: libio bugs

Re: libio bugs

[Jeffrey A Law]

1998-01-20  Andreas Schwab  

        * iostream.cc (istream::operator>>(long double&))
        [!_G_HAVE_LONG_DOUBLE_IO]: Scan value into separate variable, in
        case long double is bigger than double.
        (ostream::operator<<(double)) [_G_HAVE_PRINTF_FP]: Fix order of
        initializers of struct printf_info to match declaration order,
        to work around g++ bug.
        (ostream::operator<<(long double)) [_G_HAVE_PRINTF_FP]: Likewise.

        * gen-params: Add missing quotes.  Avoid useless use of command
        substitution.
Thanks.  I've installed this patch.

jeff

Re: libio bugs

[Mark Mitchell]

>>>>> "Jeffrey" == Jeffrey A Law <law@cygnus.com> writes:

    Jeffrey> (ostream::operator<<(double)) [_G_HAVE_PRINTF_FP]: Fix
    Jeffrey> order of initializers of struct printf_info to match
    Jeffrey> declaration order, to work around g++ bug.
    Jeffrey> (ostream::operator<<(long double)) [_G_HAVE_PRINTF_FP]:
    Jeffrey> Likewise.

Since the order of initializers isn't supposed to matter, there is 
definitely a bug here if this fix makes a difference.  Does anyone
have a simple test-case for this bug?

-- 
Mark Mitchell		mmitchell@usa.net
Stanford University	http://www.stanford.edu

Re: libio bugs

[Andreas Schwab]

Mark Mitchell <mmitchell@usa.net> writes:

|>>>>>> "Jeffrey" == Jeffrey A Law <law@cygnus.com> writes:

|>     Jeffrey> (ostream::operator<<(double)) [_G_HAVE_PRINTF_FP]: Fix
|>     Jeffrey> order of initializers of struct printf_info to match
|>     Jeffrey> declaration order, to work around g++ bug.
|>     Jeffrey> (ostream::operator<<(long double)) [_G_HAVE_PRINTF_FP]:
|>     Jeffrey> Likewise.

|> Since the order of initializers isn't supposed to matter, there is 
|> definitely a bug here if this fix makes a difference.  Does anyone
|> have a simple test-case for this bug?

G++ simply ignores all initializer labels.  I have already send a bug
report, here it is again:

$ cat init.cc
struct X { int a, b; };

struct X f (int a, int b)
{
  struct X y = { b: b, a: a }; // labels ignored by g++
  return y;
}

int main ()
{
  struct X x = f (42, 0);
  if (x.a != 42 || x.b != 0)
    abort ();
  exit (0);
}
$ gcc init.cc
$ ./a.out
Aborted (core dumped)

-- 
Andreas Schwab                                      "And now for something
schwab@issan.informatik.uni-dortmund.de              completely different"
schwab@gnu.org

Re: libio bugs

[Mark Mitchell]

    Andreas> |> Since the order of initializers isn't supposed to
    Andreas> matter, there is |> definitely a bug here if this fix
    Andreas> makes a difference.  Does anyone |> have a simple
    Andreas> test-case for this bug?

    Andreas> G++ simply ignores all initializer labels.  I have
    Andreas> already send a bug report, here it is again:

Ah, I see.  But initializer labels are a g++ extension.  Perhaps it
would be simpler simply to issue an error message on any use of
initializer labels?  Is the point of this to allow out-of-order
initialization of the members?  Why is there code in libio that uses
this extension?  Is there other C++ code that uses this extension?

    Andreas> $ cat init.cc struct X { int a, b; };

    Andreas> struct X f (int a, int b) { struct X y = { b: b, a: a };
    Andreas> // labels ignored by g++ return y; }

    Andreas> int main () { struct X x = f (42, 0); if (x.a != 42 ||
    Andreas> x.b != 0) abort (); exit (0); } $ gcc init.cc $ ./a.out
    Andreas> Aborted (core dumped)

    Andreas> -- Andreas Schwab "And now for something
    Andreas> schwab@issan.informatik.uni-dortmund.de completely
    Andreas> different" schwab@gnu.org


-- 
Mark Mitchell		mmitchell@usa.net
Stanford University	http://www.stanford.edu

Re: libio bugs

[Andreas Schwab]

Mark Mitchell <mmitchell@usa.net> writes:

|> Ah, I see.  But initializer labels are a g++ extension.  Perhaps it
|> would be simpler simply to issue an error message on any use of
|> initializer labels?

That would probably the best until G++ is fixed, or the new C9x way of
doing it is implemented (which is already partly implemented in the C
frontend).

|>  Is the point of this to allow out-of-order
|> initialization of the members?

Yes.

|>  Why is there code in libio that uses
|> this extension?

Probably because it works well in C, and nobody has yet noticed that it is
broken in C++.

|>  Is there other C++ code that uses this extension?

I'm not aware of any.

-- 
Andreas Schwab                                      "And now for something
schwab@issan.informatik.uni-dortmund.de              completely different"
schwab@gnu.org

Re: libio bugs

[Mark Mitchell]

>>>>> "Andreas" == Andreas Schwab <schwab@issan.informatik.uni-dortmund.de> writes:

    Andreas> Mark Mitchell <mmitchell@usa.net> writes:

    Andreas> |> Ah, I see.  But initializer labels are a g++
    Andreas> extension.  Perhaps it |> would be simpler simply to
    Andreas> issue an error message on any use of |> initializer
    Andreas> labels?

    Andreas> That would probably the best until G++ is fixed, or the
    Andreas> new C9x way of doing it is implemented (which is already
    Andreas> partly implemented in the C frontend).

    Andreas> |> Is the point of this to allow out-of-order |>
    Andreas> initialization of the members?

I'm about to submit a patch that disables initializer labels in C++
(since they are a non-working non-standard extension), and remove the
uses of them in libio.  Is that OK?  Are there problems with
link-compatibility, i.e., does naming the fields in struct printf_info
make sure that they are initialized in some order different than the
order that would be specified by the usual initialization syntax?  (I
note that the current initialization order is exactly the same as that
in GLIBC 2.)

    Andreas> Yes.

    Andreas> |> Why is there code in libio that uses |> this
    Andreas> extension?

    Andreas> Probably because it works well in C, and nobody has yet
    Andreas> noticed that it is broken in C++.

    Andreas> |> Is there other C++ code that uses this extension?

    Andreas> I'm not aware of any.

    Andreas> -- Andreas Schwab "And now for something
    Andreas> schwab@issan.informatik.uni-dortmund.de completely
    Andreas> different" schwab@gnu.org

-- 
Mark Mitchell		mmitchell@usa.net
Stanford University	http://www.stanford.edu

Re: libio bugs

[Andreas Schwab]

Mark Mitchell <mmitchell@usa.net> writes:

|>>>>>> "Andreas" == Andreas Schwab <schwab@issan.informatik.uni-dortmund.de> writes:

|>     Andreas> Mark Mitchell <mmitchell@usa.net> writes:

|>     Andreas> |> Ah, I see.  But initializer labels are a g++
|>     Andreas> extension.  Perhaps it |> would be simpler simply to
|>     Andreas> issue an error message on any use of |> initializer
|>     Andreas> labels?

|>     Andreas> That would probably the best until G++ is fixed, or the
|>     Andreas> new C9x way of doing it is implemented (which is already
|>     Andreas> partly implemented in the C frontend).

|>     Andreas> |> Is the point of this to allow out-of-order |>
|>     Andreas> initialization of the members?

|> I'm about to submit a patch that disables initializer labels in C++
|> (since they are a non-working non-standard extension), and remove the
|> uses of them in libio.  Is that OK?  Are there problems with
|> link-compatibility, i.e., does naming the fields in struct printf_info
|> make sure that they are initialized in some order different than the
|> order that would be specified by the usual initialization syntax?  (I
|> note that the current initialization order is exactly the same as that
|> in GLIBC 2.)

The current development version of glibc 2 adds another bitfield before
the last field in struct printf_info, it now looks like this (is_char is
new):

struct printf_info
{
  int prec;			/* Precision.  */
  int width;			/* Width.  */
  wchar_t spec;			/* Format letter.  */
  unsigned int is_long_double:1;/* L flag.  */
  unsigned int is_short:1;	/* h flag.  */
  unsigned int is_long:1;	/* l flag.  */
  unsigned int alt:1;		/* # flag.  */
  unsigned int space:1;		/* Space flag.  */
  unsigned int left:1;		/* - flag.  */
  unsigned int showsign:1;	/* + flag.  */
  unsigned int group:1;		/* ' flag.  */
  unsigned int extra:1;		/* For special use.  */
  unsigned int is_char:1;	/* hh flag.  */
  wchar_t pad;			/* Padding character.  */
};


-- 
Andreas Schwab                                      "And now for something
schwab@issan.informatik.uni-dortmund.de              completely different"
schwab@gnu.org

Re: libio bugs

[Mark Mitchell]

    Mark Mitchell <mmitchell@usa.net> writes:

    |>>>>>> "Andreas" == Andreas Schwab <schwab@issan.informatik.uni-dortmund.de> writes:

    |> uses of them in libio.  Is that OK?  Are there problems with
    |> link-compatibility, i.e., does naming the fields in struct printf_info
    |> make sure that they are initialized in some order different than the
    |> order that would be specified by the usual initialization syntax?  (I
    |> note that the current initialization order is exactly the same as that
    |> in GLIBC 2.)

    The current development version of glibc 2 adds another bitfield before
    the last field in struct printf_info, it now looks like this (is_char is
    new):

Well, my patch is in.  So, somebody needs to modify the two uses of
printf_info in iostream.cc to initialize the is_char flag
appropriately, for those versions of glibc for which it should be
initialized.  Andreas, are you willing to do this?  You've probably
got a better understanding of how to test for the right versions of
glibc and so forth.

    struct printf_info
    {
      int prec;			/* Precision.  */
      int width;			/* Width.  */
      wchar_t spec;			/* Format letter.  */
      unsigned int is_long_double:1;/* L flag.  */
      unsigned int is_short:1;	/* h flag.  */
      unsigned int is_long:1;	/* l flag.  */
      unsigned int alt:1;		/* # flag.  */
      unsigned int space:1;		/* Space flag.  */
      unsigned int left:1;		/* - flag.  */
      unsigned int showsign:1;	/* + flag.  */
      unsigned int group:1;		/* ' flag.  */
      unsigned int extra:1;		/* For special use.  */
      unsigned int is_char:1;	/* hh flag.  */
      wchar_t pad;			/* Padding character.  */
    };

-- 
Mark Mitchell		mmitchell@usa.net
Stanford University	http://www.stanford.edu

Re: libio bugs

[H.J. Lu]

> 
> This patch fixes some bugs in libio.
> 
> Btw, why does _IO_wchar_t in streambuf.h default to short and not
> _G_wchar_t?  And why does ios::fill(_IO_wchar_t) cast its argument to
> char?  Unfortunately, changing that now breaks binary compatibility on
> Linux.
> 

We can still change it for glibc 2.1. glibc 2.1 will change the
iostream ABI. glibc 2.1 has

#define _G_IO_IO_FILE_VERSION 0x20001

in _G_config.h. It can be used to identify glibc 2.1.


H.J.

libio bugs

[Andreas Schwab]

This patch fixes some bugs in libio.

Btw, why does _IO_wchar_t in streambuf.h default to short and not
_G_wchar_t?  And why does ios::fill(_IO_wchar_t) cast its argument to
char?  Unfortunately, changing that now breaks binary compatibility on
Linux.


1998-01-20  Andreas Schwab  <schwab@issan.informatik.uni-dortmund.de>

	* iostream.cc (istream::operator>>(long double&))
	[!_G_HAVE_LONG_DOUBLE_IO]: Scan value into separate variable, in
	case long double is bigger than double.
	(ostream::operator<<(double)) [_G_HAVE_PRINTF_FP]: Fix order of
	initializers of struct printf_info to match declaration order,
	to work around g++ bug.
	(ostream::operator<<(long double)) [_G_HAVE_PRINTF_FP]: Likewise.

	* gen-params: Add missing quotes.  Avoid useless use of command
	substitution.

*** egcs-980115/libio/gen-params.~1~	Thu Nov 27 00:54:21 1997
--- egcs-980115/libio/gen-params	Tue Dec 23 20:48:12 1997
*************** fi
*** 277,283 ****
  tr '	' ' ' <TMP >dummy.out
  
  for TYPE in dev_t clock_t fpos_t gid_t ino_t mode_t nlink_t off_t pid_t ptrdiff_t sigset_t size_t ssize_t time_t uid_t va_list wchar_t wint_t int16_t uint16_t int32_t uint_32_t u_int16_t u_int32_t; do
!     IMPORTED=`eval 'echo $'"$TYPE"`
      if [ -n "${IMPORTED}" ] ; then
  	eval "$TYPE='$IMPORTED'"
      else
--- 277,283 ----
  tr '	' ' ' <TMP >dummy.out
  
  for TYPE in dev_t clock_t fpos_t gid_t ino_t mode_t nlink_t off_t pid_t ptrdiff_t sigset_t size_t ssize_t time_t uid_t va_list wchar_t wint_t int16_t uint16_t int32_t uint_32_t u_int16_t u_int32_t; do
!     eval IMPORTED=\$$TYPE
      if [ -n "${IMPORTED}" ] ; then
  	eval "$TYPE='$IMPORTED'"
      else
*************** done
*** 318,326 ****
  
  # Look for some standard macros.
  for NAME in BUFSIZ FOPEN_MAX FILENAME_MAX NULL; do
!     IMPORTED=`eval 'echo $'"$NAME"`
      if [ -n "${IMPORTED}" ] ; then
! 	eval "$NAME='$IMPORTED /* specified */"
      else
  	rm -f TMP
  	${SED} -n -e 's| *;|;|g' -e "s|long X${NAME}= *\(.*\);|\1|w TMP" \
--- 318,326 ----
  
  # Look for some standard macros.
  for NAME in BUFSIZ FOPEN_MAX FILENAME_MAX NULL; do
!     eval IMPORTED=\$$NAME
      if [ -n "${IMPORTED}" ] ; then
! 	eval "$NAME='$IMPORTED /* specified */'"
      else
  	rm -f TMP
  	${SED} -n -e 's| *;|;|g' -e "s|long X${NAME}= *\(.*\);|\1|w TMP" \
*************** done
*** 334,342 ****
  
  # These macros must be numerical constants; strip any trailing 'L's.
  for NAME in SHRT_MAX INT_MAX LONG_MAX LONG_LONG_MAX; do
!     IMPORTED=`eval 'echo $'"$NAME"`
      if [ -n "${IMPORTED}" ] ; then
! 	eval "$NAME='$IMPORTED /* specified */"
      else
  	rm -f TMP
  	${SED} -n -e 's| *;|;|g' -e "s|long X${NAME}= *\([0-9]*\)L* *;|\1|w TMP" \
--- 334,342 ----
  
  # These macros must be numerical constants; strip any trailing 'L's.
  for NAME in SHRT_MAX INT_MAX LONG_MAX LONG_LONG_MAX; do
!     eval IMPORTED=\$$NAME
      if [ -n "${IMPORTED}" ] ; then
! 	eval "$NAME='$IMPORTED /* specified */'"
      else
  	rm -f TMP
  	${SED} -n -e 's| *;|;|g' -e "s|long X${NAME}= *\([0-9]*\)L* *;|\1|w TMP" \
*** egcs-980115/libio/iostream.cc.~1~	Fri Oct  3 19:14:02 1997
--- egcs-980115/libio/iostream.cc	Fri Jan 16 20:32:18 1998
*************** READ_INT(bool)
*** 333,343 ****
  istream& istream::operator>>(long double& x)
  {
      if (ipfx0())
  #if _G_HAVE_LONG_DOUBLE_IO
  	scan("%Lg", &x);
  #else
! 	scan("%lg", &x);
  #endif
      return *this;
  }
  
--- 333,347 ----
  istream& istream::operator>>(long double& x)
  {
      if (ipfx0())
+       {
  #if _G_HAVE_LONG_DOUBLE_IO
  	scan("%Lg", &x);
  #else
! 	double y;
! 	scan("%lg", &y);
! 	x = y;
  #endif
+       }
      return *this;
  }
  
*************** ostream& ostream::operator<<(double n)
*** 628,637 ****
  				      left: (flags() & ios::left) != 0,
  				      showsign: (flags() & ios::showpos) != 0,
  				      group: 0,
- 				      pad: fill()
  #if defined __GLIBC__ && __GLIBC__ >= 2
! 				      , extra: 0
  #endif
  	  };
  	  const void *ptr = (const void *) &n;
  	  if (__printf_fp (rdbuf(), &info, &ptr) < 0)
--- 632,641 ----
  				      left: (flags() & ios::left) != 0,
  				      showsign: (flags() & ios::showpos) != 0,
  				      group: 0,
  #if defined __GLIBC__ && __GLIBC__ >= 2
! 				      extra: 0,
  #endif
+ 				      pad: fill()
  	  };
  	  const void *ptr = (const void *) &n;
  	  if (__printf_fp (rdbuf(), &info, &ptr) < 0)
*************** ostream& ostream::operator<<(long double
*** 731,740 ****
  				  left: (flags() & ios::left) != 0,
  				  showsign: (flags() & ios::showpos) != 0,
  				  group: 0,
- 				  pad: fill()
  #if defined __GLIBC__ && __GLIBC__ >= 2
! 				  , extra: 0
  #endif
        };
  
        const void *ptr = (const void *) &n;
--- 735,744 ----
  				  left: (flags() & ios::left) != 0,
  				  showsign: (flags() & ios::showpos) != 0,
  				  group: 0,
  #if defined __GLIBC__ && __GLIBC__ >= 2
! 				  extra: 0,
  #endif
+ 				  pad: fill()
        };
  
        const void *ptr = (const void *) &n;