[Bug libstdc++/13864] New: stl_hastable.h Line 527

[Bug libstdc++/13864] New: stl_hastable.h Line 527

[greg dot bertin at solacesystems dot com]

_Construct(&__n->_M_val, __obj); being called with an unitialized memory 
pointer (__n) being passed as the first parameter (i.e. it is memory allocated 
from an allocator in line 524). Memory should be default constructed first to 
ensure the class' copy constructor behaves properly:

    try {
      _Construct(&__n->_M_val);
      _Construct(&__n->_M_val, __obj);
      return __n;
    }

-- 
           Summary: stl_hastable.h Line 527
           Product: gcc
           Version: 3.3.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: libstdc++
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: greg dot bertin at solacesystems dot com
                CC: gcc-bugs at gcc dot gnu dot org


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

[Bug libstdc++/13864] stl_hastable.h Line 527

[gdr at integrable-solutions dot net]

------- Additional Comments From gdr at integrable-solutions dot net  2004-01-26 06:38 -------
Subject: Re:  New: stl_hastable.h Line 527

"greg dot bertin at solacesystems dot com" <gcc-bugzilla@gcc.gnu.org> writes:

| _Construct(&__n->_M_val, __obj); being called with an unitialized memory 
| pointer (__n) being passed as the first parameter (i.e. it is memory allocated 
| from an allocator in line 524). Memory should be default constructed first to 
| ensure the class' copy constructor behaves properly:
| 
|     try {
|       _Construct(&__n->_M_val);
|       _Construct(&__n->_M_val, __obj);
|       return __n;
|     }

I don't understand.  Why should a default construction be needed
before a copy-construction?  A constructor is called only once in an
object lifetime.

-- Gaby


-- 


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

[Bug libstdc++/13864] stl_hastable.h Line 527

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-01-26 13:23 -------
Here is the full source, n __n is initialized:
  _Node* _M_new_node(const value_type& __obj)
  {
    _Node* __n = _M_get_node();
    __n->_M_next = 0;
    try {
      _Construct(&__n->_M_val, __obj);
      return __n;
    }
    catch(...)
      {
        _M_put_node(__n);
        __throw_exception_again;
      }
  }

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |INVALID


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

[Bug libstdc++/13864] stl_hastable.h Line 527

[greg dot bertin at solacesystems dot com]

------- Additional Comments From greg dot bertin at solacesystems dot com  2004-01-26 14:20 -------
Subject: RE:  stl_hastable.h Line 527

Hi pinskia ...

__n is initialized but the memory being pointed at by __n is not. It is
memory allocated via _M_get_node():

  _Node* _M_get_node() { return _M_node_allocator.allocate(1); } 

Allocators return uninitialized memory blocks. This is a problem because
_Construct():

  template <class _T1, class _T2>
    inline void
    _Construct(_T1* __p, const _T2& __value)
    { new (static_cast<void*>(__p)) _T1(__value); }

assumes that __p points to a properly initialized T1 object since it
invokes T1's copy constructor. T1's copy constructor will assume that
T1's members (e.g. pointer members that are not NULL and thus need to be
deallocate()'d/delete()'d/free()'d/whatever) have been properly
initialized. The other uses of _Construct() in the standard STL that I
checked all pass a properly initialized __p to _Construct and thus
behave correctly.


Thanks for looking at this!!

> -----Original Message-----
> From: pinskia at gcc dot gnu dot org [mailto:gcc-bugzilla@gcc.gnu.org]
> Sent: Monday, January 26, 2004 8:24 AM
> To: greg.bertin@solacesystems.com
> Subject: [Bug libstdc++/13864] stl_hastable.h Line 527
> 
> 
> ------- Additional Comments From pinskia at gcc dot gnu dot org
2004-01-
> 26 13:23 -------
> Here is the full source, n __n is initialized:
>   _Node* _M_new_node(const value_type& __obj)
>   {
>     _Node* __n = _M_get_node();
>     __n->_M_next = 0;
>     try {
>       _Construct(&__n->_M_val, __obj);
>       return __n;
>     }
>     catch(...)
>       {
>         _M_put_node(__n);
>         __throw_exception_again;
>       }
>   }
> 
> --
>            What    |Removed                     |Added
>
------------------------------------------------------------------------
--
> --
>              Status|UNCONFIRMED                 |RESOLVED
>          Resolution|                            |INVALID
> 
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13864
> 
> ------- You are receiving this mail because: -------
> You reported the bug, or are watching the reporter.




-- 


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

[Bug libstdc++/13864] stl_hastable.h Line 527

[greg dot bertin at solacesystems dot com]

------- Additional Comments From greg dot bertin at solacesystems dot com  2004-01-26 14:26 -------
Subject: RE:  stl_hastable.h Line 527

Ignore this last one ... my idiocy ... copy constructors initialize
uninitialized memory ... one of these days I will learn C++ :-)

