Replace &__rhs with _VSTD::addressof(__rhs) to guard against ADL hijacking
of operator& in operator=. Thanks to @CaseyCarter for bringing it to our
attention.
Similar issues with hijacking operator& still exist, they will be
addressed separately.
Paths
| Differential D110852
[libc++] Use addressof in assignment operator. ClosedPublic Authored by Mordante on Sep 30 2021, 10:04 AM.
Details
Summary Replace &__rhs with _VSTD::addressof(__rhs) to guard against ADL hijacking Similar issues with hijacking operator& still exist, they will be
Diff Detail
Unit TestsFailed Event Timelinejloser added inline comments.
• Quuxplusone added inline comments.
This revision now requires changes to proceed.Sep 30 2021, 10:35 AM Comment Actions Thanks for the feedback. At the moment it seems the pre-merge checks are disable. So I'll fix it when the CI is in the air again. Mordante added inline comments.
Mordante marked 3 inline comments as done. Comment ActionsAddresses review comments. Comment Actions LGTM % priority_queue.
Comment Actions Thanks for tackling this! LGTM but I do have a few questions.
This revision now requires changes to proceed.Oct 5 2021, 11:14 AM
Comment Actions Awesome, thanks for the patch. Can you rename the new tests to e.g. assign_copy.addressof.compile.pass.cpp so we can tell at a glance that it's testing some sort of addressof-related issue, or operator highjacking? This revision is now accepted and ready to land.Oct 6 2021, 11:00 AM Closed by commit rGb8608b87239c: [libc++] Use addressof in assignment operator. (authored by Mordante). · Explain WhyOct 7 2021, 9:10 AM This revision was automatically updated to reflect the committed changes. Mordante mentioned this in D111564: [libc++] Use addressof to fix debug tests..Oct 11 2021, 10:47 AM
Revision Contents
Diff 376980 libcxx/include/__hash_table
libcxx/include/__tree
libcxx/include/deque
libcxx/include/forward_list
libcxx/include/list
libcxx/include/map
libcxx/include/unordered_map
libcxx/include/valarray
libcxx/include/vector
libcxx/test/std/containers/associative/map/map.cons/copy_assign.compile.pass.cpp
libcxx/test/std/containers/associative/multimap/multimap.cons/copy_assign.compile.pass.cpp
libcxx/test/std/containers/associative/multiset/multiset.cons/copy_assign.compile.pass.cpp
libcxx/test/std/containers/associative/set/set.cons/copy_assign.compile.pass.cpp
libcxx/test/std/containers/container.adaptors/priority.queue/priqueue.cons/assign_copy.compile.pass.cpp
libcxx/test/std/containers/container.adaptors/queue/queue.defn/assign_copy.compile.pass.cpp
libcxx/test/std/containers/sequences/array/array.cons/implicit_copy.compile.pass.cpp
libcxx/test/std/containers/sequences/deque/deque.cons/move_assign.compile.pass.cpp
libcxx/test/std/containers/sequences/forwardlist/forwardlist.cons/assign_copy.compile.pass.cpp
libcxx/test/std/containers/sequences/list/list.cons/assign_copy.compile.pass.cpp
libcxx/test/std/containers/sequences/vector/vector.cons/assign_copy.compile.pass.cpp
libcxx/test/std/containers/unord/unord.map/unord.map.cnstr/assign_copy.compile.pass.cpp
libcxx/test/std/containers/unord/unord.multimap/unord.multimap.cnstr/assign_copy.compile.pass.cpp
libcxx/test/std/containers/unord/unord.multiset/unord.multiset.cnstr/assign_copy.compile.pass.cpp
libcxx/test/std/containers/unord/unord.set/unord.set.cnstr/assign_copy.compile.pass.cpp
libcxx/test/std/numerics/numarray/template.valarray/valarray.assign/value_assign.compile.pass.cpp
libcxx/test/support/deleted_operator_ampersand.h
|
The pattern I use for "ADL-proofing tests" is find libcxx/test/ -name robust_against_adl.pass.cpp:
However, the problem here is that std::map actually is not fully ADL-proof, and cannot be made fully ADL-proof, because it relies on comparing allocator<pair<int, Holder<Incomplete>*>> objects with ADL operator!=. So I guess the deleted-specific-function version is the best we can do.