RFR: 8341916: Remove ProtectionDomain related hotspot code and tests

[David Holmes]

On Wed, 13 Nov 2024 11:42:11 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

> Remove Hotspot code that passes protection_domain around class loading so that checkPackageAccess can be called and the result stored.  With the removal of the Security Manager in JEP 486, this code no longer does anything.
> 
> Tested with tier1-4.

This is a great cleanup!

I may have missed something, but it seems to me that `java_security_AccessControlContext` is all dead code now too.

Thanks

src/hotspot/share/ci/ciEnv.cpp line 1613:

> 1611: 
> 1612:   // The very first entry is the InstanceKlass of the root method of the current compilation in order to get the right
> 1613:   // (class loader???) protection domain to load subsequent classes during replay compilation.

Suggestion: simply have:

 // The very first entry is the InstanceKlass of the root method of the current compilation .

The rest of the comment doesn't really make sense even before your change as this method basically just prints the class name

src/hotspot/share/classfile/dictionary.cpp line 80:

> 78: 
> 79: void Dictionary::Config::free_node(void* context, void* memory, Value const& value) {
> 80:   delete value; // Call DictionaryEntry destructor

`using Value = XXX` seems like an unwanted/unnecessary abstraction in this code, because depending on what `XX` is you either will or won't need to call `delete`. That is a more general cleanup though.

src/hotspot/share/classfile/javaClasses.hpp line 1545:

> 1543:   static int _static_security_offset;
> 1544:   static int _static_allow_security_offset;
> 1545:   static int _static_never_offset;

Guess these were missed by the main PR as they are unused. :)

src/hotspot/share/classfile/systemDictionary.hpp line 239:

> 237:   // compute java_mirror (java.lang.Class instance) for a type ("I", "[[B", "LFoo;", etc.)
> 238:   // Either the accessing_klass or the CL can be non-null, but not both.
> 239:   // callee will fill in CL from AK, if they are needed

Suggestion:

  // Callee will fill in CL from accessing_klass, if they are needed.

src/hotspot/share/logging/logTag.hpp line 163:

> 161:   LOG_TAG(preview)   /* Trace loading of preview feature types */ \
> 162:   LOG_TAG(promotion) \
> 163:   LOG_TAG(protectiondomain) /* "Trace protection domain verification" */ \

Not 100% sure about this. We don't really have a policy for "deprecating" or removing log tags. I think it unlikely anyone enables this logging "just because", so it seems okay for this case.

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

PR Review: https://git.openjdk.org/jdk/pull/22064#pullrequestreview-2435096096
PR Review Comment: https://git.openjdk.org/jdk/pull/22064#discussion_r1841595529
PR Review Comment: https://git.openjdk.org/jdk/pull/22064#discussion_r1841597831
PR Review Comment: https://git.openjdk.org/jdk/pull/22064#discussion_r1841688595
PR Review Comment: https://git.openjdk.org/jdk/pull/22064#discussion_r1841691487
PR Review Comment: https://git.openjdk.org/jdk/pull/22064#discussion_r1841698260