RFR: 8303512: Race condition when computing is_loaded property of TypePtr::InterfaceSet [v3]

[Tobias Hartmann]

> [JDK-8297933](https://bugs.openjdk.org/browse/JDK-8297933) introduced a race condition among compiler threads computing `TypePtr::InterfaceSet::_is_loaded` for shared types (for example, for [TypeInstPtr::NOTNULL](https://github.com/openjdk/jdk/blob/5726d31e56530bbe7dee61ae04b126e20cb3611d/src/hotspot/share/opto/type.cpp#L545)). One thread can set `_is_loaded_computed` before setting `_is_loaded`:
> 
> https://github.com/openjdk/jdk/blob/5726d31e56530bbe7dee61ae04b126e20cb3611d/src/hotspot/share/opto/type.cpp#L3473-L3482
> 
> while another thread can already access `is_loaded()` and wrongly observe `_is_loaded = false`:
> 
> https://github.com/openjdk/jdk/blob/5726d31e56530bbe7dee61ae04b126e20cb3611d/src/hotspot/share/opto/type.cpp#L3464-L3468
> 
> As a result, the klass of a `TypePtr` can be loaded while the interfaces it implements appear to be not loaded.
> 
> Another problem is that `TypePtr::InterfaceSet::eq` does not take the `_is_loaded` / `_is_loaded_computed` fields into account:
> 
> https://github.com/openjdk/jdk/blob/5726d31e56530bbe7dee61ae04b126e20cb3611d/src/hotspot/share/opto/type.cpp#L3290-L3301
> 
> A loaded type can therefore be replaced by an unloaded type during GVN.
> 
> In the case of the failure reported by this bug, `LibraryCallKit::inline_native_hashcode` first null checks the receiver and updates the type. Due to the issues described above, the null-free type is GVN'ed with it's unloaded counterpart and propagated to another, redundant null check emitted by `LibraryCallKit::generate_method_call` (I filed [JDK-8307625](https://bugs.openjdk.org/browse/JDK-8307625) to remove it). Since the type is now unloaded, an uncommon trap is emitted and parsing is stopped(). We crash when trying to de-reference `GraphKit::_map->_jvms` which is NULL (in debug we would hit an assert).
> 
> Another failure mode is reported by [JDK-8305339](https://bugs.openjdk.org/browse/JDK-8305339). Both failures are extremely intermittent and I was never able to reproduce them.
> 
> The fix I propose is to completely remove the `_is_loaded` logic because if a klass is loaded, the interface that it implements should be loaded as well. I added an assert to verify this. Higher tier testing is still running.
> 
> In addition, I noticed some `#ifdef DEBUG` checks where `DEBUG` is never defined. I replaced them by `#ifdef ASSERT` and fixed the verification code that failed to build.
> 
> Thanks,
> Tobias

Tobias Hartmann has updated the pull request incrementally with one additional commit since the last revision:

  Refactoring

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/13868/files
  - new: https://git.openjdk.org/jdk/pull/13868/files/27d502d0..d0dce7b6

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=13868&range=02
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=13868&range=01-02

  Stats: 52 lines in 3 files changed: 19 ins; 15 del; 18 mod
  Patch: https://git.openjdk.org/jdk/pull/13868.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/13868/head:pull/13868

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