Fri, 30 May 2014 06:50:38 +0200
8011646: SEGV in compiled code with loop predication
Summary: Remove control edge of load node to ensure that castPP removal sets the control edge correctly
Reviewed-by: kvn, roland
src/share/vm/opto/library_call.cpp | file | annotate | diff | comparison | revisions | |
test/compiler/intrinsics/hashcode/TestHashCode.java | file | annotate | diff | comparison | revisions |
1.1 --- a/src/share/vm/opto/library_call.cpp Fri May 30 10:43:51 2014 +0200 1.2 +++ b/src/share/vm/opto/library_call.cpp Fri May 30 06:50:38 2014 +0200 1.3 @@ -3978,8 +3978,11 @@ 1.4 } 1.5 1.6 1.7 -//------------------------------inline_native_hashcode-------------------- 1.8 -// Build special case code for calls to hashCode on an object. 1.9 +/** 1.10 + * Build special case code for calls to hashCode on an object. This call may 1.11 + * be virtual (invokevirtual) or bound (invokespecial). For each case we generate 1.12 + * slightly different code. 1.13 + */ 1.14 bool LibraryCallKit::inline_native_hashcode(bool is_virtual, bool is_static) { 1.15 assert(is_static == callee()->is_static(), "correct intrinsic selection"); 1.16 assert(!(is_virtual && is_static), "either virtual, special, or static"); 1.17 @@ -3987,11 +3990,9 @@ 1.18 enum { _slow_path = 1, _fast_path, _null_path, PATH_LIMIT }; 1.19 1.20 RegionNode* result_reg = new(C) RegionNode(PATH_LIMIT); 1.21 - PhiNode* result_val = new(C) PhiNode(result_reg, 1.22 - TypeInt::INT); 1.23 + PhiNode* result_val = new(C) PhiNode(result_reg, TypeInt::INT); 1.24 PhiNode* result_io = new(C) PhiNode(result_reg, Type::ABIO); 1.25 - PhiNode* result_mem = new(C) PhiNode(result_reg, Type::MEMORY, 1.26 - TypePtr::BOTTOM); 1.27 + PhiNode* result_mem = new(C) PhiNode(result_reg, Type::MEMORY, TypePtr::BOTTOM); 1.28 Node* obj = NULL; 1.29 if (!is_static) { 1.30 // Check for hashing null object 1.31 @@ -4017,12 +4018,6 @@ 1.32 return true; 1.33 } 1.34 1.35 - // After null check, get the object's klass. 1.36 - Node* obj_klass = load_object_klass(obj); 1.37 - 1.38 - // This call may be virtual (invokevirtual) or bound (invokespecial). 1.39 - // For each case we generate slightly different code. 1.40 - 1.41 // We only go to the fast case code if we pass a number of guards. The 1.42 // paths which do not pass are accumulated in the slow_region. 1.43 RegionNode* slow_region = new (C) RegionNode(1); 1.44 @@ -4035,19 +4030,24 @@ 1.45 // guard for non-virtual calls -- the caller is known to be the native 1.46 // Object hashCode(). 1.47 if (is_virtual) { 1.48 + // After null check, get the object's klass. 1.49 + Node* obj_klass = load_object_klass(obj); 1.50 generate_virtual_guard(obj_klass, slow_region); 1.51 } 1.52 1.53 // Get the header out of the object, use LoadMarkNode when available 1.54 Node* header_addr = basic_plus_adr(obj, oopDesc::mark_offset_in_bytes()); 1.55 - Node* header = make_load(control(), header_addr, TypeX_X, TypeX_X->basic_type(), MemNode::unordered); 1.56 + // The control of the load must be NULL. Otherwise, the load can move before 1.57 + // the null check after castPP removal. 1.58 + Node* no_ctrl = NULL; 1.59 + Node* header = make_load(no_ctrl, header_addr, TypeX_X, TypeX_X->basic_type(), MemNode::unordered); 1.60 1.61 // Test the header to see if it is unlocked. 1.62 - Node *lock_mask = _gvn.MakeConX(markOopDesc::biased_lock_mask_in_place); 1.63 - Node *lmasked_header = _gvn.transform(new (C) AndXNode(header, lock_mask)); 1.64 - Node *unlocked_val = _gvn.MakeConX(markOopDesc::unlocked_value); 1.65 - Node *chk_unlocked = _gvn.transform(new (C) CmpXNode( lmasked_header, unlocked_val)); 1.66 - Node *test_unlocked = _gvn.transform(new (C) BoolNode( chk_unlocked, BoolTest::ne)); 1.67 + Node* lock_mask = _gvn.MakeConX(markOopDesc::biased_lock_mask_in_place); 1.68 + Node* lmasked_header = _gvn.transform(new (C) AndXNode(header, lock_mask)); 1.69 + Node* unlocked_val = _gvn.MakeConX(markOopDesc::unlocked_value); 1.70 + Node* chk_unlocked = _gvn.transform(new (C) CmpXNode( lmasked_header, unlocked_val)); 1.71 + Node* test_unlocked = _gvn.transform(new (C) BoolNode( chk_unlocked, BoolTest::ne)); 1.72 1.73 generate_slow_guard(test_unlocked, slow_region); 1.74 1.75 @@ -4055,19 +4055,19 @@ 1.76 // We depend on hash_mask being at most 32 bits and avoid the use of 1.77 // hash_mask_in_place because it could be larger than 32 bits in a 64-bit 1.78 // vm: see markOop.hpp. 1.79 - Node *hash_mask = _gvn.intcon(markOopDesc::hash_mask); 1.80 - Node *hash_shift = _gvn.intcon(markOopDesc::hash_shift); 1.81 - Node *hshifted_header= _gvn.transform(new (C) URShiftXNode(header, hash_shift)); 1.82 + Node* hash_mask = _gvn.intcon(markOopDesc::hash_mask); 1.83 + Node* hash_shift = _gvn.intcon(markOopDesc::hash_shift); 1.84 + Node* hshifted_header= _gvn.transform(new (C) URShiftXNode(header, hash_shift)); 1.85 // This hack lets the hash bits live anywhere in the mark object now, as long 1.86 // as the shift drops the relevant bits into the low 32 bits. Note that 1.87 // Java spec says that HashCode is an int so there's no point in capturing 1.88 // an 'X'-sized hashcode (32 in 32-bit build or 64 in 64-bit build). 1.89 hshifted_header = ConvX2I(hshifted_header); 1.90 - Node *hash_val = _gvn.transform(new (C) AndINode(hshifted_header, hash_mask)); 1.91 - 1.92 - Node *no_hash_val = _gvn.intcon(markOopDesc::no_hash); 1.93 - Node *chk_assigned = _gvn.transform(new (C) CmpINode( hash_val, no_hash_val)); 1.94 - Node *test_assigned = _gvn.transform(new (C) BoolNode( chk_assigned, BoolTest::eq)); 1.95 + Node* hash_val = _gvn.transform(new (C) AndINode(hshifted_header, hash_mask)); 1.96 + 1.97 + Node* no_hash_val = _gvn.intcon(markOopDesc::no_hash); 1.98 + Node* chk_assigned = _gvn.transform(new (C) CmpINode( hash_val, no_hash_val)); 1.99 + Node* test_assigned = _gvn.transform(new (C) BoolNode( chk_assigned, BoolTest::eq)); 1.100 1.101 generate_slow_guard(test_assigned, slow_region); 1.102
2.1 --- /dev/null Thu Jan 01 00:00:00 1970 +0000 2.2 +++ b/test/compiler/intrinsics/hashcode/TestHashCode.java Fri May 30 06:50:38 2014 +0200 2.3 @@ -0,0 +1,73 @@ 2.4 +/* 2.5 + * Copyright (c) 2014, Oracle and/or its affiliates. All rights reserved. 2.6 + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. 2.7 + * 2.8 + * This code is free software; you can redistribute it and/or modify it 2.9 + * under the terms of the GNU General Public License version 2 only, as 2.10 + * published by the Free Software Foundation. 2.11 + * 2.12 + * This code is distributed in the hope that it will be useful, but WITHOUT 2.13 + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or 2.14 + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License 2.15 + * version 2 for more details (a copy is included in the LICENSE file that 2.16 + * accompanied this code). 2.17 + * 2.18 + * You should have received a copy of the GNU General Public License version 2.19 + * 2 along with this work; if not, write to the Free Software Foundation, 2.20 + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. 2.21 + * 2.22 + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA 2.23 + * or visit www.oracle.com if you need additional information or have any 2.24 + * questions. 2.25 + */ 2.26 + 2.27 +/* 2.28 + * @test 2.29 + * @bug 8011646 2.30 + * @summary SEGV in compiled code with loop predication 2.31 + * @run main/othervm -XX:-TieredCompilation -XX:CompileOnly=TestHashCode.m1,Object.hashCode TestHashCode 2.32 + * 2.33 + */ 2.34 + 2.35 +public class TestHashCode { 2.36 + static class A { 2.37 + int i; 2.38 + } 2.39 + 2.40 + static class B extends A { 2.41 + } 2.42 + 2.43 + static boolean crash = false; 2.44 + 2.45 + static A m2() { 2.46 + if (crash) { 2.47 + return null; 2.48 + } 2.49 + return new A(); 2.50 + } 2.51 + 2.52 + static int m1(A aa) { 2.53 + int res = 0; 2.54 + for (int i = 0; i < 10; i++) { 2.55 + A a = m2(); 2.56 + int j = a.i; 2.57 + if (aa instanceof B) { 2.58 + } 2.59 + res += a.hashCode(); 2.60 + } 2.61 + return res; 2.62 + } 2.63 + 2.64 + public static void main(String[] args) { 2.65 + A a = new A(); 2.66 + for (int i = 0; i < 20000; i++) { 2.67 + m1(a); 2.68 + } 2.69 + crash = true; 2.70 + try { 2.71 + m1(a); 2.72 + } catch (NullPointerException e) { 2.73 + System.out.println("Test passed"); 2.74 + } 2.75 + } 2.76 +}