[llvm-bugs] [Bug 47956] New: std::map:operator= does assignment on the underlying value type, dereferencing references!

[via llvm-bugs]

https://bugs.llvm.org/show_bug.cgi?id=47956

            Bug ID: 47956
           Summary: std::map:operator= does assignment on the underlying
                    value type, dereferencing references!
           Product: libc++
           Version: 11.0
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P
         Component: All Bugs
          Assignee: unassignedclangbugs at nondot.org
          Reporter: greg at dignus.com
                CC: llvm-bugs at lists.llvm.org, mclow.lists at gmail.com

libc++'s std::map::operator= implementation re-uses the existing tree nodes in
the destination map to avoid unnecessary memory allocations.  To accomplish
this, it uses assignment to overwrite the nodes' contents.  If the value type
(template argument to std::map) is a reference, then that assignment is
__value_type::operator=, which calls std::pair::operator=, which does an actual
value assignment on the reference.  This has two downsides: it means a "const
type &" reference can't be used for the value, and it means that it will
overwrite the value itself for mutable references!

Here is an example:

   #include <map>
   #include <iostream>
   int arr[5] = {100,101,102,103,104};
   typedef std::map<int, int&> m;
   typedef m::value_type p;
   int main(void) {
        m a,b;
        a.insert({ p(200,arr[0]), p(201,arr[1]), p(202,arr[2]),
                p(203,arr[3]), p(204,arr[4]) });
        b = a;
        a = b;          /* a's old nodes are now being overwritten! */
        for (m::const_iterator i = a.begin(); i != a.end(); i++) {
                std::cout<<i->first<<" "<<i->second<<" "<<
                        ((int *)&i->second - arr)<<std::endl;
        }
   }

I have used that with libc++ versions 3.8.0, 9.0, and 11.0, and this is the
result I get from all versions:

   200 100 0
   201 101 2
   202 101 4
   203 103 3
   204 101 1

The original arr[] elements have been overwritten.  Most operations on the map
initialize the reference (taking the address of the value), but map assignment
assigns the reference (dereferencing it).

This result is astonishing to the user, but I'm not certain it's forbidden by
the standard.  std::pair::operator= is required to do a value assignment, but
the requirements for containers are not as clear to me.  Anyways, I'm quite
confident that this isn't the result a user would expect or desire.

I have a patch against version 3.8.0 that "fixes" the problem by implementing
__value_type::operator= in <map> using placement new (initializing the
reference) instead of assignment:

     __value_type& operator=(const __value_type& __v)
        { __cc.~value_type(); new((void*)&__cc) value_type(__v.__cc);
          return *this; }

But I don't really want to hold that up as an ideal fix, it's just the hack I
came up with.  I figure there is a reason __value_type contorts the way that it
does, but I don't know it.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-bugs/attachments/20201023/17e8f4c0/attachment.html>