[lworld] RFR: 8376045: [lworld] Treat @NullRestricted fields as ACC_STRICT_INIT on load [v6]

Frederic Parain fparain at openjdk.org
Tue Jan 27 17:55:29 UTC 2026


On Tue, 27 Jan 2026 15:41:33 GMT, Chen Liang <liach at openjdk.org> wrote:

>> This is the 2nd PR in the strict removal series. 1st PR is #1952, and 3rd PR is #1959.
>> 
>> Automatically inject strict for NR annotations, and remove tests that previously kept them separate. Also add behavior where NR is only accepted for preview class files.
>
> Chen Liang has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains eight additional commits since the last revision:
> 
>  - Correctly fix resource leak this time
>  - Merge branch 'lworld' of https://github.com/openjdk/valhalla into feature/cfp-nr-auto-strict
>  - Fix conditionals
>  - Missing returns?
>  - Try again?
>  - Require NR users to be preview
>  - Merge branch 'lworld' of https://github.com/openjdk/valhalla into feature/cfp-nr-auto-strict
>  - 8376045: [lworld] Treat @NullRestricted fields as ACC_STRICT_INIT on load

Could you try the patch below that enabled early returns in the parse_fields() method by removing the assumption in Annotations deallocation code that the arrays are fully populated, and by ensuring the ownership transfer of field_annotations is completed before all return to prevent a double free.
It has been tested with only the two tests of this PR, so more testing is needed.


diff --git a/src/hotspot/share/classfile/classFileParser.cpp b/src/hotspot/share/classfile/classFileParser.cpp
index 8af14a0d984..0cd354401e3 100644
--- a/src/hotspot/share/classfile/classFileParser.cpp
+++ b/src/hotspot/share/classfile/classFileParser.cpp
@@ -1461,6 +1461,7 @@ void ClassFileParser::parse_fields(const ClassFileStream* const cfs,
                                              CHECK);
         }
         _fields_annotations->at_put(n, parsed_annotations.field_annotations());
+        parsed_annotations.set_field_annotations(nullptr);
         if (parsed_annotations.has_annotation(AnnotationCollector::_jdk_internal_NullRestricted)) {
           if (!Signature::has_envelope(sig)) {
             Exceptions::fthrow(
@@ -1468,6 +1469,7 @@ void ClassFileParser::parse_fields(const ClassFileStream* const cfs,
               vmSymbols::java_lang_ClassFormatError(),
               "Illegal use of @jdk.internal.vm.annotation.NullRestricted annotation on field %s.%s with signature %s (primitive types can never be null)",
               class_name()->as_C_string(), name->as_C_string(), sig->as_C_string());
+              return;
           }
           if (!supports_inline_types()) {
             Exceptions::fthrow(
@@ -1475,9 +1477,10 @@ void ClassFileParser::parse_fields(const ClassFileStream* const cfs,
               vmSymbols::java_lang_ClassFormatError(),
               "Illegal use of @jdk.internal.vm.annotation.NullRestricted annotation on field %s.%s in non-preview class file",
               class_name()->as_C_string(), name->as_C_string());
+              return;
           }
           const bool is_strict = (flags & JVM_ACC_STRICT) != 0;
-          if (!is_strict && !HAS_PENDING_EXCEPTION) {
+          if (!is_strict) {
             // Inject STRICT_INIT and validate in context
             const jint patched_flags = flags | JVM_ACC_STRICT;
             verify_legal_field_modifiers(patched_flags, class_access_flags, CHECK);
@@ -1485,7 +1488,6 @@ void ClassFileParser::parse_fields(const ClassFileStream* const cfs,
           }
           is_null_restricted = true;
         }
-        parsed_annotations.set_field_annotations(nullptr);
       }
       if (parsed_annotations.field_type_annotations() != nullptr) {
         if (_fields_type_annotations == nullptr) {
diff --git a/src/hotspot/share/oops/annotations.cpp b/src/hotspot/share/oops/annotations.cpp
index c4e49d52383..8fb6f3b7824 100644
--- a/src/hotspot/share/oops/annotations.cpp
+++ b/src/hotspot/share/oops/annotations.cpp
@@ -41,7 +41,9 @@ Annotations* Annotations::allocate(ClassLoaderData* loader_data, TRAPS) {
 void Annotations::free_contents(ClassLoaderData* loader_data, Array<AnnotationArray*>* p) {
   if (p != nullptr) {
     for (int i = 0; i < p->length(); i++) {
-      MetadataFactory::free_array<u1>(loader_data, p->at(i));
+      if (p->at(i) != nullptr) {
+        MetadataFactory::free_array<u1>(loader_data, p->at(i));
+      }
     }
     MetadataFactory::free_array<AnnotationArray*>(loader_data, p);
   }

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

PR Comment: https://git.openjdk.org/valhalla/pull/1951#issuecomment-3806624375


More information about the valhalla-dev mailing list