> -----Original Message-----
> From: Greg Bertin [mailto:greg.bertin@solacesystems.com]
> Sent: Monday, January 26, 2004 9:23 AM
> To: 'gcc-bugzilla@gcc.gnu.org'
> Subject: RE: [Bug libstdc++/13864] stl_hastable.h Line 527
> 
> Hi pinskia ...
> 
> __n is initialized but the memory being pointed at by __n is not. It
is
> memory allocated via _M_get_node():
> 
>   _Node* _M_get_node() { return _M_node_allocator.allocate(1); }
> 
> Allocators return uninitialized memory blocks. This is a problem
because
> _Construct():
> 
>   template <class _T1, class _T2>
>     inline void
>     _Construct(_T1* __p, const _T2& __value)
>     { new (static_cast<void*>(__p)) _T1(__value); }
> 
> assumes that __p points to a properly initialized T1 object since it
> invokes T1's copy constructor. T1's copy constructor will assume that
T1's
> members (e.g. pointer members that are not NULL and thus need to be
> deallocate()'d/delete()'d/free()'d/whatever) have been properly
> initialized. The other uses of _Construct() in the standard STL that I
> checked all pass a properly initialized __p to _Construct and thus
behave
> correctly.
> 
> 
> Thanks for looking at this!!
> 
> > -----Original Message-----
> > From: pinskia at gcc dot gnu dot org
[mailto:gcc-bugzilla@gcc.gnu.org]
> > Sent: Monday, January 26, 2004 8:24 AM
> > To: greg.bertin@solacesystems.com
> > Subject: [Bug libstdc++/13864] stl_hastable.h Line 527
> >
> >
> > ------- Additional Comments From pinskia at gcc dot gnu dot org
2004-
> 01-
> > 26 13:23 -------
> > Here is the full source, n __n is initialized:
> >   _Node* _M_new_node(const value_type& __obj)
> >   {
> >     _Node* __n = _M_get_node();
> >     __n->_M_next = 0;
> >     try {
> >       _Construct(&__n->_M_val, __obj);
> >       return __n;
> >     }
> >     catch(...)
> >       {
> >         _M_put_node(__n);
> >         __throw_exception_again;
> >       }
> >   }
> >
> > --
> >            What    |Removed                     |Added
> >
------------------------------------------------------------------------
> --
> > --
> >              Status|UNCONFIRMED                 |RESOLVED
> >          Resolution|                            |INVALID
> >
> >
> > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13864
> >
> > ------- You are receiving this mail because: -------
> > You reported the bug, or are watching the reporter.




-- 


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

[Bug libstdc++/13864] stl_hastable.h Line 527

[greg dot bertin at solacesystems dot com]

------- Additional Comments From greg dot bertin at solacesystems dot com  2004-01-26 14:28 -------
Subject: RE:  stl_hastable.h Line 527

Actually this is my idiocy ... you are quite correct. Copy constructors
must assume they are working with uninitialized memory ... my fault ...
sorry!!

> -----Original Message-----
> From: gdr at integrable-solutions dot net [mailto:gcc-
> bugzilla@gcc.gnu.org]
> Sent: Monday, January 26, 2004 1:39 AM
> To: greg.bertin@solacesystems.com
> Subject: [Bug libstdc++/13864] stl_hastable.h Line 527
> 
> 
> ------- Additional Comments From gdr at integrable-solutions dot net
> 2004-01-26 06:38 -------
> Subject: Re:  New: stl_hastable.h Line 527
> 
> "greg dot bertin at solacesystems dot com" <gcc-bugzilla@gcc.gnu.org>
> writes:
> 
> | _Construct(&__n->_M_val, __obj); being called with an unitialized
memory
> | pointer (__n) being passed as the first parameter (i.e. it is memory
> allocated
> | from an allocator in line 524). Memory should be default constructed
> first to
> | ensure the class' copy constructor behaves properly:
> |
> |     try {
> |       _Construct(&__n->_M_val);
> |       _Construct(&__n->_M_val, __obj);
> |       return __n;
> |     }
> 
> I don't understand.  Why should a default construction be needed
> before a copy-construction?  A constructor is called only once in an
> object lifetime.
> 
> -- Gaby
> 
> 
> --
> 
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13864
> 
> ------- You are receiving this mail because: -------
> You reported the bug, or are watching the reporter.




-- 


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

[Bug libstdc++/13864] stl_hastable.h Line 527

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2004-01-26 14:29 -------
(In reply to comment #3)
> Subject: RE:  stl_hastable.h Line 527
> 
> Hi pinskia ...
> 
> __n is initialized but the memory being pointed at by __n is not. It is
> memory allocated via _M_get_node():
> 
>   _Node* _M_get_node() { return _M_node_allocator.allocate(1); } 
> 
> Allocators return uninitialized memory blocks. This is a problem because
> _Construct():
> 
>   template <class _T1, class _T2>
>     inline void
>     _Construct(_T1* __p, const _T2& __value)
>     { new (static_cast<void*>(__p)) _T1(__value); }
> 
> assumes that __p points to a properly initialized T1 object since it
> invokes T1's copy constructor. T1's copy constructor will assume that
> T1's members (e.g. pointer members that are not NULL and thus need to be
> deallocate()'d/delete()'d/free()'d/whatever) have been properly
> initialized. 
No, the copy constructor does not assume anything for the new object.
And also this is using inplacement operator new which just uses the memory location
that you are taking and makes a new object out of that location.  In this case it points to
&__n->_M_val (notice the & which means take the address and not something __n->_M_val points 
to so that address is always valid as long as __n is which in this case is true).

You need to go read a book about C++ and C for more information about this, this is still valid and
correct C++.

-- 


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

[Bug libstdc++/13864] stl_hastable.h Line 527

[gdr at integrable-solutions dot net]

------- Additional Comments From gdr at integrable-solutions dot net  2004-01-26 14:41 -------
Subject: Re:  stl_hastable.h Line 527

"greg dot bertin at solacesystems dot com" <gcc-bugzilla@gcc.gnu.org> writes:

| Allocators return uninitialized memory blocks. This is a problem because
| _Construct():
| 
|   template <class _T1, class _T2>
|     inline void
|     _Construct(_T1* __p, const _T2& __value)
|     { new (static_cast<void*>(__p)) _T1(__value); }
| 
| assumes that __p points to a properly initialized T1 object since it
| invokes T1's copy constructor.

No, it does not.  Please look up "new with placement" in a any decent
book on C++.

The PR is invalid.  It is user confusion. 


-- 


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