RFR: JDK-8261552: s390: MacroAssembler::encode_klass_not_null() may produce wrong results for non-zero values of narrow klass base

[Thomas Stuefe]

If Compressed class pointer base has a non-zero value it may cause MacroAssembler::encode_klass_not_null() to encode a Klass pointer to a wrong narrow pointer.

This can be reproduced by starting the VM with
-Xshare:dump -XX:HeapBaseMinAddress=2g -Xmx128m
but CDS is not involved. It is only relevant insofar as this is the only way to get the following combination:
- heap is allocated at 0x800_0000. It is small and ends at 0x8800_0000.
- class space follows at 0x8800_0000
- the narrow klass pointer base points to the start of the class space at 0x8800_0000.

In MacroAssembler::encode_klass_not_null(), there is the following section:

  if (base != NULL) {
    unsigned int base_h = ((unsigned long)base)>>32;
    unsigned int base_l = (unsigned int)((unsigned long)base);
    if ((base_h != 0) && (base_l == 0) && VM_Version::has_HighWordInstr()) {
      lgr_if_needed(dst, current);
      z_aih(dst, -((int)base_h));     // Base has no set bits in lower half.
    } else if ((base_h == 0) && (base_l != 0)) {   (A)
      lgr_if_needed(dst, current);                
      z_agfi(dst, -(int)base_l);                   (B)
    } else {
      load_const(Z_R0, base);
      lgr_if_needed(dst, current);
      z_sgr(dst, Z_R0);
    }
    current = dst;
  }

We enter the condition at (A) if the narrow klass pointer base is non-zero but fits into 32bit. At (B), we want to substract the base from the Klass pointer; we do this by calculating the 32bit twos-complement of the base and add it with AGFI. AGFI adds a 32bit immediate to a 64bit register. In this case, it produces the wrong result if the base is >0x800_0000:

In the case of the crash, we have:
base: 				  8800_0000
klass pointer: 			  8804_1040
32bit two's complement of base:   7800_0000
added to the klass pointer: 	1_0004_1040

So the result of the "substraction" is 1_0004_1040, it should be 4_1040, which would be the correct offset of the Klass* pointer within the ccs.

This bug has been dormant; was activated by JDK-8250989 which changed the way class space reservation happens at CDS dump time. It surfaced first as crash in a CDS-specific jtreg test (JDK-8261552).

================

Fix:

I changed the AGFI instruction to a pure 32bit add (AFI). That works as long as the Klass pointer also fits into 32bit. So I narrowed the condition at (A) to only fire if it can be ensured that both narrow base and Klass* pointers fit into 32bit.

I also added a runtime verification in that case that any Klass pointer passed down is indeed a 32bit pointer. However, I am not really sure this is useful, or that this is the best way to do this (using TMHH and TMHL). I was looking for something like TMH or TML to check whole 32bit words but could not find any.

----

Tests:

I manually tested that the crash disappears, which it does. I stepped through the encoding code and the values now look right.

I also did build a VM with the ability to override both class space start address and the narrow klass pointer base to exact values (see https://github.com/openjdk/jdk/compare/master...tstuefe:override-ccs-start-and-base). 

I used this method to test various combinations: 
- narrow klass pointer base > 0 < 4g + ccs end < 4g  (we hit our branch doing AFI)
- narrow klass pointer base > 0 < 4g + ccs end > 4g  (we hit the fallback doing SGR with r0)
- narrow klass pointer base = 0                      (we dont do anything)

(would this override-feature be useful? We could do better testing).

Thanks, Thomas

-------------

Commit messages:
 - Remove whitespace
 - start

Changes: https://git.openjdk.java.net/jdk/pull/2595/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2595&range=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8261552
  Stats: 42 lines in 3 files changed: 37 ins; 0 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2595.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2595/head:pull/2595

PR: https://git.openjdk.java.net/jdk/pull/2595