RFR: JDK-8302595: use-after-free related to GraphKit::clone_map [v2]

[Vladimir Kozlov]

On Wed, 15 Feb 2023 20:01:16 GMT, Justin King <jcking at openjdk.org> wrote:

>> `GraphKit::clone_map` duplicates `SafePointNode` and calls `Compile::record_for_igvn`. In some cases `SafePointNode` is not used so `Node::destruct` is called to cleanup. The `Unique_Node_List` returned by `Compile::for_igvn` still references the node which resides in freed memory which may or may not have been reused. We additionally need to remove the node from `Unique_Node_List` as well to prevent this from happening.
>> 
>> I introduced `GraphKit::destruct_map_clone` which undoes `GraphKit::clone_map`. It even clears the type, though I am not sure if this is necessary so feel free to suggest otherwise. Additionally it calls `delete` on `JVMState`, which is a noop, but it seems like the correct thing to do in case its ever changed.
>
> Justin King has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Update based on review
>   
>   Signed-off-by: Justin King <jcking at google.com>

src/hotspot/share/opto/compile.hpp line 940:

> 938:   Unique_Node_List* for_igvn()                  { return _for_igvn; }
> 939:   inline void       record_for_igvn(Node* n);   // Body is after class Unique_Node_List.
> 940:   inline void       remove_for_igvn(Node* n);   // Body is after class Unique_Node_List.

May be change comment.

src/hotspot/share/opto/graphKit.cpp line 743:

> 741: // to destruct/free/delete in the exact opposite order as clone_map().
> 742: void GraphKit::destruct_map_clone(SafePointNode* m) {
> 743:   if (m == nullptr) return;

Please rename `m` to `sfp`. `m` is for `memory`.

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

PR: https://git.openjdk.org/jdk/pull/12578