[Bug libstdc++/104909] New: Potential use-after-move in _Hashtable constructor

[Bug libstdc++/104909] New: Potential use-after-move in _Hashtable constructor

[gjasny at googlemail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104909

            Bug ID: 104909
           Summary: Potential use-after-move in _Hashtable constructor
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: gjasny at googlemail dot com
  Target Milestone: ---

Hello,

Coverity complains about one of the _Hashtable constructors in
bits/hashtable.h: 

 541       // Use delegating constructors.
 542       _Hashtable(_Hashtable&& __ht)
 543         noexcept(_S_nothrow_move())
 544       : _Hashtable(std::move(__ht), std::move(__ht._M_node_allocator()),
 545                    true_type{})
 546       { }

https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/include/bits/hashtable.h;h=5e1a417f7cda010002f65c763c29be75703ee380;hb=refs/heads/master#l544

It complains that __ht._M_node_allocator() is accessed after the whole __ht
object has been moved. What's your opinion on that?

Thanks,
Gregor

[Bug libstdc++/104909] Potential use-after-move in _Hashtable constructor

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=104909

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
It looks wrong, but it's not. 

The std::move calls do not actually alter anything, they only cast the argument
to an rvalue reference.  Then those rvalue reference are passed to this
constructor:

    _Hashtable<_Key, _Value, _Alloc, _ExtractKey, _Equal,
               _Hash, _RangeHash, _Unused, _RehashPolicy, _Traits>::
    _Hashtable(_Hashtable&& __ht, __node_alloc_type&& __a,
               true_type /* alloc always equal */)
    noexcept(_S_nothrow_move())
    : __hashtable_base(__ht),
      __map_base(__ht),
      __rehash_base(__ht),
      __hashtable_alloc(std::move(__a)),

The base classes are copied from __ht, then the allocator is moved. That means
that __ht has not been altered by the time we read from the allocator.

The code works correctly.