Use the right symbol for constant string placeholder

[liqunl]

Constant string can be used as a placeholder and be patched to other
object. Set the flag on the symbol according to the actual object type.

Signed-off-by: Liqun Liu liqunl@ca.ibm.com

[DanHeidinga]

@liqunl my understanding was that other values besides MHs can be stored in these "string" constants as the Unsafe.defineAnonymousClass allows any object to be stored there. The bytecode pattern will be something like:

ldc  #42 // Constant_String but patched to some other object
instanceof <real type>
....

We want to be careful to not rely on the resolved value only being a string or a MH.

@fengxue-IS / @babsingh to confirm

[fengxue-IS]

for Java 8-14 where the InvokerBytecodeGenerator uses cp patching to reference dynamic object, the constant string placeholder can contain non-MH objects.
for Java 15+, the patch objects are stored into hiddenclass's classData field, so doesn't require checkcast.
@babsingh to confirm about the hiddenclass classData access

[babsingh]

to confirm about the hiddenclass classData access

In JDK15+, InvokerBytecodeGenerator uses the below-mentioned fields of ClassData to generate the appropriate bytecodes with the required checkcast instructions. In the below bytecodes, the static init block loads the value from classData into the field (_DATA_LambdaForm_0) with the correct checkcast. So, a checkcast won't be needed during invocation in JDK15+.

An instance of ClassData contains:

        // Used to generate the field to access the object; e.g. _DATA_LambdaForm_0.
        final String name;

        // Describes if the object is a Class, MethodHandle, LambdaForm or Object.
        // Used to add a checkcast before storing the object value into the generated field.
        final String desc;

        // Stored object value.
        final Object value;

Sample generated bytecodes to access classData in JDK16:

  static {};
    descriptor: ()V
    flags: (0x0008) ACC_STATIC
    Code:
      stack=2, locals=1, args_size=0
         0: ldc           #2                  // class java/lang/invoke/LambdaForm$BMH003
         2: invokestatic  #32                 // Method java/lang/invoke/MethodHandleNatives.classData:(Ljava/lang/Class;)Ljava/lang/Object;
         5: checkcast     #34                 // class java/util/List
         8: astore_0
         9: aload_0
        10: iconst_0
        11: invokeinterface #38,  2           // InterfaceMethod java/util/List.get:(I)Ljava/lang/Object;
        16: checkcast     #40                 // class java/lang/invoke/LambdaForm
        19: putstatic     #42                 // Field _DATA_LambdaForm_0:Ljava/lang/invoke/LambdaForm;
        22: return

[liqunl]

@DanHeidinga @fengxue-IS @babsingh Thanks for pointing that out. Will change the code to load any objects.

[liqunl]

Depends on eclipse-omr/omr#5664

[liqunl]

@andrewcraik @DanHeidinga May I ask for your review?
@dmitry-ten Could you review the JITServer change?

[andrewcraik]

The non-JITServer parts look good to me

[dmitry-ten]

Overall LGTM,
please increment MINOR_NUMBER in JITServer::CommunicationStream and update message numbers.

[dmitry-ten]

This commit results in 2 additional messages when a string is unresolved but if the increase is significant we'll deal with it later.

[liqunl]

This commit results in 2 additional messages when a string is unresolved but if the increase is significant we'll deal with it later.

It's 2 additional messages when the cp entry is resolved. I can fix it by checking if the symref is first created, only do the check if symref is first created.

[dmitry-ten]

It's 2 additional messages when the cp entry is resolved. I can fix it by checking if the symref is first created, only do the check if symref is first created.
If that's something that would potentially reduce the number of messages then sure, it'd be nice.

[liqunl]

Looking at a failure in my pbuild after updating the commits, WIP the PR now.

[liqunl]

Fixed the pbuild failure. @dmitry-ten @andrewcraik Could you take a look again?

[dmitry-ten]

Please increment the minor number here:
https://github.com/eclipse/openj9/blob/547bb8e04985d8043443b977ef979eb183139da6/runtime/compiler/net/CommunicationStream.hpp#L112
Otherwise, LGTM

[liqunl]

@dmitry-ten Fixed the minor number

[dmitry-ten]

LGTM

[liqunl]

@DanHeidinga I have made changes to support any type of patched objects, could you review again?

[DanHeidinga]

@DanHeidinga I have made changes to support any type of patched objects, could you review again?

Design wise, this seems right to me. I'll defer to @andrewcraik's already provided review.

[andrewcraik]

LGTM

[liqunl]

@DanHeidinga Andrew has approved this PR, may I ask you to help launching the PR build and merge if it passes?

[DanHeidinga]

Jenkins test sanity all jdk11

[DanHeidinga]

Builds have passed and given @andrewcraik's approval above, merging