Skip to content

Commit

Permalink
[ConstraintElim] Use MulOverflow to avoid UB on signed overflow.
Browse files Browse the repository at this point in the history
This fixes an UBSan failure after 359bc5c. For inbounds GEP with
index sizes <= 64, having the coefficients overflowing is fine.
  • Loading branch information
fhahn committed Oct 13, 2022
1 parent 4265f78 commit 019049a
Showing 1 changed file with 17 additions and 6 deletions.
23 changes: 17 additions & 6 deletions llvm/lib/Transforms/Scalar/ConstraintElimination.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,13 @@ static bool canUseSExt(ConstantInt *CI) {
return Val.sgt(MinSignedConstraintValue) && Val.slt(MaxConstraintValue);
}

// A helper to multiply 2 signed integers where overflowing is allowed.
static int64_t multiplyWithOverflow(int64_t A, int64_t B) {
int64_t Result;
MulOverflow(A, B, Result);
return Result;
}

static SmallVector<DecompEntry, 4>
decomposeGEP(GetElementPtrInst &GEP,
SmallVector<PreconditionTy, 4> &Preconditions, bool IsSigned,
Expand All @@ -214,7 +221,7 @@ decomposeGEP(GetElementPtrInst &GEP,
isa<ConstantInt>(GEP.getOperand(1))) {
APInt Offset = cast<ConstantInt>(GEP.getOperand(1))->getValue();
auto Result = decompose(InnerGEP, Preconditions, IsSigned, DL);
Result[0].Coefficient += Scale * Offset.getSExtValue();
Result[0].Coefficient += multiplyWithOverflow(Scale, Offset.getSExtValue());
if (Offset.isNegative()) {
// Add pre-condition ensuring the GEP is increasing monotonically and
// can be de-composed.
Expand All @@ -233,10 +240,12 @@ decomposeGEP(GetElementPtrInst &GEP,
if (match(Op0, m_NUWShl(m_Value(Op1), m_ConstantInt(CI))) && canUseSExt(CI))
return {{0, nullptr},
{1, GEP.getPointerOperand()},
{Scale * int64_t(std::pow(int64_t(2), CI->getSExtValue())), Op1}};
{multiplyWithOverflow(
Scale, int64_t(std::pow(int64_t(2), CI->getSExtValue()))),
Op1}};
if (match(Op0, m_NSWAdd(m_Value(Op1), m_ConstantInt(CI))) &&
canUseSExt(CI) && match(Op0, m_NUWAdd(m_Value(), m_Value())))
return {{Scale * CI->getSExtValue(), nullptr},
return {{multiplyWithOverflow(Scale, CI->getSExtValue()), nullptr},
{1, GEP.getPointerOperand()},
{Scale, Op1}};

Expand All @@ -245,7 +254,7 @@ decomposeGEP(GetElementPtrInst &GEP,

if (match(GEP.getOperand(GEP.getNumOperands() - 1), m_ConstantInt(CI)) &&
!CI->isNegative() && canUseSExt(CI))
return {{Scale * CI->getSExtValue(), nullptr},
return {{multiplyWithOverflow(Scale, CI->getSExtValue()), nullptr},
{1, GEP.getPointerOperand()}};

SmallVector<DecompEntry, 4> Result;
Expand All @@ -254,11 +263,13 @@ decomposeGEP(GetElementPtrInst &GEP,
canUseSExt(CI))
Result = {{0, nullptr},
{1, GEP.getPointerOperand()},
{Scale * int64_t(std::pow(int64_t(2), CI->getSExtValue())), Op0}};
{multiplyWithOverflow(
Scale, int64_t(std::pow(int64_t(2), CI->getSExtValue()))),
Op0}};
else if (match(GEP.getOperand(GEP.getNumOperands() - 1),
m_NSWAdd(m_Value(Op0), m_ConstantInt(CI))) &&
canUseSExt(CI))
Result = {{Scale * CI->getSExtValue(), nullptr},
Result = {{multiplyWithOverflow(Scale, CI->getSExtValue()), nullptr},
{1, GEP.getPointerOperand()},
{Scale, Op0}};
else {
Expand Down

0 comments on commit 019049a

Please sign in to comment.