On Thu, 14 Nov 2024 05:42:51 GMT, David Holmes <dhol...@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.
>
> 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
Thanks for noticing this. Updated comment that didn't make sense to me either.
> 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. :)
Yes, they are dead code.
> 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.
fixed. All these comments could use capitalization, but I won't do that here.
> 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.
Given that I'm probably the only one that has ever used this tag (or maybe also
Ioi), I think it's safe to remove.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/22064#discussion_r1842123715
PR Review Comment: https://git.openjdk.org/jdk/pull/22064#discussion_r1842124581
PR Review Comment: https://git.openjdk.org/jdk/pull/22064#discussion_r1842126691
PR Review Comment: https://git.openjdk.org/jdk/pull/22064#discussion_r1842127430
