Skip to content
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

[clang][bytecode] Fix some shift edge cases #119895

Merged
merged 1 commit into from
Dec 14, 2024
Merged

[clang][bytecode] Fix some shift edge cases #119895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 5 additions & 2 deletions clang/lib/AST/ByteCode/Integral.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,10 @@ template <unsigned Bits, bool Signed> class Integral final {
unsigned countLeadingZeros() const {
if constexpr (!Signed)
return llvm::countl_zero<ReprT>(V);
llvm_unreachable("Don't call countLeadingZeros() on signed types.");
if (isPositive())
return llvm::countl_zero<typename AsUnsigned::ReprT>(
static_cast<typename AsUnsigned::ReprT>(V));
llvm_unreachable("Don't call countLeadingZeros() on negative values.");
}

Integral truncate(unsigned TruncBits) const {
Expand Down Expand Up @@ -210,7 +213,7 @@ template <unsigned Bits, bool Signed> class Integral final {
return Integral(Value.V);
}

static Integral zero() { return from(0); }
static Integral zero(unsigned BitWidth = 0) { return from(0); }

template <typename T> static Integral from(T Value, unsigned NumBits) {
return Integral(Value);
Expand Down
68 changes: 35 additions & 33 deletions clang/lib/AST/ByteCode/Interp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2511,50 +2511,52 @@ inline bool DoShift(InterpState &S, CodePtr OpPC, LT &LHS, RT &RHS) {
S, OpPC, LHS, RHS);
}

if constexpr (Dir == ShiftDir::Left) {
if (LHS.isNegative() && !S.getLangOpts().CPlusPlus20) {
// C++11 [expr.shift]p2: A signed left shift must have a non-negative
// operand, and must not overflow the corresponding unsigned type.
// C++2a [expr.shift]p2: E1 << E2 is the unique value congruent to
// E1 x 2^E2 module 2^N.
const SourceInfo &Loc = S.Current->getSource(OpPC);
S.CCEDiag(Loc, diag::note_constexpr_lshift_of_negative) << LHS.toAPSInt();
if (!S.noteUndefinedBehavior())
return false;
}
}

if (!CheckShift<Dir>(S, OpPC, LHS, RHS, Bits))
return false;

// Limit the shift amount to Bits - 1. If this happened,
// it has already been diagnosed by CheckShift() above,
// but we still need to handle it.
// Note that we have to be extra careful here since we're doing the shift in
// any case, but we need to adjust the shift amount or the way we do the shift
// for the potential error cases.
typename LT::AsUnsigned R;
unsigned MaxShiftAmount = LHS.bitWidth() - 1;
if constexpr (Dir == ShiftDir::Left) {
if (RHS > RT::from(Bits - 1, RHS.bitWidth()))
LT::AsUnsigned::shiftLeft(LT::AsUnsigned::from(LHS),
LT::AsUnsigned::from(Bits - 1), Bits, &R);
else
if (Compare(RHS, RT::from(MaxShiftAmount, RHS.bitWidth())) ==
ComparisonCategoryResult::Greater) {
if (LHS.isNegative())
R = LT::AsUnsigned::zero(LHS.bitWidth());
else {
RHS = RT::from(LHS.countLeadingZeros(), RHS.bitWidth());
LT::AsUnsigned::shiftLeft(LT::AsUnsigned::from(LHS),
LT::AsUnsigned::from(RHS, Bits), Bits, &R);
}
} else if (LHS.isNegative()) {
if (LHS.isMin()) {
R = LT::AsUnsigned::zero(LHS.bitWidth());
} else {
// If the LHS is negative, perform the cast and invert the result.
typename LT::AsUnsigned LHSU = LT::AsUnsigned::from(-LHS);
LT::AsUnsigned::shiftLeft(LHSU, LT::AsUnsigned::from(RHS, Bits), Bits,
&R);
R = -R;
}
} else {
// The good case, a simple left shift.
LT::AsUnsigned::shiftLeft(LT::AsUnsigned::from(LHS),
LT::AsUnsigned::from(RHS, Bits), Bits, &R);
}
} else {
if (RHS > RT::from(Bits - 1, RHS.bitWidth()))
LT::AsUnsigned::shiftRight(LT::AsUnsigned::from(LHS),
LT::AsUnsigned::from(Bits - 1), Bits, &R);
else
LT::AsUnsigned::shiftRight(LT::AsUnsigned::from(LHS),
LT::AsUnsigned::from(RHS, Bits), Bits, &R);
}

// We did the shift above as unsigned. Restore the sign bit if we need to.
if constexpr (Dir == ShiftDir::Right) {
if (LHS.isSigned() && LHS.isNegative()) {
typename LT::AsUnsigned SignBit;
LT::AsUnsigned::shiftLeft(LT::AsUnsigned::from(1, Bits),
LT::AsUnsigned::from(Bits - 1, Bits), Bits,
&SignBit);
LT::AsUnsigned::bitOr(R, SignBit, Bits, &R);
// Right shift.
if (Compare(RHS, RT::from(MaxShiftAmount, RHS.bitWidth())) ==
ComparisonCategoryResult::Greater) {
R = LT::AsUnsigned::from(-1);
} else {
// Do the shift on potentially signed LT, then convert to unsigned type.
LT A;
LT::shiftRight(LHS, LT::from(RHS, Bits), Bits, &A);
R = LT::AsUnsigned::from(A);
}
}

Expand Down
41 changes: 25 additions & 16 deletions clang/test/AST/ByteCode/shifts.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,27 +21,15 @@ namespace shifts {
c = 1 << 0;
c = 1 << -0;
c = 1 >> -0;
c = 1 << -1; // expected-warning {{shift count is negative}} \
// expected-note {{negative shift count -1}} \
// cxx17-note {{negative shift count -1}} \
// cxx17-warning {{shift count is negative}} \
// ref-warning {{shift count is negative}} \
// ref-note {{negative shift count -1}} \
// ref-cxx17-warning {{shift count is negative}} \
// ref-cxx17-note {{negative shift count -1}}
c = 1 << -1; // all-warning {{shift count is negative}} \
// all-note {{negative shift count -1}}

c = 1 >> -1; // expected-warning {{shift count is negative}} \
// cxx17-warning {{shift count is negative}} \
// ref-warning {{shift count is negative}} \
// ref-cxx17-warning {{shift count is negative}}
c = 1 << (unsigned)-1; // expected-warning {{shift count >= width of type}} \
// expected-warning {{implicit conversion from 'int' to 'char' changes value from -2147483648 to 0}} \
// cxx17-warning {{shift count >= width of type}} \
// cxx17-warning {{implicit conversion from 'int' to 'char' changes value from -2147483648 to 0}} \
// ref-warning {{shift count >= width of type}} \
// ref-warning {{implicit conversion from 'int' to 'char' changes value from -2147483648 to 0}} \
// ref-cxx17-warning {{shift count >= width of type}} \
// ref-cxx17-warning {{implicit conversion from 'int' to 'char' changes value from -2147483648 to 0}}
c = 1 << (unsigned)-1; // all-warning {{shift count >= width of type}} \
// all-warning {{implicit conversion from 'int' to 'char' changes value from -2147483648 to 0}}
c = 1 >> (unsigned)-1; // expected-warning {{shift count >= width of type}} \
// cxx17-warning {{shift count >= width of type}} \
// ref-warning {{shift count >= width of type}} \
Expand Down Expand Up @@ -212,3 +200,24 @@ enum shiftof {
X3 = (1<<32) // all-error {{expression is not an integral constant expression}} \
// all-note {{shift count 32 >= width of type 'int'}}
};

#if __WCHAR_WIDTH__ == 32
static_assert(((wchar_t)-1U >> 31) == -1);
#endif

#if __INT_WIDTH__ == 32
static_assert(((int)-1U >> 32) == -1); // all-error {{not an integral constant expression}} \
// all-note {{shift count 32 >= width of type 'int' (32 bits)}}
#endif

static_assert((-4 << 32) == 0); // all-error {{not an integral constant expression}} \
// all-note {{shift count}}

static_assert((-4 << 1) == -8); // ref-cxx17-error {{not an integral constant expression}} \
// ref-cxx17-note {{left shift of negative value -4}} \
// cxx17-error {{not an integral constant expression}} \
// cxx17-note {{left shift of negative value -4}}
static_assert((-4 << 31) == 0); // ref-cxx17-error {{not an integral constant expression}} \
// ref-cxx17-note {{left shift of negative value -4}} \
// cxx17-error {{not an integral constant expression}} \
// cxx17-note {{left shift of negative value -4}}
Loading