8144212: JDK 9 b93 breaks Apache Lucene due to compact strings

String compress/inflate intrinsics need to capture char and byte memory.

Reviewed-by: aph, roland, kvn

hotspot/src/share/vm/opto/graphKit.cpp

@@ -4340,20 +4340,51 @@ void GraphKit::store_String_coder(Node* ctrl, Node* str, Node* value) {
                   value, T_BYTE, coder_field_idx, MemNode::unordered);
 }
 
-Node* GraphKit::compress_string(Node* src, Node* dst, Node* count) {
+// Capture src and dst memory state with a MergeMemNode
+Node* GraphKit::capture_memory(const TypePtr* src_type, const TypePtr* dst_type) {
+  if (src_type == dst_type) {
+    // Types are equal, we don't need a MergeMemNode
+    return memory(src_type);
+  }
+  MergeMemNode* merge = MergeMemNode::make(map()->memory());
+  record_for_igvn(merge); // fold it up later, if possible
+  int src_idx = C->get_alias_index(src_type);
+  int dst_idx = C->get_alias_index(dst_type);
+  merge->set_memory_at(src_idx, memory(src_idx));
+  merge->set_memory_at(dst_idx, memory(dst_idx));
+  return merge;
+}
+
+Node* GraphKit::compress_string(Node* src, const TypeAryPtr* src_type, Node* dst, Node* count) {
   assert(Matcher::match_rule_supported(Op_StrCompressedCopy), "Intrinsic not supported");
-  uint idx = C->get_alias_index(TypeAryPtr::BYTES);
+  assert(src_type == TypeAryPtr::BYTES || src_type == TypeAryPtr::CHARS, "invalid source type");
-  StrCompressedCopyNode* str = new StrCompressedCopyNode(control(), memory(idx), src, dst, count);
+  // If input and output memory types differ, capture both states to preserve
+  // the dependency between preceding and subsequent loads/stores.
+  // For example, the following program:
+  //  StoreB
+  //  compress_string
+  //  LoadB
+  // has this memory graph (use->def):
+  //  LoadB -> compress_string -> CharMem
+  //             ... -> StoreB -> ByteMem
+  // The intrinsic hides the dependency between LoadB and StoreB, causing
+  // the load to read from memory not containing the result of the StoreB.
+  // The correct memory graph should look like this:
+  //  LoadB -> compress_string -> MergeMem(CharMem, StoreB(ByteMem))
+  Node* mem = capture_memory(src_type, TypeAryPtr::BYTES);
+  StrCompressedCopyNode* str = new StrCompressedCopyNode(control(), mem, src, dst, count);
   Node* res_mem = _gvn.transform(new SCMemProjNode(str));
-  set_memory(res_mem, idx);
+  set_memory(res_mem, TypeAryPtr::BYTES);
   return str;
 }
 
-void GraphKit::inflate_string(Node* src, Node* dst, Node* count) {
+void GraphKit::inflate_string(Node* src, Node* dst, const TypeAryPtr* dst_type, Node* count) {
   assert(Matcher::match_rule_supported(Op_StrInflatedCopy), "Intrinsic not supported");
-  uint idx = C->get_alias_index(TypeAryPtr::BYTES);
+  assert(dst_type == TypeAryPtr::BYTES || dst_type == TypeAryPtr::CHARS, "invalid dest type");
-  StrInflatedCopyNode* str = new StrInflatedCopyNode(control(), memory(idx), src, dst, count);
+  // Capture src and dst memory (see comment in 'compress_string').
-  set_memory(_gvn.transform(str), idx);
+  Node* mem = capture_memory(TypeAryPtr::BYTES, dst_type);
+  StrInflatedCopyNode* str = new StrInflatedCopyNode(control(), mem, src, dst, count);
+  set_memory(_gvn.transform(str), dst_type);
 }
 
 void GraphKit::inflate_string_slow(Node* src, Node* dst, Node* start, Node* count) {

hotspot/src/share/vm/opto/graphKit.hpp

@@ -881,8 +881,9 @@ class GraphKit : public Phase {
   Node* load_String_coder(Node* ctrl, Node* str);
   void store_String_value(Node* ctrl, Node* str, Node* value);
   void store_String_coder(Node* ctrl, Node* str, Node* value);
-  Node* compress_string(Node* src, Node* dst, Node* count);
+  Node* capture_memory(const TypePtr* src_type, const TypePtr* dst_type);
-  void inflate_string(Node* src, Node* dst, Node* count);
+  Node* compress_string(Node* src, const TypeAryPtr* src_type, Node* dst, Node* count);
+  void inflate_string(Node* src, Node* dst, const TypeAryPtr* dst_type, Node* count);
   void inflate_string_slow(Node* src, Node* dst, Node* start, Node* count);
 
   // Handy for making control flow

hotspot/src/share/vm/opto/library_call.cpp

@@ -1359,9 +1359,9 @@ bool LibraryCallKit::inline_string_copy(bool compress) {
   // 'dst_start' points to dst array + scaled offset
   Node* count = NULL;
   if (compress) {
-    count = compress_string(src_start, dst_start, length);
+    count = compress_string(src_start, TypeAryPtr::get_array_body_type(src_elem), dst_start, length);
   } else {
-    inflate_string(src_start, dst_start, length);
+    inflate_string(src_start, dst_start, TypeAryPtr::get_array_body_type(dst_elem), length);
   }
 
   if (alloc != NULL) {
@@ -1587,7 +1587,7 @@ bool LibraryCallKit::inline_string_char_access(bool is_store) {
     (void) store_to_memory(control(), adr, ch, T_CHAR, TypeAryPtr::BYTES, MemNode::unordered,
                            false, false, true /* mismatched */);
   } else {
-    ch = make_load(control(), adr, TypeInt::CHAR, T_CHAR, MemNode::unordered,
+    ch = make_load(control(), adr, TypeInt::CHAR, T_CHAR, TypeAryPtr::BYTES, MemNode::unordered,
                    LoadNode::DependsOnlyOnTest, false, false, true /* mismatched */);
     set_result(ch);
   }

hotspot/src/share/vm/opto/stringopts.cpp

@@ -1466,7 +1466,7 @@ void PhaseStringOpts::copy_latin1_string(GraphKit& kit, IdealKit& ideal, Node* s
       // Use fast intrinsic
       Node* src = kit.array_element_address(src_array, kit.intcon(0), T_BYTE);
       Node* dst = kit.array_element_address(dst_array, start, T_BYTE);
-      kit.inflate_string(src, dst, __ value(count));
+      kit.inflate_string(src, dst, TypeAryPtr::BYTES, __ value(count));
     } else {
       // No intrinsic available, use slow method
       kit.inflate_string_slow(src_array, dst_array, start, __ value(count));

hotspot/test/compiler/intrinsics/string/TestStringIntrinsicMemoryFlow.java

@@ -0,0 +1,83 @@
+/*
+ * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+import jdk.test.lib.Asserts;
+
+/*
+ * @test
+ * @bug 8144212
+ * @summary Check for correct memory flow with the String compress/inflate intrinsics.
+ * @library /testlibrary
+ * @run main TestStringIntrinsicMemoryFlow
+ */
+public class TestStringIntrinsicMemoryFlow {
+
+    public static void main(String[] args) {
+        for (int i = 0; i < 100_000; ++i) {
+            String s = "MyString";
+            char[] c = {'M'};
+            char res = testInflate1(s);
+            Asserts.assertEquals(res, 'M', "testInflate1 failed");
+            res = testInflate2(s);
+            Asserts.assertEquals(res, (char)42, "testInflate2 failed");
+            res = testCompress1(c);
+            Asserts.assertEquals(res, 'M', "testCompress1 failed");
+            byte resB = testCompress2(c);
+            Asserts.assertEquals(resB, (byte)42, "testCompress2 failed");
+        }
+    }
+
+    private static char testInflate1(String s) {
+        char c[] = new char[1];
+        // Inflate String from byte[] to char[]
+        s.getChars(0, 1, c, 0);
+        // Read char[] memory written by inflate intrinsic
+        return c[0];
+    }
+
+    private static char testInflate2(String s) {
+        char c1[] = new char[1];
+        char c2[] = new char[1];
+        c2[0] = 42;
+        // Inflate String from byte[] to char[]
+        s.getChars(0, 1, c1, 0);
+        // Read char[] memory written before inflation
+        return c2[0];
+    }
+
+    private static char testCompress1(char[] c) {
+        // Compress String from char[] to byte[]
+        String s = new String(c);
+        // Read the memory written by compress intrinsic
+        return s.charAt(0);
+    }
+
+    private static byte testCompress2(char[] c) {
+        byte b1[] = new byte[1];
+        b1[0] = 42;
+        // Compress String from char[] to byte[]
+        new String(c);
+        // Read byte[] memory written before compression
+        return b1[0];
+    }
+}