Relo records not being created properly with inlining and findOrCreateShadowSymbol

[cjjdespres]

While running acmeair with -Xaot:forceAOT, I noticed that adding disableInlining in the cold run reduced the number of AOT failures by around 200, including roughly 150 compilationSymbolValidationManagerFailure failures (which are assertion failures in the SVM). If I run with TR_svmAssertionsAreFatal=1, I get a crash in TR_J9SharedCacheVM::getClassFromSignature() in this method:

https://github.com/eclipse-openj9/openj9/blob/eae6d58876cfd797d0913c623775ce172fa4f850/runtime/compiler/compile/J9SymbolReferenceTable.cpp#L791

Compiling this method without fatal assertions does show that the failure occurs during

[     4] O^O INLINER: Inlining qwerty java/util/Locale.equals(Ljava/lang/Object;)Z into java/util/ResourceBundle$CacheKey.equals(Ljava/lang/Object;)Z with a virtual guard kind=DirectMethodGuard type=NonoverriddenTest partialInline=0

The assertion is "method ... should have already been validated", and from looking at findOrFabricateShadowSymbol() it does seem like the first method of containingClass wouldn't necessarily have been validated already. A relo record should be created for this method, or some other way should be found to get declaringClass.

---

Actually, this might be related to https://github.com/eclipse-openj9/openj9/issues/20529#issuecomment-2484538171 as well. I was confused why certain values couldn't be looked up in a beholder constant pool while TR_RelocationRecordValidateStaticClassFromCP was being validated, and noticed that there weren't any prior records with an internalVMFunctions call that would actually resolve the entry at that index, unlike other collections of records. I suspect that if the first method of containingClass coincidentally happens to be validated already, the SVM compilation will succeed, but then will fail to load because a record isn't created for definingClass and some time later something called classOfStatic() on definingClass. I mention this because that ValidateStaticClassFromCP record was created while inlining java/util/Locale.equals(Ljava/lang/Object;)Z too, in what looks like the same the same place as the assertion failure here. (The method being compiled was different, of course).

Now that I think about it, getClassFromSignature() isn't called with isVettedForAOT=true and that would cause an arbitrary class validation to be created without SVM, so the declaringClass in findOrFabricateShadowSymbol() is probably missing a validation record as well.

[github-actions[bot]]

Issue Number: 20669 Status: Open Recommended Components: comp:vm, comp:test, comp:gc

[cjjdespres]

@mpirvu This is the failure I was seeing during my testing, and @dsouzai this could be related to the constant pool lookup failures I was talking about, which might have been a bug after all.

[dsouzai]

I suspect that if the first method of containingClass coincidentally happens to be validated already, the SVM compilation will succeed, but then will fail to load because a record isn't created for definingClass

I'm not sure how that could happen. The reason is because, if a method gets a SVM validation record, it is done via TR::SymbolValidationManager::addMethodRecord which calls appendClassChainInfoRecords(record->definingClass(), chainInfo); [1]. If the defining class was not already validated, this will define a new ID as part of the ClassChainRecord record.

That said, I had run into a "should have already been validated" error as part of my AOT MH work, because there was a mismatch between the JIT and AOT FrontEnd/ResolvedMethod APIs [2]. Maybe something similar is happening here? You'd have to use a trace log to find the source of the method that is missing the validation.

[1] https://github.com/eclipse-openj9/openj9/blob/ff435327b1ea3f7ee9593545b0ab27c92673edae/runtime/compiler/runtime/SymbolValidationManager.cpp#L742 [2] https://github.com/eclipse-openj9/openj9/pull/20272/commits/9a779e1b096fb80f822c800f825b8ea456e0a439

[dsouzai]

Now that I think about it, getClassFromSignature() isn't called with isVettedForAOT=true and that would cause an arbitrary class validation to be created without SVM, so the declaringClass in findOrFabricateShadowSymbol() is probably missing a validation record as well.

Because getClassFromSignature() is not called with isVettedForAOT=true, in non-SVM AOT it will return NULL, which I think just means the compiler treats this symbol as unresolved. So, it's fine that there's no validation record here.