Skip to content

Commit 73056eb

Browse files
committedSep 17, 2024
8233364: Fix undefined behavior in Canonicalizer::do_ShiftOp
Add java_shift_xxx helpers and use them. Reviewed-by: sgehwolf, roland Backport-of: 89e3782f218a54c91cbab9f06e75825b90dd2fb7
1 parent b734886 commit 73056eb

File tree

2 files changed

+46
-14
lines changed

2 files changed

+46
-14
lines changed
 

‎hotspot/src/share/vm/c1/c1_Canonicalizer.cpp

+10-14
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 1999, 2016, Oracle and/or its affiliates. All rights reserved.
2+
* Copyright (c) 1999, 2019, Oracle and/or its affiliates. All rights reserved.
33
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
44
*
55
* This code is free software; you can redistribute it and/or modify it
@@ -303,24 +303,20 @@ void Canonicalizer::do_ShiftOp (ShiftOp* x) {
303303
}
304304
if (t2->is_constant()) {
305305
if (t->tag() == intTag) {
306-
int value = t->as_IntConstant()->value();
307-
int shift = t2->as_IntConstant()->value() & 31;
308-
jint mask = ~(~0 << (32 - shift));
309-
if (shift == 0) mask = ~0;
306+
jint value = t->as_IntConstant()->value();
307+
jint shift = t2->as_IntConstant()->value();
310308
switch (x->op()) {
311-
case Bytecodes::_ishl: set_constant(value << shift); return;
312-
case Bytecodes::_ishr: set_constant(value >> shift); return;
313-
case Bytecodes::_iushr: set_constant((value >> shift) & mask); return;
309+
case Bytecodes::_ishl: set_constant(java_shift_left(value, shift)); return;
310+
case Bytecodes::_ishr: set_constant(java_shift_right(value, shift)); return;
311+
case Bytecodes::_iushr: set_constant(java_shift_right_unsigned(value, shift)); return;
314312
}
315313
} else if (t->tag() == longTag) {
316314
jlong value = t->as_LongConstant()->value();
317-
int shift = t2->as_IntConstant()->value() & 63;
318-
jlong mask = ~(~jlong_cast(0) << (64 - shift));
319-
if (shift == 0) mask = ~jlong_cast(0);
315+
jint shift = t2->as_IntConstant()->value();
320316
switch (x->op()) {
321-
case Bytecodes::_lshl: set_constant(value << shift); return;
322-
case Bytecodes::_lshr: set_constant(value >> shift); return;
323-
case Bytecodes::_lushr: set_constant((value >> shift) & mask); return;
317+
case Bytecodes::_lshl: set_constant(java_shift_left(value, shift)); return;
318+
case Bytecodes::_lshr: set_constant(java_shift_right(value, shift)); return;
319+
case Bytecodes::_lushr: set_constant(java_shift_right_unsigned(value, shift)); return;
324320
}
325321
}
326322
}

‎hotspot/src/share/vm/utilities/globalDefinitions.hpp

+36
Original file line numberDiff line numberDiff line change
@@ -1280,6 +1280,42 @@ inline intx byte_size(void* from, void* to) {
12801280
return (address)to - (address)from;
12811281
}
12821282

1283+
#ifdef ASSERT
1284+
#define RHS_MASK_ASSERT(rhs_mask) \
1285+
if (rhs_mask != 31 && rhs_mask != 63) { \
1286+
basic_fatal("rhs_mask assertion failed."); \
1287+
}
1288+
#else
1289+
#define RHS_MASK_ASSERT(rhs_mask)
1290+
#endif
1291+
1292+
// Provide integer shift operations with Java semantics. No overflow
1293+
// issues - left shifts simply discard shifted out bits. No undefined
1294+
// behavior for large or negative shift quantities; instead the actual
1295+
// shift distance is the argument modulo the lhs value's size in bits.
1296+
// No undefined or implementation defined behavior for shifting negative
1297+
// values; left shift discards bits, right shift sign extends. We use
1298+
// the same safe conversion technique as above for java_add and friends.
1299+
#define JAVA_INTEGER_SHIFT_OP(OP, NAME, TYPE, XTYPE) \
1300+
inline TYPE NAME (TYPE lhs, jint rhs) { \
1301+
const uint rhs_mask = (sizeof(TYPE) * 8) - 1; \
1302+
RHS_MASK_ASSERT(rhs_mask) \
1303+
XTYPE xres = static_cast<XTYPE>(lhs); \
1304+
xres OP ## = (rhs & rhs_mask); \
1305+
return reinterpret_cast<TYPE&>(xres); \
1306+
}
1307+
1308+
JAVA_INTEGER_SHIFT_OP(<<, java_shift_left, jint, juint)
1309+
JAVA_INTEGER_SHIFT_OP(<<, java_shift_left, jlong, julong)
1310+
// For signed shift right, assume C++ implementation >> sign extends.
1311+
JAVA_INTEGER_SHIFT_OP(>>, java_shift_right, jint, jint)
1312+
JAVA_INTEGER_SHIFT_OP(>>, java_shift_right, jlong, jlong)
1313+
// For >>> use C++ unsigned >>.
1314+
JAVA_INTEGER_SHIFT_OP(>>, java_shift_right_unsigned, jint, juint)
1315+
JAVA_INTEGER_SHIFT_OP(>>, java_shift_right_unsigned, jlong, julong)
1316+
1317+
#undef JAVA_INTEGER_SHIFT_OP
1318+
12831319
//----------------------------------------------------------------------------------------------------
12841320
// Avoid non-portable casts with these routines (DEPRECATED)
12851321

1 commit comments

Comments
 (1)

openjdk-notifier[bot] commented on Sep 17, 2024

@openjdk-notifier[bot]
Please sign in to comment.