Skip to content

Commit c7b2f06

Browse files
committed
[APInt] Assert correct values in APInt constructor
If the uint64_t constructor is used, assert that the value is actuall a signed or unsigned N-bit integer depending on whether the isSigned flag is set. Currently, we allow values to be silently truncated, which is a constant source of subtle bugs -- a particularly common mistake is to create -1 values without setting the isSigned flag, which will work fine for all common bit widths (<= 64-bit) and miscompile for larger integers.
1 parent 4f381af commit c7b2f06

37 files changed

+389
-349
lines changed

llvm/include/llvm/ADT/APFixedPoint.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,9 @@ class APFixedPoint {
160160
}
161161

162162
APFixedPoint(uint64_t Val, const FixedPointSemantics &Sema)
163-
: APFixedPoint(APInt(Sema.getWidth(), Val, Sema.isSigned()), Sema) {}
163+
: APFixedPoint(APInt(Sema.getWidth(), Val, Sema.isSigned(),
164+
/*implicitTrunc*/ true),
165+
Sema) {}
164166

165167
// Zero initialization.
166168
APFixedPoint(const FixedPointSemantics &Sema) : APFixedPoint(0, Sema) {}

llvm/include/llvm/ADT/APInt.h

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,11 +106,26 @@ class [[nodiscard]] APInt {
106106
/// \param numBits the bit width of the constructed APInt
107107
/// \param val the initial value of the APInt
108108
/// \param isSigned how to treat signedness of val
109-
APInt(unsigned numBits, uint64_t val, bool isSigned = false)
109+
/// \param implicitTrunc allow implicit truncation of non-zero/sign bits of
110+
/// val beyond the range of numBits
111+
APInt(unsigned numBits, uint64_t val, bool isSigned = false,
112+
bool implicitTrunc = false)
110113
: BitWidth(numBits) {
114+
if (!implicitTrunc) {
115+
if (BitWidth == 0) {
116+
assert(val == 0 && "Value must be zero for 0-bit APInt");
117+
} else if (isSigned) {
118+
assert(llvm::isIntN(BitWidth, val) &&
119+
"Value is not an N-bit signed value");
120+
} else {
121+
assert(llvm::isUIntN(BitWidth, val) &&
122+
"Value is not an N-bit unsigned value");
123+
}
124+
}
111125
if (isSingleWord()) {
112126
U.VAL = val;
113-
clearUnusedBits();
127+
if (implicitTrunc || isSigned)
128+
clearUnusedBits();
114129
} else {
115130
initSlowCase(val, isSigned);
116131
}

llvm/lib/Analysis/ConstantFolding.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -889,7 +889,8 @@ Constant *SymbolicallyEvaluateGEP(const GEPOperator *GEP,
889889
APInt Offset = APInt(
890890
BitWidth,
891891
DL.getIndexedOffsetInType(
892-
SrcElemTy, ArrayRef((Value *const *)Ops.data() + 1, Ops.size() - 1)));
892+
SrcElemTy, ArrayRef((Value *const *)Ops.data() + 1, Ops.size() - 1)),
893+
/*isSigned*/ true, /*implicitTrunc*/ true);
893894

894895
// If this is a GEP of a GEP, fold it all into a single GEP.
895896
while (auto *GEP = dyn_cast<GEPOperator>(Ptr)) {
@@ -3322,8 +3323,9 @@ ConstantFoldScalarFrexpCall(Constant *Op, Type *IntTy) {
33223323

33233324
// The exponent is an "unspecified value" for inf/nan. We use zero to avoid
33243325
// using undef.
3325-
Constant *Result1 = FrexpMant.isFinite() ? ConstantInt::get(IntTy, FrexpExp)
3326-
: ConstantInt::getNullValue(IntTy);
3326+
Constant *Result1 = FrexpMant.isFinite()
3327+
? ConstantInt::getSigned(IntTy, FrexpExp)
3328+
: ConstantInt::getNullValue(IntTy);
33273329
return {Result0, Result1};
33283330
}
33293331

llvm/lib/Analysis/MemoryBuiltins.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -675,7 +675,7 @@ Value *llvm::lowerObjectSizeCall(
675675
if (!MustSucceed)
676676
return nullptr;
677677

678-
return ConstantInt::get(ResultType, MaxVal ? -1ULL : 0);
678+
return ConstantInt::get(ResultType, MaxVal ? -1ULL : 0, true);
679679
}
680680

681681
STATISTIC(ObjectVisitorArgument,

llvm/lib/Analysis/ScalarEvolution.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1458,7 +1458,7 @@ bool ScalarEvolution::proveNoWrapByVaryingStart(const SCEV *Start,
14581458

14591459
APInt StartAI = StartC->getAPInt();
14601460

1461-
for (unsigned Delta : {-2, -1, 1, 2}) {
1461+
for (int Delta : {-2, -1, 1, 2}) {
14621462
const SCEV *PreStart = getConstant(StartAI - Delta);
14631463

14641464
FoldingSetNodeID ID;
@@ -1473,7 +1473,7 @@ bool ScalarEvolution::proveNoWrapByVaryingStart(const SCEV *Start,
14731473
// Give up if we don't already have the add recurrence we need because
14741474
// actually constructing an add recurrence is relatively expensive.
14751475
if (PreAR && PreAR->getNoWrapFlags(WrapType)) { // proves (2)
1476-
const SCEV *DeltaS = getConstant(StartC->getType(), Delta);
1476+
const SCEV *DeltaS = getConstant(StartC->getType(), Delta, true);
14771477
ICmpInst::Predicate Pred = ICmpInst::BAD_ICMP_PREDICATE;
14781478
const SCEV *Limit = ExtendOpTraits<ExtendOpTy>::getOverflowLimitForStep(
14791479
DeltaS, &Pred, this);

llvm/lib/Analysis/ValueTracking.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8698,7 +8698,7 @@ static ConstantRange getRangeForIntrinsic(const IntrinsicInst &II) {
86988698
case Intrinsic::cttz:
86998699
// Maximum of set/clear bits is the bit width.
87008700
return ConstantRange::getNonEmpty(APInt::getZero(Width),
8701-
APInt(Width, Width + 1));
8701+
APInt(Width, Width) + 1);
87028702
case Intrinsic::uadd_sat:
87038703
// uadd.sat(x, C) produces [C, UINT_MAX].
87048704
if (match(II.getOperand(0), m_APInt(C)) ||
@@ -8849,7 +8849,7 @@ static void setLimitForFPToI(const Instruction *I, APInt &Lower, APInt &Upper) {
88498849
if (!I->getOperand(0)->getType()->getScalarType()->isHalfTy())
88508850
return;
88518851
if (isa<FPToSIInst>(I) && BitWidth >= 17) {
8852-
Lower = APInt(BitWidth, -65504);
8852+
Lower = APInt(BitWidth, -65504, true);
88538853
Upper = APInt(BitWidth, 65505);
88548854
}
88558855

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3062,7 +3062,7 @@ Error BitcodeReader::parseConstants() {
30623062
case bitc::CST_CODE_INTEGER: // INTEGER: [intval]
30633063
if (!CurTy->isIntegerTy() || Record.empty())
30643064
return error("Invalid integer const record");
3065-
V = ConstantInt::get(CurTy, decodeSignRotatedValue(Record[0]));
3065+
V = ConstantInt::getSigned(CurTy, decodeSignRotatedValue(Record[0]));
30663066
break;
30673067
case bitc::CST_CODE_WIDE_INTEGER: {// WIDE_INTEGER: [n x intval]
30683068
if (!CurTy->isIntegerTy() || Record.empty())

llvm/lib/CodeGen/CodeGenPrepare.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1645,7 +1645,7 @@ static bool matchUAddWithOverflowConstantEdgeCases(CmpInst *Cmp,
16451645
if (Pred == ICmpInst::ICMP_EQ && match(B, m_AllOnes()))
16461646
B = ConstantInt::get(B->getType(), 1);
16471647
else if (Pred == ICmpInst::ICMP_NE && match(B, m_ZeroInt()))
1648-
B = ConstantInt::get(B->getType(), -1);
1648+
B = Constant::getAllOnesValue(B->getType());
16491649
else
16501650
return false;
16511651

llvm/lib/CodeGen/ExpandMemCmp.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -590,7 +590,7 @@ void MemCmpExpansion::emitMemCmpResultBlock() {
590590
ResBlock.PhiSrc2);
591591

592592
Value *Res =
593-
Builder.CreateSelect(Cmp, ConstantInt::get(Builder.getInt32Ty(), -1),
593+
Builder.CreateSelect(Cmp, Constant::getAllOnesValue(Builder.getInt32Ty()),
594594
ConstantInt::get(Builder.getInt32Ty(), 1));
595595

596596
PhiRes->addIncoming(Res, ResBlock.BB);

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1603,7 +1603,10 @@ SDValue SelectionDAG::getConstant(uint64_t Val, const SDLoc &DL, EVT VT,
16031603
assert((EltVT.getSizeInBits() >= 64 ||
16041604
(uint64_t)((int64_t)Val >> EltVT.getSizeInBits()) + 1 < 2) &&
16051605
"getConstant with a uint64_t value that doesn't fit in the type!");
1606-
return getConstant(APInt(EltVT.getSizeInBits(), Val), DL, VT, isT, isO);
1606+
// TODO: Avoid implicit trunc?
1607+
return getConstant(
1608+
APInt(EltVT.getSizeInBits(), Val, false, /*implicitTrunc*/ true), DL, VT,
1609+
isT, isO);
16071610
}
16081611

16091612
SDValue SelectionDAG::getConstant(const APInt &Val, const SDLoc &DL, EVT VT,

0 commit comments

Comments
 (0)