RFR: 8320530: has_resolved_ref_index flag not restored after resetting entry

[Ioi Lam]

On Thu, 23 Nov 2023 05:46:27 GMT, David Holmes <dholmes at openjdk.org> wrote:

> Change seems fine but what was the effect of not restoring the flag? Does this cause failures or just unnecessary re-resolution, or?
> 
> Thanks

The code works somehow, but in an unsafe manner. We are reading from `resolved_references_index()` even when the `has_resolved_ref_shift` bit has been (improperly) cleared. Adding the following assert makes it impossible to start jtreg:


diff --git a/src/hotspot/share/oops/cpCache.cpp b/src/hotspot/share/oops/cpCache.cpp
index daa094baa7e..760c5268c88 100644
--- a/src/hotspot/share/oops/cpCache.cpp
+++ b/src/hotspot/share/oops/cpCache.cpp
@@ -310,6 +310,9 @@ ResolvedMethodEntry* ConstantPoolCache::set_method_handle(int method_index, cons
   // Store appendix, if any.
   if (has_appendix) {
+    assert(method_entry->has_resolved_ref_index(), "huh");
     const int appendix_index = method_entry->resolved_references_index();
     objArrayOop resolved_references = constant_pool()->resolved_references();


I think we should take this as a chance to tighten up the code in resolvedMethodEntry.hpp:

- `resolved_references_index()` should assert that `has_resolved_ref_index()`.
- The following three functions should assert for mutual exclusivity. I.e., you can't call set_klass() and then call  set_resolved_references_index(). Probably the easiest is to add two more bits: `_has_klass_shift` and `_has_table_index_shift`. At entry of these three setters, we should assert that all three klass/table_index/resolved_references_index bits are cleared.


  void set_klass(InstanceKlass* klass) {
    _entry_specific._interface_klass = klass;
  }
  void set_resolved_references_index(u2 ref_index) {
    set_flags(1 << has_resolved_ref_shift);
    _entry_specific._resolved_references_index = ref_index;
  }
  void set_table_index(u2 table_index) {
    _entry_specific._table_index = table_index;
  }


Also, `has_resolved_ref_index` should be renamed to `has_resolved_reference_index`; otherwise it's difficult to search for all code related to `resolved_refenece_index`.

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

PR Comment: https://git.openjdk.org/jdk/pull/16769#issuecomment-1827119981