RFR: 8302670: use-after-free related to PhaseIterGVN interaction with Unique_Node_List and Node_Stack [v10]

[Kim Barrett]

On Thu, 18 May 2023 10:06:59 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> src/hotspot/share/libadt/dict.hpp line 65:
>> 
>>> 63: 
>>> 64:   // Allow move constructor for && (eg. capture return of function)
>>> 65:   Dict(Dict&&) = default;
>> 
>> Nit: You might consider invalidating the other dict being moved from, to catch accidental use-after-move. Could be punted to a future change.
>
> The only way to get use-after-move is with an explicit `std::move` or equivalent.
> 
> But shouldn't there also be a move-assign operator?  I think at some point the standard mandates
> some amount of the Rule of Five (previously the Rule of Three), and some violations are already
> deprecated in C++14 (and gcc warns about them).
> 
> OTOH, I think the rationale of needing a move constructor to permit returning noncopyable objects from functions is eliminated by C++17's guaranteed copy elision.  So it might be that these move constructors are only needed until we upgrade the language standard we use.  (I hope that will be soon-ish, but it's not imminent.)

The main reason to do some cleanup of the moved-from object is if it owns resources that are being transferred
by the move.  You don't want the destruction of the moved-from object trashing stuff in the moved-to object.
All of the classes having move-constructors added by this PR have default or empty destructors.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13833#discussion_r1197840725