On Tue, 21 Apr 2026 06:36:28 GMT, Axel Boldt-Christmas <[email protected]> wrote:
>> Hello, >> >> When looking at the usages of >> `InstanceKlass::get_inline_type_field_klass_or_null` I saw that >> `SystemDictionary::try_preload_from_loadable_descriptors` uses the returned >> value even though it may be null, which can resolve in a SIGSGV. This change >> aligns well with what is done just above in >> `SystemDictionary::preload_from_null_free_field`, using the local variable >> `name` instead of getting the name from the klass. >> >> >> Given the right conditions this will result in a crash, but it seems rare as >> it requires "the stars to be aligned just right" for it to happen. >> >> I'm not sure how much testing testing there is for classes which have >> inlined and non-inlined fields in them in connection to the code being >> changed. Such classes would result in an inline_layout_info_array having >> null-entires, since it is a sparse array. >> >> Testing: >> * Oracle's tier1-3 >> * Testing with a local reproducer that this does not crash with the changes >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > src/hotspot/share/classfile/systemDictionary.cpp line 1167: > >> 1165: "app substituted a different version >> of %s", >> 1166: name->as_C_string(), >> ik->name()->as_C_string(), >> 1167: name->as_C_string()); > > What is the difference between `name` and `k->name` here. It sounds like > `name` originates from a signature. Not sure what the value is in printing > `name` twice. > > I'd expect something more along the lines of the following, even if we expect > that `name` and `k->name` are the same symbol, it would leave breadcrumbs > incase they ever diverged. > > Suggestion: > > k != nullptr ? k->name->as_C_string() : > "[name unavailable]"); Yes, name is from a signature TempNewSymbol name = Signature::strip_envelope(sig); I don't think `name` and `k->name()` should ever diverge since both the signature and the klass from `get_inline_type_field_klass_or_null` are from the same "field_index" in the caller. If anything, I'd expect a stronger invariant, maybe with an assert, that they are the same if k is non-null. I agree that the print could do without printing the name twice, but I don't have anything against the current version. ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/2344#discussion_r3115596677
