[Bug libstdc++/11584] New: ios::iword() fails to zero-initialize storage on failure

[Bug libstdc++/11584] New: ios::iword() fails to zero-initialize storage on failure

[sebor at roguewave dot com]

PLEASE REPLY TO gcc-bugzilla@gcc.gnu.org ONLY, *NOT* gcc-bugs@gcc.gnu.org.

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=11584

           Summary: ios::iword() fails to zero-initialize storage on failure
           Product: gcc
           Version: 3.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: libstdc++
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: sebor at roguewave dot com
                CC: gcc-bugs at gcc dot gnu dot org

According to 27.4.2.5, p3, of ISO/IEC 14882:2003, the program below is expected
to exit with the status of 0. It exits with a non-0 status when compiled with
gcc 3.2.

Regards
Martin

#include <stdlib.h>
#include <iostream>
#include <new>

int new_fails;

void* operator new (size_t n)
{
    if (new_fails)
        throw std::bad_alloc ();

    return malloc (n);
}

void operator delete (void *p) { free (p); }
void* operator new[] (size_t n) { return operator new (n); }
void operator delete[] (void *p) { operator delete (p); }

int main ()
{
    const int i = std::ios::xalloc ();

    new_fails = 1;

    std::cout.iword (i) = 0xdeadbeef;

    return std::cout.iword (i);
}

[Bug libstdc++/11584] ios::iword() fails to zero-initialize storage on failure

[ehrhardt at mathematik dot uni-ulm dot de]

PLEASE REPLY TO gcc-bugzilla@gcc.gnu.org ONLY, *NOT* gcc-bugs@gcc.gnu.org.

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=11584


ehrhardt at mathematik dot uni-ulm dot de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|                            |1
   Last reconfirmed|0000-00-00 00:00:00         |2003-07-29 16:56:56
               date|                            |
            Version|3.2                         |3.3


------- Additional Comments From ehrhardt at mathematik dot uni-ulm dot de  2003-07-29 16:56 -------
I think I can confirm this. 27.4.2.5 states about the iword function:

| Returns: On success iarray[ idx]. On failure, a valid long&
| initialized to 0.

Currently _M_grow_words returns a reference to _M_word_zero which is
initially set to zero but there is no guarantee that this continues
to be the case. The solution is to set _M_word_zero to 0 in _M_grow_words
before returning a reference to it.

This is ok because p2 states:

| The reference returned may become invalid after another call to the
| object's iword member with a different index, after a call to its
| copyfmt member, or when the object is destroyed.

I'll attach a proposed patch that should fix this. However, I can't
regtest 3.4 at the moment due to libiconv Problems with the testsuite.

[Bug libstdc++/11584] ios::iword() fails to zero-initialize storage on failure

[dhazeghi at yahoo dot com]

PLEASE REPLY TO gcc-bugzilla@gcc.gnu.org ONLY, *NOT* gcc-bugs@gcc.gnu.org.

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=11584


dhazeghi at yahoo dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bkoz at redhat dot com
           Keywords|                            |wrong-code
   Target Milestone|3.4                         |---


------- Additional Comments From dhazeghi at yahoo dot com  2003-08-23 00:52 -------
Ben, can you take a look at the attached patch for this bug? Thanks.

[Bug libstdc++/11584] ios::iword() fails to zero-initialize storage on failure

[bkoz at gcc dot gnu dot org]

PLEASE REPLY TO gcc-bugzilla@gcc.gnu.org ONLY, *NOT* gcc-bugs@gcc.gnu.org.

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=11584



------- Additional Comments From bkoz at gcc dot gnu dot org  2003-09-10 14:46 -------

On today's gcc, I am getting at least

FAIL: 22_locale/messages/members/char/1.cc execution test
FAIL: 22_locale/messages/members/char/2.cc execution test
FAIL: 22_locale/messages/members/char/wrapped_env.cc execution test
FAIL: 22_locale/messages/members/char/wrapped_locale.cc execution test
FAIL: 22_locale/messages_byname/1.cc execution test
FAIL: 24_iterators/istream_iterator.cc execution test
...

These new fails. The build-test cycle is pretty painfully slow at the moment, so
I don't have complete results.

This does indeed look like a valid bug, but the solution appears off. I'll look
at this tonight and will post more on Friday.

-benjamin

