From d22f5c54b51a07dafae3069a4f485f7e703d8d18 Mon Sep 17 00:00:00 2001 From: Emanuel Peter Date: Fri, 19 Jun 2026 06:55:12 +0000 Subject: [PATCH] 8386591: C2: wrong result because of broken truncation check in CountedLoopConverter::TruncatedIncrement::build Reviewed-by: roland, kvn, qamai --- src/hotspot/share/opto/loopnode.cpp | 28 +++-- src/hotspot/share/opto/loopnode.hpp | 1 - .../TestTruncationWrapBadCharWrap.java | 115 ++++++++++++++++++ 3 files changed, 133 insertions(+), 11 deletions(-) create mode 100644 test/hotspot/jtreg/compiler/loopopts/TestTruncationWrapBadCharWrap.java diff --git a/src/hotspot/share/opto/loopnode.cpp b/src/hotspot/share/opto/loopnode.cpp index 7c62e313803..1018a8ae9ab 100644 --- a/src/hotspot/share/opto/loopnode.cpp +++ b/src/hotspot/share/opto/loopnode.cpp @@ -3129,8 +3129,12 @@ Node* LoopLimitNode::Identity(PhaseGVN* phase) { return this; } -// Match increment with optional truncation: -// CHAR: (i+1)&0x7fff, BYTE: ((i+1)<<8)>>8, or SHORT: ((i+1)<<16)>>16 +// CHAR: (i+1)&0x7fff Note: does NOT work for char cast (0xffff) +// BYTE: ((i+1)<<8)>>8 Note: does NOT work for byte cast (<< 24 >> 24) +// SHORT: ((i+1)<<16)>>16 +// +// Note: in the future, we should fix both the BYTE and the CHAR case, +// to allow proper optimization of byte/char cast truncation. void CountedLoopConverter::TruncatedIncrement::build(Node* expr) { _is_valid = false; @@ -3146,15 +3150,19 @@ void CountedLoopConverter::TruncatedIncrement::build(Node* expr) { const TypeInteger* trunc_t = TypeInteger::bottom(_bt); if (_bt == T_INT) { - // Try to strip (n1 & M) or (n1 << N >> N) from n1. if (n1op == Op_AndI && - n1->in(2)->is_Con() && - n1->in(2)->bottom_type()->is_int()->get_con() == 0x7fff) { - // %%% This check should match any mask of 2**K-1. - t1 = n1; - n1 = t1->in(1); - n1op = n1->Opcode(); - trunc_t = TypeInt::CHAR; + n1->in(2)->is_Con()) { + // Unsigned truncation. + // Pattern: ((i+1) & mask) + jint mask = n1->in(2)->bottom_type()->is_int()->get_con(); + switch (mask) { + case 0x7fff: // Unsigned 15-bit truncation. For historical reasons. + t1 = n1; + n1 = t1->in(1); + n1op = n1->Opcode(); + trunc_t = TypeInt::make_unsigned(0, mask, 0); + break; + } } else if (n1op == Op_RShiftI && n1->in(1) != nullptr && n1->in(1)->Opcode() == Op_LShiftI && diff --git a/src/hotspot/share/opto/loopnode.hpp b/src/hotspot/share/opto/loopnode.hpp index 9f841d958ec..71a159352a4 100644 --- a/src/hotspot/share/opto/loopnode.hpp +++ b/src/hotspot/share/opto/loopnode.hpp @@ -2105,7 +2105,6 @@ class CountedLoopConverter { bool is_valid() const { return _is_valid; } Node* incr() const { return _incr; } - // Optional truncation for: CHAR: (i+1)&0x7fff, BYTE: ((i+1)<<8)>>8, or SHORT: ((i+1)<<16)>>16 Node* outer_trunc() const { return _outer_trunc; } // the outermost truncating node (either the & or the final >>) Node* inner_trunc() const { return _inner_trunc; } // the inner truncating node, if applicable (the << in a <> pair) const TypeInteger* trunc_type() const { return _trunc_type; } diff --git a/test/hotspot/jtreg/compiler/loopopts/TestTruncationWrapBadCharWrap.java b/test/hotspot/jtreg/compiler/loopopts/TestTruncationWrapBadCharWrap.java new file mode 100644 index 00000000000..ed279347762 --- /dev/null +++ b/test/hotspot/jtreg/compiler/loopopts/TestTruncationWrapBadCharWrap.java @@ -0,0 +1,115 @@ +/* + * Copyright (c) 2026, 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. + */ + +package compiler.loopopts; + +/* + * @test + * @bug 8386591 + * @summary Test case for TruncatedIncrement::build / + * CountedLoopConverter::has_truncation_wrap where we got wrong + * results, because we confused "& 0x7fff" as range [0..65535] + * instead of [0..32767]. + * @library /test/lib / + * @run main/othervm -Xcomp + * -XX:-TieredCompilation + * -XX:CompileCommand=compileonly,${test.main.class}::test* + * ${test.main.class} + * @run main ${test.main.class} + */ + +public class TestTruncationWrapBadCharWrap { + interface TestMethod { + int call(); + } + + public static void main(String[] args) { + int failures = 0; + + failures += run("test1", () -> test1(), 1402); + failures += run("test2", () -> test2(), 2037); + failures += run("test3", () -> test3(), 171761184); + + if (failures > 0) { + throw new RuntimeException("failures: " + failures); + } + } + + static int run(String name, TestMethod t, int expected) { + for (int i = 0; i < 10_000; i++) { + int result = t.call(); + if (result != expected) { + System.out.println(name + " wrong result: " + result + " vs " + expected); + return 1; + } + } + return 0; + } + + + static int test1() { + int sum = 0; + // The entry value is outside the range [0..32767], but inside [0..65535]. + int i = (char)38405; + while (32 < i) { + sum++; + // Ignoring truncation would require values to be in range [0..32767]. + // But unfortunately, we classified this as CHAR, and checked for [0..65535]. + i = (i - 4) & 0x7fff; + } + return sum; + } + + static int test2() { + int sum = 0; + // We have 32767 - 32758 = 9 < 48, so the limit is too close to the wrap + // limit, and wrap is possible. But since 0x7fff got mapped to CHAR, + // we accidentally checked 65535 - 32758 < 48, and conclude wrap is not + // possible. + for (int i = 519; i < 32758; i = (i + 48) & 0x7fff) { + sum++; + } + return sum; + } + + static int opaqueCounter; + + static boolean opaqueCheck() { + return opaqueCounter++ > 10448; + } + + static int test3() { + opaqueCounter = 0; + int sum = 0; + int i; + // Similar as with test2: + // We should check 32767 - 32766 = 1 < 50, so wrap possible. But we + // wrongly classified it as CHAR and checked 65535 - 32766 < 50, and + // concluded there is no wrap. + for (i = 2046; i <= 32766; i = (i + 50) & 0x7fff) { + sum += i + 1; + if (opaqueCheck()) { break; } + } + return sum + i; + } +}