Skip to content

Commit aff4143

Browse files
TobiHartmannTobiHartmann
committedDec 3, 2019
8234617: C1: Incorrect result of field load due to missing narrowing conversion
Emit an explicit conversion to get the correct field value after the write. Reviewed-by: vlivanov, mdoerr
1 parent c7a2e3b commit aff4143

File tree

3 files changed

+207
-0
lines changed

3 files changed

+207
-0
lines changed
 

‎src/hotspot/share/c1/c1_GraphBuilder.cpp

+17
Original file line numberDiff line numberDiff line change
@@ -1725,6 +1725,23 @@ void GraphBuilder::access_field(Bytecodes::Code code) {
17251725
Value replacement = !needs_patching ? _memory->load(load) : load;
17261726
if (replacement != load) {
17271727
assert(replacement->is_linked() || !replacement->can_be_linked(), "should already by linked");
1728+
// Writing an (integer) value to a boolean, byte, char or short field includes an implicit narrowing
1729+
// conversion. Emit an explicit conversion here to get the correct field value after the write.
1730+
BasicType bt = field->type()->basic_type();
1731+
switch (bt) {
1732+
case T_BOOLEAN:
1733+
case T_BYTE:
1734+
replacement = append(new Convert(Bytecodes::_i2b, replacement, as_ValueType(bt)));
1735+
break;
1736+
case T_CHAR:
1737+
replacement = append(new Convert(Bytecodes::_i2c, replacement, as_ValueType(bt)));
1738+
break;
1739+
case T_SHORT:
1740+
replacement = append(new Convert(Bytecodes::_i2s, replacement, as_ValueType(bt)));
1741+
break;
1742+
default:
1743+
break;
1744+
}
17281745
push(type, replacement);
17291746
} else {
17301747
push(type, append(load));

‎test/hotspot/jtreg/compiler/conversions/Conversion.jasm

+130
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
/*
2+
* Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
package compiler/conversions;
25+
26+
public class Conversion
27+
version 52:0
28+
{
29+
Field booleanFld:Z;
30+
Field byteFld:B;
31+
Field charFld:C;
32+
Field shortFld:S;
33+
Field intFld:I;
34+
35+
public Method "<init>":"()V"
36+
stack 1 locals 1
37+
{
38+
aload_0;
39+
invokespecial Method java/lang/Object."<init>":"()V";
40+
return;
41+
}
42+
43+
public Method testBooleanConst:"()I"
44+
stack 5 locals 1
45+
{
46+
aload_0;
47+
ldc_w int 2; // 2^1 (maximum boolean value is 1)
48+
putfield Field booleanFld:"Z";
49+
aload_0;
50+
getfield Field booleanFld:"Z";
51+
ireturn;
52+
}
53+
54+
public Method testBoolean:"(I)I"
55+
stack 5 locals 2
56+
{
57+
aload_0;
58+
iload_1;
59+
putfield Field booleanFld:"Z";
60+
aload_0;
61+
getfield Field booleanFld:"Z";
62+
ireturn;
63+
}
64+
65+
public Method testByteConst:"()I"
66+
stack 5 locals 1
67+
{
68+
aload_0;
69+
ldc_w int 256; // 2^8 (maximum byte value is 2^7-1)
70+
putfield Field byteFld:"B";
71+
aload_0;
72+
getfield Field byteFld:"B";
73+
ireturn;
74+
}
75+
76+
public Method testByte:"(I)I"
77+
stack 5 locals 2
78+
{
79+
aload_0;
80+
iload_1;
81+
putfield Field byteFld:"B";
82+
aload_0;
83+
getfield Field byteFld:"B";
84+
ireturn;
85+
}
86+
87+
public Method testCharConst:"()I"
88+
stack 5 locals 1
89+
{
90+
aload_0;
91+
ldc_w int 131072; // 2^17 (maximum char value is 2^16-1)
92+
putfield Field charFld:"C";
93+
aload_0;
94+
getfield Field charFld:"C";
95+
ireturn;
96+
}
97+
98+
public Method testChar:"(I)I"
99+
stack 5 locals 2
100+
{
101+
aload_0;
102+
iload_1;
103+
putfield Field charFld:"C";
104+
aload_0;
105+
getfield Field charFld:"C";
106+
ireturn;
107+
}
108+
109+
public Method testShortConst:"()I"
110+
stack 5 locals 1
111+
{
112+
aload_0;
113+
ldc_w int 65536; // 2^16 (maximum short value is 2^15-1)
114+
putfield Field shortFld:"S";
115+
aload_0;
116+
getfield Field shortFld:"S";
117+
ireturn;
118+
}
119+
120+
public Method testShort:"(I)I"
121+
stack 5 locals 2
122+
{
123+
aload_0;
124+
iload_1;
125+
putfield Field shortFld:"S";
126+
aload_0;
127+
getfield Field shortFld:"S";
128+
ireturn;
129+
}
130+
}

‎test/hotspot/jtreg/compiler/conversions/TestPrimitiveConversions.java

+60
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
/*
2+
* Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
3+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
4+
*
5+
* This code is free software; you can redistribute it and/or modify it
6+
* under the terms of the GNU General Public License version 2 only, as
7+
* published by the Free Software Foundation.
8+
*
9+
* This code is distributed in the hope that it will be useful, but WITHOUT
10+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
11+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
12+
* version 2 for more details (a copy is included in the LICENSE file that
13+
* accompanied this code).
14+
*
15+
* You should have received a copy of the GNU General Public License version
16+
* 2 along with this work; if not, write to the Free Software Foundation,
17+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
18+
*
19+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
20+
* or visit www.oracle.com if you need additional information or have any
21+
* questions.
22+
*/
23+
24+
package compiler.conversions;
25+
26+
import jdk.test.lib.Asserts;
27+
28+
/*
29+
* @test
30+
* @bug 8234617
31+
* @summary Test implicit narrowing conversion of primivite values at putfield.
32+
* @library /test/lib /
33+
* @compile Conversion.jasm
34+
* @run main/othervm -Xbatch -XX:CompileCommand=dontinline,compiler.conversions.Conversion::*
35+
* compiler.conversions.TestPrimitiveConversions
36+
*/
37+
public class TestPrimitiveConversions {
38+
39+
public static void main(String[] args) {
40+
Conversion conv = new Conversion();
41+
for (int i = 0; i < 100_000; ++i) {
42+
int res = conv.testBooleanConst();
43+
Asserts.assertEquals(res, 0);
44+
res = conv.testBoolean(2); // 2^1 (maximum boolean value is 1)
45+
Asserts.assertEquals(res, 0);
46+
res = conv.testByteConst();
47+
Asserts.assertEquals(res, 0);
48+
res = conv.testByte(256); // 2^8 (maximum byte value is 2^7-1)
49+
Asserts.assertEquals(res, 0);
50+
res = conv.testCharConst();
51+
Asserts.assertEquals(res, 0);
52+
res = conv.testChar(131072); // 2^17 (maximum char value is 2^16-1)
53+
Asserts.assertEquals(res, 0);
54+
res = conv.testShortConst();
55+
Asserts.assertEquals(res, 0);
56+
res = conv.testShort(65536); // 2^16 (maximum short value is 2^15-1)
57+
Asserts.assertEquals(res, 0);
58+
}
59+
}
60+
}

0 commit comments

Comments
 (0)
Failed to load comments.