Skip to content

Commit

Permalink
Fix a couple of places where range selects were not properly checked …
Browse files Browse the repository at this point in the history
…for overflow
  • Loading branch information
MikePopoloski committed Apr 6, 2024
1 parent afe0248 commit 6fcb6d7
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 15 deletions.
15 changes: 11 additions & 4 deletions include/slang/numeric/ConstantValue.h
Original file line number Diff line number Diff line change
Expand Up @@ -316,10 +316,17 @@ struct SLANG_EXPORT ConstantRange {

/// Gets the width of the range, regardless of the order in which
/// the bounds are specified.
bitwidth_t width() const {
auto ul = uint32_t(left);
auto ur = uint32_t(right);
return bitwidth_t(left > right ? ul - ur : ur - ul) + 1;
///
/// @note the width is truncated to fit in a bitwidth_t. Use @a fullWidth
/// if there is a possibility of overflow.
bitwidth_t width() const { return bitwidth_t(fullWidth()); }

/// Gets the full width of the range, which may not fit into a
/// 32-bit integer.
uint64_t fullWidth() const {
auto ul = uint64_t(left);
auto ur = uint64_t(right);
return (left > right ? ul - ur : ur - ul) + 1;
}

/// Gets the lower bound of the range, regardless of the order in which
Expand Down
20 changes: 11 additions & 9 deletions source/ast/ASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -523,15 +523,17 @@ void ASTContext::evalRangeDimension(const SelectorSyntax& syntax, bool isPacked,
addDiag(diag::PackedDimsRequireFullRange, syntax.sourceRange());
result.kind = DimensionKind::Unknown;
}
else if (isPacked && result.range.width() > SVInt::MAX_BITS) {
addDiag(diag::PackedTypeTooLarge, syntax.sourceRange())
<< result.range.width() << (int)SVInt::MAX_BITS;
result.kind = DimensionKind::Unknown;
}
else if (!isPacked && result.range.width() > INT32_MAX) {
addDiag(diag::ArrayDimTooLarge, syntax.sourceRange())
<< result.range.width() << INT32_MAX;
result.kind = DimensionKind::Unknown;
else {
auto fullWidth = result.range.fullWidth();
if (isPacked && fullWidth > SVInt::MAX_BITS) {
addDiag(diag::PackedTypeTooLarge, syntax.sourceRange())
<< fullWidth << (int)SVInt::MAX_BITS;
result.kind = DimensionKind::Unknown;
}
else if (!isPacked && fullWidth > INT32_MAX) {
addDiag(diag::ArrayDimTooLarge, syntax.sourceRange()) << fullWidth << INT32_MAX;
result.kind = DimensionKind::Unknown;
}
}
}
}
Expand Down
14 changes: 14 additions & 0 deletions source/ast/expressions/SelectExpressions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -417,6 +417,15 @@ void ElementSelectExpression::serializeTo(ASTSerializer& serializer) const {
serializer.write("selector", selector());
}

template<typename TContext>
static bool checkRangeOverflow(ConstantRange range, TContext& context, SourceRange sourceRange) {
if (range.fullWidth() > INT32_MAX) {
context.addDiag(diag::RangeWidthOverflow, sourceRange);
return true;
}
return false;
}

Expression& RangeSelectExpression::fromSyntax(Compilation& compilation, Expression& value,
const RangeSelectSyntax& syntax,
SourceRange fullRange, const ASTContext& context) {
Expand Down Expand Up @@ -528,6 +537,9 @@ Expression& RangeSelectExpression::fromSyntax(Compilation& compilation, Expressi
return badExpr(compilation, result);

selectionRange = {*lv, *rv};
if (checkRangeOverflow(selectionRange, context, errorRange))
return badExpr(compilation, result);

if (selectionRange.isLittleEndian() != valueRange.isLittleEndian() &&
selectionRange.width() > 1) {
auto& diag = context.addDiag(diag::SelectEndianMismatch, errorRange);
Expand Down Expand Up @@ -740,6 +752,8 @@ std::optional<ConstantRange> RangeSelectExpression::evalRange(EvalContext& conte
ConstantRange result;
if (selectionKind == RangeSelectionKind::Simple) {
result = {*li, *ri};
if (checkRangeOverflow(result, context, sourceRange))
return std::nullopt;
}
else {
bool isLittleEndian = false;
Expand Down
16 changes: 16 additions & 0 deletions tests/unittests/ast/ExpressionTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3631,3 +3631,19 @@ endmodule
CHECK(diags[1].code == diag::AutoVarToRefStatic);
CHECK(diags[2].code == diag::AutoVarToRefStatic);
}

TEST_CASE("Out of range select regress") {
auto tree = SyntaxTree::fromText(R"(
localparam p = -4'd8;
$info(p[-2147483648:-2147483649]);
)");

Compilation compilation;
compilation.addSyntaxTree(tree);

auto& diags = compilation.getAllDiagnostics();
REQUIRE(diags.size() == 3);
CHECK(diags[0].code == diag::RangeWidthOverflow);
CHECK(diags[1].code == diag::SignedIntegerOverflow);
CHECK(diags[2].code == diag::SignedIntegerOverflow);
}
19 changes: 17 additions & 2 deletions tests/unittests/ast/TypeTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2037,7 +2037,7 @@ module m;
struct packed { logic [16777214:0] a; logic [16777214:0] b; } boz;
initial begin
$display(foo[2147483647:-2147483647]);
$display(foo[2147483647:1]);
$display(bar[-2147483647:2147483647]);
end
endmodule
Expand Down Expand Up @@ -2189,7 +2189,7 @@ parameter real r = $;
CHECK(diags[0].code == diag::BadAssignment);
}

TEST_CASE("Soft packed unions") {
TEST_CASE("v1800-2023: Soft packed unions") {
auto options = optionsFor(LanguageVersion::v1800_2023);
auto tree = SyntaxTree::fromText(R"(
function automatic logic[7:0] foo;
Expand All @@ -2209,3 +2209,18 @@ endmodule
compilation.addSyntaxTree(tree);
NO_COMPILATION_ERRORS;
}

TEST_CASE("Type dimension overflow regress") {
auto tree = SyntaxTree::fromText(R"(
logic [-2147483648:-2147483649] a;
)");

Compilation compilation;
compilation.addSyntaxTree(tree);

auto& diags = compilation.getAllDiagnostics();
REQUIRE(diags.size() == 3);
CHECK(diags[0].code == diag::PackedTypeTooLarge);
CHECK(diags[1].code == diag::SignedIntegerOverflow);
CHECK(diags[2].code == diag::SignedIntegerOverflow);
}
2 changes: 2 additions & 0 deletions tests/unittests/util/NumericTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ TEST_CASE("SVInt to string (and back)") {
checkRoundTrip("-999989", LiteralBase::Decimal);
checkRoundTrip("12'b101x101z1", LiteralBase::Binary);
checkRoundTrip("999999999", LiteralBase::Decimal);
checkRoundTrip("-2147483648", LiteralBase::Decimal);

std::ostringstream ss;
ss << "96'd192834"_si;
Expand Down Expand Up @@ -391,6 +392,7 @@ TEST_CASE("Division") {
CHECK("-50"_si / "25"_si == -2);
CHECK("-50"_si % "-40"_si == -10);
CHECK("-50"_si % "40"_si == -10);
CHECK("-4'sd8"_si / "-4'sd7"_si == 1);

SVInt v7 = "19823'd234098234098234098234"_si;
CHECK("300'd0"_si / "10"_si == 0);
Expand Down

0 comments on commit 6fcb6d7

Please sign in to comment.