[Bug libstdc++/11584] ios::iword() fails to zero-initialize storage on failure

[bkoz at gcc dot gnu dot org]

PLEASE REPLY TO gcc-bugzilla@gcc.gnu.org ONLY, *NOT* gcc-bugs@gcc.gnu.org.

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=11584



------- Additional Comments From bkoz at gcc dot gnu dot org  2003-09-17 23:45 -------

On closer examination, I don't think this is a bug. 

For one thing:

    int i = std::ios::xalloc();
    std::cout.iword (i) = 0xdeadbeef;
    long l = std::cout.iword (i);
    l == 0xdeadbeef;

is valid, based on the fact that the index is the same (regardless of the
replacement operator new). 

If, instead:

void test01()
{
  bool test = true;
  new_fails = 1;

  const int i = std::ios::xalloc();
  std::cout.iword(i) = 0xdeadbeef;
  long l1 = std::cout.iword(i);
  VERIFY( l1 == 0xdeadbeef);
 
  long l2 = std::cout.iword(i * 3);
  VERIFY( l2 == 0 );

  // The reference returned may become invalid after another call to
  // the object's iword member with a different index.  NB: Note the
  // 'may': it's not required, and in this case should not be
  // required, as the i size array is still allocated.
  long l3 = std::cout.iword(i);
  VERIFY( l3 == 0 || l3 == 0xdeadbeef);
}

-benjamin

Re: [Bug libstdc++/11584] ios::iword() fails to zero-initialize storage on failure

[Christian Ehrhardt]

On Wed, Sep 17, 2003 at 11:45:29PM -0000, bkoz at gcc dot gnu dot org wrote:
> On closer examination, I don't think this is a bug. 

I don't agree. See below.

> For one thing:
> 
>     int i = std::ios::xalloc();
>     std::cout.iword (i) = 0xdeadbeef;
>     long l = std::cout.iword (i);
>     l == 0xdeadbeef;
> is valid, based on the fact that the index is the same (regardless of the
> replacement operator new). 

First note that iword must try to realloc the array if it is called
with an out of bounds index. This is what happens here and the case
where iword succeeds seems to be standard conforming.

But the bug report is about the case where iword _fails_ to extend the
array. 27.4.2.5[3] clearly states for this case:

| Returns: [...] On failure, a valid long& initialized to 0.

The comparions in the above testcase MUST be false in the case where
iword can't allocate enough memory. The reason is that both iword calls
fail and consequently both calls should return 0.

