Sign extend 32bit offsets in inlineIntrinsicInflate

[klangman]

Without sign extending the 32bit offsets, it is possible for the upper 32bits of the offset registers to contain garbage bits that will cause the address calculations to produce incorrect (unaddressable) results.

[klangman]

@IBMJimmyk @zl-wang This fixes the problem that we talked about a few weeks back. My personal build (for all POWER platforms) passed.

[IBMJimmyk]

It looks good to me.

[zl-wang]

Did you find out the specific situation where garbage upper 32bits made its way into register?
This PR certainly fixed your particular issue on hand, but it might miss the real culprit at large. Appreciate it if that can be found out. I think codegen should be expected to produce correct values in full register.

[klangman]

Did you find out the specific situation where garbage upper 32bits made its way into register?

In the failing case the caller of String.getChars() calculates the 1st parameter (srcBegin) to be the return from String.lastIndexOf() using a parameter string that is not contained in the 'this' String, then it adds one. Since lastIndexOf() returns -1 (0xffffffff), and then we add 1 we get (0x100000000) in the 64bit register. This value is passed to String.getChars() which needs to be 32bit sign extended to be a proper 64bit 0.

Here is the test-case:

public class getCharsCrash {
   public static void main(String argv[]){
      for( int i=0 ; i< 5000 ; i++ ) {
         test(argv[0]);
      }
   }

   static void test(String str) {
      char dest[] = new char[128];
      int begin = str.lastIndexOf("missing");
      System.out.println("begin: " + begin);
      begin++;
      str.getChars(begin, str.length()-1, dest, 0);
      System.out.println("dest: " + new String(dest));
   }
}

With the right inlining restrictions this test-case crashes in String.getChars().

[zl-wang]

ah .. i recalled it now: the original issue involved an int load-back (from stack) with lwz, right?
let me merge this fix in first, while thinking about a complete fix (e.g. lwa for the load-back, it has perf implications though).

[zl-wang]

Jenkins test sanity aix,plinux jdk8,jdk23

[klangman]

ah .. i recalled it now: the original issue involved an int load-back (from stack) with lwz, right? let me merge this fix in first, while thinking about a complete fix (e.g. lwa for the load-back, it has perf implications though).

There was no load here. It was String.lastIndexOf() returning -1 (0xffffffff) which is turned into 0x100000000 when we add 1 to that number. This value is then moved to a parameter passing register for a call to String.getChars() which uses it in an address calculation without sign extending the parameter register.

[zl-wang]

I am curious how the original 0xffffffff (without sign-extension) got into the register as the return value.

[klangman]

I am curious how the original 0xffffffff (without sign-extension) got into the register as the return value.

I guess a function executing return(-1); returns as a 32bit -1 rather than a 64bit -1. I can look at the core and check the code returning -1 from lastIndexOf().

[klangman]

Found it... Here is how the value returned from lastIndexOf() was turned from a 64bit -1 into a 32bit -1.

0x10011705054 {org/jboss/modules/PathUtils.canonicalize} +86   -1:313  |    4bc0ddc9 bl        0x10011312e1c ^{java/lang/String.lastIndexOf} +3    // invokevirtual 59 {java/lang/String.lastIndexOf(II)I}
0x10011705058 {org/jboss/modules/PathUtils.canonicalize} +87           |    906e0068 stw       r3, 0x68(r14)
0x1001170505c {org/jboss/modules/PathUtils.canonicalize} +88           |    806e0068 lwz       r3, 0x68(r14) <<< ^+208

The JIT stored the return value on to the stack as a 32bit value (which is a bit odd??).. Then we use an lwz to load it back into the same register.

[zl-wang]

odd indeed. as i expected, it was due to lwz. that is an obvious LHS (loadHitStore) too ... bad for performance. maybe gen a log to chase it further.