[Bug libstdc++/17012] [DR 526] std::list's function, remove, looks like it is reading memory that has been freed.

[Bug libstdc++/17012] [DR 526] std::list's function, remove, looks like it is reading memory that has been freed.

[pcarlini at suse dot de]

------- Comment #10 from pcarlini at suse dot de  2006-01-21 10:12 -------
Now DR 526.


-- 

pcarlini at suse dot de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |SUSPENDED
            Summary|std::list's function,       |[DR 526] std::list's
                   |remove, looks like it is    |function, remove, looks like
                   |reading memory that has been|it is reading memory that
                   |freed.                      |has been freed.


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

[Bug libstdc++/17012] [DR 526] std::list's function, remove, looks like it is reading memory that has been freed.

[mec at google dot com]

------- Comment #11 from mec at google dot com  2006-01-21 22:04 -------
You can make this visible at the C++ program level with a Key class that has a
signature field.  Init the signature in the constructor, clear the signature in
the destructor, and check the signature in operator==.


-- 


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

[Bug libstdc++/17012] [DR 526] std::list's function, remove, looks like it is reading memory that has been freed.

[hhinnant at apple dot com]

------- Comment #12 from hhinnant at apple dot com  2007-02-07 16:46 -------
At the ad-hoc LWG meeting in Batavia, Jan 22-24, 2007, the LWG decided that
self referencing code in list::remove must work.  Here is a preview of the
issue which is currently set to become official at the Spring '07 meeting:

http://home.twcny.rr.com/hinnant/cpp_extensions/issues_preview/lwg-active.html#526

Here is a patch to mainline to make it work.  I believe the approach taken by
this patch is superior to copying the value as it currently looks like a future
standard will not require that the value_type be copyable.  Additionally the
cost of copying the value_type can be arbitrarily large.

If we have a utility similar to boost::address_of, that might be better than
using operator& to get the address of the value_type (to accommodate types
which overload operator&).

Index: libstdc++-v3/include/bits/list.tcc
===================================================================
--- libstdc++-v3/include/bits/list.tcc  (revision 121691)
+++ libstdc++-v3/include/bits/list.tcc  (working copy)
@@ -176,14 +176,22 @@
     {
       iterator __first = begin();
       iterator __last = end();
+      iterator __extra = __last;
       while (__first != __last)
        {
          iterator __next = __first;
          ++__next;
          if (*__first == __value)
-           _M_erase(__first);
+         {
+           if (&__value != &*__first)
+             _M_erase(__first);
+           else
+             __extra = __first;
+         }
          __first = __next;
        }
+      if (__extra != __last)
+        _M_erase(__extra);
     }

   template<typename _Tp, typename _Alloc>


-- 


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

[Bug libstdc++/17012] [DR 526] std::list's function, remove, looks like it is reading memory that has been freed.

[hhinnant at apple dot com]

------- Comment #13 from hhinnant at apple dot com  2007-02-07 16:59 -------
(In reply to comment #12)

> If we have a utility similar to boost::address_of, that might be better than
> using operator& to get the address of the value_type (to accommodate types
> which overload operator&).

Oops, I forgot about allocator::address and:

http://home.twcny.rr.com/hinnant/cpp_extensions/issues_preview/lwg-active.html#580

New patch uses allocator::address:

Index: libstdc++-v3/include/bits/list.tcc
===================================================================
--- libstdc++-v3/include/bits/list.tcc  (revision 121691)
+++ libstdc++-v3/include/bits/list.tcc  (working copy)
@@ -176,14 +176,23 @@
     {
       iterator __first = begin();
       iterator __last = end();
+      iterator __extra = __last;
+      allocator_type __a = get_allocator();
       while (__first != __last)
        {
          iterator __next = __first;
          ++__next;
          if (*__first == __value)
-           _M_erase(__first);
+         {
+           if (__a.address(__value) != __a.address(*__first))
+             _M_erase(__first);
+           else
+             __extra = __first;
+         }
          __first = __next;
        }
+      if (__extra != __last)
+        _M_erase(__extra);
     }

   template<typename _Tp, typename _Alloc>


-- 


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

[Bug libstdc++/17012] [DR 526] std::list's function, remove, looks like it is reading memory that has been freed.

[chris at bubblescope dot net]

------- Comment #14 from chris at bubblescope dot net  2007-02-07 17:12 -------
Unless __value comes from the list, does the standard require
__a.address(__value) to work?


-- 


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

[Bug libstdc++/17012] [DR 526] std::list's function, remove, looks like it is reading memory that has been freed.

[hhinnant at apple dot com]

------- Comment #15 from hhinnant at apple dot com  2007-02-07 17:22 -------
(In reply to comment #14)
> Unless __value comes from the list, does the standard require
> __a.address(__value) to work?
> 

That's a good question Chris.  The way I read the current draft, I believe the
answer is no.  This looks like another defect report to me.  In the definition
of "r" and "s" in [allocator.requirements] I see no reason to have the clause
"obtained by the expression *p".  But I'll open a DR (635) and let the LWG
decide.


-- 


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

[Bug libstdc++/17012] [DR 526] std::list's function, remove, looks like it is reading memory that has been freed.

[pcarlini at suse dot de]

------- Comment #16 from pcarlini at suse dot de  2007-02-08 09:17 -------
Many thanks Howard and Chris: I had a quick look to the tentatively ready
issues and didn't notice this one. Anyway, in my understanding, DR 580 (thus
the consistent use of allocator::address and the new issues ;) is largely
orthogonal to this one and we can make good progress by simply applying Howard'
first fix. I'm going to set it up as a full patch, and send it to the list, to
close soon this PR.


-- 

pcarlini at suse dot de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|SUSPENDED                   |ASSIGNED
   Last reconfirmed|2004-08-12 20:11:36         |2007-02-08 09:17:16
               date|                            |


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

[Bug libstdc++/17012] [DR 526] std::list's function, remove, looks like it is reading memory that has been freed.

[paolo at gcc dot gnu dot org]

------- Comment #17 from paolo at gcc dot gnu dot org  2007-02-09 01:00 -------
Subject: Bug 17012

Author: paolo
Date: Fri Feb  9 01:00:25 2007
New Revision: 121735

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=121735
Log:
2007-02-08  Howard Hinnant  <hhinnant@apple.com>

        PR libstdc++/17012
        * include/bits/list.tcc (list<>::remove): Take care of
        &*__first == &__value.
        * docs/html/ext/howto.html: Add an entry for DR 526.

Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/docs/html/ext/howto.html
    trunk/libstdc++-v3/include/bits/list.tcc


-- 


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

[Bug libstdc++/17012] [DR 526] std::list's function, remove, looks like it is reading memory that has been freed.

[pcarlini at suse dot de]

------- Comment #18 from pcarlini at suse dot de  2007-02-09 08:52 -------
Fixed.


-- 

pcarlini at suse dot de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|---                         |4.3.0


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