[lworld] RFR: 8349945: Implement strict static fields (proposed JVM feature)

[Dan Heidinga]

On Tue, 20 May 2025 22:15:29 GMT, Matias Saavedra Silva <matsaave at openjdk.org> wrote:

> This patch enables and implements verification for fields with the ACC_STATIC and ACC_STRICT modifiers. To enforce strictness on static fields, the reads and writes on the field are tracked dynamically to ensure that the field is written before being read and written to before.

src/hotspot/share/oops/instanceKlass.cpp line 1641:

> 1639:     }
> 1640:   } else {
> 1641:     // Experimentally, enforce additional proposed conditions after the first write.

Suggestion:

    // Ensure no write after read for final strict statics

I think this comment may be left over from the multi-option initial prototype.  It might be worth going over all the comments and making sure they still apply now that we've settled on a single approach

src/hotspot/share/oops/instanceKlass.cpp line 1651:

> 1649:         throw_strict_static_exception(bad_strict_static, "is set after read (as final) in", CHECK);
> 1650:       } else if (!is_writing && fs.is_strict_static_unread()) {
> 1651:         // log the read (this requires an extra status bit, with extra tests on it)

Suggestion:


This comment also seems like a holdover from the prototype

src/hotspot/share/oops/instanceKlassFlags.hpp line 62:

> 60:     flag(must_be_atomic                     , 1 << 17) /* doesn't allow tearing */ \
> 61:     flag(has_loosely_consistent_annotation  , 1 << 18) /* the class has the LooselyConsistentValue annotation WARNING: it doesn't automatically mean that the class allows tearing */ \
> 62:     flag(is_implicitly_constructible        , 1 << 19) /* the class has the ImplicitlyConstrutible annotation */ \

Did this get accidentally propogated into this patch from somewhere else?

src/hotspot/share/prims/jni.cpp line 2062:

> 2060:       !InstanceKlass::cast(k)->find_field(fieldname, signame, true, &fd) ||
> 2061:       // strict fields need special tracking during <clinit>; do not hand them out so early
> 2062:       (fd.access_flags().is_strict() && !InstanceKlass::cast(k)->is_initialized())) {

Should this be an assert rather than a runtime check given that we force the class to be initialized a few lines above?

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1465#discussion_r2102433487
PR Review Comment: https://git.openjdk.org/valhalla/pull/1465#discussion_r2102436621
PR Review Comment: https://git.openjdk.org/valhalla/pull/1465#discussion_r2102439672
PR Review Comment: https://git.openjdk.org/valhalla/pull/1465#discussion_r2102444109