Ensure addClassFromCPRecord emits CP record

[cjjdespres]

The addClassFromCPRecord method of the symbol validation manager should make sure that a ClassFromCPRecord is always added to the current compilation.

[cjjdespres]

@dsouzai I'm fairly sure that a ClassFromCPRecord needs to be added here if the class hasn't already been remembered. I can't see a reason why it can be skipped, at least.

[cjjdespres]

I have tested this to see if it helped with #20529 (comment), and unfortunately it does not. I'm curious - if this record does need to be added, do we need to make sure that a ClassFromCPRecord is added for these as well?

openj9/runtime/compiler/runtime/SymbolValidationManager.cpp

Lines 853 to 873 in 395aab5

	bool
	TR::SymbolValidationManager::addDefiningClassFromCPRecord(TR_OpaqueClassBlock *clazz, J9ConstantPool *constantPoolOfBeholder, uint32_t cpIndex, bool isStatic)
	{
	TR_OpaqueClassBlock *beholder = _fej9->getClassFromCP(constantPoolOfBeholder);
	SVM_ASSERT_ALREADY_VALIDATED(this, beholder);
	if (skipFieldRefClassRecord(clazz, beholder, cpIndex))
	return true;
	else
	return addClassRecord(clazz, new (_region) DefiningClassFromCPRecord(clazz, beholder, cpIndex, isStatic));
	}
	bool
	TR::SymbolValidationManager::addStaticClassFromCPRecord(TR_OpaqueClassBlock *clazz, J9ConstantPool *constantPoolOfBeholder, uint32_t cpIndex)
	{
	TR_OpaqueClassBlock *beholder = _fej9->getClassFromCP(constantPoolOfBeholder);
	SVM_ASSERT_ALREADY_VALIDATED(this, beholder);
	if (skipFieldRefClassRecord(clazz, beholder, cpIndex))
	return true;
	else
	return addClassRecord(clazz, new (_region) StaticClassFromCPRecord(clazz, beholder, cpIndex));
	}

In the compilation in the linked comment, a DefiningClassFromCPRecord was created for a class/beholder/cp index triple without a ClassFromCPRecord being created for it.

[cjjdespres]

I tried #20680 (comment), but it seemed to result in a very large number of class from CP validation failures at load time. It might be DeclaringClassFromFieldOrStatic that could use a DefiningClassFromCPRecord, if that's appropriate.

[cjjdespres]

I should test this a bit more.

[dsouzai]

@dsouzai I'm fairly sure that a ClassFromCPRecord needs to be added here if the class hasn't already been remembered. I can't see a reason why it can be skipped, at least.

This was done in #4694; as per the commit message:

If a class is first encountered by the symbol validation manager as the
result of ClassFromCP, it is necessary to generate both the originally
requested ClassFromCP record and a ClassChain validation record. A
ClassByName with the same beholder has the same results as ClassFromCP
would, and it subsumes ClassChain, so that only ClassByName is needed.
The ClassByName record is larger than the ClassFromCP alone, but smaller
than the two records would have been together.

[cjjdespres]

That does make sense. I noticed that class from CP records were often skipped. I'm trying something else to try to resolve the validation failures related to CP entries with dependency tracking; since the lack of record is intentional I'll likely just close this.