Skip to content

8370459: C2: CompressBitsNode::Value produces wrong result on Windows (1UL vs 1ULL), found by ExpressionFuzzer #28062

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 15 commits into from
+111 −15
Closed

8370459: C2: CompressBitsNode::Value produces wrong result on Windows (1UL vs 1ULL), found by ExpressionFuzzer #28062

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
f62bb5f
JDK-8370459
eme64 Oct 29, 2025
cff00bb
x2
eme64 Oct 30, 2025
92601f6
x3
eme64 Oct 30, 2025
5c970c5
x4
eme64 Oct 30, 2025
fb45482
x5
eme64 Oct 30, 2025
c3bdbc2
x6
eme64 Oct 30, 2025
c8eeb2f
x7
eme64 Oct 30, 2025
16312f3
more tests
eme64 Oct 30, 2025
351aa02
rm templated test
eme64 Oct 30, 2025
100b98f
fix v1
eme64 Oct 30, 2025
794ef7b
the constant case
eme64 Oct 31, 2025
41b8c36
Apply suggestions from code review
eme64 Oct 31, 2025
5b592b3
use right_n_bits_typed for Dean
eme64 Oct 31, 2025
d2b88be
Merge branch 'JDK-8370459-expression-fuzz-failure' of https://github.…
eme64 Oct 31, 2025
8bf89e9
fix last commit
eme64 Oct 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/hotspot/share/opto/intrinsicnode.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,7 @@ static const Type* bitshuffle_value(const TypeInteger* src_type, const TypeInteg
// result.lo = 0
if (maskcon != -1L) {
int bitcount = population_count(static_cast<julong>(bt == T_INT ? maskcon & 0xFFFFFFFFL : maskcon));
hi = (1UL << bitcount) - 1;
hi = right_n_bits_typed<jlong>(bitcount);
lo = 0L;
} else {
// preserve originally assigned hi (MAX_INT/LONG) and lo (MIN_INT/LONG) values
Expand Down Expand Up @@ -376,7 +376,7 @@ static const Type* bitshuffle_value(const TypeInteger* src_type, const TypeInteg
// Rule 3:
// We can further constrain the upper bound of bit compression if the number of bits
// which can be set(one) is less than the maximum number of bits of integral type.
hi = MIN2((jlong)((1UL << result_bit_width) - 1L), hi);
hi = MIN2(right_n_bits_typed<jlong>(result_bit_width), hi);
}
} else {
assert(opc == Op_ExpandBits, "");
Expand Down
122 changes: 109 additions & 13 deletions test/hotspot/jtreg/compiler/c2/gvn/TestBitCompressValueTransform.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

/*
* @test
* @bug 8350896
* @bug 8350896 8370459
* @library /test/lib /
* @summary C2: wrong result: Integer/Long.compress gets wrong type from CompressBitsNode::Value.
* @run driver compiler.c2.gvn.TestBitCompressValueTransform
Expand Down Expand Up @@ -62,11 +62,15 @@ public class TestBitCompressValueTransform {
public final long LIMIT_L7 = GEN_L.next();
public final long LIMIT_L8 = GEN_L.next();

public final int BOUND_LO_I = GEN_I.next();
public final int BOUND_HI_I = GEN_I.next();
public final int BOUND1_LO_I = GEN_I.next();
public final int BOUND2_LO_I = GEN_I.next();
public final int BOUND1_HI_I = GEN_I.next();
public final int BOUND2_HI_I = GEN_I.next();

public final long BOUND_LO_L = GEN_L.next();
public final long BOUND_HI_L = GEN_L.next();
public final long BOUND1_LO_L = GEN_L.next();
public final long BOUND2_LO_L = GEN_L.next();
public final long BOUND1_HI_L = GEN_L.next();
public final long BOUND2_HI_L = GEN_L.next();

@Test
@IR (counts = { IRNode.COMPRESS_BITS, " >0 " }, applyIfCPUFeature = { "bmi2", "true" })
Expand Down Expand Up @@ -327,7 +331,8 @@ public void run15() {

@DontCompile
public int test16_interpreted(int src, int mask) {
src = Math.max(BOUND_LO_I, Math.min(src, BOUND_HI_I));
src = Math.max(BOUND1_LO_I, Math.min(src, BOUND1_HI_I));
mask = Math.max(BOUND2_LO_I, Math.min(mask, BOUND2_HI_I));
int res = Integer.compress(src, mask);

if (res > LIMIT_I1) {
Expand Down Expand Up @@ -360,7 +365,8 @@ public int test16_interpreted(int src, int mask) {
@Test
@IR (counts = { IRNode.COMPRESS_BITS, " >0 " }, applyIfCPUFeature = {"bmi2" , "true"})
public int test16(int src, int mask) {
src = Math.max(BOUND_LO_I, Math.min(src, BOUND_HI_I));
src = Math.max(BOUND1_LO_I, Math.min(src, BOUND1_HI_I));
mask = Math.max(BOUND2_LO_I, Math.min(mask, BOUND2_HI_I));
int res = Integer.compress(src, mask);

// Check the result with some random value ranges, if any of the
Expand Down Expand Up @@ -411,7 +417,8 @@ public void run16() {

@DontCompile
public int test17_interpreted(int src, int mask) {
src = Math.max(BOUND_LO_I, Math.min(src, BOUND_HI_I));
src = Math.max(BOUND1_LO_I, Math.min(src, BOUND1_HI_I));
mask = Math.max(BOUND2_LO_I, Math.min(mask, BOUND2_HI_I));
int res = Integer.expand(src, mask);

if (res > LIMIT_I1) {
Expand Down Expand Up @@ -444,7 +451,8 @@ public int test17_interpreted(int src, int mask) {
@Test
@IR (counts = { IRNode.EXPAND_BITS, " >0 " }, applyIfCPUFeature = {"bmi2" , "true"})
public int test17(int src, int mask) {
src = Math.max(BOUND_LO_I, Math.min(src, BOUND_HI_I));
src = Math.max(BOUND1_LO_I, Math.min(src, BOUND1_HI_I));
mask = Math.max(BOUND2_LO_I, Math.min(mask, BOUND2_HI_I));
int res = Integer.expand(src, mask);

// Check the result with some random value ranges, if any of the
Expand Down Expand Up @@ -495,7 +503,8 @@ public void run17() {

@DontCompile
public long test18_interpreted(long src, long mask) {
src = Math.max(BOUND_LO_L, Math.min(src, BOUND_HI_L));
src = Math.max(BOUND1_LO_L, Math.min(src, BOUND1_HI_L));
mask = Math.max(BOUND2_LO_L, Math.min(mask, BOUND2_HI_L));
long res = Long.compress(src, mask);

if (res > LIMIT_L1) {
Expand Down Expand Up @@ -528,7 +537,8 @@ public long test18_interpreted(long src, long mask) {
@Test
@IR (counts = { IRNode.COMPRESS_BITS, " >0 " }, applyIfCPUFeature = {"bmi2" , "true"})
public long test18(long src, long mask) {
src = Math.max(BOUND_LO_L, Math.min(src, BOUND_HI_L));
src = Math.max(BOUND1_LO_L, Math.min(src, BOUND1_HI_L));
mask = Math.max(BOUND2_LO_L, Math.min(mask, BOUND2_HI_L));
long res = Long.compress(src, mask);

// Check the result with some random value ranges, if any of the
Expand Down Expand Up @@ -579,7 +589,8 @@ public void run18() {

@DontCompile
public long test19_interpreted(long src, long mask) {
src = Math.max(BOUND_LO_L, Math.min(src, BOUND_HI_L));
src = Math.max(BOUND1_LO_L, Math.min(src, BOUND1_HI_L));
mask = Math.max(BOUND2_LO_L, Math.min(mask, BOUND2_HI_L));
long res = Long.expand(src, mask);

if (res > LIMIT_L1) {
Expand Down Expand Up @@ -612,7 +623,8 @@ public long test19_interpreted(long src, long mask) {
@Test
@IR (counts = { IRNode.EXPAND_BITS, " >0 " }, applyIfCPUFeature = {"bmi2" , "true"})
public long test19(long src, long mask) {
src = Math.max(BOUND_LO_L, Math.min(src, BOUND_HI_L));
src = Math.max(BOUND1_LO_L, Math.min(src, BOUND1_HI_L));
mask = Math.max(BOUND2_LO_L, Math.min(mask, BOUND2_HI_L));
long res = Long.expand(src, mask);

// Check the result with some random value ranges, if any of the
Expand Down Expand Up @@ -661,6 +673,90 @@ public void run19() {
Asserts.assertEQ(actual, expected);
}

@Test
@IR (counts = { IRNode.COMPRESS_BITS, " >0 " }, applyIfCPUFeature = { "bmi2", "true" })
public static long test20(int x) {
// Analysis of when this is used to produce wrong results on Windows:
//
// src = -2683206580L = ffff_ffff_6011_844c
// mask = 0..maxuint, at runtime: 4294950911 = 0xffff_bfff
//
// Hence we go to the B) case of CompressBits in bitshuffle_value
//
// mask_bit_width = 64
// clz = 32
// result_bit_width = 32
//
// So we have result_bit_width < mask_bit_width
//
// And we do:
// lo = result_bit_width == mask_bit_width ? lo : 0L;
// -> lo = 0
//
// And we do:
// hi = MIN2((jlong)((1UL << result_bit_width) - 1L), hi);
//
// But watch out: on windows 1UL is only a 32 bit value. Intended was probably 1ULL.
// So when we calculate "1UL << 32", we just get 1. And so then hi would be 0 now.
// If we instead did "1ULL << 32", we would get 0x1_0000_0000, and hi = 0xffff_ffff.
//
// We create type [lo, hi]:
// Windows: [0, 0] -> constant zero
// correct: [0, 0xffff_ffff] -> does not constant fold. At runtime: 0x3008_c44c
return Long.compress(-2683206580L, Integer.toUnsignedLong(x));
}

@DontCompile
public static long test20_interpreted(int x) {
return Long.compress(-2683206580L, Integer.toUnsignedLong(x));
}

@Run (test = "test20")
public void run20() {
for (int i = 0; i < 100; i++) {
int arg = GEN_I.next();

long actual = test20(arg);
long expected = test20_interpreted(arg);
Asserts.assertEQ(actual, expected);
}
}

@Test
@IR (counts = { IRNode.COMPRESS_BITS, " >0 " }, applyIfCPUFeature = { "bmi2", "true" })
public static long test21(long x) {
// Analysis of when this is used to produce wrong results on Windows:
//
// Very similar to case in test20, but this time we go into the A) case.
//
// maskcon = 0xffff_ffff
// bitcount = 32
//
// And now the problematic part:
// hi = (1UL << bitcount) - 1;
//
// On Windows, this becomes 0 (but it should be 0xffff_ffff).
// Hence, the range wrongly collapses to [0, 0], and the CompressBits node
// is wrongly replaced with a zero constant.
return Long.compress(x, 0xffff_ffffL);
}

@DontCompile
public static long test21_interpreted(long x) {
return Long.compress(x, 0xffff_ffffL);
}

@Run (test = "test21")
public void run21() {
for (int i = 0; i < 100; i++) {
int arg = GEN_I.next();

long actual = test21(arg);
long expected = test21_interpreted(arg);
Asserts.assertEQ(actual, expected);
}
}

public static void main(String[] args) {
TestFramework.run(TestBitCompressValueTransform.class);
}
Expand Down