Skip to content

Commit

Permalink
Merge branch 'master' into netlist
Browse files Browse the repository at this point in the history
* master:
  Add checking for streaming operators that produce values larger than the maximum object size
  Type::getBitstreamWidth can now return a uint32_t, no possibility for overflow
  Rename bitstreamWidth -> getBitstreamWidth
  Enforce bitstream width limits for classes inside structs, unpacked arrays
  SmallVector.h: avoid casting away constness (MikePopoloski#821)
  Win32: add missing manifest to tool unittest executables
  Fix gcc + clang warnings
  chore: update pre-commit hooks (MikePopoloski#820)
  Tweak message for ObjectTooLarge diagnostic
  Error if a declared class type is too large
  Remove unreachable diagnostic path for bits of dynamic type
  Remove wchar special cases for Windows now that it can use UTF-8 as the active code page
  Win32: add a manifest to set the active code page to utf-8
  • Loading branch information
jameshanlon committed Sep 16, 2023
2 parents 2c19b0e + cf39a99 commit 9aa4199
Show file tree
Hide file tree
Showing 46 changed files with 328 additions and 358 deletions.
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ repos:

# Black, the python code formatter
- repo: https://github.com/psf/black
rev: 23.7.0
rev: 23.9.1
hooks:
- id: black
exclude: ^(docs)
Expand Down
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ set(CMAKE_CXX_EXTENSIONS OFF)
# target_ specific commands.
if(CMAKE_SYSTEM_NAME MATCHES "Windows")
add_compile_definitions(WIN32 _WINDOWS NTDDI_VERSION=0x06010000
_WIN32_WINNT=0x0601 UNICODE _UNICODE)
_WIN32_WINNT=0x0601)
add_compile_definitions(_SCL_SECURE_NO_WARNINGS _CRT_SECURE_NO_WARNINGS)
add_compile_definitions(_CRT_SECURE_NO_DEPRECATE _CRT_NONSTDC_NO_WARNINGS)
endif()
Expand Down
3 changes: 2 additions & 1 deletion bindings/python/ASTBindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,8 @@ void registerAST(py::module_& m) {
m, "StreamingConcatenationExpression");
streamConcatExpr.def_readonly("sliceSize", &StreamingConcatenationExpression::sliceSize)
.def_property_readonly("isFixedSize", &StreamingConcatenationExpression::isFixedSize)
.def_property_readonly("bitstreamWidth", &StreamingConcatenationExpression::bitstreamWidth)
.def_property_readonly("bitstreamWidth",
&StreamingConcatenationExpression::getBitstreamWidth)
.def_property_readonly("streams", &StreamingConcatenationExpression::streams);

py::class_<StreamingConcatenationExpression::StreamExpression>(streamConcatExpr,
Expand Down
2 changes: 1 addition & 1 deletion bindings/python/TypeBindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ void registerTypes(py::module_& m) {
py::class_<Type, Symbol>(m, "Type")
.def_property_readonly("canonicalType", &Type::getCanonicalType)
.def_property_readonly("bitWidth", &Type::getBitWidth)
.def_property_readonly("bitstreamWidth", &Type::bitstreamWidth)
.def_property_readonly("bitstreamWidth", &Type::getBitstreamWidth)
.def_property_readonly("selectableWidth", &Type::getSelectableWidth)
.def_property_readonly("isSigned", &Type::isSigned)
.def_property_readonly("isFourState", &Type::isFourState)
Expand Down
9 changes: 5 additions & 4 deletions include/slang/ast/expressions/OperatorExpressions.h
Original file line number Diff line number Diff line change
Expand Up @@ -253,22 +253,22 @@ class SLANG_EXPORT StreamingConcatenationExpression : public Expression {
public:
/// The size of the blocks to slice and reorder: if 0, this is a left-to-right
/// concatenation. Otherwise, it's a right-to-left concatenation.
const size_t sliceSize;
const uint32_t sliceSize;

struct StreamExpression {
not_null<const Expression*> operand;
const Expression* withExpr;
std::optional<bitwidth_t> constantWithWidth;
};

StreamingConcatenationExpression(const Type& type, size_t sliceSize,
StreamingConcatenationExpression(const Type& type, uint32_t sliceSize, uint32_t bitstreamWidth,
std::span<const StreamExpression> streams,
SourceRange sourceRange) :
Expression(ExpressionKind::Streaming, type, sourceRange),
sliceSize(sliceSize), streams_(streams) {}
sliceSize(sliceSize), streams_(streams), bitstreamWidth(bitstreamWidth) {}

bool isFixedSize() const;
size_t bitstreamWidth() const;
uint32_t getBitstreamWidth() const { return bitstreamWidth; }

std::span<const StreamExpression> streams() const { return streams_; }

Expand All @@ -293,6 +293,7 @@ class SLANG_EXPORT StreamingConcatenationExpression : public Expression {

private:
std::span<const StreamExpression> streams_;
uint32_t bitstreamWidth;
};

/// Denotes a range of values by providing expressions for the lower and upper
Expand Down
11 changes: 10 additions & 1 deletion include/slang/ast/symbols/ClassSymbols.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,13 @@ class SLANG_EXPORT ClassType : public Type, public Scope {
/// invoke that method. Otherwise returns nullptr.
const Expression* getBaseConstructorCall() const;

/// Gets $bits of the type. Returns zero if the type does not have a statically known size.
uint32_t getBitstreamWidth() const {
if (!cachedBitstreamWidth)
computeSize();
return *cachedBitstreamWidth;
}

static const Symbol& fromSyntax(const Scope& scope,
const syntax::ClassDeclarationSyntax& syntax);

Expand Down Expand Up @@ -113,13 +120,15 @@ class SLANG_EXPORT ClassType : public Type, public Scope {
void handleImplements(const syntax::ImplementsClauseSyntax& implementsClause,
const ASTContext& context,
function_ref<void(const Symbol&)> insertCB) const;
void computeSize() const;

mutable const Type* baseClass = nullptr;
mutable const Symbol* baseConstructor = nullptr;
mutable std::optional<const Expression*> baseConstructorCall;
mutable const ForwardingTypedefSymbol* firstForward = nullptr;
mutable std::span<const Type* const> implementsIfaces;
mutable std::span<const Type* const> declaredIfaces;
mutable std::optional<const Expression*> baseConstructorCall;
mutable std::optional<uint32_t> cachedBitstreamWidth;
SymbolIndex headerIndex;
};

Expand Down
5 changes: 4 additions & 1 deletion include/slang/ast/types/AllTypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -164,9 +164,10 @@ class SLANG_EXPORT FixedSizeUnpackedArrayType : public Type {
const Type& elementType;
ConstantRange range;
uint32_t selectableWidth;
uint32_t bitstreamWidth;

FixedSizeUnpackedArrayType(const Type& elementType, ConstantRange range,
uint32_t selectableWidth);
uint32_t selectableWidth, uint32_t bitstreamWidth);

static const Type& fromDims(const Scope& scope, const Type& elementType,
std::span<const ConstantRange> dimensions,
Expand Down Expand Up @@ -253,6 +254,7 @@ class SLANG_EXPORT UnpackedStructType : public Type, public Scope {
public:
std::span<const FieldSymbol* const> fields;
uint32_t selectableWidth = 0;
uint32_t bitstreamWidth = 0;
int systemId;

UnpackedStructType(Compilation& compilation, SourceLocation loc, const ASTContext& context);
Expand Down Expand Up @@ -287,6 +289,7 @@ class SLANG_EXPORT UnpackedUnionType : public Type, public Scope {
public:
std::span<const FieldSymbol* const> fields;
uint32_t selectableWidth = 0;
uint32_t bitstreamWidth = 0;
int systemId;
bool isTagged;

Expand Down
5 changes: 4 additions & 1 deletion include/slang/ast/types/Type.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ SLANG_BITMASK(IntegralFlags, Reg)
///
class SLANG_EXPORT Type : public Symbol {
public:
/// The maximum size in bits of any fixed size type.
static constexpr uint32_t MaxBitWidth = uint32_t(INT32_MAX);

/// Gets the canonical type for this type, which involves unwrapping any type aliases.
const Type& getCanonicalType() const {
if (!canonical)
Expand All @@ -59,7 +62,7 @@ class SLANG_EXPORT Type : public Symbol {
bitwidth_t getBitWidth() const;

/// Gets $bits of the type. Returns zero if the type does not have a statically known size.
size_t bitstreamWidth() const;
uint32_t getBitstreamWidth() const;

/// Gets the "selectable" width of the type. This is the size of the object when determining
/// whether assignments to the static portions overlap with each other. Dynamically sized
Expand Down
6 changes: 0 additions & 6 deletions include/slang/util/CommandLine.h
Original file line number Diff line number Diff line change
Expand Up @@ -290,12 +290,6 @@ class SLANG_EXPORT CommandLine {
/// @return true on success, false if an errors occurs.
bool parse(int argc, const char* const argv[]);

#if defined(_WIN32)
/// Parse the provided command line (MSVC wchar-style).
/// @return true on success, false if an errors occurs.
bool parse(int argc, const wchar_t* const argv[]);
#endif

/// Contains various options to control parsing of command flags.
struct ParseOptions {
/// If set to true, comments will be parsed and ignored.
Expand Down
6 changes: 6 additions & 0 deletions include/slang/util/OS.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ namespace slang {
/// A collection of various OS-specific utility functions.
class SLANG_EXPORT OS {
public:
/// Does initial one-time setup of OS console.
static void setupConsole();

/// Tries to enable color output support for stdout and stderr.
/// @return true if successful and false otherwise.
static bool tryEnableColors();
Expand All @@ -39,6 +42,9 @@ class SLANG_EXPORT OS {
/// Note that the buffer will be null-terminated.
static std::error_code readFile(const std::filesystem::path& path, SmallVector<char>& buffer);

/// Writes the given contents to the specified file.
static void writeFile(const std::filesystem::path& path, std::string_view contents);

/// Prints text to stdout.
static void print(std::string_view text);

Expand Down
2 changes: 1 addition & 1 deletion include/slang/util/SmallVector.h
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ class SmallVectorBase {

/// Indicates whether we are still "small", which means we are still on the stack.
[[nodiscard]] constexpr bool isSmall() const noexcept {
return (void*)data_ == (void*)firstElement;
return (const void*)data_ == (const void*)firstElement;
}

protected:
Expand Down
24 changes: 7 additions & 17 deletions include/slang/util/String.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,28 +75,18 @@ inline char charToLower(char c) {
int editDistance(std::string_view left, std::string_view right, bool allowReplacements = true,
int maxDistance = 0);

/// C++20 is dumb and provides no way to get a std::string with the UTF-8
/// contents of a fs::path, so we have to use this method to copy the chars :(
std::string getU8Str(const std::filesystem::path& path);

#if defined(_WIN32)

/// Widens the provided UTF8 string into UTF16 wchars.
SLANG_EXPORT std::wstring widen(std::string_view str);

/// Narrows the provided UTF16 string into UTF8.
SLANG_EXPORT std::string narrow(std::wstring_view str);
/// Gets a string representation of the given path, in UTF-8 encoding.
inline std::string getU8Str(const std::filesystem::path& path) {
return path.string();
}

#else

/// Widens the provided UTF8 string into UTF16 wchars.
inline std::string_view widen(std::string_view str) {
return str;
}

/// Narrows the provided UTF16 string into UTF8.
inline std::string_view narrow(std::string_view str) {
return str;
/// Gets a string representation of the given path, in UTF-8 encoding.
inline const std::string& getU8Str(const std::filesystem::path& path) {
return path.native();
}

#endif
Expand Down
3 changes: 1 addition & 2 deletions scripts/diagnostics.txt
Original file line number Diff line number Diff line change
Expand Up @@ -793,7 +793,7 @@ error InvalidAssociativeIndexType "index type cannot be or contain floating poin
error PackedArrayNotIntegral "packed array elements must be of integral type (not {})"
error PackedTypeTooLarge "packed type of {} bits is too large; maximum width is {} bits"
error ArrayDimTooLarge "array dimension of {} is too large; maximum number of elements is {}"
error ObjectTooLarge "object size of {} bits exceeds implementation limit of {} bits"
error ObjectTooLarge "object size exceeds implementation limit of {} bits"
error InvalidUnionMember "untagged unions cannot have members of type {}"
error VirtualInterfaceUnionMember "virtual interfaces cannot be used as members of unions"
error InvalidArrayElemType "cannot declare an array with elements of type {}"
Expand Down Expand Up @@ -926,7 +926,6 @@ subsystem ConstEval
note NoteInCallTo "in call to '{}'"
note NoteSkippingFrames "(skipping {} calls in backtrace; use --constexpr-backtrace-limit=0 to see all)"
error ConstEvalNonConstVariable "reference to non-constant variable '{}' is not allowed in a constant expression"
error ConstEvalBitsNotFixedSize "$bits argument type {} is not fixed-size in a constant expression"
error ConstEvalBitstreamCastSize "bit-stream casting argument size {} cannot fit casting type {} in a constant expression"
error ConstEvalReplicationCountInvalid "string replication count {} is invalid in a constant expression"
error ConstEvalHierarchicalName "reference to '{}' by hierarchical name is not allowed in a constant expression"
Expand Down
9 changes: 9 additions & 0 deletions scripts/win32.manifest
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<assembly manifestVersion="1.0" xmlns="urn:schemas-microsoft-com:asm.v1">
<assemblyIdentity type="win32" name="..." version="6.0.0.0"/>
<application>
<windowsSettings>
<activeCodePage xmlns="http://schemas.microsoft.com/SMI/2019/WindowsSettings">UTF-8</activeCodePage>
</windowsSettings>
</application>
</assembly>
30 changes: 15 additions & 15 deletions source/ast/Bitstream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ static std::pair<size_t, size_t> dynamicBitstreamSize(const Type& type, Bitstrea
static std::pair<size_t, size_t> dynamicBitstreamSize(
const StreamingConcatenationExpression& concat, BitstreamSizeMode mode) {
if (concat.isFixedSize())
return {0, concat.bitstreamWidth()};
return {0, concat.getBitstreamWidth()};

size_t multiplier = 0, fixedSize = 0;
for (auto& stream : concat.streams()) {
Expand All @@ -117,7 +117,6 @@ static std::pair<size_t, size_t> dynamicBitstreamSize(
}
else {
multiplierStream = fixedSizeElem;
fixedSizeStream = fixedSizeElem;
}
}
else {
Expand Down Expand Up @@ -197,7 +196,7 @@ bool Bitstream::dynamicSizesMatch(const T1& destination, const T2& source) {
template<typename T>
static size_t bitstreamCastRemainingSize(const T& destination, size_t srcSize) {
if (destination.isFixedSize()) {
auto destSize = destination.bitstreamWidth();
auto destSize = destination.getBitstreamWidth();
if (destSize != srcSize)
return srcSize + 1; // cannot fit

Expand Down Expand Up @@ -375,7 +374,7 @@ static ConstantValue unpackBitstream(const Type& type, PackIterator& iter,
// empty.
if (dynamicSize > 0) {
auto elemWidth = type.getArrayElementType()->isFixedSize()
? type.getArrayElementType()->bitstreamWidth()
? type.getArrayElementType()->getBitstreamWidth()
: dynamicSize;

// If element is dynamically sized, num = 1
Expand Down Expand Up @@ -428,7 +427,7 @@ ConstantValue Bitstream::evaluateCast(const Type& type, ConstantValue&& value,
}
}
else { // implicit streaming concatenation conversion
auto targetWidth = type.bitstreamWidth();
auto targetWidth = type.getBitstreamWidth();
if (targetWidth < srcSize) {
if (type.isFixedSize()) {
context.addDiag(diag::BadStreamSize, sourceRange) << targetWidth << srcSize;
Expand Down Expand Up @@ -488,20 +487,20 @@ bool Bitstream::canBeTarget(const StreamingConcatenationExpression& lhs, const E
if (context.inUnevaluatedBranch())
return true; // No size check in an unevaluated conditional branch

size_t targetWidth = lhs.bitstreamWidth();
size_t targetWidth = lhs.getBitstreamWidth();
size_t sourceWidth;
bool good = true;

if (rhs.kind != ExpressionKind::Streaming) {
if (!rhs.type->isFixedSize())
return true; // Sizes checked at constant evaluation or runtime

sourceWidth = rhs.type->bitstreamWidth();
sourceWidth = rhs.type->getBitstreamWidth();
good = targetWidth <= sourceWidth;
}
else {
auto& source = rhs.as<StreamingConcatenationExpression>();
sourceWidth = source.bitstreamWidth();
sourceWidth = source.getBitstreamWidth();
if (lhs.isFixedSize() && source.isFixedSize())
good = targetWidth == sourceWidth;
else
Expand Down Expand Up @@ -538,8 +537,8 @@ bool Bitstream::canBeSource(const Type& target, const StreamingConcatenationExpr
if (!target.isFixedSize())
return true; // Sizes checked at constant evaluation or runtime

auto targetWidth = target.bitstreamWidth();
auto sourceWidth = rhs.bitstreamWidth();
auto targetWidth = target.getBitstreamWidth();
auto sourceWidth = rhs.getBitstreamWidth();
if (targetWidth < sourceWidth) {
auto& diag = context.addDiag(diag::BadStreamSize, assignLoc) << targetWidth << sourceWidth;
diag << rhs.sourceRange;
Expand All @@ -554,7 +553,7 @@ bool Bitstream::isBitstreamCast(const Type& type, const StreamingConcatenationEx
return false;

if (type.isFixedSize() && arg.isFixedSize())
return type.bitstreamWidth() == arg.bitstreamWidth();
return type.getBitstreamWidth() == arg.getBitstreamWidth();

return dynamicSizesMatch(type, arg);
}
Expand Down Expand Up @@ -705,9 +704,9 @@ static bool unpackConcatenation(const StreamingConcatenationExpression& lhs, Pac
auto elemType = arrayType.getArrayElementType();
SLANG_ASSERT(elemType);

// TODO: overflow
auto withSize = elemType->getBitstreamWidth() * with.width();
if (dynamicSize > 0 && !stream.constantWithWidth) {
// TODO: overflow
auto withSize = elemType->bitstreamWidth() * with.width();
if (withSize >= dynamicSize)
dynamicSize = 0;
else
Expand All @@ -721,7 +720,8 @@ static bool unpackConcatenation(const StreamingConcatenationExpression& lhs, Pac
// We already checked for overflow earlier so it's fine to create this
// temporary array result type as-is.
FixedSizeUnpackedArrayType rvalueType(
*elemType, with, elemType->getSelectableWidth() * with.width());
*elemType, with, elemType->getSelectableWidth() * with.width(),
(uint32_t)withSize);

rvalue = unpackBitstream(rvalueType, iter, iterEnd, bitOffset, dynamicSize);
}
Expand Down Expand Up @@ -777,7 +777,7 @@ ConstantValue Bitstream::evaluateTarget(const StreamingConcatenationExpression&
return nullptr;

auto srcSize = rvalue.bitstreamWidth();
auto targetWidth = lhs.bitstreamWidth();
auto targetWidth = lhs.getBitstreamWidth();
size_t dynamicSize = 0;

if (rhs.kind == ExpressionKind::Streaming) {
Expand Down
3 changes: 2 additions & 1 deletion source/ast/ElabVisitors.h
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ struct DiagnosticVisitor : public ASTVisitor<DiagnosticVisitor, false, false> {
return;

symbol.getBaseConstructorCall();
symbol.getBitstreamWidth();
}

void handle(const CovergroupType& symbol) {
Expand Down Expand Up @@ -385,7 +386,7 @@ struct DiagnosticVisitor : public ASTVisitor<DiagnosticVisitor, false, false> {
// Once everything has been visited, go back over and check things that might
// have been influenced by visiting later symbols. Unfortunately visiting
// a specialization can trigger more specializations to be made for the
// same or other generic classs, so we need to be careful here when iterating.
// same or other generic class, so we need to be careful here when iterating.
SmallSet<const Type*, 8> visitedSpecs;
SmallVector<const Type*> toVisit;
bool didSomething;
Expand Down
Loading

0 comments on commit 9aa4199

Please sign in to comment.