Skip to content

Commit

Permalink
Avoid silently truncating BigInt to Int
Browse files Browse the repository at this point in the history
- Introduce internal helper `castToInt`, which issues an error when the input
BigInt can't be represented as Int.

- Use `castToInt` wherever we were using `toInt` in a potentially unsafe way.
  • Loading branch information
aswaterman authored and edwardcwang committed Apr 15, 2019
1 parent 0028c64 commit a4a29e2
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 10 deletions.
16 changes: 8 additions & 8 deletions chiselFrontend/src/main/scala/chisel3/core/Bits.scala
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ sealed abstract class Bits(private[chisel3] val width: Width) extends Element wi
// This preserves old behavior while a more more consistent API is under debate
// See https://github.com/freechipsproject/chisel3/issues/867
litOption.map { value =>
(((value >> x.toInt) & 1) == 1).asBool
(((value >> castToInt(x, "Index")) & 1) == 1).asBool
}.getOrElse {
requireIsHardware(this, "bits to be indexed")
pushOp(DefPrim(sourceInfo, Bool(), BitsExtractOp, this.ref, ILit(x), ILit(x)))
Expand Down Expand Up @@ -238,7 +238,7 @@ sealed abstract class Bits(private[chisel3] val width: Width) extends Element wi

/** @group SourceInfoTransformMacro */
final def do_apply(x: BigInt, y: BigInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): UInt =
apply(x.toInt, y.toInt)
apply(castToInt(x, "High index"), castToInt(y, "Low index"))

private[core] def unop[T <: Data](sourceInfo: SourceInfo, dest: T, op: PrimOp): T = {
requireIsHardware(this, "bits operated on")
Expand Down Expand Up @@ -866,13 +866,13 @@ sealed class UInt private[core] (width: Width) extends Bits(width) with Num[UInt
override def do_<< (that: Int)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): UInt =
binop(sourceInfo, UInt(this.width + that), ShiftLeftOp, validateShiftAmount(that))
override def do_<< (that: BigInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): UInt =
this << that.toInt
this << castToInt(that, "Shift amount")
override def do_<< (that: UInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): UInt =
binop(sourceInfo, UInt(this.width.dynamicShiftLeft(that.width)), DynamicShiftLeftOp, that)
override def do_>> (that: Int)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): UInt =
binop(sourceInfo, UInt(this.width.shiftRight(that)), ShiftRightOp, validateShiftAmount(that))
override def do_>> (that: BigInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): UInt =
this >> that.toInt
this >> castToInt(that, "Shift amount")
override def do_>> (that: UInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): UInt =
binop(sourceInfo, UInt(this.width), DynamicShiftRightOp, that)

Expand Down Expand Up @@ -1155,13 +1155,13 @@ sealed class SInt private[core] (width: Width) extends Bits(width) with Num[SInt
override def do_<< (that: Int)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): SInt =
binop(sourceInfo, SInt(this.width + that), ShiftLeftOp, validateShiftAmount(that))
override def do_<< (that: BigInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): SInt =
this << that.toInt
this << castToInt(that, "Shift amount")
override def do_<< (that: UInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): SInt =
binop(sourceInfo, SInt(this.width.dynamicShiftLeft(that.width)), DynamicShiftLeftOp, that)
override def do_>> (that: Int)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): SInt =
binop(sourceInfo, SInt(this.width.shiftRight(that)), ShiftRightOp, validateShiftAmount(that))
override def do_>> (that: BigInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): SInt =
this >> that.toInt
this >> castToInt(that, "Shift amount")
override def do_>> (that: UInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): SInt =
binop(sourceInfo, SInt(this.width), DynamicShiftRightOp, that)

Expand Down Expand Up @@ -1602,13 +1602,13 @@ sealed class FixedPoint private (width: Width, val binaryPoint: BinaryPoint)
override def do_<< (that: Int)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): FixedPoint =
binop(sourceInfo, FixedPoint(this.width + that, this.binaryPoint), ShiftLeftOp, validateShiftAmount(that))
override def do_<< (that: BigInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): FixedPoint =
(this << that.toInt).asFixedPoint(this.binaryPoint)
(this << castToInt(that, "Shift amount")).asFixedPoint(this.binaryPoint)
override def do_<< (that: UInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): FixedPoint =
binop(sourceInfo, FixedPoint(this.width.dynamicShiftLeft(that.width), this.binaryPoint), DynamicShiftLeftOp, that)
override def do_>> (that: Int)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): FixedPoint =
binop(sourceInfo, FixedPoint(this.width.shiftRight(that), this.binaryPoint), ShiftRightOp, validateShiftAmount(that))
override def do_>> (that: BigInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): FixedPoint =
(this >> that.toInt).asFixedPoint(this.binaryPoint)
(this >> castToInt(that, "Shift amount")).asFixedPoint(this.binaryPoint)
override def do_>> (that: UInt)(implicit sourceInfo: SourceInfo, compileOptions: CompileOptions): FixedPoint =
binop(sourceInfo, FixedPoint(this.width, this.binaryPoint), DynamicShiftRightOp, that)

Expand Down
9 changes: 9 additions & 0 deletions chiselFrontend/src/main/scala/chisel3/internal/Builder.scala
Original file line number Diff line number Diff line change
Expand Up @@ -378,3 +378,12 @@ object DynamicNamingStack {
prefixRef
}
}

/** Casts BigInt to Int, issuing an error when the input isn't representable. */
private[chisel3] object castToInt {
def apply(x: BigInt, msg: String): Int = {
val res = x.toInt
require(x == res, s"$msg $x is too large to be represented as Int")
res
}
}
4 changes: 2 additions & 2 deletions src/main/scala/chisel3/internal/firrtl/Converter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import chisel3.core.{SpecifiedDirection, EnumType}
import chisel3.experimental._
import chisel3.internal.sourceinfo.{NoSourceInfo, SourceLine, SourceInfo}
import firrtl.{ir => fir}
import chisel3.internal.throwException
import chisel3.internal.{castToInt, throwException}

import scala.annotation.tailrec
import scala.collection.immutable.{Queue}
Expand Down Expand Up @@ -50,7 +50,7 @@ private[chisel3] object Converter {
case Slot(imm, name) =>
fir.SubField(convert(imm, ctx), name, fir.UnknownType)
case Index(imm, ILit(idx)) =>
fir.SubIndex(convert(imm, ctx), idx.toInt, fir.UnknownType)
fir.SubIndex(convert(imm, ctx), castToInt(idx, "Index"), fir.UnknownType)
case Index(imm, value) =>
fir.SubAccess(convert(imm, ctx), convert(value, ctx), fir.UnknownType)
case ModuleIO(mod, name) =>
Expand Down

0 comments on commit a4a29e2

Please sign in to comment.