Skip to content

Commit ca2fc22

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 73d835c commit ca2fc22

33 files changed

+377
-339
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
@@ -108,11 +108,26 @@ class [[nodiscard]] APInt {
108108
/// \param numBits the bit width of the constructed APInt
109109
/// \param val the initial value of the APInt
110110
/// \param isSigned how to treat signedness of val
111-
APInt(unsigned numBits, uint64_t val, bool isSigned = false)
111+
/// \param implicitTrunc allow implicit truncation of non-zero/sign bits of
112+
/// val beyond the range of numBits
113+
APInt(unsigned numBits, uint64_t val, bool isSigned = false,
114+
bool implicitTrunc = false)
112115
: BitWidth(numBits) {
116+
if (!implicitTrunc) {
117+
if (BitWidth == 0) {
118+
assert(val == 0 && "Value must be zero for 0-bit APInt");
119+
} else if (isSigned) {
120+
assert(llvm::isIntN(BitWidth, val) &&
121+
"Value is not an N-bit signed value");
122+
} else {
123+
assert(llvm::isUIntN(BitWidth, val) &&
124+
"Value is not an N-bit unsigned value");
125+
}
126+
}
113127
if (isSingleWord()) {
114128
U.VAL = val;
115-
clearUnusedBits();
129+
if (implicitTrunc || isSigned)
130+
clearUnusedBits();
116131
} else {
117132
initSlowCase(val, isSigned);
118133
}

llvm/lib/Analysis/ConstantFolding.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -887,7 +887,8 @@ Constant *SymbolicallyEvaluateGEP(const GEPOperator *GEP,
887887
APInt Offset = APInt(
888888
BitWidth,
889889
DL.getIndexedOffsetInType(
890-
SrcElemTy, ArrayRef((Value *const *)Ops.data() + 1, Ops.size() - 1)));
890+
SrcElemTy, ArrayRef((Value *const *)Ops.data() + 1, Ops.size() - 1)),
891+
/*isSigned*/ true, /*implicitTrunc*/ true);
891892

892893
std::optional<ConstantRange> InRange = GEP->getInRange();
893894
if (InRange)

llvm/lib/Analysis/MemoryBuiltins.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -660,7 +660,7 @@ Value *llvm::lowerObjectSizeCall(
660660
if (!MustSucceed)
661661
return nullptr;
662662

663-
return ConstantInt::get(ResultType, MaxVal ? -1ULL : 0);
663+
return ConstantInt::get(ResultType, MaxVal ? -1ULL : 0, true);
664664
}
665665

666666
STATISTIC(ObjectVisitorArgument,

llvm/lib/Analysis/ScalarEvolution.cpp

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

14821482
APInt StartAI = StartC->getAPInt();
14831483

1484-
for (unsigned Delta : {-2, -1, 1, 2}) {
1484+
for (int Delta : {-2, -1, 1, 2}) {
14851485
const SCEV *PreStart = getConstant(StartAI - Delta);
14861486

14871487
FoldingSetNodeID ID;
@@ -1496,7 +1496,7 @@ bool ScalarEvolution::proveNoWrapByVaryingStart(const SCEV *Start,
14961496
// Give up if we don't already have the add recurrence we need because
14971497
// actually constructing an add recurrence is relatively expensive.
14981498
if (PreAR && PreAR->getNoWrapFlags(WrapType)) { // proves (2)
1499-
const SCEV *DeltaS = getConstant(StartC->getType(), Delta);
1499+
const SCEV *DeltaS = getConstant(StartC->getType(), Delta, true);
15001500
ICmpInst::Predicate Pred = ICmpInst::BAD_ICMP_PREDICATE;
15011501
const SCEV *Limit = ExtendOpTraits<ExtendOpTy>::getOverflowLimitForStep(
15021502
DeltaS, &Pred, this);

llvm/lib/Analysis/ValueTracking.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9516,7 +9516,7 @@ static ConstantRange getRangeForIntrinsic(const IntrinsicInst &II) {
95169516
case Intrinsic::cttz:
95179517
// Maximum of set/clear bits is the bit width.
95189518
return ConstantRange::getNonEmpty(APInt::getZero(Width),
9519-
APInt(Width, Width + 1));
9519+
APInt(Width, Width) + 1);
95209520
case Intrinsic::uadd_sat:
95219521
// uadd.sat(x, C) produces [C, UINT_MAX].
95229522
if (match(II.getOperand(0), m_APInt(C)) ||
@@ -9671,7 +9671,7 @@ static void setLimitForFPToI(const Instruction *I, APInt &Lower, APInt &Upper) {
96719671
if (!I->getOperand(0)->getType()->getScalarType()->isHalfTy())
96729672
return;
96739673
if (isa<FPToSIInst>(I) && BitWidth >= 17) {
9674-
Lower = APInt(BitWidth, -65504);
9674+
Lower = APInt(BitWidth, -65504, true);
96759675
Upper = APInt(BitWidth, 65505);
96769676
}
96779677

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3190,7 +3190,7 @@ Error BitcodeReader::parseConstants() {
31903190
case bitc::CST_CODE_INTEGER: // INTEGER: [intval]
31913191
if (!CurTy->isIntOrIntVectorTy() || Record.empty())
31923192
return error("Invalid integer const record");
3193-
V = ConstantInt::get(CurTy, decodeSignRotatedValue(Record[0]));
3193+
V = ConstantInt::getSigned(CurTy, decodeSignRotatedValue(Record[0]));
31943194
break;
31953195
case bitc::CST_CODE_WIDE_INTEGER: {// WIDE_INTEGER: [n x intval]
31963196
if (!CurTy->isIntOrIntVectorTy() || Record.empty())

llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1628,7 +1628,10 @@ SDValue SelectionDAG::getConstant(uint64_t Val, const SDLoc &DL, EVT VT,
16281628
assert((EltVT.getSizeInBits() >= 64 ||
16291629
(uint64_t)((int64_t)Val >> EltVT.getSizeInBits()) + 1 < 2) &&
16301630
"getConstant with a uint64_t value that doesn't fit in the type!");
1631-
return getConstant(APInt(EltVT.getSizeInBits(), Val), DL, VT, isT, isO);
1631+
// TODO: Avoid implicit trunc?
1632+
return getConstant(
1633+
APInt(EltVT.getSizeInBits(), Val, false, /*implicitTrunc*/ true), DL, VT,
1634+
isT, isO);
16321635
}
16331636

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

llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2142,7 +2142,9 @@ ScheduleDAGSDNodes *SelectionDAGISel::CreateScheduler() {
21422142
bool SelectionDAGISel::CheckAndMask(SDValue LHS, ConstantSDNode *RHS,
21432143
int64_t DesiredMaskS) const {
21442144
const APInt &ActualMask = RHS->getAPIntValue();
2145-
const APInt &DesiredMask = APInt(LHS.getValueSizeInBits(), DesiredMaskS);
2145+
// TODO: Avoid implicit trunc?
2146+
const APInt &DesiredMask = APInt(LHS.getValueSizeInBits(), DesiredMaskS,
2147+
false, /*implicitTrunc*/ true);
21462148

21472149
// If the actual mask exactly matches, success!
21482150
if (ActualMask == DesiredMask)

llvm/lib/FuzzMutate/OpDescriptor.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ void fuzzerop::makeConstantsWithType(Type *T, std::vector<Constant *> &Cs) {
2222
uint64_t W = IntTy->getBitWidth();
2323
Cs.push_back(ConstantInt::get(IntTy, 0));
2424
Cs.push_back(ConstantInt::get(IntTy, 1));
25-
Cs.push_back(ConstantInt::get(IntTy, 42));
25+
Cs.push_back(ConstantInt::get(
26+
IntTy, APInt(W, 42, /*isSigned*/ false, /*implicitTrunc*/ true)));
2627
Cs.push_back(ConstantInt::get(IntTy, APInt::getMaxValue(W)));
2728
Cs.push_back(ConstantInt::get(IntTy, APInt::getMinValue(W)));
2829
Cs.push_back(ConstantInt::get(IntTy, APInt::getSignedMaxValue(W)));

0 commit comments

Comments
 (0)