RFR: Merge jdk-21-ga

[Man Cao]

On Tue, 9 Apr 2024 00:16:14 GMT, Jiangli Zhou <jiangli at openjdk.org> wrote:

> JDK 21 merge command: `$ git merge --no-ff jdk-21-ga`
> 
> Most manual merge for conflicts are mechanical.
> 
> - src/hotspot/cpu/aarch64/templateTable_aarch64.cpp
> - src/hotspot/cpu/x86/templateTable_x86.cpp
> Add extra `noreg` arg to `access_store_at` in `TemplateTable::lastore` and friends.
> 
> - src/hotspot/share/runtime/mutexLocker.cpp
> Update TsanOopMap_lock to use MUTEX_DEFN.
> 
> - src/hotspot/share/runtime/sharedRuntime.cpp
> Update unused_reg_map() to reflect RegisterMap change.
> 
> Additional updates:
> 
> - Field access flags related (related to https://github.com/openjdk/jdk21/commit/bfb812a8ff8bca70aed7695c73f019ae66ac6f33 change)
> Change TSAN specific field access flag, JVM_ACC_FIELD_TSAN_IGNORE to be a FieldFlags, _ff_tsan_ignore. Add is_tsan_ignore() and update_tsan_ignore in FieldFlags and replace the existing usages in various places to use the FieldFlags functions for the tsan_ignore flag. Remove the old is_tsan_ignore() and set_tsan_ignore() from FieldInfo.
> 
> - init_lock related (related to https://github.com/jianglizhou/tsan/commit/9099f3e7b58deec50cd8af9db49b2b2e564a6bfe changes)
> In instanceKlass.cpp, change to do SharedRuntime::tsan_acquire and SharedRuntime::tsan_release on the class mirror. 
> 
> In JDK 21, there is no eager_initialization (removed by https://github.com/openjdk/jdk/commit/cec23b1b078cd0c82063dda1af5a54ea561855c9). So we now only have SharedRuntime::tsan_acquire in InstanceKlass::initialize and SharedRuntime::tsan_release in InstanceKlass::set_initialization_state_and_notify with the latest version.
> 
> In InstanceKlass::set_initialization_state_and_notify, only do SharedRuntime::tsan_release for the mirror if
>   the state is fully_initialized.
> 
> Remove the code for loading the address of init_lock in src/hotspot/cpu/x86/templateTable_x86.cpp and
>   src/hotspot/cpu/aarch64/templateTable_aarch64.cpp. SharedRuntime::tsan_acquire calls are done with 
>   `obj`.
> 
> Testing:
> 
> Built with following config:
> 
> 
> $ bash configure --with-boot-jdk=/usr/local/google/home/jianglizhou/openjdk/jdk-21.0.1 --with-debug-level=release --disable-warnings-as-errors --with-jtreg=/usr/local/google/home/jianglizhou/github/jtreg/build/images/jtreg --with-stdc++lib=static --disable-precompiled-headers --enable-unlimited-crypto --with-native-debug-symbols=internal --with-default-make-target=jdk-image --disable-warnings-as-errors --with-toolchain-type=clang --disable-warnings-as-errors
> 
> 
> 
> $ make test TEST=hotspot/jtreg/tsan
> 
> 
> 
> ==============================
> Test summary
> ==================...

Great job, and thanks for this!
One additional note is that the TSAN changes to 
`SystemDictionary::double_lock_wait()` and `ObjectMonitor::complete_exit()` are removed, because JDK has removed those methods. They are for the workaround of deadlocks on the class loader lock objects.
This merge seems to also fix a bug from [JDK 17 merge PR](https://github.com/openjdk/tsan/pull/17): in `sharedRuntime_aarch64.cpp`, above the instrumentation for `SharedRuntime::tsan_oop_unlock`, there was an unnecessary `__ resolve(IS_NOT_NULL, obj_reg);`. Good that it is deleted now.

Below are some small actionable feedback:

In `javaClasses.hpp`, the empty block of `#if INCLUDE_TSAN` with a FIXME line above can be removed.

In `fieldInfo.hpp`, the removal of empty line above the following context is not needed:

 // Gadget for decoding and reading the stream of field records.
 class FieldInfoReader {


In `instanceKlass.cpp`, `InstanceKlass::initialize`, the new sentence in comment is a bit hard to understand:
`// This is to read/write of natives related to class static initializer.`
How about "Native memory accesses in JNI code invoked from class static initializer could rely on this happens-before edge to avoid data races."?
Alternatively, we can leave the old comment as-is, and create a separate PR later to add the new sentence, to make this change more conspicuous.

In `InstanceKlass::set_initialization_state_and_notify()`, it might be better to wrap the `if (state == fully_initialized)` check inside `TSAN_RUNTIME_ONLY` macro. Also, the indention seems off inside the if block.

Change to `jvmtiTagMap.hpp` in unnecessary (added an empty line).

In `unsafe.cpp`, in `Unsafe_CompareAndExchangeInt`, it should be `ScopedReleaseAcquire releaseAcquire(addr)` instead of `ScopedReleaseAcquire releaseAcquire(p, offset)`.

In `tsanOopMap.cpp`, I can see it is difficult to get `GCTraceCPUTime` working, so it is fine to remove it. But for `GCTraceTime`, do they trigger a build error if they are left as-is? It is better to remove `GCTraceTime` in the later change that fixes the issues with tsanOopMap.

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

PR Comment: https://git.openjdk.org/tsan/pull/18#issuecomment-2060654960