> If, instead:
> 
> void test01()
> {
>   bool test = true;
>   new_fails = 1;
> 
>   const int i = std::ios::xalloc();
>   std::cout.iword(i) = 0xdeadbeef;
>   long l1 = std::cout.iword(i);
>   VERIFY( l1 == 0xdeadbeef);

This is correct for the success case but false for the failure case.

> [ ... ]
>   // The reference returned may become invalid after another call to
>   // the object's iword member with a different index.  NB: Note the
>   // 'may': it's not required, and in this case should not be
>   // required, as the i size array is still allocated.
>   long l3 = std::cout.iword(i);
>   VERIFY( l3 == 0 || l3 == 0xdeadbeef);

Here we must have l3 == 0xdeadbeef iff iword succeeds. l3 == 0 is correct
iff iword fails!

Note that the term ``reference may become invalid'' above does NOT
mean that the value for a certain index may change. It only means that the
_location_ where that value is stored may change (do to realloc).

The bug is in the following code

   const int i = <very large value, e.g. 0x7FFFFF0>
   std::cout.iword (i) = 0xdeadbeef;
   std::cerr << "expected 0 got " << std::iword (i+1) << std::endl;
   assert (std::cout.iword (i) == 0 || !std::cout.badbit);

which outputs:

   expected 0 got -559038737
   Assertion failed: !std::cout.badbit || std::cout.iword (i) == 0 ...

instead of just

   expected 0 got 0 

The remaining problematic case is: What do we expect from this code:

   const int i = <very large value, e.g. 0x7FFFFF0>
   long & ref =  std::cout.iword (i);
   assert (std::cout.badbit);
   ref = 0xdeadbeef;
   std::cout.iword (i+1) = 0x12345678;
   std::cerr << "ref is " << ref << std::endl;

IMHO it is unreasonable to expect ref to have the value 0xdeadbeef. We
can justify this with the ``reference becomes invalid'' clause cited
above. However, invoking this clause is misleading and doesn't help in
this case, which is even worse:

   const int i = <very large value, e.g. 0x7FFFFF0>
   long & ref =  std::cout.iword (i);
   assert (std::cout.badbit);
   ref = 0xdeadbeef;
   long val = std::cout.iword (i); /* Reference does NOT become invalid */
   assert ((ref == 0xdeadbeef) && (val == 0));

IMHO given the words of the standard the assertion should hold.

  regards  Christian

[Bug libstdc++/11584] ios::iword() fails to zero-initialize storage on failure

[ehrhardt at mathematik dot uni-ulm dot de]

PLEASE REPLY TO gcc-bugzilla@gcc.gnu.org ONLY, *NOT* gcc-bugs@gcc.gnu.org.

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=11584



------- Additional Comments From ehrhardt at mathematik dot uni-ulm dot de  2003-09-18 11:56 -------
Subject: Re:  ios::iword() fails to zero-initialize storage on failure

On Wed, Sep 17, 2003 at 11:45:29PM -0000, bkoz at gcc dot gnu dot org wrote:
> On closer examination, I don't think this is a bug. 

I don't agree. See below.

> For one thing:
> 
>     int i = std::ios::xalloc();
>     std::cout.iword (i) = 0xdeadbeef;
>     long l = std::cout.iword (i);
>     l == 0xdeadbeef;
> is valid, based on the fact that the index is the same (regardless of the
> replacement operator new). 

First note that iword must try to realloc the array if it is called
with an out of bounds index. This is what happens here and the case
where iword succeeds seems to be standard conforming.

But the bug report is about the case where iword _fails_ to extend the
array. 27.4.2.5[3] clearly states for this case:

| Returns: [...] On failure, a valid long& initialized to 0.

The comparions in the above testcase MUST be false in the case where
iword can't allocate enough memory. The reason is that both iword calls
fail and consequently both calls should return 0.

> If, instead:
> 
> void test01()
> {
>   bool test = true;
>   new_fails = 1;
> 
>   const int i = std::ios::xalloc();
>   std::cout.iword(i) = 0xdeadbeef;
>   long l1 = std::cout.iword(i);
>   VERIFY( l1 == 0xdeadbeef);

This is correct for the success case but false for the failure case.

> [ ... ]
>   // The reference returned may become invalid after another call to
>   // the object's iword member with a different index.  NB: Note the
>   // 'may': it's not required, and in this case should not be
>   // required, as the i size array is still allocated.
>   long l3 = std::cout.iword(i);
>   VERIFY( l3 == 0 || l3 == 0xdeadbeef);

Here we must have l3 == 0xdeadbeef iff iword succeeds. l3 == 0 is correct
iff iword fails!

Note that the term ``reference may become invalid'' above does NOT
mean that the value for a certain index may change. It only means that the
_location_ where that value is stored may change (do to realloc).

The bug is in the following code

   const int i = <very large value, e.g. 0x7FFFFF0>
   std::cout.iword (i) = 0xdeadbeef;
   std::cerr << "expected 0 got " << std::iword (i+1) << std::endl;
   assert (std::cout.iword (i) == 0 || !std::cout.badbit);

which outputs:

   expected 0 got -559038737
   Assertion failed: !std::cout.badbit || std::cout.iword (i) == 0 ...

instead of just

   expected 0 got 0 

The remaining problematic case is: What do we expect from this code:

   const int i = <very large value, e.g. 0x7FFFFF0>
   long & ref =  std::cout.iword (i);
   assert (std::cout.badbit);
   ref = 0xdeadbeef;
   std::cout.iword (i+1) = 0x12345678;
   std::cerr << "ref is " << ref << std::endl;

IMHO it is unreasonable to expect ref to have the value 0xdeadbeef. We
can justify this with the ``reference becomes invalid'' clause cited
above. However, invoking this clause is misleading and doesn't help in
this case, which is even worse:

   const int i = <very large value, e.g. 0x7FFFFF0>
   long & ref =  std::cout.iword (i);
   assert (std::cout.badbit);
   ref = 0xdeadbeef;
   long val = std::cout.iword (i); /* Reference does NOT become invalid */
   assert ((ref == 0xdeadbeef) && (val == 0));

IMHO given the words of the standard the assertion should hold.

  regards  Christian

[Bug libstdc++/11584] ios::iword() fails to zero-initialize storage on failure

[jlquinn at optonline dot net]

------- Additional Comments From jlquinn at optonline dot net  2003-12-30 20:21 -------
(In reply to comment #6)
> Subject: Re:  ios::iword() fails to zero-initialize storage on failure
> 
> On Wed, Sep 17, 2003 at 11:45:29PM -0000, bkoz at gcc dot gnu dot org wrote:
> > On closer examination, I don't think this is a bug. 
> 
> I don't agree. See below.
> 
> > For one thing:
> > 
> >     int i = std::ios::xalloc();
> >     std::cout.iword (i) = 0xdeadbeef;
> >     long l = std::cout.iword (i);
> >     l == 0xdeadbeef;
> > is valid, based on the fact that the index is the same (regardless of the
> > replacement operator new). 
> 
> First note that iword must try to realloc the array if it is called
> with an out of bounds index. This is what happens here and the case
> where iword succeeds seems to be standard conforming.
> 
> But the bug report is about the case where iword _fails_ to extend the
> array. 27.4.2.5[3] clearly states for this case:
> 
> | Returns: [...] On failure, a valid long& initialized to 0.
> 
> The comparions in the above testcase MUST be false in the case where
> iword can't allocate enough memory. The reason is that both iword calls
> fail and consequently both calls should return 0.

It seems the source of the disagreement here is the interpretation of
"initialized to 0" in the standard.  If iword(invalid) must always return a
reference to a long equal to 0, then Christian is correct and the above code
snippet should fail.  If iword(invalid) only has to return a reference to a long
that was assigned to 0 the *first* time it was referenced, then Benjamin is
correct and the above piece of code should always work.

I tend to believe Christian's interpretation, but suspect the standard is a bit
ambiguous here.

Jerry

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=11584

[Bug libstdc++/11584] ios::iword() fails to zero-initialize storage on failure

[bkoz at gcc dot gnu dot org]

------- Additional Comments From bkoz at gcc dot gnu dot org  2004-01-08 17:21 -------

Jerry you know this code better than I. The proposed patch is flawed, however. 

If you can fix this up, add the testcase and or clarify the libstdc++ docs
according to your last post, I'm up for it.

best,
benjamin

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=11584

[Bug libstdc++/11584] ios::iword() fails to zero-initialize storage on failure

[jlquinn at gcc dot gnu dot org]

------- Additional Comments From jlquinn at gcc dot gnu dot org  2004-01-26 01:45 -------
The only thing I really see wrong with the patch is that it zeros both the iword
and pword failure storage, rather than the one being accessed.  I'm going to
post a modified patch as soon as I update the test case.

BTW, it doesn't seem to me to be a requirement that test case in this bug produce
0.  The standard says that a failure to allocate space should return 0 storage,
but it doesn't say how that space has to be allocated.  Our implementation has a
small amount of preallocated space, so accessing slot 1 doesn't fail.  The
problem that was raised is still generally valid, though.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2003-12-17 02:04:29         |2004-01-26 01:45:36
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=11584

[Bug libstdc++/11584] ios::iword() fails to zero-initialize storage on failure

[bkoz at redhat dot com]

------- Additional Comments From bkoz at redhat dot com  2004-01-26 20:36 -------
Subject: Re:  ios::iword() fails to zero-initialize
 storage on failure


>The only thing I really see wrong with the patch is that it zeros both
>the iword and pword failure storage, rather than the one being
>accessed.  I'm going to post a modified patch as soon as I update the
>test case.

Ok.

>BTW, it doesn't seem to me to be a requirement that test case in this
>bug produce 0.  The standard says that a failure to allocate space
>should return 0 storage, but it doesn't say how that space has to be
>allocated.  Our implementation has a small amount of preallocated
>space, so accessing slot 1 doesn't fail.  The problem that was raised
>is still generally valid, though.

Right. The issue, I think, is that the standard is under-specified. 

What we should do, in the meantime, is try to come up with sensible
behavior. We have some test cases in this PR already, but what we should
do is document the new, "sensible" libstdc++ behavior in test cases.

Then, perhaps Martin could tell us if the sensible libstdc++ behavior,
as defined in these test cases, is sensible to him, as another library
vendor.

Once we have behavior that RW and libstdc++ agree is sensible, one of us
should submit a defect report.

-benjamin


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=11584

[Bug libstdc++/11584] ios::iword() fails to zero-initialize storage on failure

[jlquinn at gcc dot gnu dot org]

------- Additional Comments From jlquinn at gcc dot gnu dot org  2004-01-27 02:32 -------
I posted a patch for this in http://gcc.gnu.org/ml/libstdc++/2004-01/msg00289.html


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=11584

[Bug libstdc++/11584] ios::iword() fails to zero-initialize storage on failure

[jlquinn at gcc dot gnu dot org]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |WAITING


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=11584

[Bug libstdc++/11584] ios::iword() fails to zero-initialize storage on failure

[bkoz at redhat dot com]

------- Additional Comments From bkoz at redhat dot com  2004-01-27 04:50 -------
Subject: Re:  ios::iword() fails to zero-initialize
 storage on failure

Additional Comments From jlquinn at gcc dot gnu dot org  2004-01-27 02:32 -------
>I posted a patch for this in http://gcc.gnu.org/ml/libstdc++/2004-01/msg00289.html

This looks good please put it on 3.4 and mainline.

-benjamin


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=11584

[Bug libstdc++/11584] ios::iword() fails to zero-initialize storage on failure

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2004-01-27 15:45 -------
Subject: Bug 11584

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	jlquinn@gcc.gnu.org	2004-01-27 15:45:43

Modified files:
	libstdc++-v3   : ChangeLog 
	libstdc++-v3/include/bits: ios_base.h 
	libstdc++-v3/src: ios.cc 
Added files:
	libstdc++-v3/testsuite/27_io/ios_base/storage: 11584.cc 

Log message:
	2004-01-27  Jerry Quinn  <jlquinn@optonline.net>
	
	PR libstdc++/11584
	* include/bits/ios_base.h (ios_base::_M_grow_words):  Add
	iword/pword selector.
	(ios_base::iword, ios_base::pword):  Use it.
	* src/ios.cc (ios_base::_M_grow_words):  Clear _M_word_zero
	iword or pword member on alloc failure.
	* testsuite/27_io/ios_base/storage/11584.cc:  New test.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/ChangeLog.diff?cvsroot=gcc&r1=1.2259&r2=1.2260
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/include/bits/ios_base.h.diff?cvsroot=gcc&r1=1.37&r2=1.38
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/src/ios.cc.diff?cvsroot=gcc&r1=1.53&r2=1.54
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/testsuite/27_io/ios_base/storage/11584.cc.diff?cvsroot=gcc&r1=NONE&r2=1.1



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=11584

[Bug libstdc++/11584] ios::iword() fails to zero-initialize storage on failure

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2004-01-27 15:54 -------
Subject: Bug 11584

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-3_4-branch
Changes by:	jlquinn@gcc.gnu.org	2004-01-27 15:54:04

Modified files:
	libstdc++-v3   : ChangeLog 
	libstdc++-v3/include/bits: ios_base.h 
	libstdc++-v3/src: ios.cc 
Added files:
	libstdc++-v3/testsuite/27_io/ios_base/storage: 11584.cc 

Log message:
	2004-01-27  Jerry Quinn  <jlquinn@optonline.net>
	
	PR libstdc++/11584
	* include/bits/ios_base.h (ios_base::_M_grow_words):  Add
	iword/pword selector.
	(ios_base::iword, ios_base::pword):  Use it.
	* src/ios.cc (ios_base::_M_grow_words):  Clear _M_word_zero
	iword or pword member on alloc failure.
	* testsuite/27_io/ios_base/storage/11584.cc:  New test.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.2224.2.11&r2=1.2224.2.12
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/include/bits/ios_base.h.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.37&r2=1.37.4.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/src/ios.cc.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.52&r2=1.52.4.1
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/libstdc++-v3/testsuite/27_io/ios_base/storage/11584.cc.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=NONE&r2=1.1.2.1



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=11584

[Bug libstdc++/11584] ios::iword() fails to zero-initialize storage on failure

[jlquinn at gcc dot gnu dot org]

------- Additional Comments From jlquinn at gcc dot gnu dot org  2004-01-27 15:55 -------
Fix was committed.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=11584

[Bug libstdc++/11584] ios::iword() fails to zero-initialize storage on failure

[pinskia at gcc dot gnu dot org]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |3.4.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=11584