From 65dc21f5877fb4bb5f6378fb7be88005ece5cdfa Mon Sep 17 00:00:00 2001 From: Thorsten de Buhr Date: Thu, 15 Oct 2026 15:33:00 +0200 Subject: [PATCH 01/26] fix(svdconv): guard bit masks and bound dimension expansion --- tools/svdconv/SVDConv/README.md | 9 + tools/svdconv/SVDConv/src/SVDConv.cpp | 13 +- tools/svdconv/SVDConv/src/SVDConv_Msgs.cpp | 3 + .../svdconv/SVDGenerator/include/HeaderData.h | 2 +- .../SVDGenerator/include/HeaderGenerator.h | 2 +- .../SVDGenerator/src/HeaderData_Field.cpp | 9 +- .../SVDGenerator/src/HeaderData_PosMask.cpp | 5 +- .../SVDGenerator/src/HeaderGenerator.cpp | 18 +- tools/svdconv/SVDGenerator/src/SfdData.cpp | 50 +- .../SVDGenerator/src/SfdData_SingleItems.cpp | 18 +- .../svdconv/SVDGenerator/src/SvdGenerator.cpp | 4 +- tools/svdconv/SVDModel/include/SvdDimension.h | 5 +- tools/svdconv/SVDModel/include/SvdItem.h | 7 + tools/svdconv/SVDModel/include/SvdUtils.h | 2 + tools/svdconv/SVDModel/src/SvdCExpression.cpp | 11 +- tools/svdconv/SVDModel/src/SvdCluster.cpp | 14 +- tools/svdconv/SVDModel/src/SvdDerivedFrom.cpp | 3 +- tools/svdconv/SVDModel/src/SvdDimension.cpp | 86 +++- tools/svdconv/SVDModel/src/SvdField.cpp | 26 +- tools/svdconv/SVDModel/src/SvdInterrupt.cpp | 20 +- tools/svdconv/SVDModel/src/SvdItem.cpp | 70 ++- tools/svdconv/SVDModel/src/SvdModel.cpp | 1 + tools/svdconv/SVDModel/src/SvdPeripheral.cpp | 20 +- tools/svdconv/SVDModel/src/SvdRegister.cpp | 79 ++- tools/svdconv/SVDModel/src/SvdUtils.cpp | 10 + .../test/integtests/src/SvdConvIntegTests.cpp | 82 +++ tools/svdconv/test/unittests/CMakeLists.txt | 2 + .../test/unittests/src/SvdBitWidthTest.cpp | 465 ++++++++++++++++++ .../unittests/src/SvdDimensionBoundsTest.cpp | 254 ++++++++++ 29 files changed, 1199 insertions(+), 91 deletions(-) create mode 100644 tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp create mode 100644 tools/svdconv/test/unittests/src/SvdDimensionBoundsTest.cpp diff --git a/tools/svdconv/SVDConv/README.md b/tools/svdconv/SVDConv/README.md index 3ad89c2db..4e62beeae 100644 --- a/tools/svdconv/SVDConv/README.md +++ b/tools/svdconv/SVDConv/README.md @@ -48,6 +48,13 @@ due to backward compatibility reasons. All newly developed/updated SVD files sho in the same directory as `svdconv.exe`. On Linux and macOS, SVDConv reports error M133 and returns exit code 2. The intermediate `.sfd` file may still be generated. Use `--generate=sfd` to generate SFD output on any supported platform. +SFD/SFR output supports register and field ranges within 32 bits. Wider ranges report M226 before SFD file creation; +SFR compilation is skipped when SFD generation fails. Header masks support valid ranges up to 64 bits. + +Dimension expansion has a cumulative limit of 100,000 generated indices and model copies per input file. +Nested arrays, inherited content, and repeated expansion share this limit. Exceeding it reports M249 and stops +processing that input before any output generator runs. This bounds expansion work, not the XML file size or total +process memory. Invalid dimension index ranges and overflowing expansion addresses report M250. ## Return Codes @@ -287,6 +294,8 @@ The following table shows the errors and warnings issued by svdconv. | M241 | ERROR | Parse Error: 'VALUE' invalid for Array generation | Correct SVD.| | M242 | WARNING | 'LEVEL' 'NAME' `` found, but no `` | Correct SVD.| | M243 | WARNING | 'LEVEL' 'NAME' `` found, but `` does not describe an array | Correct SVD.| +| M249 | ERROR | Dimension expansion exceeds the per-input limit of 'NUM' generated indices and model copies. | Reduce the total expanded model size.| +| M250 | ERROR | Dimension 'NAME' has an invalid or overflowing index/address range. | Correct the dimension range or increment.| ### Data Check Errors diff --git a/tools/svdconv/SVDConv/src/SVDConv.cpp b/tools/svdconv/SVDConv/src/SVDConv.cpp index af71e11de..188b315a2 100644 --- a/tools/svdconv/SVDConv/src/SVDConv.cpp +++ b/tools/svdconv/SVDConv/src/SVDConv.cpp @@ -253,6 +253,13 @@ SVD_ERR SvdConv::CheckSvdFile() if(success) { LogMsg("M040", NAME("Deleting XML Tree"), TIME(t2)); } else { LogMsg("M111", NAME("Deleting XML Tree")); } + // A partial model from an exhausted expansion budget must never reach a generator. + if(m_svdModel->HasExpansionError()) { + delete m_svdModel; + m_svdModel = nullptr; + return SVD_ERR_VERIFY; + } + // ---------------------- Calculate Model ---------------------- t1 = CrossPlatformUtils::ClockInMsec(); success = m_svdModel->CalculateModel(); @@ -332,13 +339,15 @@ SVD_ERR SvdConv::CheckSvdFile() } // ---------------------- Generate SFD File ---------------------- + bool sfdGenerated = false; if(m_svdOptions.IsGenerateSfd()) { t1 = CrossPlatformUtils::ClockInMsec(); if(device) { generator->SetSvdFileName(path); generator->SetProgramInfo(version, descr, copyright); - success = generator->SfdFile(device, outDir); + sfdGenerated = generator->SfdFile(device, outDir); } + success = sfdGenerated; t2 = CrossPlatformUtils::ClockInMsec() - t1; if(success) { LogMsg("M040", NAME("Generate System Viewer SFD File"), TIME(t2)); } @@ -346,7 +355,7 @@ SVD_ERR SvdConv::CheckSvdFile() } // ---------------------- Generate SFR File ---------------------- - if(m_svdOptions.IsGenerateSfr()) { + if(m_svdOptions.IsGenerateSfr() && sfdGenerated) { t1 = CrossPlatformUtils::ClockInMsec(); if(device) { generator->SetSvdFileName(path); diff --git a/tools/svdconv/SVDConv/src/SVDConv_Msgs.cpp b/tools/svdconv/SVDConv/src/SVDConv_Msgs.cpp index 7344d361e..78fafc55e 100644 --- a/tools/svdconv/SVDConv/src/SVDConv_Msgs.cpp +++ b/tools/svdconv/SVDConv/src/SVDConv_Msgs.cpp @@ -114,6 +114,9 @@ const MsgTable SvdConv::msgTable = { { "M247", { MsgLevel::LEVEL_ERROR, CRLF_B, "C Expression: Error occurred during generation!" } }, { "M248", { MsgLevel::LEVEL_ERROR, CRLF_B, "C Expression: Referenced Object must be Register or Field!" } }, + { "M249", { MsgLevel::LEVEL_ERROR, CRLF_B, "Dimension expansion exceeds the per-input limit of %NUM% generated indices and model copies." } }, + { "M250", { MsgLevel::LEVEL_ERROR, CRLF_B, "Dimension '%NAME%' has an invalid or overflowing index/address range." } }, + // 300... Data Check Errors { "M300", { MsgLevel::LEVEL_ERROR, CRLF_B, "" } }, diff --git a/tools/svdconv/SVDGenerator/include/HeaderData.h b/tools/svdconv/SVDGenerator/include/HeaderData.h index ea5702149..4ec541726 100644 --- a/tools/svdconv/SVDGenerator/include/HeaderData.h +++ b/tools/svdconv/SVDGenerator/include/HeaderData.h @@ -19,7 +19,7 @@ #include struct ONESTRUCT { - uint32_t mask; + uint64_t mask; std::map fields; ONESTRUCT() : mask(0) {} diff --git a/tools/svdconv/SVDGenerator/include/HeaderGenerator.h b/tools/svdconv/SVDGenerator/include/HeaderGenerator.h index 9f3afcef1..9a969cd60 100644 --- a/tools/svdconv/SVDGenerator/include/HeaderGenerator.h +++ b/tools/svdconv/SVDGenerator/include/HeaderGenerator.h @@ -74,7 +74,7 @@ class HeaderGenerator : public CodeGenerator { void MakeEnumStructUnionEndArray (const std::string& textBuf, c_header::Index elementIndex, c_header::Special specialType, c_header::Additional langAdditional, uint32_t num); void MakeFieldPosMask (const std::string& field, const std::string& peri, const std::string& reg, uint32_t pos, uint32_t mask); void MakeFieldPosMask2 (const std::string& field, const std::string& peri, const std::string& reg, uint32_t pos, uint32_t mask); - void MakeFieldPosMask3 (const std::string& name, const std::string& field, uint32_t pos, uint32_t mask); + void MakeFieldPosMask3 (const std::string& name, const std::string& field, uint32_t pos, uint64_t mask); void CreateDoxyComment (const std::string& str); void CreateDoxyCommentStar (const std::string& str); void CreateDoxyCommentLineBreak (const std::string& str); diff --git a/tools/svdconv/SVDGenerator/src/HeaderData_Field.cpp b/tools/svdconv/SVDGenerator/src/HeaderData_Field.cpp index 473a62870..64e6322ed 100644 --- a/tools/svdconv/SVDGenerator/src/HeaderData_Field.cpp +++ b/tools/svdconv/SVDGenerator/src/HeaderData_Field.cpp @@ -75,9 +75,12 @@ bool HeaderData::AddFields(SvdItem* container, FIELDMAPLIST& sortedFields) } const auto name = field->GetNameCalculated(); - uint32_t offs = (uint32_t) field->GetOffset(); + const auto offs = field->GetOffset(); uint32_t width = (uint32_t) field->GetBitWidth(); - uint32_t mask = ((1<mask |= mask; - oneStruct->fields[offs] = field; + oneStruct->fields[static_cast(offs)] = field; } return true; diff --git a/tools/svdconv/SVDGenerator/src/HeaderData_PosMask.cpp b/tools/svdconv/SVDGenerator/src/HeaderData_PosMask.cpp index 5f27e8444..8a721f496 100644 --- a/tools/svdconv/SVDGenerator/src/HeaderData_PosMask.cpp +++ b/tools/svdconv/SVDGenerator/src/HeaderData_PosMask.cpp @@ -220,7 +220,10 @@ bool HeaderData::CreateFieldPosMask(SvdField* field, PosMaskNames *posMaskNames) const auto fieldName = field->GetHierarchicalNameResulting(); uint32_t firstBit = (uint32_t)field->GetOffset(); uint32_t bitWidth = field->GetEffectiveBitWidth(); - uint32_t bitMaxNum = (uint32_t) ((((uint64_t)(1) << bitWidth) -1)); + const auto bitMaxNum = SvdUtils::CreateBitMask(bitWidth); + if(!SvdUtils::CreateBitMask(bitWidth, field->GetOffset())) { + return false; + } if(!alternateGroup.empty()) m_gen->Generate("%s_%s", name.c_str(), firstBit, bitMaxNum, fieldName.c_str(), alternateGroup.c_str()); diff --git a/tools/svdconv/SVDGenerator/src/HeaderGenerator.cpp b/tools/svdconv/SVDGenerator/src/HeaderGenerator.cpp index 172a2ee9a..7fafa5e86 100644 --- a/tools/svdconv/SVDGenerator/src/HeaderGenerator.cpp +++ b/tools/svdconv/SVDGenerator/src/HeaderGenerator.cpp @@ -804,9 +804,12 @@ void HeaderGenerator::MakeFieldPosMask2(const std::string& field, const std::str } } -void HeaderGenerator::MakeFieldPosMask3(const std::string& pName, const std::string& field, uint32_t pos, uint32_t mask) +void HeaderGenerator::MakeFieldPosMask3(const std::string& pName, const std::string& field, uint32_t pos, uint64_t mask) { - uint32_t calcMask; + if(pos >= 64 || mask > (UINT64_MAX >> pos)) { + LogMsg("M220", MSG("Field mask exceeds 64 bits")); + return; + } Generate("#define %s%s ", pName, m_definePosStr); GenerateH_CRTabOffset(HEADER_POSMAK_VAL_OFFSET); @@ -818,14 +821,19 @@ void HeaderGenerator::MakeFieldPosMask3(const std::string& pName, const std::str GenerateH_CRTabOffset(HEADER_POSMAK_COMMENTEND_OFFSET2); Generate(""); - calcMask = mask << pos; + const auto calcMask = mask << pos; Generate("#define %s%s ", pName, m_defineMaskStr); GenerateH_CRTabOffset(HEADER_POSMAK_VAL_OFFSET); - Generate("(0x%xUL) ", calcMask); + if(calcMask <= UINT32_MAX) { + Generate("(0x%xUL) ", static_cast(calcMask)); + } + else { + Generate("(0x%llxULL) ", static_cast(calcMask)); + } // Doxy Comment GenerateH_CRTabOffset(HEADER_POSMAK_COMMENT_OFFSET2); - Generate("%s (Bitfield-Mask: 0x%02x)", field, mask); + Generate("%s (Bitfield-Mask: 0x%02llx)", field, static_cast(mask)); GenerateH_CRTabOffset(HEADER_POSMAK_COMMENTEND_OFFSET2); Generate(""); diff --git a/tools/svdconv/SVDGenerator/src/SfdData.cpp b/tools/svdconv/SVDGenerator/src/SfdData.cpp index 92f49690b..d3eab5cf9 100644 --- a/tools/svdconv/SVDGenerator/src/SfdData.cpp +++ b/tools/svdconv/SVDGenerator/src/SfdData.cpp @@ -10,9 +10,44 @@ #include "SvdItem.h" #include "FileIo.h" #include "SvdDevice.h" +#include "SvdDimension.h" +#include "SvdField.h" +#include "SvdRegister.h" +#include "SvdUtils.h" +#include "ErrLog.h" using namespace std; +namespace { + +bool CheckSfdBitRanges(SvdItem* item) +{ + if(!item->IsValid()) { + return true; + } + + const auto reg = dynamic_cast(item); + const auto field = dynamic_cast(item); + if(reg || field) { + const auto width = item->GetEffectiveBitWidth(); + const auto mask = SvdUtils::CreateBitMask(width, field ? field->GetOffset() : 0); + if(!mask || mask > UINT32_MAX) { + LogMsg("M226", MSG("SFD supports only bit ranges within 32 bits: '" + item->GetNameCalculated() + "'"), item->GetLineNumber()); + return false; + } + } + + for(const auto child : item->GetChildren()) { + if(!CheckSfdBitRanges(child)) { + return false; + } + } + const auto dim = item->GetDimension(); + return !dim || CheckSfdBitRanges(dim); +} + +} // namespace + SfdData::SfdData(const FileHeaderInfo &fileHeaderInfo, const SvdOptions& options): m_options(options), @@ -45,12 +80,21 @@ bool SfdData::Create(SvdItem *item, const string &fileName) return false; } - m_fileIo->Create(fileName); - const auto device = dynamic_cast(item); - if(!device) { + if(!device || !CheckSfdBitRanges(device)) { return false; } + for(const auto& [name, reg] : device->GetExpressionRegistersList()) { + if(!reg || !reg->IsValid()) { + LogMsg("M226", MSG("Invalid register referenced by an SFD expression: '" + name + "'")); + return false; + } + if(!CheckSfdBitRanges(reg)) { + return false; + } + } + + m_fileIo->Create(fileName); CreateDevice(device); diff --git a/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp b/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp index 68481d1d0..9ef9de2a3 100644 --- a/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp +++ b/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp @@ -873,10 +873,16 @@ bool SfdData::CreateFieldItem(SvdField* field) const auto dispName = field->GetDisplayNameCalculated(); const auto fieldDescr = SvdUtils::CheckTextGeneric_SfrCC2(field->GetDescriptionCalculated()); const auto regName = field->GetParentRegisterNameHierarchical(); - const auto firstBit = (uint32_t)field->GetOffset(); + const auto offset = field->GetOffset(); const auto bitWidth = (uint32_t)field->GetEffectiveBitWidth(); const auto accessType = field->GetEffectiveAccess(); const auto regAddress = (uint32_t)field->GetAbsoluteAddress(); + const auto fieldMask = SvdUtils::CreateBitMask(bitWidth, offset); + if(!fieldMask || fieldMask > UINT32_MAX) { + LogMsg("M226", MSG("SFD supports only bit ranges within 32 bits: '" + fieldName + "'"), field->GetLineNumber()); + return false; + } + const auto firstBit = static_cast(offset); uint32_t regBitWidth = 32; const auto parent = field->GetParent(); @@ -895,7 +901,7 @@ bool SfdData::CreateFieldItem(SvdField* field) valuesDescr = SvdUtils::CheckTextGeneric_SfrCC2(valuesDescr); uint32_t lastBit = firstBit + bitWidth -1; - uint32_t bitMaxNum = (uint32_t) (((uint64_t)(1) << bitWidth) -1); + const auto bitMaxNum = static_cast(SvdUtils::CreateBitMask(bitWidth)); CreateItemDescription(field, "Item"); m_gen->Generate("SFDITEM_FIELD__%s", itemName.c_str()); @@ -1006,6 +1012,11 @@ bool SfdData::CreateEnumValues(const list& conts, SvdField* f bool SfdData::CreateEnumValueSet(SvdEnumContainer *cont, SvdField* field) { + const auto bitWidth = field->GetEffectiveBitWidth(); + if(bitWidth == 0 || bitWidth > MAX_BITWIDTH_FOR_COMBO) { + return false; + } + const auto& childs = cont->GetChildren(); if(childs.empty()) { return true; @@ -1028,8 +1039,7 @@ bool SfdData::CreateEnumValueSet(SvdEnumContainer *cont, SvdField* field) enumValues[val.u32] = enu; } - const auto bitWidth = field->GetEffectiveBitWidth(); - const auto bitMaxNum = (uint32_t)((1UL << (uint64_t)bitWidth)); + const auto bitMaxNum = uint32_t{1} << bitWidth; for(uint32_t bitNum=0; bitNumCreate(device, fileName); + const bool success = sfdData->Create(device, fileName); delete sfdData; - return true; + return success; } bool SvdGenerator::SfrFile(SvdDevice *device, const string &path) diff --git a/tools/svdconv/SVDModel/include/SvdDimension.h b/tools/svdconv/SVDModel/include/SvdDimension.h index f4e13eb67..a5619ba7a 100644 --- a/tools/svdconv/SVDModel/include/SvdDimension.h +++ b/tools/svdconv/SVDModel/include/SvdDimension.h @@ -95,8 +95,9 @@ class SvdDimension : public SvdItem bool SetDimIndexList (const std::list& dimIndexList) { m_dimIndexList = dimIndexList; return true; } bool SetDimName (const std::string& dimName ) { m_dimName = dimName ; return true; } - int32_t GetAddressBitsUnits (); - int32_t CalcAddressIncrement (); + uint32_t GetAddressBitsUnits (); + uint64_t CalcAddressIncrement (); + bool CheckExpansionRange (uint64_t start, uint64_t increment); bool IsTagAllowed (const std::string& tag); bool InitAllowedTags (); diff --git a/tools/svdconv/SVDModel/include/SvdItem.h b/tools/svdconv/SVDModel/include/SvdItem.h index 1e026383a..eb1ecf284 100644 --- a/tools/svdconv/SVDModel/include/SvdItem.h +++ b/tools/svdconv/SVDModel/include/SvdItem.h @@ -121,6 +121,11 @@ class SvdItem : public SvdElement { static const uint32_t VALUE32_NOT_INIT; static const uint64_t VALUE64_NOT_INIT; + // Cumulative expansion work per input tree, including indices and copied items. + static constexpr size_t MAX_EXPANSION_ITEMS = 100000; + bool ReserveExpansion (size_t count); + bool HasExpansionError () const; + virtual bool Construct (XMLTreeElement* xmlElement); virtual bool ProcessXmlChildren (XMLTreeElement* xmlElement); @@ -249,6 +254,8 @@ class SvdItem : public SvdElement { std::string m_description; std::map m_attributes; + size_t m_expansionItems = 0; + bool m_expansionError = false; }; diff --git a/tools/svdconv/SVDModel/include/SvdUtils.h b/tools/svdconv/SVDModel/include/SvdUtils.h index ad243bb7e..76dd3e3e8 100644 --- a/tools/svdconv/SVDModel/include/SvdUtils.h +++ b/tools/svdconv/SVDModel/include/SvdUtils.h @@ -56,6 +56,8 @@ class SvdUtils { static bool ConvertNumberXBin (const std::string &text, std::set& numbers); static bool ConvertBitRange (const std::string &text, uint32_t& m_offset, uint32_t& m_size); + // Returns zero when the bit range is empty or does not fit in 64 bits. + static uint64_t CreateBitMask (uint32_t width, uint64_t offset = 0); static bool DoConvertNumberXBin (const std::string &text, uint32_t value, std::set& numbers); static bool ConvertCpuRevision (const std::string &text, uint32_t& rev); static bool ConvertProtectionStringType (const std::string &text, SvdTypes::ProtectionType& protection, uint32_t lineNo); diff --git a/tools/svdconv/SVDModel/src/SvdCExpression.cpp b/tools/svdconv/SVDModel/src/SvdCExpression.cpp index 3ec913b68..909ff4a9a 100644 --- a/tools/svdconv/SVDModel/src/SvdCExpression.cpp +++ b/tools/svdconv/SVDModel/src/SvdCExpression.cpp @@ -174,12 +174,19 @@ string SvdCExpression::CreateFieldExpression(SvdItem* item) } const auto regName = field->GetParentRegisterNameHierarchical(); - const auto firstBit = (uint32_t)field->GetOffset(); + const auto firstBit = field->GetOffset(); const auto bitWidth = field->GetEffectiveBitWidth(); - const auto bitMaxNum = (uint32_t) ((((uint64_t)(1) << bitWidth) -1)); + const auto fieldMask = SvdUtils::CreateBitMask(bitWidth, firstBit); + if(!field->IsValid() || !fieldMask) { + return SvdUtils::EMPTY_STRING; + } + const auto bitMaxNum = SvdUtils::CreateBitMask(bitWidth); std::stringstream ss; ss << "0x" << std::hex << bitMaxNum; + if(fieldMask > UINT32_MAX) { + ss << "ULL"; + } string expression; expression += "("; diff --git a/tools/svdconv/SVDModel/src/SvdCluster.cpp b/tools/svdconv/SVDModel/src/SvdCluster.cpp index 3b5cefd36..25d4564d8 100644 --- a/tools/svdconv/SVDModel/src/SvdCluster.cpp +++ b/tools/svdconv/SVDModel/src/SvdCluster.cpp @@ -279,7 +279,9 @@ bool SvdCluster::CalculateDim() dim->ClearChildren(); } - dim->CalculateDim(); + if(!dim->CalculateDim() || !dim->CheckExpansionRange(GetOffset(), dim->GetDimIncrement())) { + return false; + } const auto& dimIndexList = dim->GetDimIndexList(); auto offset = GetOffset(); const auto bitWidth = GetBitWidth(); @@ -287,10 +289,14 @@ bool SvdCluster::CalculateDim() uint32_t dimElementIndex = 0; for(const auto& dimIndexname : dimIndexList) { + if(!dim->ReserveExpansion(1)) { + return false; + } const auto newClust = new SvdCluster(dim); dim->AddItem(newClust); - CopyChilds(this, newClust); + if(!CopyChilds(this, newClust)) { return false; } newClust->CopyItem(this); + if(HasExpansionError()) { return false; } newClust->SetName(dim->CreateName(dimIndexname)); newClust->SetDisplayName(dim->CreateDisplayName(dimIndexname)); newClust->SetDescription(dim->CreateDescription(dimIndexname)); @@ -298,7 +304,9 @@ bool SvdCluster::CalculateDim() newClust->SetBitWidth(bitWidth); newClust->SetDimElementIndex(dimElementIndex++); - offset += dim->GetDimIncrement(); + if(dimElementIndex < dimIndexList.size()) { + offset += dim->GetDimIncrement(); + } if(dimElementIndex < 8) { if(!dimIndexText.empty()) { diff --git a/tools/svdconv/SVDModel/src/SvdDerivedFrom.cpp b/tools/svdconv/SVDModel/src/SvdDerivedFrom.cpp index d02e73b2d..ac3829af6 100644 --- a/tools/svdconv/SVDModel/src/SvdDerivedFrom.cpp +++ b/tools/svdconv/SVDModel/src/SvdDerivedFrom.cpp @@ -116,10 +116,11 @@ bool SvdDerivedFrom::DeriveItem(SvdItem *from) SetDerivedFromItem(from); // Also copy all element's childs - CopyChilds(from, parent); + if(!CopyChilds(from, parent)) { return false; } // Create a copy parent->CopyItem(from); + if(HasExpansionError()) { return false; } const auto dim = parent->GetDimension(); if(dim) { diff --git a/tools/svdconv/SVDModel/src/SvdDimension.cpp b/tools/svdconv/SVDModel/src/SvdDimension.cpp index 684d51e71..eb4a329c8 100644 --- a/tools/svdconv/SVDModel/src/SvdDimension.cpp +++ b/tools/svdconv/SVDModel/src/SvdDimension.cpp @@ -10,6 +10,8 @@ #include "XMLTree.h" #include "ErrLog.h" +#include + using namespace std; map > SvdDimension::m_allowedTagsDim; @@ -180,7 +182,14 @@ bool SvdDimension::Construct(XMLTreeElement* xmlElement) bool SvdDimension::CalculateDimIndex() { - const auto lineNo = GetParent()->GetLineNumber(); + const auto parent = GetParent(); + const auto lineNo = parent ? parent->GetLineNumber() : GetLineNumber(); + if(m_dim == 0 || m_dim == SvdItem::VALUE32_NOT_INIT) { + return false; + } + if(m_dim > MAX_EXPANSION_ITEMS) { + return ReserveExpansion(m_dim); + } if(m_dim == 1) { const auto& name = GetExpression()->GetName(); @@ -188,6 +197,8 @@ bool SvdDimension::CalculateDimIndex() } m_dimIndexList.clear(); + m_from.clear(); + m_to.clear(); // Read the Value const auto& value = GetDimIndex(); @@ -201,7 +212,9 @@ bool SvdDimension::CalculateDimIndex() } m_to = value.substr(pos, value.length()); SvdUtils::TrimWhitespace(m_to); - CalculateDimIndexFromTo(); + if(!CalculateDimIndexFromTo()) { + return false; + } } else { string index = ""; @@ -211,6 +224,10 @@ bool SvdDimension::CalculateDimIndex() } if(val == ',') { if(!index.empty()) { + if(!ReserveExpansion(1)) { + m_dimIndexList.clear(); + return false; + } m_dimIndexList.push_back(index); } index = ""; @@ -220,6 +237,10 @@ bool SvdDimension::CalculateDimIndex() } } if(!index.empty()) { + if(!ReserveExpansion(1)) { + m_dimIndexList.clear(); + return false; + } m_dimIndexList.push_back(index); // last one } } @@ -246,7 +267,14 @@ bool SvdDimension::CalculateDimIndexFromTo() return true; } - int32_t numFrom=0, numTo=0; + if(m_dim == 0 || m_dim == SvdItem::VALUE32_NOT_INIT) { + return false; + } + if(m_dim > MAX_EXPANSION_ITEMS) { + return ReserveExpansion(m_dim); + } + + uint32_t numFrom=0, numTo=0; bool numOk = true; if(!m_from.empty()) { if(!SvdUtils::ConvertNumber(m_from, numFrom)) { @@ -271,14 +299,16 @@ bool SvdDimension::CalculateDimIndexFromTo() } if(numOk) { - int32_t len = numTo - numFrom +1; - if(len < 0 || (uint32_t)len != m_dim) { + const uint64_t len = uint64_t(numTo) - numFrom + 1; + if(numTo < numFrom || len != m_dim) { + return false; + } + if(!ReserveExpansion(m_dim)) { return false; } - for(int32_t i=numFrom; i<=numTo; i++) { - auto numStr = SvdUtils::CreateDecNum(i); - m_dimIndexList.push_back(numStr); + for(uint64_t i = 0; i < len; i++) { + m_dimIndexList.push_back(SvdUtils::CreateDecNum(uint64_t(numFrom) + i)); } } else { @@ -291,6 +321,9 @@ bool SvdDimension::CalculateDimIndexFromTo() return false; } + if(!ReserveExpansion(m_dim)) { + return false; + } char res[2] = { 0 }; for(int32_t i=0; iIsValid())) { + return false; + } + if(!CalculateDimIndex()) { + if(!HasExpansionError()) { + LogMsg("M250", NAME(parent ? parent->GetName() : GetName()), GetLineNumber()); + } + Invalidate(); + if(parent) { parent->Invalidate(); } + m_dimIndexList.clear(); + return false; + } + CheckItem(); + return IsValid() && (!parent || parent->IsValid()); +} - return true; +bool SvdDimension::CheckExpansionRange(uint64_t start, uint64_t increment) +{ + const auto count = m_dimIndexList.size(); + if(count > 1 && increment > (numeric_limits::max() - start) / (count - 1)) { + const auto parent = GetParent(); + LogMsg("M250", NAME(parent ? parent->GetName() : GetName()), GetLineNumber()); + Invalidate(); + if(parent) { parent->Invalidate(); } + return false; + } + return !HasExpansionError(); } bool SvdDimension::CalculateNameFromExpression() diff --git a/tools/svdconv/SVDModel/src/SvdField.cpp b/tools/svdconv/SVDModel/src/SvdField.cpp index a15894fea..5578f1722 100644 --- a/tools/svdconv/SVDModel/src/SvdField.cpp +++ b/tools/svdconv/SVDModel/src/SvdField.cpp @@ -205,7 +205,9 @@ bool SvdField::CalculateDim() dim->ClearChildren(); } - dim->CalculateDim(); + if(!dim->CalculateDim() || !dim->CheckExpansionRange(GetOffset(), dim->CalcAddressIncrement())) { + return false; + } const auto dimExpression = dim->GetExpression(); SvdTypes::Expression exprType = SvdTypes::Expression::NONE; @@ -228,10 +230,18 @@ bool SvdField::CalculateDim() string dimIndexText; for(const auto& index : dimIndexList) { + if(!dim->ReserveExpansion(1)) { + return false; + } const auto newField = new SvdField(dim); dim->AddItem(newField); - CopyChilds(this, newField); + if(!CopyChilds(this, newField)) { + return false; + } newField->CopyItem (this); + if(HasExpansionError()) { + return false; + } newField->SetName (dim->CreateName(index)); newField->SetDisplayName (dim->CreateDisplayName(index)); newField->SetDescription (dim->CreateDescription(index)); @@ -240,7 +250,9 @@ bool SvdField::CalculateDim() newField->CheckItem(); // TODO: Field dimIncrement is in bits; CalcAddressIncrement() scales it by addressUnitBits / 8. // With 16-bit address units, increment 1 incorrectly produces bit positions 0, 2, 4. - offset += dim->CalcAddressIncrement(); + if(dimElementIndex < dimIndexList.size()) { + offset += dim->CalcAddressIncrement(); + } if(dimElementIndex < 8) { if(!dimIndexText.empty()) dimIndexText += ","; @@ -495,7 +507,7 @@ bool SvdField::CheckItem() SetOffset(FIELD_MAX_OFFSET); Invalidate(); } - if(width > FIELD_MAX_BITWIDTH) { + if(width == 0 || width > FIELD_MAX_BITWIDTH) { LogMsg("M310", NAME(name), NUM(width), lineNo); SetBitWidth(FIELD_MAX_BITWIDTH); Invalidate(); @@ -505,6 +517,9 @@ bool SvdField::CheckItem() SetBitWidth(1); Invalidate(); } + if(!IsValid()) { + return true; + } auto parent = GetParent(); if(parent) { @@ -522,6 +537,7 @@ bool SvdField::CheckItem() if(width > (int64_t)regWidth || offs+width > regWidth) { LogMsg("M324", NAME(name), BITRANGE(offs+width-1, offs, true), NAME2(reg->GetName()), NUM(regWidth), LINE2(l), lineNo); Invalidate(); + return true; } } @@ -534,7 +550,7 @@ bool SvdField::CheckItem() } map enumContMap; - uint64_t fieldMaxVal = (1ULL << (uint64_t)width) -1; + const auto fieldMaxVal = SvdUtils::CreateBitMask(width); const auto& enumConts = GetEnumContainer(); if(enumConts.empty() && width < 6) { // message "please add enum values" diff --git a/tools/svdconv/SVDModel/src/SvdInterrupt.cpp b/tools/svdconv/SVDModel/src/SvdInterrupt.cpp index aadcb7528..f5eaa2b44 100644 --- a/tools/svdconv/SVDModel/src/SvdInterrupt.cpp +++ b/tools/svdconv/SVDModel/src/SvdInterrupt.cpp @@ -65,7 +65,7 @@ bool SvdInterrupt::CalculateDim() if(parent) { const auto parentDim = parent->GetDimension(); if(parentDim) { - CopyDim(this, parent); + if(!CopyDim(this, parent)) { return false; } dim = GetDimension(); dim->SetDimIncrement(1); } @@ -83,24 +83,36 @@ bool SvdInterrupt::CalculateDim() dim->ClearChildren(); } - dim->CalculateDim(); + if(!dim->CalculateDim()) { return false; } const auto& dimIndexList = dim->GetDimIndexList(); uint32_t value = GetValue(); uint32_t dimElementIndex = 0; string dimIndexText; + const auto increment = dim->GetDimIncrement(); + const auto count = dimIndexList.size(); + if(value == SvdItem::VALUE32_NOT_INIT || + (count > 1 && increment > (uint64_t(UINT32_MAX - 1) - value) / (count - 1))) { + LogMsg("M250", NAME(GetName()), GetLineNumber()); + Invalidate(); + return false; + } for(const auto& dimIndex : dimIndexList) { + if(!dim->ReserveExpansion(1)) { return false; } const auto newIrq = new SvdInterrupt(dim); dim->AddItem(newIrq); - CopyChilds(this, newIrq); + if(!CopyChilds(this, newIrq)) { return false; } newIrq->CopyItem (this); + if(HasExpansionError()) { return false; } newIrq->SetName (dim->CreateName(dimIndex)); newIrq->SetDescription (dim->CreateDescription(dimIndex)); newIrq->SetValue (value); newIrq->SetDimElementIndex (dimElementIndex++); - value += dim->GetDimIncrement(); + if(dimElementIndex < dimIndexList.size()) { + value += increment; + } } string description = dim->CreateDescription(""); diff --git a/tools/svdconv/SVDModel/src/SvdItem.cpp b/tools/svdconv/SVDModel/src/SvdItem.cpp index 380973615..ab8c7a65b 100644 --- a/tools/svdconv/SVDModel/src/SvdItem.cpp +++ b/tools/svdconv/SVDModel/src/SvdItem.cpp @@ -183,7 +183,13 @@ bool SvdItem::Construct(XMLTreeElement* xmlElement) bool success = ProcessXmlAttributes(xmlElement); success = ProcessXmlChildren(xmlElement); + if(HasExpansionError()) { + return false; + } CalculateDim(); + if(HasExpansionError()) { + return false; + } Calculate(); CheckItem(); @@ -199,6 +205,9 @@ bool SvdItem::ProcessXmlChildren(XMLTreeElement* xmlElement) // process children const auto& childs = xmlElement->GetChildren(); for(const auto child : childs) { + if(HasExpansionError()) { + return false; + } if(!ProcessXmlElement(child)) { return false; } @@ -207,6 +216,33 @@ bool SvdItem::ProcessXmlChildren(XMLTreeElement* xmlElement) return true; } +bool SvdItem::ReserveExpansion(size_t count) +{ + auto root = this; + while(root->m_parent) { + root = root->m_parent; + } + if(root->m_expansionError) { + return false; + } + if(count > MAX_EXPANSION_ITEMS - root->m_expansionItems) { + root->m_expansionError = true; + LogMsg("M249", NUM(MAX_EXPANSION_ITEMS), GetLineNumber()); + return false; + } + root->m_expansionItems += count; + return true; +} + +bool SvdItem::HasExpansionError() const +{ + auto root = this; + while(root->m_parent) { + root = root->m_parent; + } + return root->m_expansionError; +} + bool SvdItem::ProcessXmlElement(XMLTreeElement* xmlElement) { static uint32_t tagCnt=0; @@ -919,8 +955,9 @@ bool SvdItem::CopyItem(SvdItem *from) } SetTag (tag); - CopyDerivedFrom (this, from); - CopyDim (this, from); + if(!CopyDerivedFrom(this, from) || !CopyDim(this, from)) { + return false; + } SetCopiedFrom ( from); return true; @@ -940,11 +977,15 @@ bool SvdItem::CopyChilds(SvdItem *from, SvdItem *hook) const auto lv = copy->GetSvdLevel(); + if(!hook->ReserveExpansion(1)) { + return false; + } + // Container if(lv == L_EnumeratedValues) { // more containers can be set! const auto nItem = new SvdEnumContainer(hook); hook->AddItem(nItem); - CopyChilds(copy, nItem); + if(!CopyChilds(copy, nItem)) { return false; } nItem->CopyItem(copy); } else if(lv == L_Fields) { @@ -957,7 +998,7 @@ bool SvdItem::CopyChilds(SvdItem *from, SvdItem *hook) else { nItem = (SvdFieldContainer *)*(hook->GetChildren().begin()); } - CopyChilds(copy, nItem); + if(!CopyChilds(copy, nItem)) { return false; } } else if(lv == L_Registers) { SvdRegisterContainer *nItem; @@ -969,40 +1010,43 @@ bool SvdItem::CopyChilds(SvdItem *from, SvdItem *hook) else { nItem = (SvdRegisterContainer *)*(hook->GetChildren().begin()); } - CopyChilds(copy, nItem); + if(!CopyChilds(copy, nItem)) { return false; } } // Elements else if(lv == L_EnumeratedValue) { const auto nItem = new SvdEnum(hook); hook->AddItem(nItem); - CopyChilds(copy, nItem); + if(!CopyChilds(copy, nItem)) { return false; } nItem->CopyItem(copy); } else if(lv == L_Field) { const auto nItem = new SvdField(hook); hook->AddItem(nItem); - CopyChilds(copy, nItem); + if(!CopyChilds(copy, nItem)) { return false; } nItem->CopyItem(copy); } else if(lv == L_Register) { const auto nItem = new SvdRegister(hook); hook->AddItem(nItem); - CopyChilds(copy, nItem); + if(!CopyChilds(copy, nItem)) { return false; } nItem->CopyItem(copy); } else if(lv == L_Peripheral) { const auto nItem = new SvdPeripheral(hook); hook->AddItem(nItem); - CopyChilds(copy, nItem); + if(!CopyChilds(copy, nItem)) { return false; } nItem->CopyItem(copy); } else if(lv == L_Cluster) { const auto nItem = new SvdCluster(hook); hook->AddItem(nItem); - CopyChilds(copy, nItem); + if(!CopyChilds(copy, nItem)) { return false; } nItem->CopyItem(copy); } + if(hook->HasExpansionError()) { + return false; + } } return true; @@ -1018,6 +1062,9 @@ bool SvdItem::CopyDerivedFrom(SvdItem *item, SvdItem *from) if(copyDeriveFrom) { auto deriveFrom = item->GetDerivedFrom(); if(!deriveFrom) { + if(!item->ReserveExpansion(1)) { + return false; + } deriveFrom = new SvdDerivedFrom(item); item->SetDerivedFrom(deriveFrom); auto derivedFromItem = copyDeriveFrom->GetDerivedFromItem(); @@ -1050,6 +1097,9 @@ bool SvdItem::CopyDim(SvdItem *item, SvdItem *from) if(copyDim) { auto dim = item->GetDimension(); if(!dim) { + if(!item->ReserveExpansion(1)) { + return false; + } dim = new SvdDimension(item); item->SetDimension(dim); dim->CopyItem(copyDim); diff --git a/tools/svdconv/SVDModel/src/SvdModel.cpp b/tools/svdconv/SVDModel/src/SvdModel.cpp index f42f68902..bcc0b7e8c 100644 --- a/tools/svdconv/SVDModel/src/SvdModel.cpp +++ b/tools/svdconv/SVDModel/src/SvdModel.cpp @@ -52,6 +52,7 @@ bool SvdModel::Construct(XMLTreeElement* xmlTree) if(!ok) { delete m_device; + m_device = nullptr; success = false; } } diff --git a/tools/svdconv/SVDModel/src/SvdPeripheral.cpp b/tools/svdconv/SVDModel/src/SvdPeripheral.cpp index b91233b70..3bb1e2099 100644 --- a/tools/svdconv/SVDModel/src/SvdPeripheral.cpp +++ b/tools/svdconv/SVDModel/src/SvdPeripheral.cpp @@ -373,7 +373,7 @@ bool SvdPeripheral::CopyItem(SvdItem *from) if(resetMask == 0 ) { SetResetMask (pFrom->GetResetMask ()); } if(access == SvdTypes::Access::UNDEF) { SetAccess (pFrom->GetAccess ()); } - CopyAddressBlocks(pFrom); + if(!CopyAddressBlocks(pFrom) || HasExpansionError()) { return false; } SvdItem::CopyItem(pFrom); CalculateDim(); @@ -391,6 +391,7 @@ bool SvdPeripheral::CopyAddressBlocks(SvdPeripheral *from) continue; } + if(!ReserveExpansion(1)) { return false; } const auto addressBlock = new SvdAddressBlock(this); addressBlock->CopyItem(addrBlock); AddAddressBlock(addressBlock); @@ -433,6 +434,7 @@ bool SvdPeripheral::CopyDisableCondition(SvdCExpression* from) return true; } + if(!ReserveExpansion(1)) { return false; } m_disableCondition = new SvdCExpression(*from); return true; @@ -457,7 +459,9 @@ bool SvdPeripheral::CalculateDim() dim->ClearChildren(); } - dim->CalculateDim(); + if(!dim->CalculateDim() || !dim->CheckExpansionRange(GetAbsoluteAddress(), dim->GetDimIncrement())) { + return false; + } const auto& dimIndexList = dim->GetDimIndexList(); const auto parent = dynamic_cast(dim->GetParent()); @@ -468,18 +472,23 @@ bool SvdPeripheral::CalculateDim() auto address = parent->GetAbsoluteAddress(); uint32_t dimElementIndex = 0; for(const auto& dimIndexname : dimIndexList) { + if(!dim->ReserveExpansion(1)) { + return false; + } const auto newPeri = new SvdPeripheral(dim); dim->AddItem(newPeri); - CopyDerivedFrom(newPeri, this); - CopyChilds(parent, newPeri); + if(!CopyDerivedFrom(newPeri, this) || !CopyChilds(parent, newPeri)) { return false; } newPeri->CopyItem (parent); + if(HasExpansionError()) { return false; } newPeri->SetName (dim->CreateName(dimIndexname)); newPeri->SetDisplayName (dim->CreateDisplayName(dimIndexname)); newPeri->SetDescription (dim->CreateDescription(dimIndexname)); newPeri->SetDimElementIndex (dimElementIndex++); newPeri->SetAddress (address); - address += dim->GetDimIncrement(); + if(dimElementIndex < dimIndexList.size()) { + address += dim->GetDimIncrement(); + } } string dimIndexText; @@ -1276,6 +1285,7 @@ bool SvdPeripheral::SortAddressBlocks(map& addrBlock continue; // only use "register" addressBlocks } + if(!ReserveExpansion(1)) { return false; } const auto block = new SvdAddressBlock(this); block->CopyItem(addrBlock); diff --git a/tools/svdconv/SVDModel/src/SvdRegister.cpp b/tools/svdconv/SVDModel/src/SvdRegister.cpp index 70b1e6be4..cec992458 100644 --- a/tools/svdconv/SVDModel/src/SvdRegister.cpp +++ b/tools/svdconv/SVDModel/src/SvdRegister.cpp @@ -85,6 +85,9 @@ SvdRegister::~SvdRegister() bool SvdRegister::Construct(XMLTreeElement* xmlElement) { bool success = SvdItem::Construct(xmlElement); + if(!IsValid()) { + return success; + } const auto bitWidth = GetBitWidth(); if(bitWidth == (int32_t)SvdItem::VALUE32_NOT_INIT) { // default, derive from above levels @@ -236,7 +239,9 @@ bool SvdRegister::CalculateDim() dim->ClearChildren(); } - dim->CalculateDim(); + if(!dim->CalculateDim() || !dim->CheckExpansionRange(GetOffset(), dim->CalcAddressIncrement())) { + return false; + } const auto& dimIndexList = dim->GetDimIndexList(); auto offset = GetOffset(); @@ -244,11 +249,19 @@ bool SvdRegister::CalculateDim() string dimIndexText; for(const auto& dimIndex : dimIndexList) { + if(!dim->ReserveExpansion(1)) { + return false; + } const auto newReg = new SvdRegister(dim); dim->AddItem(newReg); - CopyChilds(this, newReg); + if(!CopyChilds(this, newReg)) { + return false; + } newReg->CopyItem (this); + if(HasExpansionError()) { + return false; + } newReg->SetName (dim->CreateName(dimIndex)); newReg->SetDisplayName (dim->CreateDisplayName(dimIndex)); newReg->SetDescription (dim->CreateDescription(dimIndex)); @@ -257,7 +270,9 @@ bool SvdRegister::CalculateDim() newReg->CheckItem (); // TODO: Register dimIncrement is already in bytes; CalcAddressIncrement() scales it again. // With 16-bit address units, a requested 4-byte stride incorrectly becomes 8 bytes. - offset += dim->CalcAddressIncrement(); + if(dimElementIndex < dimIndexList.size()) { + offset += dim->CalcAddressIncrement(); + } if(dimElementIndex < 8) { if(!dimIndexText.empty()) { @@ -384,18 +399,21 @@ SvdTypes::Access SvdRegister::GetAccessCalculated() } const auto regWidth = GetEffectiveBitWidth(); - const auto mask = ((uint64_t)1 << regWidth) -1; + const auto mask = SvdUtils::CreateBitMask(regWidth); + if(!mask) { + return GetEffectiveAccess(); + } auto bits = (uint64_t)0U; auto access = SvdTypes::Access::UNDEF; for(const auto child : childs) { const auto field = dynamic_cast(child); - if(!field) { + if(!field || !field->IsValid()) { continue; } const auto fieldWidth = field->GetEffectiveBitWidth(); - bits |= ((uint64_t)1 << fieldWidth) -1; + bits |= SvdUtils::CreateBitMask(fieldWidth); SvdTypes::Access acc = field->GetEffectiveAccess(); access = SvdUtils::CalcAccessResult(acc, access); @@ -415,6 +433,17 @@ SvdTypes::Access SvdRegister::GetAccessCalculated() bool SvdRegister::CalcAccessMask() { + const auto regWidth = GetEffectiveBitWidth(); + const auto regMask = SvdUtils::CreateBitMask(regWidth); + if(!regMask) { + m_accessMaskRead = m_accessMaskWrite = 0; + m_accessMaskValid = true; + return false; + } + if(regWidth == 64) { + m_accessMaskRead = m_accessMaskWrite = regMask; + } + const auto fields = GetFieldContainer(); if(!fields) { return true; @@ -425,21 +454,21 @@ bool SvdRegister::CalcAccessMask() return true; } - const auto regWidth = GetEffectiveBitWidth(); - m_accessMaskRead = (((uint32_t)((uint64_t)1 << regWidth)-1) << 0); + m_accessMaskRead = regMask; m_accessMaskWrite = 0; for(const auto child : childs) { SvdField* field = dynamic_cast(child); - if(!field) { + if(!field || !field->IsValid()) { continue; } const auto bitWidth = field->GetEffectiveBitWidth(); const auto firstBit = field->GetOffset(); - auto accessMask = 0; - accessMask = (((uint32_t)((uint64_t)1 << bitWidth)-1) << firstBit); - accessMask &= ((uint32_t)((uint64_t)1 << regWidth)-1); + if(bitWidth == 0 || bitWidth > 64 || firstBit >= regWidth || bitWidth > regWidth - firstBit) { + continue; + } + const auto accessMask = SvdUtils::CreateBitMask(bitWidth, firstBit) & regMask; const auto accType = field->GetEffectiveAccess(); switch(accType) { @@ -485,9 +514,7 @@ uint64_t SvdRegister::GetAccessMaskWrite() uint64_t SvdRegister::GetAccessMask() { - uint64_t accessMask = ((uint32_t)((uint64_t)1 << GetEffectiveBitWidth())-1); - - return accessMask; + return SvdUtils::CreateBitMask(GetEffectiveBitWidth()); } bool SvdRegister::AddToMap(SvdField *field, map &map) @@ -630,9 +657,10 @@ bool SvdRegister::CheckFields(SvdItem* fields, uint32_t regWidth, const string& const auto fieldWidth = field->GetEffectiveBitWidth(); const auto lineNo = field->GetLineNumber(); - if(fieldOffs + fieldWidth > regWidth) { + if(fieldWidth == 0 || fieldWidth > 64 || fieldOffs >= regWidth || fieldWidth > regWidth - fieldOffs) { LogMsg("M345", NAME(fieldName), BITRANGE(fieldOffs + fieldWidth -1, fieldOffs, true), NAME2(name), NUM(regWidth), lineNo); field->Invalidate(); + continue; } const auto accType = field->GetEffectiveAccess(); @@ -688,6 +716,12 @@ bool SvdRegister::CheckItem() const auto resetValue = GetResetValue(); const auto resetMask = GetResetMask(); + if(regWidth == 0 || regWidth > 64) { + LogMsg("M302", NAME(name), NUM(regWidth), lineNo); + Invalidate(); + return true; + } + if(offset == SvdItem::VALUE64_NOT_INIT) { const auto& svdLevelStr = GetSvdLevelStr(GetSvdLevel()); LogMsg("M370", LEVEL(svdLevelStr), NAME(name), lineNo); @@ -695,11 +729,7 @@ bool SvdRegister::CheckItem() } // Officially supported register size 1..32 bits. Unofficial "support" (and max value) for 64 bits. - uint64_t maxRegValue = 0; - if(regWidth < 64) { - maxRegValue = (uint64_t)0x1 << regWidth; - } - maxRegValue -= 1; + const auto maxRegValue = SvdUtils::CreateBitMask(regWidth); if(resetValue > maxRegValue) { LogMsg("M382", LEVEL("Register"), NAME(name), NAME2("Reset Value"), HEXNUM(resetValue), NUM(regWidth), lineNo); @@ -716,7 +746,7 @@ bool SvdRegister::CheckItem() if(dim) { const auto addressBitsUnits = dim->GetAddressBitsUnits(); const auto dimInc = dim->GetDimIncrement(); - const auto addressInc = addressBitsUnits * dimInc; + const auto addressInc = uint64_t{addressBitsUnits} * dimInc; if(addressInc < regWidth) { LogMsg("M366", NAME(name), NUM(regWidth), NUM2(dimInc), lineNo); @@ -727,8 +757,9 @@ bool SvdRegister::CheckItem() if(expr) { const auto e = expr->GetType(); if(e == SvdTypes::Expression::ARRAY) { - if(dimInc*8 != regWidth) { - LogMsg("M378", NAME(name), NUM(regWidth), NUM2(dimInc*8), lineNo); + const auto incrementBits = uint64_t{dimInc} * 8; + if(incrementBits != regWidth) { + LogMsg("M378", NAME(name), NUM(regWidth), NUM2(incrementBits), lineNo); Invalidate(); } } diff --git a/tools/svdconv/SVDModel/src/SvdUtils.cpp b/tools/svdconv/SVDModel/src/SvdUtils.cpp index 9a16d391a..6a2616f5a 100644 --- a/tools/svdconv/SVDModel/src/SvdUtils.cpp +++ b/tools/svdconv/SVDModel/src/SvdUtils.cpp @@ -446,6 +446,16 @@ bool SvdUtils::ConvertNumber(const string &text, set &numbers) return true; } +uint64_t SvdUtils::CreateBitMask(uint32_t width, uint64_t offset) +{ + if(width == 0 || width > 64 || offset >= 64 || width > 64 - offset) { + return 0; + } + + const auto mask = width == 64 ? UINT64_MAX : (uint64_t{1} << width) - 1; + return mask << offset; +} + bool SvdUtils::ConvertNumber(const string &text, uint32_t &number) { uint64_t num = 0; diff --git a/tools/svdconv/test/integtests/src/SvdConvIntegTests.cpp b/tools/svdconv/test/integtests/src/SvdConvIntegTests.cpp index f61c3eff8..2eb5496f6 100644 --- a/tools/svdconv/test/integtests/src/SvdConvIntegTests.cpp +++ b/tools/svdconv/test/integtests/src/SvdConvIntegTests.cpp @@ -211,6 +211,88 @@ TEST_F(SvdConvIntegTests, ExpandsRegisterAndFieldDimensionsInMemoryMap) { } } +TEST_F(SvdConvIntegTests, ExpansionLimitStopsAllGeneratorsAndNextInputStillWorks) { + const string original = SvdConvIntegTestEnv::localtestdata_dir + "/posMaskDim/PosMaskDim.svd"; + string xml; + ASSERT_TRUE(RteFsUtils::ReadFile(original, xml)); + const string marker = "256"; + const auto position = xml.find(marker); + ASSERT_NE(string::npos, position); + + for(const auto* dimension : {"100001", "50000"}) { + SCOPED_TRACE(dimension); + const string directory = SvdConvIntegTestEnv::testoutput_dir + "/expansionLimit/" + dimension; + ASSERT_TRUE(RteFsUtils::CreateDirectories(directory)); + const string input = directory + "/PosMaskDim.svd"; + string oversized = xml; + oversized.replace(position, marker.size(), string("") + dimension + ""); + { + ofstream file(input); + ASSERT_TRUE(file.good()); + file << oversized; + ASSERT_TRUE(file.good()); + } + const string output = directory + "/out"; + Arguments args("SVDConv.exe", input); + args.add({"-o", output, "--generate=header", "--generate=sfd", "--generate=fieldMap", "--create-folder"}); + SvdConv converter; + EXPECT_EQ(2, converter.Check(args, args, nullptr)); + string messages; + for(const auto& message : ErrLog::Get()->GetLogMessages()) { messages += message; } + EXPECT_NE(string::npos, messages.find("M249")); + for(const auto* suffix : {".h", ".sfd", "_MapFields.txt"}) { + EXPECT_FALSE(RteFsUtils::Exists(output + "/PosMaskDim" + suffix)); + } + ErrLog::Get()->ClearLogMessages(); + } + + // A rejected conversion must not poison the expansion budget of a new input. + const string output = SvdConvIntegTestEnv::testoutput_dir + "/expansionLimit/valid"; + Arguments args("SVDConv.exe", original); + args.add({"-o", output, "--generate=header", "--create-folder"}); + SvdConv converter; + EXPECT_EQ(0, converter.Check(args, args, nullptr)); + EXPECT_TRUE(RteFsUtils::Exists(output + "/PosMaskDim.h")); +} + +TEST_F(SvdConvIntegTests, RejectedSfdDoesNotCompileStaleOutput) { + string xml; + ASSERT_TRUE(RteFsUtils::ReadFile(SvdConvIntegTestEnv::localtestdata_dir + "/enumComboWidth/EnumComboWidth.svd", xml)); + xml = regex_replace(xml, regex("0x20"), "64"); + xml = regex_replace(xml, regex("0x4"), "0x8"); + const string directory = SvdConvIntegTestEnv::testoutput_dir + "/rejectedSfd"; + ASSERT_TRUE(RteFsUtils::CreateDirectories(directory)); + const string input = directory + "/EnumComboWidth.svd"; + const string output = directory + "/EnumComboWidth.sfd"; + const string previousOutput = "Previous SFD output\n"; + { + ofstream file(input); + file << xml; + ASSERT_TRUE(file.good()); + } + { + ofstream file(output); + file << previousOutput; + ASSERT_TRUE(file.good()); + } + + Arguments args("SVDConv.exe", input); + args.add({"-o", directory, "--generate=sfr", "--nocleanup", "--verbose"}); + SvdConv converter; + EXPECT_EQ(2, converter.Check(args, args, nullptr)); + string messages; + for(const auto& message : ErrLog::Get()->GetLogMessages()) { messages += message; } + EXPECT_NE(string::npos, messages.find("M226")); + EXPECT_NE(string::npos, messages.find("M111")); + EXPECT_EQ(string::npos, messages.find("Generate System Viewer SFR File")); + EXPECT_EQ(string::npos, messages.find("M133")); + EXPECT_EQ(string::npos, messages.find("SfrCC2")); + string retained; + ASSERT_TRUE(RteFsUtils::ReadFile(output, retained)); + EXPECT_EQ(previousOutput, retained); + EXPECT_FALSE(RteFsUtils::Exists(directory + "/EnumComboWidth.sfr")); +} + #ifndef _WIN32 TEST_F(SvdConvIntegTests, CheckSfrUnsupportedPlatform) { const string inFile = SvdConvIntegTestEnv::localtestdata_dir + "/option_n/option_n.svd"; diff --git a/tools/svdconv/test/unittests/CMakeLists.txt b/tools/svdconv/test/unittests/CMakeLists.txt index fa44fb8c6..180ffb8b0 100644 --- a/tools/svdconv/test/unittests/CMakeLists.txt +++ b/tools/svdconv/test/unittests/CMakeLists.txt @@ -7,6 +7,8 @@ set(TEST_SOURCE_FILES SvdCExpressionParserTest.cpp SvdOptionsTest.cpp SvdModelValidationTest.cpp + SvdBitWidthTest.cpp + SvdDimensionBoundsTest.cpp SvdSentinelModelTest.cpp SvdSentinelConsumerTest.cpp ) diff --git a/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp b/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp new file mode 100644 index 000000000..ac971a0e9 --- /dev/null +++ b/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp @@ -0,0 +1,465 @@ +/* + * Copyright (c) 2026 Arm Limited. All rights reserved. + * + * SPDX-License-Identifier: Apache-2.0 + */ +#include "SvdItem.h" +#include "SvdCExpression.h" +#include "SvdDevice.h" +#include "SvdEnum.h" +#include "SvdField.h" +#include "SvdRegister.h" +#include "HeaderData.h" +#include "SfdData.h" +#include "ErrLog.h" +#include "XMLTree.h" + +#include "gtest/gtest.h" +#include +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace std; + +namespace { + +class BitWidthHeaderData : public HeaderData { +public: + using HeaderData::HeaderData; + using HeaderData::AddFields; + using HeaderData::CreateFieldPosMask; +}; + +class BitWidthHeaderGenerator : public HeaderGenerator { +public: + using HeaderGenerator::HeaderGenerator; + using HeaderGenerator::MakeFieldPosMask3; +}; + +class BitWidthSfdData : public SfdData { +public: + using SfdData::SfdData; + using SfdData::CreateFieldItem; +}; + +class SvdBitWidthTest : public testing::Test, public IErrConsumer { +protected: + void SetUp() override { + const auto log = ErrLog::Get(); + m_previousConsumer = log->SetErrConsumer(this); + m_previousQuiet = log->IsQuietMode(); + m_previousErrors = log->GetErrCnt(); + m_previousWarnings = log->GetWarnCnt(); + log->SetQuietMode(false); + log->ResetMsgCount(); + } + + void TearDown() override { + const auto log = ErrLog::Get(); + log->SetErrConsumer(m_previousConsumer); + log->SetQuietMode(m_previousQuiet); + log->ResetMsgCount(); + for(int i = 0; i < m_previousErrors; i++) { + log->IncErrCnt(); + } + for(int i = 0; i < m_previousWarnings; i++) { + log->IncWarnCnt(); + } + if(!m_outputDirectory.empty()) { + error_code ec; + filesystem::remove(m_outputDirectory / "masks.h", ec); + filesystem::remove(m_outputDirectory / "device.sfd", ec); + filesystem::remove(m_outputDirectory, ec); + } + } + + bool Consume(const PdscMsg& msg, const string&) override { + m_messages.push_back(msg.GetMsgNum()); + return true; + } + + bool HasMessage(const string& id) const { + return find(m_messages.begin(), m_messages.end(), id) != m_messages.end(); + } + + void CreateOutputDirectory() { + const auto nonce = chrono::steady_clock::now().time_since_epoch().count(); + for(unsigned i = 0; i < 100; i++) { + const auto candidate = filesystem::temp_directory_path() / + ("svdconv-bit-width-" + to_string(nonce) + "-" + to_string(i)); + if(filesystem::create_directory(candidate)) { + m_outputDirectory = candidate; + break; + } + } + ASSERT_FALSE(m_outputDirectory.empty()); + } + + static string DefineValue(const string& text, const string& name) { + istringstream lines(text); + string line; + while(getline(lines, line)) { + istringstream tokens(line); + string directive, symbol, value; + if(tokens >> directive >> symbol >> value && directive == "#define" && symbol == name) { + return value; + } + } + return {}; + } + + static XMLTreeElement* ConfigureRegister(XMLTreeElement& element, const string& width) { + element.CreateElement("name", "CTRL"); + element.CreateElement("description", "Control register."); + element.CreateElement("size", width); + element.CreateElement("addressOffset", "0"); + element.CreateElement("access", "read-write"); + return element.CreateElement("fields"); + } + + static XMLTreeElement* AddField(XMLTreeElement* fields, const string& name, + const string& offset, const string& width) { + const auto field = fields->CreateElement("field"); + field->CreateElement("name", name); + field->CreateElement("description", "Control field."); + field->CreateElement("bitOffset", offset); + field->CreateElement("bitWidth", width); + return field; + } + + static void AddEnum(XMLTreeElement* field, const string& value) { + const auto container = field->CreateElement("enumeratedValues"); + const auto entry = container->CreateElement("enumeratedValue"); + entry->CreateElement("name", "VALUE"); + entry->CreateElement("description", "Control value."); + entry->CreateElement("value", value); + } + + static SvdField* AddUncheckedField(SvdRegister& reg, uint64_t offset, int32_t width) { + auto fields = reg.GetFieldContainer(); + if(!fields) { + fields = new SvdFieldContainer(®); + reg.AddItem(fields); + } + const auto field = new SvdField(fields); + fields->AddItem(field); + field->SetName("FIELD"); + field->SetOffset(offset); + field->SetBitWidth(width); + field->SetAccess(SvdTypes::Access::READWRITE); + return field; + } + + vector m_messages; + filesystem::path m_outputDirectory; + +private: + IErrConsumer* m_previousConsumer = nullptr; + bool m_previousQuiet = false; + int m_previousErrors = 0; + int m_previousWarnings = 0; +}; + +TEST_F(SvdBitWidthTest, FullWidthFieldsPreserveEnumsAndAccessMasks) { + for(const auto width : {32, 64}) { + SCOPED_TRACE(width); + m_messages.clear(); + XMLTreeElement element(nullptr, "register"); + const auto fields = ConfigureRegister(element, to_string(width)); + const auto fieldElement = AddField(fields, "FULL", "0", to_string(width)); + fieldElement->CreateElement("access", "read-only"); + AddEnum(fieldElement, "4294967295"); + SvdRegister reg(nullptr); + + ASSERT_TRUE(reg.Construct(&element)); + ASSERT_TRUE(reg.IsValid()); + ASSERT_NE(nullptr, reg.GetFieldContainer()); + ASSERT_EQ(1U, reg.GetFieldContainer()->GetChildCount()); + const auto field = dynamic_cast(reg.GetFieldContainer()->GetChildren().front()); + ASSERT_NE(nullptr, field); + EXPECT_TRUE(field->IsValid()); + ASSERT_EQ(1U, field->GetEnumContainer().size()); + const auto enumContainer = field->GetEnumContainer().front(); + ASSERT_EQ(1U, enumContainer->GetChildCount()); + EXPECT_TRUE(enumContainer->GetChildren().front()->IsValid()); + EXPECT_FALSE(HasMessage("M335")); + + const uint64_t expectedMask = width == 64 ? UINT64_MAX : UINT32_MAX; + EXPECT_EQ(expectedMask, reg.GetAccessMask()); + EXPECT_EQ(expectedMask, reg.GetAccessMaskRead()); + EXPECT_EQ(0U, reg.GetAccessMaskWrite()); + EXPECT_EQ(SvdTypes::Access::READONLY, reg.GetAccessCalculated()); + } +} + +TEST_F(SvdBitWidthTest, UpperBoundaryFieldsProduceOnlyTheirOwnWriteBit) { + for(const auto width : {32, 64}) { + SCOPED_TRACE(width); + XMLTreeElement element(nullptr, "register"); + const auto fields = ConfigureRegister(element, to_string(width)); + AddField(fields, "TOP", to_string(width - 1), "1"); + SvdRegister reg(nullptr); + + ASSERT_TRUE(reg.Construct(&element)); + ASSERT_TRUE(reg.IsValid()); + ASSERT_NE(nullptr, reg.GetFieldContainer()); + EXPECT_TRUE(reg.GetFieldContainer()->GetChildren().front()->IsValid()); + EXPECT_EQ(uint64_t{1} << (width - 1), reg.GetAccessMaskWrite()); + EXPECT_EQ(width == 64 ? UINT64_MAX : uint64_t{UINT32_MAX}, reg.GetAccessMaskRead()); + } +} + +TEST_F(SvdBitWidthTest, InvalidFieldWidthsAreRejectedBeforeEnumChecks) { + struct WidthCase { + const char* width; + const char* diagnostic; + }; + for(const auto& input : {WidthCase{"0", "M310"}, WidthCase{"65", "M310"}, + WidthCase{"129", "M310"}, WidthCase{"4294967294", "M313"}, + WidthCase{"-2", "M310"}}) { + SCOPED_TRACE(input.width); + m_messages.clear(); + XMLTreeElement element(nullptr, "register"); + const auto fields = ConfigureRegister(element, "64"); + AddEnum(AddField(fields, "BAD", "0", input.width), "1"); + AddField(fields, "VALID", "1", "1"); + SvdRegister reg(nullptr); + + ASSERT_TRUE(reg.Construct(&element)); + ASSERT_TRUE(reg.IsValid()); + ASSERT_NE(nullptr, reg.GetFieldContainer()); + ASSERT_EQ(2U, reg.GetFieldContainer()->GetChildCount()); + EXPECT_FALSE(reg.GetFieldContainer()->GetChildren().front()->IsValid()); + EXPECT_TRUE(reg.GetFieldContainer()->GetChildren().back()->IsValid()); + EXPECT_TRUE(HasMessage(input.diagnostic)); + EXPECT_FALSE(HasMessage("M335")); + EXPECT_EQ(2U, reg.GetAccessMaskWrite()); + } +} + +TEST_F(SvdBitWidthTest, FieldsOutsideOffsetAndRegisterBoundsAreRejected) { + struct RangeCase { + const char* registerWidth; + const char* offset; + const char* width; + const char* diagnostic; + }; + for(const auto& input : {RangeCase{"64", "64", "1", "M309"}, + RangeCase{"64", "18446744073709551614", "1", "M309"}, + RangeCase{"64", "63", "2", "M324"}, + RangeCase{"32", "31", "2", "M324"}}) { + SCOPED_TRACE(input.offset); + SCOPED_TRACE(input.width); + m_messages.clear(); + XMLTreeElement element(nullptr, "register"); + const auto fields = ConfigureRegister(element, input.registerWidth); + AddEnum(AddField(fields, "BAD", input.offset, input.width), "1"); + AddField(fields, "VALID", "0", "1"); + SvdRegister reg(nullptr); + + ASSERT_TRUE(reg.Construct(&element)); + ASSERT_NE(nullptr, reg.GetFieldContainer()); + ASSERT_EQ(2U, reg.GetFieldContainer()->GetChildCount()); + EXPECT_FALSE(reg.GetFieldContainer()->GetChildren().front()->IsValid()); + EXPECT_TRUE(reg.GetFieldContainer()->GetChildren().back()->IsValid()); + EXPECT_TRUE(HasMessage(input.diagnostic)); + EXPECT_FALSE(HasMessage("M335")); + EXPECT_EQ(1U, reg.GetAccessMaskWrite()); + } +} + +TEST_F(SvdBitWidthTest, AccessMaskSkipsInvalidAndUncheckedFieldRanges) { + SvdRegister reg(nullptr); + reg.SetBitWidth(64); + reg.SetAccess(SvdTypes::Access::READWRITE); + AddUncheckedField(reg, 63, 1); + AddUncheckedField(reg, 0, 0); + AddUncheckedField(reg, 0, 65); + AddUncheckedField(reg, 0, -2); + AddUncheckedField(reg, 64, 1); + AddUncheckedField(reg, UINT64_MAX - 1, 1); + AddUncheckedField(reg, 63, 2); + AddUncheckedField(reg, 0, 64)->Invalidate(); + + EXPECT_TRUE(reg.CalcAccessMask()); + EXPECT_EQ(uint64_t{1} << 63, reg.GetAccessMaskWrite()); + EXPECT_EQ(UINT64_MAX, reg.GetAccessMaskRead()); + EXPECT_EQ(SvdTypes::Access::READWRITE, reg.GetAccessCalculated()); +} + +TEST_F(SvdBitWidthTest, InvalidRegisterWidthsAreRejectedBeforeMaskCalculations) { + for(const auto* width : {"0", "65", "4294967294"}) { + SCOPED_TRACE(width); + m_messages.clear(); + XMLTreeElement element(nullptr, "register"); + const auto fields = ConfigureRegister(element, width); + AddField(fields, "BIT", "0", "1"); + SvdRegister reg(nullptr); + + ASSERT_TRUE(reg.Construct(&element)); + EXPECT_FALSE(reg.IsValid()); + EXPECT_EQ(1, count(m_messages.begin(), m_messages.end(), "M302")); + EXPECT_EQ(0U, reg.GetAccessMask()); + EXPECT_EQ(0U, reg.GetAccessMaskRead()); + EXPECT_EQ(0U, reg.GetAccessMaskWrite()); + } +} + +TEST_F(SvdBitWidthTest, MaskQueriesHandleUncheckedInvalidRegisterWidths) { + for(const auto width : {0, 65, -2}) { + SCOPED_TRACE(width); + SvdRegister reg(nullptr); + reg.SetBitWidth(width); + reg.SetAccess(SvdTypes::Access::READWRITE); + AddUncheckedField(reg, 0, 1); + + EXPECT_EQ(0U, reg.GetAccessMask()); + EXPECT_FALSE(reg.CalcAccessMask()); + EXPECT_EQ(0U, reg.GetAccessMaskRead()); + EXPECT_EQ(0U, reg.GetAccessMaskWrite()); + EXPECT_EQ(SvdTypes::Access::READWRITE, reg.GetAccessCalculated()); + } +} + +TEST_F(SvdBitWidthTest, EnumValuesAboveTheFieldWidthRemainInvalid) { + XMLTreeElement element(nullptr, "register"); + const auto fields = ConfigureRegister(element, "32"); + AddEnum(AddField(fields, "VALUE", "0", "31"), "2147483648"); + SvdRegister reg(nullptr); + + ASSERT_TRUE(reg.Construct(&element)); + ASSERT_NE(nullptr, reg.GetFieldContainer()); + const auto field = dynamic_cast(reg.GetFieldContainer()->GetChildren().front()); + ASSERT_NE(nullptr, field); + ASSERT_EQ(1U, field->GetEnumContainer().size()); + const auto enumContainer = field->GetEnumContainer().front(); + ASSERT_EQ(1U, enumContainer->GetChildCount()); + EXPECT_FALSE(enumContainer->GetChildren().front()->IsValid()); + EXPECT_TRUE(HasMessage("M335")); +} + +TEST_F(SvdBitWidthTest, FullWidthRegisterWithoutFieldsPreservesAllAccessBits) { + SvdRegister reg(nullptr); + reg.SetBitWidth(64); + EXPECT_EQ(UINT64_MAX, reg.GetAccessMaskRead()); + EXPECT_EQ(UINT64_MAX, reg.GetAccessMaskWrite()); +} + +TEST_F(SvdBitWidthTest, HeaderFieldGroupsDetectFullWidthAndUpperBitOverlaps) { + SvdOptions options; + FileHeaderInfo info; + BitWidthHeaderData header(info, options); + for(const auto width : {32, 64}) { + SCOPED_TRACE(width); + SvdRegister reg(nullptr); + reg.SetBitWidth(width); + const auto full = AddUncheckedField(reg, 0, width); + const auto upper = AddUncheckedField(reg, width - 1, 1); + FIELDMAPLIST groups; + ASSERT_TRUE(header.AddFields(reg.GetFieldContainer(), groups)); + ASSERT_EQ(2U, groups.size()); + EXPECT_EQ(width == 64 ? UINT64_MAX : uint64_t{UINT32_MAX}, groups.front().mask); + EXPECT_EQ(full, groups.front().fields.at(0)); + EXPECT_EQ(uint64_t{1} << (width - 1), groups.back().mask); + EXPECT_EQ(upper, groups.back().fields.at(width - 1)); + } +} + +TEST_F(SvdBitWidthTest, HeaderMasksPreserve32BitTextAndRepresent64BitBoundaries) { + CreateOutputDirectory(); + FileIo output; + output.SetFileName((m_outputDirectory / "masks.h").string()); + BitWidthHeaderGenerator generator(&output); + generator.MakeFieldPosMask3("FULL32", "Full 32-bit field", 0, UINT32_MAX); + generator.MakeFieldPosMask3("TOP32", "Top 32-bit field", 31, 1); + generator.MakeFieldPosMask3("FULL64", "Full 64-bit field", 0, UINT64_MAX); + generator.MakeFieldPosMask3("TOP64", "Top 64-bit field", 63, 1); + ASSERT_TRUE(output.Flush()); + ifstream input(m_outputDirectory / "masks.h"); + ASSERT_TRUE(input.is_open()); + const string text{istreambuf_iterator(input), istreambuf_iterator()}; + EXPECT_EQ("(0xffffffffUL)", DefineValue(text, "FULL32_Msk")); + EXPECT_EQ("(0x80000000UL)", DefineValue(text, "TOP32_Msk")); + EXPECT_EQ("(0xffffffffffffffffULL)", DefineValue(text, "FULL64_Msk")); + EXPECT_EQ("(0x8000000000000000ULL)", DefineValue(text, "TOP64_Msk")); + + SvdOptions options; + FileHeaderInfo info; + BitWidthHeaderData header(info, options); + SvdRegister reg(nullptr); + reg.SetBitWidth(64); + const auto full = AddUncheckedField(reg, 0, 64); + PosMaskNames names; + EXPECT_TRUE(header.CreateFieldPosMask(full, &names)); + EXPECT_FALSE(HasMessage("M220")); + EXPECT_FALSE(header.CreateFieldPosMask(AddUncheckedField(reg, 63, 2), &names)); +} + +TEST_F(SvdBitWidthTest, FieldExpressionsUse64BitLiteralsOnlyWhenNeeded) { + SvdRegister reg(nullptr); + reg.SetName("CTRL"); + reg.SetBitWidth(64); + SvdCExpression expression; + EXPECT_EQ("(CTRL & (0xffffffff << 0))", expression.CreateFieldExpression(AddUncheckedField(reg, 0, 32))); + EXPECT_EQ("(CTRL & (0xffffffffffffffffULL << 0))", expression.CreateFieldExpression(AddUncheckedField(reg, 0, 64))); + EXPECT_EQ("(CTRL & (0x1ULL << 63))", expression.CreateFieldExpression(AddUncheckedField(reg, 63, 1))); + EXPECT_TRUE(expression.CreateFieldExpression(AddUncheckedField(reg, 64, 1)).empty()); + EXPECT_TRUE(expression.CreateFieldExpression(AddUncheckedField(reg, 0, 65)).empty()); + EXPECT_TRUE(expression.CreateFieldExpression(AddUncheckedField(reg, 0, 0)).empty()); +} + +TEST_F(SvdBitWidthTest, SfdRejectsUnsupported64BitRegisterBeforeCreatingOutput) { + CreateOutputDirectory(); + SvdDevice device(nullptr); + const auto reg = new SvdRegister(&device); + device.AddItem(reg); + reg->SetName("CTRL"); + reg->SetBitWidth(64); + SvdOptions options; + FileHeaderInfo info; + SfdData sfd(info, options); + const auto output = m_outputDirectory / "device.sfd"; + EXPECT_FALSE(sfd.Create(&device, output.string())); + EXPECT_TRUE(HasMessage("M226")); + EXPECT_FALSE(filesystem::exists(output)); +} + +TEST_F(SvdBitWidthTest, SfdFieldMasksAccept32BitsAndRejectWiderRanges) { + SvdOptions options; + FileHeaderInfo info; + BitWidthSfdData sfd(info, options); + SvdRegister reg(nullptr); + reg.SetBitWidth(32); + EXPECT_TRUE(sfd.CreateFieldItem(AddUncheckedField(reg, 0, 32))); + EXPECT_TRUE(sfd.CreateFieldItem(AddUncheckedField(reg, 31, 1))); + EXPECT_FALSE(HasMessage("M226")); + EXPECT_FALSE(sfd.CreateFieldItem(AddUncheckedField(reg, 0, 64))); + EXPECT_FALSE(sfd.CreateFieldItem(AddUncheckedField(reg, 32, 1))); + EXPECT_TRUE(HasMessage("M226")); +} + +TEST_F(SvdBitWidthTest, SfdRejectsInvalidExpressionReferencesBeforeCreatingOutput) { + CreateOutputDirectory(); + SvdDevice device(nullptr); + const auto reg = new SvdRegister(&device); + device.AddItem(reg); + reg->SetName("CTRL"); + reg->SetBitWidth(1024); + reg->Invalidate(); + device.GetExpressionRegistersList()["CTRL"] = reg; + SvdOptions options; + FileHeaderInfo info; + SfdData sfd(info, options); + const auto output = m_outputDirectory / "device.sfd"; + EXPECT_FALSE(sfd.Create(&device, output.string())); + EXPECT_TRUE(HasMessage("M226")); + EXPECT_FALSE(filesystem::exists(output)); +} + +} // namespace diff --git a/tools/svdconv/test/unittests/src/SvdDimensionBoundsTest.cpp b/tools/svdconv/test/unittests/src/SvdDimensionBoundsTest.cpp new file mode 100644 index 000000000..2755f0f38 --- /dev/null +++ b/tools/svdconv/test/unittests/src/SvdDimensionBoundsTest.cpp @@ -0,0 +1,254 @@ +/* + * Copyright (c) 2026 Arm Limited. All rights reserved. + * + * SPDX-License-Identifier: Apache-2.0 + */ +#include "SvdItem.h" +#include "SvdCluster.h" +#include "SvdDevice.h" +#include "SvdDimension.h" +#include "SvdInterrupt.h" +#include "SvdPeripheral.h" +#include "SvdRegister.h" +#include "ErrLog.h" +#include "XMLTree.h" + +#include "gtest/gtest.h" +#include +#include +#include +#include + +using namespace std; + +namespace { + +class SvdDimensionBoundsTest : public testing::Test, public IErrConsumer { +protected: + void SetUp() override { + auto log = ErrLog::Get(); + m_previousConsumer = log->SetErrConsumer(this); + m_previousQuiet = log->IsQuietMode(); + m_previousErrors = log->GetErrCnt(); + m_previousWarnings = log->GetWarnCnt(); + log->SetQuietMode(false); + log->ResetMsgCount(); + } + + void TearDown() override { + auto log = ErrLog::Get(); + log->SetErrConsumer(m_previousConsumer); + log->SetQuietMode(m_previousQuiet); + log->ResetMsgCount(); + for(int i = 0; i < m_previousErrors; i++) { log->IncErrCnt(); } + for(int i = 0; i < m_previousWarnings; i++) { log->IncWarnCnt(); } + } + + bool Consume(const PdscMsg& msg, const string&) override { + m_messages.push_back(msg.GetMsgNum()); + return true; + } + + static SvdDimension* Dimension(SvdItem& item, uint32_t count, uint32_t increment = 4, + const string& indices = "") { + item.SetName("ITEM%s"); + auto dim = new SvdDimension(&item); + item.SetDimension(dim); + dim->SetDim(count); + dim->SetDimIncrement(increment); + dim->SetDimIndex(indices); + return dim; + } + + bool HasMessage(const string& id) const { + return find(m_messages.begin(), m_messages.end(), id) != m_messages.end(); + } + + vector m_messages; + +private: + IErrConsumer* m_previousConsumer = nullptr; + bool m_previousQuiet = false; + int m_previousErrors = 0; + int m_previousWarnings = 0; +}; + +TEST_F(SvdDimensionBoundsTest, NumericRangesIncludeMaximumEndpointsWithoutOverflow) { + for(const auto end : {uint32_t(INT32_MAX), UINT32_MAX}) { + for(const uint32_t count : {1U, 2U}) { + SCOPED_TRACE(end); + SCOPED_TRACE(count); + SvdRegister reg(nullptr); + const auto first = end - count + 1; + const auto dim = Dimension(reg, count, 4, to_string(first) + "-" + to_string(end)); + ASSERT_TRUE(dim->CalculateDim()); + ASSERT_EQ(count, dim->GetDimIndexList().size()); + EXPECT_EQ(to_string(first), dim->GetDimIndexList().front()); + EXPECT_EQ(to_string(end), dim->GetDimIndexList().back()); + EXPECT_FALSE(reg.HasExpansionError()); + } + } +} + +TEST_F(SvdDimensionBoundsTest, InvalidAndOversizedCountsDoNotAllocateIndices) { + for(const uint32_t count : {0U, UINT32_MAX, uint32_t(SvdItem::MAX_EXPANSION_ITEMS + 1)}) { + SCOPED_TRACE(count); + SvdRegister reg(nullptr); + const auto dim = Dimension(reg, count); + EXPECT_FALSE(reg.CalculateDim()); + EXPECT_TRUE(dim->GetDimIndexList().empty()); + EXPECT_TRUE(dim->GetChildren().empty()); + EXPECT_FALSE(reg.IsValid()); + } +} + +TEST_F(SvdDimensionBoundsTest, OversizedExplicitListIsStoppedBeforeExpansion) { + SvdRegister reg(nullptr); + string indices; + for(size_t i = 0; i <= SvdItem::MAX_EXPANSION_ITEMS; i++) { indices += "A,"; } + const auto dim = Dimension(reg, 2, 4, indices); + EXPECT_FALSE(reg.CalculateDim()); + EXPECT_TRUE(reg.HasExpansionError()); + EXPECT_TRUE(dim->GetDimIndexList().empty()); + EXPECT_TRUE(dim->GetChildren().empty()); + EXPECT_EQ(1, count(m_messages.begin(), m_messages.end(), "M249")); +} + +TEST_F(SvdDimensionBoundsTest, IndexBudgetIsSharedBySiblingDimensions) { + SvdItem root(nullptr); + SvdRegister first(&root), second(&root), third(&root); + const auto half = uint32_t(SvdItem::MAX_EXPANSION_ITEMS / 2); + ASSERT_TRUE(Dimension(first, half)->CalculateDimIndex()); + ASSERT_TRUE(Dimension(second, half)->CalculateDimIndex()); + EXPECT_FALSE(root.HasExpansionError()); + const auto last = Dimension(third, 1); + EXPECT_FALSE(last->CalculateDimIndex()); + EXPECT_TRUE(last->GetDimIndexList().empty()); + EXPECT_TRUE(first.HasExpansionError()); + EXPECT_TRUE(root.HasExpansionError()); +} + +TEST_F(SvdDimensionBoundsTest, RecalculationDoesNotResetBudgetAndNewInputIsIndependent) { + SvdRegister reg(nullptr); + const auto dim = Dimension(reg, uint32_t(SvdItem::MAX_EXPANSION_ITEMS / 2 + 1)); + ASSERT_TRUE(dim->CalculateDimIndex()); + EXPECT_FALSE(dim->CalculateDimIndex()); + EXPECT_TRUE(reg.HasExpansionError()); + EXPECT_TRUE(dim->GetDimIndexList().empty()); + + SvdRegister nextInput(nullptr); + const auto nextDim = Dimension(nextInput, 2); + EXPECT_TRUE(nextDim->CalculateDimIndex()); + EXPECT_EQ(2U, nextDim->GetDimIndexList().size()); + EXPECT_FALSE(nextInput.HasExpansionError()); +} + +TEST_F(SvdDimensionBoundsTest, NestedDimensionsShareCopiedDescendantBudget) { + XMLTreeElement element(nullptr, "cluster"); + element.CreateElement("name", "CLUSTER[%s]"); + element.CreateElement("description", "Cluster array."); + element.CreateElement("addressOffset", "0"); + element.CreateElement("dim", "1000"); + element.CreateElement("dimIncrement", "4096"); + auto reg = element.CreateElement("register"); + reg->CreateElement("name", "REG[%s]"); + reg->CreateElement("description", "Register array."); + reg->CreateElement("addressOffset", "0"); + reg->CreateElement("size", "32"); + reg->CreateElement("access", "read-write"); + reg->CreateElement("dim", "1000"); + reg->CreateElement("dimIncrement", "4"); + SvdCluster cluster(nullptr); + + EXPECT_FALSE(cluster.Construct(&element)); + EXPECT_TRUE(cluster.HasExpansionError()); + ASSERT_NE(nullptr, cluster.GetDimension()); + EXPECT_LT(cluster.GetDimension()->GetChildCount(), 1000U); + EXPECT_EQ(1, count(m_messages.begin(), m_messages.end(), "M249")); +} + +TEST_F(SvdDimensionBoundsTest, IncrementMultiplicationUsesWideIntermediate) { + SvdDevice device(nullptr); + XMLTreeElement unitBits(nullptr, "addressUnitBits"); + unitBits.SetText("32"); + ASSERT_TRUE(device.ProcessXmlElement(&unitBits)); + SvdRegister reg(&device); + const auto dim = Dimension(reg, 2, UINT32_MAX - 1); + EXPECT_EQ(uint64_t(UINT32_MAX - 1) * 4, dim->CalcAddressIncrement()); +} + +TEST_F(SvdDimensionBoundsTest, RegisterStrideValidationDoesNotWrapProducts) { + SvdDevice device(nullptr); + XMLTreeElement unitBits(nullptr, "addressUnitBits"); + unitBits.SetText("8"); + ASSERT_TRUE(device.ProcessXmlElement(&unitBits)); + + SvdRegister extended(&device); + extended.SetBitWidth(32); + extended.SetOffset(0); + ASSERT_TRUE(Dimension(extended, 2, 0x20000000)->CalculateDim()); + extended.CheckItem(); + EXPECT_TRUE(extended.IsValid()); + EXPECT_FALSE(HasMessage("M366")); + + SvdRegister array(&device); + array.SetBitWidth(32); + array.SetOffset(0); + const auto dim = Dimension(array, 2, 0x20000004); + array.SetName("ITEM[%s]"); + ASSERT_TRUE(dim->CalculateDim()); + array.CheckItem(); + EXPECT_FALSE(array.IsValid()); + EXPECT_TRUE(HasMessage("M378")); +} + +TEST_F(SvdDimensionBoundsTest, LastAddressIsCheckedBeforeGeneratingCopies) { + for(const uint64_t start : {UINT64_MAX - 5, UINT64_MAX - 3}) { + SvdRegister reg(nullptr); + reg.SetBitWidth(32); + reg.SetAccess(SvdTypes::Access::READWRITE); + reg.SetOffset(start); + const auto dim = Dimension(reg, 2, 4); + const bool fits = start == UINT64_MAX - 5; + EXPECT_EQ(fits, reg.CalculateDim()); + EXPECT_EQ(fits ? 2U : 0U, dim->GetChildCount()); + if(fits) { + const auto last = dynamic_cast(dim->GetChildren().back()); + ASSERT_NE(nullptr, last); + EXPECT_EQ(UINT64_MAX - 1, last->GetOffset()); + EXPECT_TRUE(last->IsValid()); + } + else { + EXPECT_FALSE(reg.IsValid()); + EXPECT_TRUE(HasMessage("M250")); + } + } +} + +TEST_F(SvdDimensionBoundsTest, ClusterAndPeripheralAddressWrapIsRejected) { + SvdCluster cluster(nullptr); + cluster.SetOffset(UINT64_MAX - 1); + const auto clusterDim = Dimension(cluster, 2, 4); + EXPECT_FALSE(cluster.CalculateDim()); + EXPECT_TRUE(clusterDim->GetChildren().empty()); + + SvdPeripheral peripheral(nullptr); + peripheral.SetAddress(UINT64_MAX - 1); + const auto peripheralDim = Dimension(peripheral, 2, 4); + EXPECT_FALSE(peripheral.CalculateDim()); + EXPECT_TRUE(peripheralDim->GetChildren().empty()); +} + +TEST_F(SvdDimensionBoundsTest, InterruptDimensionCannotWrapItsValue) { + SvdPeripheral peripheral(nullptr); + Dimension(peripheral, 3, 4); + SvdInterrupt irq(&peripheral); + irq.SetName("IRQ%s"); + irq.SetValue(UINT32_MAX - 2); + EXPECT_FALSE(irq.CalculateDim()); + EXPECT_FALSE(irq.IsValid()); + ASSERT_NE(nullptr, irq.GetDimension()); + EXPECT_TRUE(irq.GetDimension()->GetChildren().empty()); +} + +} // namespace From 68e527e7b54443de1e469e24b426ed88499e86cf Mon Sep 17 00:00:00 2001 From: Thorsten de Buhr Date: Thu, 15 Oct 2026 15:41:30 +0200 Subject: [PATCH 02/26] fix(svdconv): guard missing register ancestors in field validation --- tools/svdconv/SVDModel/src/SvdField.cpp | 13 ++++++------- .../svdconv/test/unittests/src/SvdBitWidthTest.cpp | 14 ++++++++++++++ 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/tools/svdconv/SVDModel/src/SvdField.cpp b/tools/svdconv/SVDModel/src/SvdField.cpp index 5578f1722..1c5cda5c8 100644 --- a/tools/svdconv/SVDModel/src/SvdField.cpp +++ b/tools/svdconv/SVDModel/src/SvdField.cpp @@ -530,7 +530,7 @@ bool SvdField::CheckItem() parent = parent->GetParent(); } } - const auto reg = dynamic_cast(parent->GetParent()); + const auto reg = parent ? dynamic_cast(parent->GetParent()) : nullptr; if(reg) { const auto regWidth = reg->GetEffectiveBitWidth(); const auto l = reg->GetLineNumber(); @@ -539,13 +539,12 @@ bool SvdField::CheckItem() Invalidate(); return true; } - } - - const auto regAccess = reg->GetEffectiveAccess(); - const auto fieldAccess = GetEffectiveAccess(); + const auto regAccess = reg->GetEffectiveAccess(); + const auto fieldAccess = GetEffectiveAccess(); - if(!SvdUtils::IsMatchAccess(fieldAccess, regAccess)) { - LogMsg("M367", lineNo); + if(!SvdUtils::IsMatchAccess(fieldAccess, regAccess)) { + LogMsg("M367", lineNo); + } } } diff --git a/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp b/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp index ac971a0e9..923e39aff 100644 --- a/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp +++ b/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp @@ -6,6 +6,7 @@ #include "SvdItem.h" #include "SvdCExpression.h" #include "SvdDevice.h" +#include "SvdDimension.h" #include "SvdEnum.h" #include "SvdField.h" #include "SvdRegister.h" @@ -351,6 +352,19 @@ TEST_F(SvdBitWidthTest, FullWidthRegisterWithoutFieldsPreservesAllAccessBits) { EXPECT_EQ(UINT64_MAX, reg.GetAccessMaskWrite()); } +TEST_F(SvdBitWidthTest, FieldValidationHandlesMissingRegisterAncestors) { + SvdFieldContainer fields(nullptr); + SvdDimension dimension(nullptr); + for(SvdItem* parent : {static_cast(&fields), static_cast(&dimension)}) { + SvdField field(parent); + field.SetName("BIT"); + field.SetOffset(0); + field.SetBitWidth(1); + EXPECT_TRUE(field.CheckItem()); + EXPECT_TRUE(field.IsValid()); + } +} + TEST_F(SvdBitWidthTest, HeaderFieldGroupsDetectFullWidthAndUpperBitOverlaps) { SvdOptions options; FileHeaderInfo info; From b53d44fc0308064c25d967c1d38ab4bbf95f9535 Mon Sep 17 00:00:00 2001 From: Thorsten de Buhr Date: Thu, 15 Oct 2026 15:52:44 +0200 Subject: [PATCH 03/26] test(svdconv): preserve fixture bytes on Windows --- tools/svdconv/test/integtests/src/SvdConvIntegTests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/svdconv/test/integtests/src/SvdConvIntegTests.cpp b/tools/svdconv/test/integtests/src/SvdConvIntegTests.cpp index 2eb5496f6..f05c54934 100644 --- a/tools/svdconv/test/integtests/src/SvdConvIntegTests.cpp +++ b/tools/svdconv/test/integtests/src/SvdConvIntegTests.cpp @@ -271,7 +271,7 @@ TEST_F(SvdConvIntegTests, RejectedSfdDoesNotCompileStaleOutput) { ASSERT_TRUE(file.good()); } { - ofstream file(output); + ofstream file(output, ios::binary); file << previousOutput; ASSERT_TRUE(file.good()); } From 6032d9087b309440839b2414ebf0dd07cbcc9e29 Mon Sep 17 00:00:00 2001 From: Thorsten de Buhr Date: Thu, 15 Oct 2026 16:01:36 +0200 Subject: [PATCH 04/26] test(svdconv): cover expansion and mask rejection boundaries --- .../test/unittests/src/SvdBitWidthTest.cpp | 116 +++++++++ .../unittests/src/SvdDimensionBoundsTest.cpp | 221 ++++++++++++++++++ 2 files changed, 337 insertions(+) diff --git a/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp b/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp index 923e39aff..6788942c1 100644 --- a/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp +++ b/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp @@ -47,6 +47,7 @@ class BitWidthSfdData : public SfdData { public: using SfdData::SfdData; using SfdData::CreateFieldItem; + using SfdData::CreateEnumValueSet; }; class SvdBitWidthTest : public testing::Test, public IErrConsumer { @@ -476,4 +477,119 @@ TEST_F(SvdBitWidthTest, SfdRejectsInvalidExpressionReferencesBeforeCreatingOutpu EXPECT_FALSE(filesystem::exists(output)); } +TEST_F(SvdBitWidthTest, HeaderFieldGroupsSkipUncheckedInvalidRanges) { + SvdOptions options; + FileHeaderInfo info; + BitWidthHeaderData header(info, options); + SvdRegister reg(nullptr); + reg.SetBitWidth(64); + AddUncheckedField(reg, 0, 0); + AddUncheckedField(reg, 0, 65); + AddUncheckedField(reg, 64, 1); + AddUncheckedField(reg, 63, 2); + const auto upper = AddUncheckedField(reg, 63, 1); + + FIELDMAPLIST groups; + ASSERT_TRUE(header.AddFields(reg.GetFieldContainer(), groups)); + ASSERT_EQ(1U, groups.size()); + EXPECT_EQ(uint64_t{1} << 63, groups.front().mask); + ASSERT_EQ(1U, groups.front().fields.size()); + EXPECT_EQ(upper, groups.front().fields.at(63)); +} + +TEST_F(SvdBitWidthTest, HeaderRejectsInvalidShiftRangesWithoutPartialMacros) { + CreateOutputDirectory(); + FileIo output; + output.SetFileName((m_outputDirectory / "masks.h").string()); + BitWidthHeaderGenerator generator(&output); + generator.MakeFieldPosMask3("INVALID_POSITION", "Position outside register", 64, 1); + generator.MakeFieldPosMask3("INVALID_MASK", "Mask crosses register boundary", 63, 3); + EXPECT_EQ(2, count(m_messages.begin(), m_messages.end(), "M220")); + generator.MakeFieldPosMask3("VALID", "Top 64-bit field", 63, 1); + ASSERT_TRUE(output.Flush()); + + ifstream input(m_outputDirectory / "masks.h"); + ASSERT_TRUE(input.is_open()); + const string text{istreambuf_iterator(input), istreambuf_iterator()}; + EXPECT_EQ(string::npos, text.find("INVALID_POSITION")); + EXPECT_EQ(string::npos, text.find("INVALID_MASK")); + EXPECT_EQ("(63UL)", DefineValue(text, "VALID_Pos")); + EXPECT_EQ("(0x8000000000000000ULL)", DefineValue(text, "VALID_Msk")); +} + +TEST_F(SvdBitWidthTest, SfdRejectsUnsupportedExpressionReferenceOutsideDeviceChildren) { + CreateOutputDirectory(); + SvdDevice device(nullptr); + SvdRegister reg(&device); + reg.SetName("CTRL"); + reg.SetBitWidth(64); + device.GetExpressionRegistersList()["CTRL"] = ® + SvdOptions options; + FileHeaderInfo info; + SfdData sfd(info, options); + const auto output = m_outputDirectory / "device.sfd"; + + ASSERT_TRUE(reg.IsValid()); + ASSERT_TRUE(device.GetChildren().empty()); + EXPECT_FALSE(sfd.Create(&device, output.string())); + EXPECT_TRUE(HasMessage("M226")); + EXPECT_FALSE(filesystem::exists(output)); +} + +TEST_F(SvdBitWidthTest, SfdEnumSetsRejectWidthsOutsideComboRange) { + SvdOptions options; + FileHeaderInfo info; + BitWidthSfdData sfd(info, options); + SvdRegister reg(nullptr); + reg.SetBitWidth(64); + const auto field = AddUncheckedField(reg, 0, 6); + SvdEnumContainer values(field); + const auto entry = new SvdEnum(&values); + values.AddItem(entry); + entry->SetName("LAST"); + entry->SetValue(63); + EXPECT_TRUE(sfd.CreateEnumValueSet(&values, field)); + + for(const auto width : {0, 7, 64}) { + SCOPED_TRACE(width); + field->SetBitWidth(width); + EXPECT_FALSE(sfd.CreateEnumValueSet(&values, field)); + } + EXPECT_TRUE(entry->IsValid()); + EXPECT_EQ(63U, entry->GetValue().u32); +} + +TEST_F(SvdBitWidthTest, FullWidthFieldsStillReportIncompatibleRegisterAccess) { + for(const auto width : {32, 64}) { + SCOPED_TRACE(width); + m_messages.clear(); + SvdRegister reg(nullptr); + reg.SetName("CTRL"); + reg.SetBitWidth(width); + reg.SetAccess(SvdTypes::Access::READONLY); + const auto field = AddUncheckedField(reg, 0, width); + + EXPECT_TRUE(field->CheckItem()); + EXPECT_TRUE(field->IsValid()); + EXPECT_TRUE(HasMessage("M367")); + EXPECT_EQ(SvdTypes::Access::READWRITE, field->GetAccess()); + } +} + +TEST_F(SvdBitWidthTest, RegisterSkipsRejectedRangesBeforeCheckingSiblingConflicts) { + SvdRegister reg(nullptr); + reg.SetName("CTRL"); + reg.SetBitWidth(64); + const auto invalid = AddUncheckedField(reg, 63, 2); + const auto valid = AddUncheckedField(reg, 63, 1); + + EXPECT_TRUE(reg.CheckFields(reg.GetFieldContainer(), 64, reg.GetName())); + EXPECT_FALSE(invalid->IsValid()); + EXPECT_TRUE(valid->IsValid()); + EXPECT_EQ(1, count(m_messages.begin(), m_messages.end(), "M345")); + EXPECT_FALSE(HasMessage("M336")); + EXPECT_FALSE(HasMessage("M338")); + EXPECT_EQ(uint64_t{1} << 63, reg.GetAccessMaskWrite()); +} + } // namespace diff --git a/tools/svdconv/test/unittests/src/SvdDimensionBoundsTest.cpp b/tools/svdconv/test/unittests/src/SvdDimensionBoundsTest.cpp index 2755f0f38..bcccb61d2 100644 --- a/tools/svdconv/test/unittests/src/SvdDimensionBoundsTest.cpp +++ b/tools/svdconv/test/unittests/src/SvdDimensionBoundsTest.cpp @@ -5,8 +5,10 @@ */ #include "SvdItem.h" #include "SvdCluster.h" +#include "SvdDerivedFrom.h" #include "SvdDevice.h" #include "SvdDimension.h" +#include "SvdField.h" #include "SvdInterrupt.h" #include "SvdPeripheral.h" #include "SvdRegister.h" @@ -114,6 +116,139 @@ TEST_F(SvdDimensionBoundsTest, OversizedExplicitListIsStoppedBeforeExpansion) { EXPECT_EQ(1, count(m_messages.begin(), m_messages.end(), "M249")); } +TEST_F(SvdDimensionBoundsTest, FinalExplicitIndexCannotExceedTheSharedBudget) { + SvdRegister reg(nullptr); + ASSERT_TRUE(reg.ReserveExpansion(SvdItem::MAX_EXPANSION_ITEMS - 1)); + const auto dim = Dimension(reg, 2, 4, "A,B"); + + EXPECT_FALSE(reg.CalculateDim()); + EXPECT_TRUE(reg.HasExpansionError()); + EXPECT_TRUE(dim->GetDimIndexList().empty()); + EXPECT_TRUE(dim->GetChildren().empty()); + EXPECT_EQ(1, count(m_messages.begin(), m_messages.end(), "M249")); +} + +TEST_F(SvdDimensionBoundsTest, DirectRangeCalculationRejectsInvalidCountsBeforeAllocation) { + for(const uint32_t count : {0U, UINT32_MAX, uint32_t(SvdItem::MAX_EXPANSION_ITEMS + 1)}) { + SCOPED_TRACE(count); + SvdRegister reg(nullptr); + const auto dim = Dimension(reg, count); + + EXPECT_FALSE(dim->CalculateDimIndexFromTo()); + EXPECT_TRUE(dim->GetDimIndexList().empty()); + EXPECT_EQ(count == SvdItem::MAX_EXPANSION_ITEMS + 1, reg.HasExpansionError()); + } +} + +TEST_F(SvdDimensionBoundsTest, AlphabeticRangeIsReservedBeforeAnyIndexIsAdded) { + for(const size_t remaining : {size_t{1}, size_t{2}}) { + SCOPED_TRACE(remaining); + SvdRegister reg(nullptr); + ASSERT_TRUE(reg.ReserveExpansion(SvdItem::MAX_EXPANSION_ITEMS - remaining)); + const auto dim = Dimension(reg, 2, 4, "A-B"); + + EXPECT_EQ(remaining == 2, dim->CalculateDim()); + EXPECT_EQ(remaining == 1, reg.HasExpansionError()); + if(remaining == 2) { + ASSERT_EQ(2U, dim->GetDimIndexList().size()); + EXPECT_EQ("A", dim->GetDimIndexList().front()); + EXPECT_EQ("B", dim->GetDimIndexList().back()); + } + else { + EXPECT_TRUE(dim->GetDimIndexList().empty()); + } + } +} + +TEST_F(SvdDimensionBoundsTest, ExhaustedBudgetRemainsStickyAcrossDescendants) { + SvdItem root(nullptr); + SvdRegister reg(&root); + ASSERT_TRUE(root.ReserveExpansion(SvdItem::MAX_EXPANSION_ITEMS)); + EXPECT_FALSE(reg.ReserveExpansion(1)); + EXPECT_FALSE(root.ReserveExpansion(0)); + EXPECT_FALSE(reg.ReserveExpansion(1)); + EXPECT_TRUE(root.HasExpansionError()); + EXPECT_TRUE(reg.HasExpansionError()); + EXPECT_EQ(1, count(m_messages.begin(), m_messages.end(), "M249")); +} + +TEST_F(SvdDimensionBoundsTest, InheritedMetadataMustFitTheDestinationBudget) { + SvdItem source(nullptr); + Dimension(source, 2); + source.SetDerivedFrom(new SvdDerivedFrom(&source)); + + for(const size_t remaining : {size_t{0}, size_t{1}, size_t{2}}) { + SCOPED_TRACE(remaining); + SvdItem target(nullptr); + ASSERT_TRUE(target.ReserveExpansion(SvdItem::MAX_EXPANSION_ITEMS - remaining)); + + EXPECT_EQ(remaining == 2, target.CopyItem(&source)); + EXPECT_EQ(remaining < 2, target.HasExpansionError()); + if(remaining > 0) { + ASSERT_NE(nullptr, target.GetDerivedFrom()); + EXPECT_EQ(&source, target.GetDerivedFrom()->GetDerivedFromItem()); + } + else { + EXPECT_EQ(nullptr, target.GetDerivedFrom()); + } + if(remaining == 2) { + ASSERT_NE(nullptr, target.GetDimension()); + EXPECT_EQ(2U, target.GetDimension()->GetDim()); + EXPECT_EQ(&source, target.GetCopiedFrom()); + } + else { + EXPECT_EQ(nullptr, target.GetDimension()); + EXPECT_EQ(nullptr, target.GetCopiedFrom()); + } + } +} + +TEST_F(SvdDimensionBoundsTest, CopiedPeripheralDescendantsShareOneBudget) { + SvdPeripheralContainer source(nullptr); + const auto peripheral = new SvdPeripheral(&source); + source.AddItem(peripheral); + peripheral->SetName("PERIPHERAL"); + peripheral->SetAddress(0x40000000); + const auto registers = new SvdRegisterContainer(peripheral); + peripheral->AddItem(registers); + const auto cluster = new SvdCluster(registers); + registers->AddItem(cluster); + cluster->SetName("GROUP"); + cluster->SetOffset(0); + const auto reg = new SvdRegister(cluster); + cluster->AddItem(reg); + reg->SetName("DATA"); + reg->SetOffset(4); + reg->SetBitWidth(32); + + for(const size_t remaining : {size_t{3}, size_t{4}}) { + SCOPED_TRACE(remaining); + SvdPeripheralContainer target(nullptr); + ASSERT_TRUE(target.ReserveExpansion(SvdItem::MAX_EXPANSION_ITEMS - remaining)); + + EXPECT_EQ(remaining == 4, target.CopyChilds(&source, &target)); + EXPECT_EQ(remaining == 3, target.HasExpansionError()); + ASSERT_EQ(1U, target.GetChildCount()); + const auto copiedPeripheral = target.GetChildren().front(); + ASSERT_EQ(1U, copiedPeripheral->GetChildCount()); + const auto copiedRegisters = copiedPeripheral->GetChildren().front(); + ASSERT_EQ(1U, copiedRegisters->GetChildCount()); + const auto copiedCluster = copiedRegisters->GetChildren().front(); + if(remaining == 4) { + ASSERT_EQ(1U, copiedCluster->GetChildCount()); + const auto copiedRegister = dynamic_cast(copiedCluster->GetChildren().front()); + ASSERT_NE(nullptr, copiedRegister); + EXPECT_EQ("DATA", copiedRegister->GetName()); + EXPECT_EQ(4U, copiedRegister->GetOffset()); + EXPECT_EQ(32U, copiedRegister->GetEffectiveBitWidth()); + EXPECT_EQ(0x40000004U, copiedRegister->GetAbsoluteAddress()); + } + else { + EXPECT_TRUE(copiedCluster->GetChildren().empty()); + } + } +} + TEST_F(SvdDimensionBoundsTest, IndexBudgetIsSharedBySiblingDimensions) { SvdItem root(nullptr); SvdRegister first(&root), second(&root), third(&root); @@ -239,6 +374,92 @@ TEST_F(SvdDimensionBoundsTest, ClusterAndPeripheralAddressWrapIsRejected) { EXPECT_TRUE(peripheralDim->GetChildren().empty()); } +TEST_F(SvdDimensionBoundsTest, FieldDimensionsRejectInvalidCountsAndAddressWrapBeforeCopying) { + for(const bool invalidCount : {false, true}) { + SCOPED_TRACE(invalidCount); + SvdField field(nullptr); + field.SetOffset(invalidCount ? 0 : UINT64_MAX - 1); + field.SetBitWidth(1); + const auto dim = Dimension(field, invalidCount ? 0 : 2, 4); + + EXPECT_FALSE(field.CalculateDim()); + EXPECT_FALSE(field.IsValid()); + EXPECT_TRUE(dim->GetChildren().empty()); + EXPECT_FALSE(field.HasExpansionError()); + } +} + +TEST_F(SvdDimensionBoundsTest, FieldAndRegisterExpansionStopsWhenInheritedMetadataCannotBeCopied) { + for(const bool isField : {false, true}) { + SCOPED_TRACE(isField); + SvdRegister reg(nullptr); + reg.SetBitWidth(32); + reg.SetOffset(0); + SvdFieldContainer fields(®); + SvdField field(&fields); + field.SetOffset(0); + field.SetBitWidth(1); + SvdItem& source = isField ? static_cast(field) : static_cast(reg); + const auto dim = Dimension(source, 2, isField ? 1 : 4); + source.SetDerivedFrom(new SvdDerivedFrom(&source)); + // Two indices and one copied item fit; its inherited metadata does not. + ASSERT_TRUE(source.ReserveExpansion(SvdItem::MAX_EXPANSION_ITEMS - 3)); + + EXPECT_FALSE(source.CalculateDim()); + EXPECT_TRUE(source.HasExpansionError()); + ASSERT_EQ(1U, dim->GetChildCount()); + EXPECT_EQ(nullptr, dim->GetChildren().front()->GetDerivedFrom()); + EXPECT_EQ(nullptr, dim->GetChildren().front()->GetCopiedFrom()); + } +} + +TEST_F(SvdDimensionBoundsTest, PeripheralExpansionPreservesAddressesAndStopsAtTheBudgetBoundary) { + for(const size_t remaining : {size_t{3}, size_t{4}}) { + SCOPED_TRACE(remaining); + SvdPeripheral peripheral(nullptr); + peripheral.SetAddress(0x40000000); + const auto dim = Dimension(peripheral, 2, 0x1000); + ASSERT_TRUE(peripheral.ReserveExpansion(SvdItem::MAX_EXPANSION_ITEMS - remaining)); + + EXPECT_EQ(remaining == 4, peripheral.CalculateDim()); + EXPECT_EQ(remaining == 3, peripheral.HasExpansionError()); + ASSERT_EQ(remaining - 2, dim->GetChildCount()); + const auto first = dynamic_cast(dim->GetChildren().front()); + ASSERT_NE(nullptr, first); + EXPECT_EQ("ITEM0", first->GetName()); + EXPECT_EQ(0x40000000U, first->GetAddress()); + if(remaining == 4) { + const auto last = dynamic_cast(dim->GetChildren().back()); + ASSERT_NE(nullptr, last); + EXPECT_EQ("ITEM1", last->GetName()); + EXPECT_EQ(0x40001000U, last->GetAddress()); + } + } +} + +TEST_F(SvdDimensionBoundsTest, InterruptDimensionPreservesTheLargestInitializedValue) { + SvdPeripheral peripheral(nullptr); + Dimension(peripheral, 2, 4); + SvdInterrupt irq(&peripheral); + irq.SetName("IRQ%s"); + irq.SetValue(UINT32_MAX - 2); + + ASSERT_TRUE(irq.CalculateDim()); + EXPECT_FALSE(irq.HasExpansionError()); + ASSERT_NE(nullptr, irq.GetDimension()); + const auto dim = irq.GetDimension(); + ASSERT_EQ(2U, dim->GetChildCount()); + const auto first = dynamic_cast(dim->GetChildren().front()); + const auto last = dynamic_cast(dim->GetChildren().back()); + ASSERT_NE(nullptr, first); + ASSERT_NE(nullptr, last); + EXPECT_EQ("IRQ0", first->GetName()); + EXPECT_EQ("IRQ1", last->GetName()); + EXPECT_EQ(UINT32_MAX - 2, first->GetValue()); + EXPECT_EQ(UINT32_MAX - 1, last->GetValue()); + EXPECT_FALSE(HasMessage("M250")); +} + TEST_F(SvdDimensionBoundsTest, InterruptDimensionCannotWrapItsValue) { SvdPeripheral peripheral(nullptr); Dimension(peripheral, 3, 4); From 93c33b3380238cc99a0eb28c43818912ceda0480 Mon Sep 17 00:00:00 2001 From: Thorsten de Buhr Date: Thu, 15 Oct 2026 16:34:16 +0200 Subject: [PATCH 05/26] test(svdconv): own detached expression register --- tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp b/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp index 6788942c1..dc9725724 100644 --- a/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp +++ b/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -519,17 +520,18 @@ TEST_F(SvdBitWidthTest, HeaderRejectsInvalidShiftRangesWithoutPartialMacros) { TEST_F(SvdBitWidthTest, SfdRejectsUnsupportedExpressionReferenceOutsideDeviceChildren) { CreateOutputDirectory(); + // The separate owner outlives the device's non-owning expression-reference map. + const auto reg = make_unique(nullptr); SvdDevice device(nullptr); - SvdRegister reg(&device); - reg.SetName("CTRL"); - reg.SetBitWidth(64); - device.GetExpressionRegistersList()["CTRL"] = ® + reg->SetName("CTRL"); + reg->SetBitWidth(64); + device.GetExpressionRegistersList()["CTRL"] = reg.get(); SvdOptions options; FileHeaderInfo info; SfdData sfd(info, options); const auto output = m_outputDirectory / "device.sfd"; - ASSERT_TRUE(reg.IsValid()); + ASSERT_TRUE(reg->IsValid()); ASSERT_TRUE(device.GetChildren().empty()); EXPECT_FALSE(sfd.Create(&device, output.string())); EXPECT_TRUE(HasMessage("M226")); From 2ddda56e8f91335bfba805377f10480cc4982a5d Mon Sep 17 00:00:00 2001 From: Thorsten de Buhr Date: Tue, 20 Oct 2026 11:05:53 +0200 Subject: [PATCH 06/26] fix(svdconv): reject invalid SFD conditions and output failures Propagate SFD file creation failures and validate actual expression operands before opening output. Discard the entire condition when an operand cannot be generated. Add regressions for invalid field and register operands, failed output creation, and stale SFD preservation. Release and UBSan pass 210 active unit tests and 48 integration tests. --- tools/svdconv/SVDGenerator/src/SfdData.cpp | 40 +++++- tools/svdconv/SVDModel/src/SvdCExpression.cpp | 10 +- .../test/integtests/src/SvdConvIntegTests.cpp | 106 ++++++++++++++++ .../test/unittests/src/SvdBitWidthTest.cpp | 117 ++++++++++++++++++ 4 files changed, 264 insertions(+), 9 deletions(-) diff --git a/tools/svdconv/SVDGenerator/src/SfdData.cpp b/tools/svdconv/SVDGenerator/src/SfdData.cpp index d3eab5cf9..1705c5cf1 100644 --- a/tools/svdconv/SVDGenerator/src/SfdData.cpp +++ b/tools/svdconv/SVDGenerator/src/SfdData.cpp @@ -12,6 +12,7 @@ #include "SvdDevice.h" #include "SvdDimension.h" #include "SvdField.h" +#include "SvdPeripheral.h" #include "SvdRegister.h" #include "SvdUtils.h" #include "ErrLog.h" @@ -20,12 +21,8 @@ using namespace std; namespace { -bool CheckSfdBitRanges(SvdItem* item) +bool CheckSfdBitRange(SvdItem* item) { - if(!item->IsValid()) { - return true; - } - const auto reg = dynamic_cast(item); const auto field = dynamic_cast(item); if(reg || field) { @@ -36,6 +33,35 @@ bool CheckSfdBitRanges(SvdItem* item) return false; } } + return true; +} + +bool CheckSfdBitRanges(SvdItem* item) +{ + if(!item->IsValid()) { + return true; + } + if(!CheckSfdBitRange(item)) { + return false; + } + + const auto peripheral = dynamic_cast(item); + const auto condition = peripheral ? peripheral->GetDisableCondition() : nullptr; + if(condition && condition->IsValid()) { + for(const auto& symbol : condition->GetSymbolsList()) { + if(symbol.token.type != SvdCExpressionParser::xme_identi) { + continue; + } + const auto referenced = symbol.svdItem; + if(!referenced || !referenced->IsValid()) { + LogMsg("M226", MSG("Invalid item referenced by an SFD expression: '" + condition->GetText() + "'"), condition->GetLineNumber()); + return false; + } + if(!CheckSfdBitRange(referenced)) { + return false; + } + } + } for(const auto child : item->GetChildren()) { if(!CheckSfdBitRanges(child)) { @@ -94,7 +120,9 @@ bool SfdData::Create(SvdItem *item, const string &fileName) } } - m_fileIo->Create(fileName); + if(!m_fileIo->Create(fileName)) { + return false; + } CreateDevice(device); diff --git a/tools/svdconv/SVDModel/src/SvdCExpression.cpp b/tools/svdconv/SVDModel/src/SvdCExpression.cpp index 909ff4a9a..fe4b85274 100644 --- a/tools/svdconv/SVDModel/src/SvdCExpression.cpp +++ b/tools/svdconv/SVDModel/src/SvdCExpression.cpp @@ -122,11 +122,15 @@ string SvdCExpression::GetExpressionString() for(const auto& symbol : symbols) { if(symbol.token.type == SvdCExpressionParser::xme_identi) { SvdItem* item = symbol.svdItem; - if(!item) { - return SvdUtils::EMPTY_STRING; // skip generating disable condition. Should not be reached, as LinkSymbols() already generates an error + if(!item || !item->IsValid()) { + return SvdUtils::EMPTY_STRING; } - expression += CreateObjectExpression(item); + const auto operand = CreateObjectExpression(item); + if(operand.empty()) { + return SvdUtils::EMPTY_STRING; + } + expression += operand; } else { expression += symbol.token.text; diff --git a/tools/svdconv/test/integtests/src/SvdConvIntegTests.cpp b/tools/svdconv/test/integtests/src/SvdConvIntegTests.cpp index d9fde9f4a..00fa4767d 100644 --- a/tools/svdconv/test/integtests/src/SvdConvIntegTests.cpp +++ b/tools/svdconv/test/integtests/src/SvdConvIntegTests.cpp @@ -329,6 +329,112 @@ TEST_F(SvdConvIntegTests, RejectedSfdDoesNotCompileStaleOutput) { EXPECT_FALSE(RteFsUtils::Exists(directory + "/EnumComboWidth.sfr")); } +TEST_F(SvdConvIntegTests, InvalidExpressionFieldDoesNotCompileStaleOutput) { + string xml; + ASSERT_TRUE(RteFsUtils::ReadFile(SvdConvIntegTestEnv::localtestdata_dir + "/option_n/option_n.svd", xml)); + const auto field = xml.find("CDS"); + ASSERT_NE(string::npos, field); + const string width = "1"; + const auto position = xml.find(width, field); + ASSERT_NE(string::npos, position); + ASSERT_LT(position, xml.find("", field)); + xml.replace(position, width.size(), "0"); + + const string directory = SvdConvIntegTestEnv::testoutput_dir + "/invalidExpressionField"; + ASSERT_TRUE(RteFsUtils::CreateDirectories(directory)); + const string input = directory + "/option_n.svd"; + const string output = directory + "/option_n.sfd"; + const string previousOutput = "Previous SFD output\n"; + { + ofstream file(input); + file << xml; + ASSERT_TRUE(file.good()); + } + { + ofstream file(output, ios::binary); + file << previousOutput; + ASSERT_TRUE(file.good()); + } + + Arguments args("SVDConv.exe", input); + args.add({"-o", directory, "--generate=sfr", "--nocleanup", "--verbose"}); + SvdConv converter; + EXPECT_EQ(2, converter.Check(args, args, nullptr)); + string messages; + for(const auto& message : ErrLog::Get()->GetLogMessages()) { messages += message; } + EXPECT_NE(string::npos, messages.find("M310")); + EXPECT_NE(string::npos, messages.find("M226")); + EXPECT_NE(string::npos, messages.find("M111")); + EXPECT_EQ(string::npos, messages.find("Generate System Viewer SFR File")); + EXPECT_EQ(string::npos, messages.find("M133")); + EXPECT_EQ(string::npos, messages.find("SfrCC2")); + string retained; + ASSERT_TRUE(RteFsUtils::ReadFile(output, retained)); + EXPECT_EQ(previousOutput, retained); + EXPECT_FALSE(RteFsUtils::Exists(directory + "/option_n.sfr")); +} + +TEST_F(SvdConvIntegTests, FailedSfdCreationDoesNotCompileOutput) { + const string input = SvdConvIntegTestEnv::localtestdata_dir + "/option_n/option_n.svd"; + const string directory = SvdConvIntegTestEnv::testoutput_dir + "/failedSfdCreation"; + const string output = directory + "/option_n.sfd"; + ASSERT_TRUE(RteFsUtils::CreateDirectories(output)); + + Arguments args("SVDConv.exe", input); + args.add({"-o", directory, "--generate=sfr", "--nocleanup", "--verbose"}); + SvdConv converter; + EXPECT_EQ(2, converter.Check(args, args, nullptr)); + string messages; + for(const auto& message : ErrLog::Get()->GetLogMessages()) { messages += message; } + EXPECT_NE(string::npos, messages.find("M130")); + EXPECT_NE(string::npos, messages.find("M111")); + EXPECT_EQ(string::npos, messages.find("Generate System Viewer SFR File")); + EXPECT_EQ(string::npos, messages.find("M133")); + EXPECT_EQ(string::npos, messages.find("SfrCC2")); + EXPECT_TRUE(RteFsUtils::IsDirectory(output)); + EXPECT_FALSE(RteFsUtils::Exists(directory + "/option_n.sfr")); +} + +TEST_F(SvdConvIntegTests, ReadOnlySfdDoesNotCompileStaleOutput) { + const string input = SvdConvIntegTestEnv::localtestdata_dir + "/option_n/option_n.svd"; + const string directory = SvdConvIntegTestEnv::testoutput_dir + "/readOnlySfd"; + const string output = directory + "/option_n.sfd"; + const string previousOutput = "Previous SFD output\n"; + ASSERT_TRUE(RteFsUtils::CreateDirectories(directory)); + { + ofstream file(output, ios::binary); + file << previousOutput; + ASSERT_TRUE(file.good()); + } + ASSERT_TRUE(RteFsUtils::SetFileReadOnly(output, true)); + { + ofstream probe(output, ios::binary | ios::app); + if(probe.good()) { + probe.close(); + EXPECT_TRUE(RteFsUtils::SetFileReadOnly(output, false)); + GTEST_SKIP() << "The current process can write read-only files"; + } + } + + Arguments args("SVDConv.exe", input); + args.add({"-o", directory, "--generate=sfr", "--nocleanup", "--verbose"}); + SvdConv converter; + const auto result = converter.Check(args, args, nullptr); + EXPECT_TRUE(RteFsUtils::SetFileReadOnly(output, false)); + EXPECT_EQ(2, result); + string messages; + for(const auto& message : ErrLog::Get()->GetLogMessages()) { messages += message; } + EXPECT_NE(string::npos, messages.find("M130")); + EXPECT_NE(string::npos, messages.find("M111")); + EXPECT_EQ(string::npos, messages.find("Generate System Viewer SFR File")); + EXPECT_EQ(string::npos, messages.find("M133")); + EXPECT_EQ(string::npos, messages.find("SfrCC2")); + string retained; + ASSERT_TRUE(RteFsUtils::ReadFile(output, retained)); + EXPECT_EQ(previousOutput, retained); + EXPECT_FALSE(RteFsUtils::Exists(directory + "/option_n.sfr")); +} + #ifndef _WIN32 TEST_F(SvdConvIntegTests, CheckSfrUnsupportedPlatform) { const string inFile = SvdConvIntegTestEnv::localtestdata_dir + "/option_n/option_n.svd"; diff --git a/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp b/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp index dc9725724..0a3fe8da7 100644 --- a/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp +++ b/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp @@ -9,6 +9,7 @@ #include "SvdDimension.h" #include "SvdEnum.h" #include "SvdField.h" +#include "SvdPeripheral.h" #include "SvdRegister.h" #include "HeaderData.h" #include "SfdData.h" @@ -25,6 +26,7 @@ #include #include #include +#include #include using namespace std; @@ -78,6 +80,7 @@ class SvdBitWidthTest : public testing::Test, public IErrConsumer { error_code ec; filesystem::remove(m_outputDirectory / "masks.h", ec); filesystem::remove(m_outputDirectory / "device.sfd", ec); + filesystem::remove(m_outputDirectory / "input.svd", ec); filesystem::remove(m_outputDirectory, ec); } } @@ -159,6 +162,23 @@ class SvdBitWidthTest : public testing::Test, public IErrConsumer { return field; } + static pair AddExpressionRegister(SvdDevice& device) { + const auto peripherals = new SvdPeripheralContainer(&device); + device.AddItem(peripherals); + const auto peripheral = new SvdPeripheral(peripherals); + peripherals->AddItem(peripheral); + peripheral->SetName("TEST"); + const auto registers = new SvdRegisterContainer(peripheral); + peripheral->AddItem(registers); + const auto reg = new SvdRegister(registers); + registers->AddItem(reg); + reg->SetName("CTRL"); + reg->SetBitWidth(32); + reg->SetOffset(0); + reg->SetAccess(SvdTypes::Access::READWRITE); + return {peripheral, reg}; + } + vector m_messages; filesystem::path m_outputDirectory; @@ -430,6 +450,103 @@ TEST_F(SvdBitWidthTest, FieldExpressionsUse64BitLiteralsOnlyWhenNeeded) { EXPECT_TRUE(expression.CreateFieldExpression(AddUncheckedField(reg, 0, 0)).empty()); } +TEST_F(SvdBitWidthTest, InvalidFieldOperandsRejectTheWholeExpression) { + for(const auto text : {"TEST.CTRL.FIELD == 0", "0 == TEST.CTRL.FIELD", + "TEST.CTRL == 0 || TEST.CTRL.FIELD == 0"}) { + SCOPED_TRACE(text); + SvdDevice device(nullptr); + const auto [peripheral, reg] = AddExpressionRegister(device); + const auto field = AddUncheckedField(*reg, 0, 1); + SvdCExpression expression; + expression.SetText(text); + ASSERT_TRUE(expression.CalcExpression(peripheral)); + ASSERT_FALSE(expression.GetExpressionString().empty()); + + field->SetBitWidth(0); + ASSERT_TRUE(field->IsValid()); + EXPECT_TRUE(expression.GetExpressionString().empty()); + + field->SetBitWidth(1); + field->Invalidate(); + EXPECT_TRUE(expression.GetExpressionString().empty()); + } +} + +TEST_F(SvdBitWidthTest, InvalidRegisterOperandsRejectTheWholeExpression) { + SvdDevice device(nullptr); + const auto [peripheral, reg] = AddExpressionRegister(device); + SvdCExpression expression; + expression.SetText("0 == TEST.CTRL"); + ASSERT_TRUE(expression.CalcExpression(peripheral)); + ASSERT_EQ("0 == TEST_CTRL ", expression.GetExpressionString()); + reg->Invalidate(); + EXPECT_TRUE(expression.GetExpressionString().empty()); +} + +TEST_F(SvdBitWidthTest, SfdChecksActualFieldReferencesBeforeCreatingOutput) { + CreateOutputDirectory(); + for(const bool invalid : {false, true}) { + SCOPED_TRACE(invalid); + m_messages.clear(); + SvdDevice device(nullptr); + const auto [peripheral, reg] = AddExpressionRegister(device); + const auto field = AddUncheckedField(*reg, invalid ? 0 : 32, 1); + if(invalid) { + field->Invalidate(); + } + XMLTreeElement condition(nullptr, "disableCondition"); + condition.SetText("TEST.CTRL.FIELD == 0"); + ASSERT_TRUE(peripheral->ProcessXmlElement(&condition)); + ASSERT_TRUE(peripheral->CalcDisableCondition()); + ASSERT_TRUE(peripheral->GetDisableCondition()->IsValid()); + ASSERT_TRUE(reg->IsValid()); + + SvdOptions options; + FileHeaderInfo info; + SfdData sfd(info, options); + const auto output = m_outputDirectory / "device.sfd"; + EXPECT_FALSE(sfd.Create(&device, output.string())); + EXPECT_TRUE(HasMessage("M226")); + EXPECT_FALSE(filesystem::exists(output)); + } +} + +TEST_F(SvdBitWidthTest, SfdPreservesValidConditionsWithUnreferencedInvalidFields) { + CreateOutputDirectory(); + const auto inputFile = m_outputDirectory / "input.svd"; + { + ofstream input(inputFile); + input << "\n"; + ASSERT_TRUE(input.good()); + } + SvdDevice device(nullptr); + const auto [peripheral, reg] = AddExpressionRegister(device); + AddUncheckedField(*reg, 0, 1); + const auto unused = AddUncheckedField(*reg, 0, 0); + unused->SetName("UNUSED"); + unused->Invalidate(); + XMLTreeElement condition(nullptr, "disableCondition"); + condition.SetText("TEST.CTRL.FIELD == 0"); + ASSERT_TRUE(peripheral->ProcessXmlElement(&condition)); + ASSERT_TRUE(peripheral->CalcDisableCondition()); + ASSERT_TRUE(peripheral->GetDisableCondition()->IsValid()); + EXPECT_EQ("(TEST_CTRL & (0x1 << 0)) == 0 ", peripheral->GetDisableCondition()->GetExpressionString()); + + SvdOptions options; + FileHeaderInfo info; + info.svdFileName = inputFile.string(); + const auto output = m_outputDirectory / "device.sfd"; + { + SfdData sfd(info, options); + EXPECT_TRUE(sfd.Create(&device, output.string())); + } + EXPECT_FALSE(HasMessage("M226")); + ifstream input(output); + ASSERT_TRUE(input.is_open()); + const string text{istreambuf_iterator(input), istreambuf_iterator()}; + EXPECT_NE(string::npos, text.find(" (TEST_CTRL & (0x1 << 0)) == 0")); +} + TEST_F(SvdBitWidthTest, SfdRejectsUnsupported64BitRegisterBeforeCreatingOutput) { CreateOutputDirectory(); SvdDevice device(nullptr); From c4255c66d1b3dd7c9444909efd239fc9a200e81a Mon Sep 17 00:00:00 2001 From: Thorsten de Buhr Date: Tue, 20 Oct 2026 11:49:17 +0200 Subject: [PATCH 07/26] fix(svdconv): keep generated masks within 32 bits Retain checked wide intermediate arithmetic while restoring 32-bit header interfaces and field expressions. Remove the added 64-bit default-mask extension and document the Cortex-M scope. Cover 32-bit output boundaries, rejected wider ranges and full-width write-mask sign extension. Release and UBSan each pass 211 active unit tests and 48 integration tests. --- tools/svdconv/SVDConv/README.md | 5 +- .../svdconv/SVDGenerator/include/HeaderData.h | 2 +- .../SVDGenerator/include/HeaderGenerator.h | 2 +- .../SVDGenerator/src/HeaderData_Field.cpp | 5 +- .../SVDGenerator/src/HeaderData_PosMask.cpp | 8 +- .../SVDGenerator/src/HeaderGenerator.cpp | 15 +-- tools/svdconv/SVDModel/src/SvdCExpression.cpp | 5 +- tools/svdconv/SVDModel/src/SvdRegister.cpp | 4 - .../test/unittests/src/SvdBitWidthTest.cpp | 101 +++++++++++------- 9 files changed, 80 insertions(+), 67 deletions(-) diff --git a/tools/svdconv/SVDConv/README.md b/tools/svdconv/SVDConv/README.md index 4e62beeae..2c2abb2c9 100644 --- a/tools/svdconv/SVDConv/README.md +++ b/tools/svdconv/SVDConv/README.md @@ -48,8 +48,9 @@ due to backward compatibility reasons. All newly developed/updated SVD files sho in the same directory as `svdconv.exe`. On Linux and macOS, SVDConv reports error M133 and returns exit code 2. The intermediate `.sfd` file may still be generated. Use `--generate=sfd` to generate SFD output on any supported platform. -SFD/SFR output supports register and field ranges within 32 bits. Wider ranges report M226 before SFD file creation; -SFR compilation is skipped when SFD generation fails. Header masks support valid ranges up to 64 bits. +SVDConv supports Cortex-M devices with register and field ranges within 32 bits. Wider intermediate arithmetic +prevents overflow; it does not add support for 64-bit input or output. SFD/SFR rejects wider ranges with M226 before +SFD file creation. SFR compilation is skipped when SFD generation fails. Dimension expansion has a cumulative limit of 100,000 generated indices and model copies per input file. Nested arrays, inherited content, and repeated expansion share this limit. Exceeding it reports M249 and stops diff --git a/tools/svdconv/SVDGenerator/include/HeaderData.h b/tools/svdconv/SVDGenerator/include/HeaderData.h index 4ec541726..ea5702149 100644 --- a/tools/svdconv/SVDGenerator/include/HeaderData.h +++ b/tools/svdconv/SVDGenerator/include/HeaderData.h @@ -19,7 +19,7 @@ #include struct ONESTRUCT { - uint64_t mask; + uint32_t mask; std::map fields; ONESTRUCT() : mask(0) {} diff --git a/tools/svdconv/SVDGenerator/include/HeaderGenerator.h b/tools/svdconv/SVDGenerator/include/HeaderGenerator.h index beeb01e7a..b723b0b6e 100644 --- a/tools/svdconv/SVDGenerator/include/HeaderGenerator.h +++ b/tools/svdconv/SVDGenerator/include/HeaderGenerator.h @@ -74,7 +74,7 @@ class HeaderGenerator : public CodeGenerator { void MakeEnumStructUnionEndArray (const std::string& textBuf, c_header::Index elementIndex, c_header::Special specialType, c_header::Additional langAdditional, uint32_t num); void MakeFieldPosMask (const std::string& field, const std::string& peri, const std::string& reg, uint32_t pos, uint32_t mask); void MakeFieldPosMask2 (const std::string& field, const std::string& peri, const std::string& reg, uint32_t pos, uint32_t mask); - void MakeFieldPosMask3 (const std::string& name, const std::string& field, uint32_t pos, uint64_t mask); + void MakeFieldPosMask3 (const std::string& name, const std::string& field, uint32_t pos, uint32_t mask); void CreateDoxyComment (const std::string& str); void CreateDoxyCommentStar (const std::string& str); void CreateDoxyCommentLineBreak (const std::string& str); diff --git a/tools/svdconv/SVDGenerator/src/HeaderData_Field.cpp b/tools/svdconv/SVDGenerator/src/HeaderData_Field.cpp index 64e6322ed..ff50c6adf 100644 --- a/tools/svdconv/SVDGenerator/src/HeaderData_Field.cpp +++ b/tools/svdconv/SVDGenerator/src/HeaderData_Field.cpp @@ -77,10 +77,11 @@ bool HeaderData::AddFields(SvdItem* container, FIELDMAPLIST& sortedFields) const auto name = field->GetNameCalculated(); const auto offs = field->GetOffset(); uint32_t width = (uint32_t) field->GetBitWidth(); - const auto mask = SvdUtils::CreateBitMask(width, offs); - if(!mask) { + const auto wideMask = SvdUtils::CreateBitMask(width, offs); + if(!wideMask || wideMask > UINT32_MAX) { continue; } + const auto mask = static_cast(wideMask); ONESTRUCT* oneStruct = nullptr; for(auto& oneStr : sortedFields) { diff --git a/tools/svdconv/SVDGenerator/src/HeaderData_PosMask.cpp b/tools/svdconv/SVDGenerator/src/HeaderData_PosMask.cpp index 8a721f496..1f9255b73 100644 --- a/tools/svdconv/SVDGenerator/src/HeaderData_PosMask.cpp +++ b/tools/svdconv/SVDGenerator/src/HeaderData_PosMask.cpp @@ -218,12 +218,14 @@ bool HeaderData::CreateFieldPosMask(SvdField* field, PosMaskNames *posMaskNames) const auto& alternateGroup = posMaskNames->alternate; const auto name = field->GetNameCalculated(); const auto fieldName = field->GetHierarchicalNameResulting(); - uint32_t firstBit = (uint32_t)field->GetOffset(); + const auto offset = field->GetOffset(); uint32_t bitWidth = field->GetEffectiveBitWidth(); - const auto bitMaxNum = SvdUtils::CreateBitMask(bitWidth); - if(!SvdUtils::CreateBitMask(bitWidth, field->GetOffset())) { + const auto fieldMask = SvdUtils::CreateBitMask(bitWidth, offset); + if(!fieldMask || fieldMask > UINT32_MAX) { return false; } + const auto firstBit = static_cast(offset); + const auto bitMaxNum = static_cast(SvdUtils::CreateBitMask(bitWidth)); if(!alternateGroup.empty()) m_gen->Generate("%s_%s", name.c_str(), firstBit, bitMaxNum, fieldName.c_str(), alternateGroup.c_str()); diff --git a/tools/svdconv/SVDGenerator/src/HeaderGenerator.cpp b/tools/svdconv/SVDGenerator/src/HeaderGenerator.cpp index 7fafa5e86..aefe67721 100644 --- a/tools/svdconv/SVDGenerator/src/HeaderGenerator.cpp +++ b/tools/svdconv/SVDGenerator/src/HeaderGenerator.cpp @@ -804,10 +804,10 @@ void HeaderGenerator::MakeFieldPosMask2(const std::string& field, const std::str } } -void HeaderGenerator::MakeFieldPosMask3(const std::string& pName, const std::string& field, uint32_t pos, uint64_t mask) +void HeaderGenerator::MakeFieldPosMask3(const std::string& pName, const std::string& field, uint32_t pos, uint32_t mask) { - if(pos >= 64 || mask > (UINT64_MAX >> pos)) { - LogMsg("M220", MSG("Field mask exceeds 64 bits")); + if(pos >= 32 || mask > (UINT32_MAX >> pos)) { + LogMsg("M220", MSG("Field mask exceeds 32 bits")); return; } @@ -824,16 +824,11 @@ void HeaderGenerator::MakeFieldPosMask3(const std::string& pName, const std::str const auto calcMask = mask << pos; Generate("#define %s%s ", pName, m_defineMaskStr); GenerateH_CRTabOffset(HEADER_POSMAK_VAL_OFFSET); - if(calcMask <= UINT32_MAX) { - Generate("(0x%xUL) ", static_cast(calcMask)); - } - else { - Generate("(0x%llxULL) ", static_cast(calcMask)); - } + Generate("(0x%xUL) ", calcMask); // Doxy Comment GenerateH_CRTabOffset(HEADER_POSMAK_COMMENT_OFFSET2); - Generate("%s (Bitfield-Mask: 0x%02llx)", field, static_cast(mask)); + Generate("%s (Bitfield-Mask: 0x%02x)", field, mask); GenerateH_CRTabOffset(HEADER_POSMAK_COMMENTEND_OFFSET2); Generate(""); diff --git a/tools/svdconv/SVDModel/src/SvdCExpression.cpp b/tools/svdconv/SVDModel/src/SvdCExpression.cpp index fe4b85274..b995c6da9 100644 --- a/tools/svdconv/SVDModel/src/SvdCExpression.cpp +++ b/tools/svdconv/SVDModel/src/SvdCExpression.cpp @@ -181,16 +181,13 @@ string SvdCExpression::CreateFieldExpression(SvdItem* item) const auto firstBit = field->GetOffset(); const auto bitWidth = field->GetEffectiveBitWidth(); const auto fieldMask = SvdUtils::CreateBitMask(bitWidth, firstBit); - if(!field->IsValid() || !fieldMask) { + if(!field->IsValid() || !fieldMask || fieldMask > UINT32_MAX) { return SvdUtils::EMPTY_STRING; } const auto bitMaxNum = SvdUtils::CreateBitMask(bitWidth); std::stringstream ss; ss << "0x" << std::hex << bitMaxNum; - if(fieldMask > UINT32_MAX) { - ss << "ULL"; - } string expression; expression += "("; diff --git a/tools/svdconv/SVDModel/src/SvdRegister.cpp b/tools/svdconv/SVDModel/src/SvdRegister.cpp index cec992458..228f5b61b 100644 --- a/tools/svdconv/SVDModel/src/SvdRegister.cpp +++ b/tools/svdconv/SVDModel/src/SvdRegister.cpp @@ -440,10 +440,6 @@ bool SvdRegister::CalcAccessMask() m_accessMaskValid = true; return false; } - if(regWidth == 64) { - m_accessMaskRead = m_accessMaskWrite = regMask; - } - const auto fields = GetFieldContainer(); if(!fields) { return true; diff --git a/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp b/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp index 0a3fe8da7..f4f354e7d 100644 --- a/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp +++ b/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp @@ -189,6 +189,8 @@ class SvdBitWidthTest : public testing::Test, public IErrConsumer { int m_previousWarnings = 0; }; +// The model retains preliminary 64-bit support. These arithmetic checks do not +// imply support for 64-bit input or generated output. TEST_F(SvdBitWidthTest, FullWidthFieldsPreserveEnumsAndAccessMasks) { for(const auto width : {32, 64}) { SCOPED_TRACE(width); @@ -238,6 +240,19 @@ TEST_F(SvdBitWidthTest, UpperBoundaryFieldsProduceOnlyTheirOwnWriteBit) { } } +TEST_F(SvdBitWidthTest, FullWidth32BitWriteMaskDoesNotSignExtend) { + XMLTreeElement element(nullptr, "register"); + const auto fields = ConfigureRegister(element, "32"); + AddField(fields, "FULL", "0", "32"); + SvdRegister reg(nullptr); + + ASSERT_TRUE(reg.Construct(&element)); + ASSERT_TRUE(reg.IsValid()); + EXPECT_EQ(uint64_t{UINT32_MAX}, reg.GetAccessMask()); + EXPECT_EQ(uint64_t{UINT32_MAX}, reg.GetAccessMaskRead()); + EXPECT_EQ(uint64_t{UINT32_MAX}, reg.GetAccessMaskWrite()); +} + TEST_F(SvdBitWidthTest, InvalidFieldWidthsAreRejectedBeforeEnumChecks) { struct WidthCase { const char* width; @@ -367,11 +382,11 @@ TEST_F(SvdBitWidthTest, EnumValuesAboveTheFieldWidthRemainInvalid) { EXPECT_TRUE(HasMessage("M335")); } -TEST_F(SvdBitWidthTest, FullWidthRegisterWithoutFieldsPreservesAllAccessBits) { +TEST_F(SvdBitWidthTest, FullWidth32BitRegisterWithoutFieldsPreservesAllAccessBits) { SvdRegister reg(nullptr); - reg.SetBitWidth(64); - EXPECT_EQ(UINT64_MAX, reg.GetAccessMaskRead()); - EXPECT_EQ(UINT64_MAX, reg.GetAccessMaskWrite()); + reg.SetBitWidth(32); + EXPECT_EQ(uint64_t{UINT32_MAX}, reg.GetAccessMaskRead()); + EXPECT_EQ(uint64_t{UINT32_MAX}, reg.GetAccessMaskWrite()); } TEST_F(SvdBitWidthTest, FieldValidationHandlesMissingRegisterAncestors) { @@ -391,63 +406,65 @@ TEST_F(SvdBitWidthTest, HeaderFieldGroupsDetectFullWidthAndUpperBitOverlaps) { SvdOptions options; FileHeaderInfo info; BitWidthHeaderData header(info, options); - for(const auto width : {32, 64}) { - SCOPED_TRACE(width); - SvdRegister reg(nullptr); - reg.SetBitWidth(width); - const auto full = AddUncheckedField(reg, 0, width); - const auto upper = AddUncheckedField(reg, width - 1, 1); - FIELDMAPLIST groups; - ASSERT_TRUE(header.AddFields(reg.GetFieldContainer(), groups)); - ASSERT_EQ(2U, groups.size()); - EXPECT_EQ(width == 64 ? UINT64_MAX : uint64_t{UINT32_MAX}, groups.front().mask); - EXPECT_EQ(full, groups.front().fields.at(0)); - EXPECT_EQ(uint64_t{1} << (width - 1), groups.back().mask); - EXPECT_EQ(upper, groups.back().fields.at(width - 1)); - } + SvdRegister reg(nullptr); + reg.SetBitWidth(32); + const auto full = AddUncheckedField(reg, 0, 32); + const auto upper = AddUncheckedField(reg, 31, 1); + FIELDMAPLIST groups; + ASSERT_TRUE(header.AddFields(reg.GetFieldContainer(), groups)); + ASSERT_EQ(2U, groups.size()); + EXPECT_EQ(UINT32_MAX, groups.front().mask); + EXPECT_EQ(full, groups.front().fields.at(0)); + EXPECT_EQ(UINT32_C(0x80000000), groups.back().mask); + EXPECT_EQ(upper, groups.back().fields.at(31)); } -TEST_F(SvdBitWidthTest, HeaderMasksPreserve32BitTextAndRepresent64BitBoundaries) { +TEST_F(SvdBitWidthTest, HeaderMasksPreserve32BitBoundaryTextAndRejectWiderRanges) { CreateOutputDirectory(); FileIo output; output.SetFileName((m_outputDirectory / "masks.h").string()); BitWidthHeaderGenerator generator(&output); generator.MakeFieldPosMask3("FULL32", "Full 32-bit field", 0, UINT32_MAX); generator.MakeFieldPosMask3("TOP32", "Top 32-bit field", 31, 1); - generator.MakeFieldPosMask3("FULL64", "Full 64-bit field", 0, UINT64_MAX); - generator.MakeFieldPosMask3("TOP64", "Top 64-bit field", 63, 1); ASSERT_TRUE(output.Flush()); ifstream input(m_outputDirectory / "masks.h"); ASSERT_TRUE(input.is_open()); const string text{istreambuf_iterator(input), istreambuf_iterator()}; EXPECT_EQ("(0xffffffffUL)", DefineValue(text, "FULL32_Msk")); EXPECT_EQ("(0x80000000UL)", DefineValue(text, "TOP32_Msk")); - EXPECT_EQ("(0xffffffffffffffffULL)", DefineValue(text, "FULL64_Msk")); - EXPECT_EQ("(0x8000000000000000ULL)", DefineValue(text, "TOP64_Msk")); + EXPECT_EQ("(0UL)", DefineValue(text, "FULL32_Pos")); + EXPECT_EQ("(31UL)", DefineValue(text, "TOP32_Pos")); SvdOptions options; FileHeaderInfo info; BitWidthHeaderData header(info, options); SvdRegister reg(nullptr); - reg.SetBitWidth(64); - const auto full = AddUncheckedField(reg, 0, 64); + reg.SetBitWidth(32); + const auto full = AddUncheckedField(reg, 0, 32); PosMaskNames names; EXPECT_TRUE(header.CreateFieldPosMask(full, &names)); EXPECT_FALSE(HasMessage("M220")); - EXPECT_FALSE(header.CreateFieldPosMask(AddUncheckedField(reg, 63, 2), &names)); + EXPECT_FALSE(header.CreateFieldPosMask(AddUncheckedField(reg, 31, 2), &names)); + EXPECT_FALSE(header.CreateFieldPosMask(AddUncheckedField(reg, 0, 33), &names)); + EXPECT_FALSE(header.CreateFieldPosMask(AddUncheckedField(reg, 0, 64), &names)); + EXPECT_FALSE(header.CreateFieldPosMask(AddUncheckedField(reg, UINT64_C(0x100000000), 1), &names)); } -TEST_F(SvdBitWidthTest, FieldExpressionsUse64BitLiteralsOnlyWhenNeeded) { +TEST_F(SvdBitWidthTest, FieldExpressionsPreserve32BitBoundaryTextAndRejectWiderRanges) { SvdRegister reg(nullptr); reg.SetName("CTRL"); - reg.SetBitWidth(64); + reg.SetBitWidth(32); SvdCExpression expression; EXPECT_EQ("(CTRL & (0xffffffff << 0))", expression.CreateFieldExpression(AddUncheckedField(reg, 0, 32))); - EXPECT_EQ("(CTRL & (0xffffffffffffffffULL << 0))", expression.CreateFieldExpression(AddUncheckedField(reg, 0, 64))); - EXPECT_EQ("(CTRL & (0x1ULL << 63))", expression.CreateFieldExpression(AddUncheckedField(reg, 63, 1))); + EXPECT_EQ("(CTRL & (0x1 << 31))", expression.CreateFieldExpression(AddUncheckedField(reg, 31, 1))); + EXPECT_TRUE(expression.CreateFieldExpression(AddUncheckedField(reg, 31, 2)).empty()); + EXPECT_TRUE(expression.CreateFieldExpression(AddUncheckedField(reg, 32, 1)).empty()); + EXPECT_TRUE(expression.CreateFieldExpression(AddUncheckedField(reg, 0, 33)).empty()); + EXPECT_TRUE(expression.CreateFieldExpression(AddUncheckedField(reg, 0, 64)).empty()); EXPECT_TRUE(expression.CreateFieldExpression(AddUncheckedField(reg, 64, 1)).empty()); EXPECT_TRUE(expression.CreateFieldExpression(AddUncheckedField(reg, 0, 65)).empty()); EXPECT_TRUE(expression.CreateFieldExpression(AddUncheckedField(reg, 0, 0)).empty()); + EXPECT_TRUE(expression.CreateFieldExpression(AddUncheckedField(reg, UINT64_C(0x100000000), 1)).empty()); } TEST_F(SvdBitWidthTest, InvalidFieldOperandsRejectTheWholeExpression) { @@ -600,19 +617,23 @@ TEST_F(SvdBitWidthTest, HeaderFieldGroupsSkipUncheckedInvalidRanges) { FileHeaderInfo info; BitWidthHeaderData header(info, options); SvdRegister reg(nullptr); - reg.SetBitWidth(64); + reg.SetBitWidth(32); AddUncheckedField(reg, 0, 0); + AddUncheckedField(reg, 0, 33); + AddUncheckedField(reg, 0, 64); AddUncheckedField(reg, 0, 65); + AddUncheckedField(reg, 32, 1); AddUncheckedField(reg, 64, 1); - AddUncheckedField(reg, 63, 2); - const auto upper = AddUncheckedField(reg, 63, 1); + AddUncheckedField(reg, 31, 2); + AddUncheckedField(reg, UINT64_C(0x100000000), 1); + const auto upper = AddUncheckedField(reg, 31, 1); FIELDMAPLIST groups; ASSERT_TRUE(header.AddFields(reg.GetFieldContainer(), groups)); ASSERT_EQ(1U, groups.size()); - EXPECT_EQ(uint64_t{1} << 63, groups.front().mask); + EXPECT_EQ(UINT32_C(0x80000000), groups.front().mask); ASSERT_EQ(1U, groups.front().fields.size()); - EXPECT_EQ(upper, groups.front().fields.at(63)); + EXPECT_EQ(upper, groups.front().fields.at(31)); } TEST_F(SvdBitWidthTest, HeaderRejectsInvalidShiftRangesWithoutPartialMacros) { @@ -620,10 +641,10 @@ TEST_F(SvdBitWidthTest, HeaderRejectsInvalidShiftRangesWithoutPartialMacros) { FileIo output; output.SetFileName((m_outputDirectory / "masks.h").string()); BitWidthHeaderGenerator generator(&output); - generator.MakeFieldPosMask3("INVALID_POSITION", "Position outside register", 64, 1); - generator.MakeFieldPosMask3("INVALID_MASK", "Mask crosses register boundary", 63, 3); + generator.MakeFieldPosMask3("INVALID_POSITION", "Position outside register", 32, 1); + generator.MakeFieldPosMask3("INVALID_MASK", "Mask crosses register boundary", 31, 3); EXPECT_EQ(2, count(m_messages.begin(), m_messages.end(), "M220")); - generator.MakeFieldPosMask3("VALID", "Top 64-bit field", 63, 1); + generator.MakeFieldPosMask3("VALID", "Top 32-bit field", 31, 1); ASSERT_TRUE(output.Flush()); ifstream input(m_outputDirectory / "masks.h"); @@ -631,8 +652,8 @@ TEST_F(SvdBitWidthTest, HeaderRejectsInvalidShiftRangesWithoutPartialMacros) { const string text{istreambuf_iterator(input), istreambuf_iterator()}; EXPECT_EQ(string::npos, text.find("INVALID_POSITION")); EXPECT_EQ(string::npos, text.find("INVALID_MASK")); - EXPECT_EQ("(63UL)", DefineValue(text, "VALID_Pos")); - EXPECT_EQ("(0x8000000000000000ULL)", DefineValue(text, "VALID_Msk")); + EXPECT_EQ("(31UL)", DefineValue(text, "VALID_Pos")); + EXPECT_EQ("(0x80000000UL)", DefineValue(text, "VALID_Msk")); } TEST_F(SvdBitWidthTest, SfdRejectsUnsupportedExpressionReferenceOutsideDeviceChildren) { From 1705825081a8b71e2b8ced674d3ce0ea0b09762a Mon Sep 17 00:00:00 2001 From: Thorsten de Buhr Date: Tue, 20 Oct 2026 12:04:38 +0200 Subject: [PATCH 08/26] fix(svdconv): guard empty containers during symbol lookup Reject references through absent register or field containers before accessing the first child. Invalid inherited fields can leave these containers empty and trigger an end-iterator assertion on MSVC. Add regression coverage for both register and peripheral lookup paths. --- tools/svdconv/SVDModel/include/SvdItem.h | 5 +++- .../test/unittests/src/SvdBitWidthTest.cpp | 28 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/tools/svdconv/SVDModel/include/SvdItem.h b/tools/svdconv/SVDModel/include/SvdItem.h index eb1ecf284..6410a65ab 100644 --- a/tools/svdconv/SVDModel/include/SvdItem.h +++ b/tools/svdconv/SVDModel/include/SvdItem.h @@ -306,7 +306,10 @@ bool SvdItem::GetDeriveItem(T *&item, const std::list& searchName, switch(lv) { case L_Peripheral: case L_Register: - parent = *(childs.begin()); + if(childs.empty()) { + return false; + } + parent = childs.front(); break; default: break; diff --git a/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp b/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp index f4f354e7d..5084ad752 100644 --- a/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp +++ b/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp @@ -500,6 +500,34 @@ TEST_F(SvdBitWidthTest, InvalidRegisterOperandsRejectTheWholeExpression) { EXPECT_TRUE(expression.GetExpressionString().empty()); } +TEST_F(SvdBitWidthTest, ExpressionLookupRejectsRegistersWithoutFieldContainers) { + SvdDevice device(nullptr); + const auto [peripheral, reg] = AddExpressionRegister(device); + ASSERT_TRUE(reg->GetChildren().empty()); + + SvdCExpression expression; + expression.SetText("TEST.CTRL.FIELD == 0"); + EXPECT_FALSE(expression.CalcExpression(peripheral)); + EXPECT_FALSE(expression.IsValid()); + EXPECT_TRUE(HasMessage("M244")); +} + +TEST_F(SvdBitWidthTest, ExpressionLookupRejectsPeripheralsWithoutRegisterContainers) { + SvdDevice device(nullptr); + const auto peripheral = AddExpressionRegister(device).first; + const auto peripherals = peripheral->GetParent(); + const auto empty = new SvdPeripheral(peripherals); + peripherals->AddItem(empty); + empty->SetName("EMPTY"); + ASSERT_TRUE(empty->GetChildren().empty()); + + SvdCExpression expression; + expression.SetText("EMPTY.CTRL == 0"); + EXPECT_FALSE(expression.CalcExpression(peripheral)); + EXPECT_FALSE(expression.IsValid()); + EXPECT_TRUE(HasMessage("M244")); +} + TEST_F(SvdBitWidthTest, SfdChecksActualFieldReferencesBeforeCreatingOutput) { CreateOutputDirectory(); for(const bool invalid : {false, true}) { From 6554ca9efa24d9dffe6222a938bc8416cd030214 Mon Sep 17 00:00:00 2001 From: Thorsten de Buhr Date: Tue, 20 Oct 2026 12:08:49 +0200 Subject: [PATCH 09/26] fix(svdconv): validate field positions before narrowing Check raw model offsets and signed widths against the 32-bit header range before converting them. Insert fields without replacing existing map entries, and cover negative widths, large aliasing offsets, and overlapping field groups. --- .../SVDGenerator/src/HeaderData_Field.cpp | 18 +++++++++++------- .../test/unittests/src/SvdBitWidthTest.cpp | 11 ++++++++++- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/tools/svdconv/SVDGenerator/src/HeaderData_Field.cpp b/tools/svdconv/SVDGenerator/src/HeaderData_Field.cpp index ff50c6adf..eec3b6c04 100644 --- a/tools/svdconv/SVDGenerator/src/HeaderData_Field.cpp +++ b/tools/svdconv/SVDGenerator/src/HeaderData_Field.cpp @@ -74,14 +74,18 @@ bool HeaderData::AddFields(SvdItem* container, FIELDMAPLIST& sortedFields) } } - const auto name = field->GetNameCalculated(); - const auto offs = field->GetOffset(); - uint32_t width = (uint32_t) field->GetBitWidth(); - const auto wideMask = SvdUtils::CreateBitMask(width, offs); - if(!wideMask || wideMask > UINT32_MAX) { + const auto offset = field->GetOffset(); + const auto bitWidth = field->GetBitWidth(); + if(offset >= 32 || bitWidth <= 0 || bitWidth > 32) { continue; } - const auto mask = static_cast(wideMask); + + const auto position = static_cast(offset); + const auto width = static_cast(bitWidth); + if(width > 32 - position) { + continue; + } + const auto mask = static_cast(SvdUtils::CreateBitMask(width, position)); ONESTRUCT* oneStruct = nullptr; for(auto& oneStr : sortedFields) { @@ -97,8 +101,8 @@ bool HeaderData::AddFields(SvdItem* container, FIELDMAPLIST& sortedFields) oneStruct = &*sortedFields.rbegin(); } + oneStruct->fields.emplace(position, field); oneStruct->mask |= mask; - oneStruct->fields[static_cast(offs)] = field; } return true; diff --git a/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp b/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp index 5084ad752..e86a534ae 100644 --- a/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp +++ b/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp @@ -410,12 +410,17 @@ TEST_F(SvdBitWidthTest, HeaderFieldGroupsDetectFullWidthAndUpperBitOverlaps) { reg.SetBitWidth(32); const auto full = AddUncheckedField(reg, 0, 32); const auto upper = AddUncheckedField(reg, 31, 1); + const auto lower = AddUncheckedField(reg, 0, 1); FIELDMAPLIST groups; ASSERT_TRUE(header.AddFields(reg.GetFieldContainer(), groups)); ASSERT_EQ(2U, groups.size()); EXPECT_EQ(UINT32_MAX, groups.front().mask); EXPECT_EQ(full, groups.front().fields.at(0)); - EXPECT_EQ(UINT32_C(0x80000000), groups.back().mask); + EXPECT_EQ(UINT32_C(0x80000001), groups.back().mask); + ASSERT_EQ(2U, groups.back().fields.size()); + EXPECT_EQ(lower, groups.back().fields.begin()->second); + EXPECT_EQ(upper, groups.back().fields.rbegin()->second); + EXPECT_EQ(lower, groups.back().fields.at(0)); EXPECT_EQ(upper, groups.back().fields.at(31)); } @@ -647,6 +652,8 @@ TEST_F(SvdBitWidthTest, HeaderFieldGroupsSkipUncheckedInvalidRanges) { SvdRegister reg(nullptr); reg.SetBitWidth(32); AddUncheckedField(reg, 0, 0); + AddUncheckedField(reg, 0, -1); + AddUncheckedField(reg, 0, INT32_MIN); AddUncheckedField(reg, 0, 33); AddUncheckedField(reg, 0, 64); AddUncheckedField(reg, 0, 65); @@ -654,6 +661,8 @@ TEST_F(SvdBitWidthTest, HeaderFieldGroupsSkipUncheckedInvalidRanges) { AddUncheckedField(reg, 64, 1); AddUncheckedField(reg, 31, 2); AddUncheckedField(reg, UINT64_C(0x100000000), 1); + AddUncheckedField(reg, UINT64_C(0x10000001f), 1); + AddUncheckedField(reg, UINT64_MAX, 1); const auto upper = AddUncheckedField(reg, 31, 1); FIELDMAPLIST groups; From b0f0f7a17e99238893ea71a4de9fcffd66f832a0 Mon Sep 17 00:00:00 2001 From: Thorsten de Buhr Date: Tue, 20 Oct 2026 13:31:02 +0200 Subject: [PATCH 10/26] Potential fix for pull request finding 'CodeQL / Inconsistent nullness check' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp b/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp index e86a534ae..5414035ef 100644 --- a/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp +++ b/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp @@ -521,6 +521,7 @@ TEST_F(SvdBitWidthTest, ExpressionLookupRejectsPeripheralsWithoutRegisterContain SvdDevice device(nullptr); const auto peripheral = AddExpressionRegister(device).first; const auto peripherals = peripheral->GetParent(); + ASSERT_NE(peripherals, nullptr); const auto empty = new SvdPeripheral(peripherals); peripherals->AddItem(empty); empty->SetName("EMPTY"); From a3bb15d73e2d7b2ce96d2e56234ce4b8d82d7b7a Mon Sep 17 00:00:00 2001 From: Thorsten de Buhr Date: Tue, 20 Oct 2026 13:44:36 +0200 Subject: [PATCH 11/26] refactor(svdconv): preserve wide field values through generation Keep field positions and masks wide across grouping, padding, generator interfaces and formatting, while preserving the 32-bit output range. Accept signed widths in the mask helper without narrowing and guard unsigned padding calculations. Remove redundant enum validation and successful 64-bit input assumptions from tests. Restore the already-correct reset-value mask calculation and retain all required expansion and output-failure checks. --- .../svdconv/SVDGenerator/include/HeaderData.h | 8 +- .../SVDGenerator/include/HeaderGenerator.h | 6 +- .../SVDGenerator/include/SfdGenerator.h | 15 +- .../SVDGenerator/src/HeaderData_Field.cpp | 54 ++--- .../SVDGenerator/src/HeaderData_PosMask.cpp | 7 +- .../src/HeaderData_RegStructure.cpp | 6 +- .../SVDGenerator/src/HeaderGenerator.cpp | 19 +- .../SVDGenerator/src/SfdData_SingleItems.cpp | 30 ++- .../svdconv/SVDGenerator/src/SfdGenerator.cpp | 22 +- tools/svdconv/SVDModel/include/SvdUtils.h | 2 +- tools/svdconv/SVDModel/src/SvdRegister.cpp | 6 +- tools/svdconv/SVDModel/src/SvdUtils.cpp | 4 +- .../test/unittests/src/SvdBitWidthTest.cpp | 201 +++++++++--------- .../unittests/src/SvdDimensionBoundsTest.cpp | 4 +- 14 files changed, 194 insertions(+), 190 deletions(-) diff --git a/tools/svdconv/SVDGenerator/include/HeaderData.h b/tools/svdconv/SVDGenerator/include/HeaderData.h index ea5702149..4c7e8d505 100644 --- a/tools/svdconv/SVDGenerator/include/HeaderData.h +++ b/tools/svdconv/SVDGenerator/include/HeaderData.h @@ -19,8 +19,8 @@ #include struct ONESTRUCT { - uint32_t mask; - std::map fields; + uint64_t mask; + std::map fields; ONESTRUCT() : mask(0) {} }; @@ -142,7 +142,7 @@ class HeaderData { void GenerateReserved (int32_t resBytes, uint32_t address, bool bGenerate = true); void GenerateReserved (); void GenerateReservedWidth (int32_t nMany, uint32_t width); - void GenerateReservedField (int32_t resBits, uint32_t regSize); + void GenerateReservedField (uint64_t resBits, uint32_t regSize); void AddReservedBytesLater (int32_t reservedBytes); uint32_t NodeHasChilds (RegTreeNode *node, uint32_t pos); bool NodeHasRegs (RegTreeNode *node); @@ -159,7 +159,7 @@ class HeaderData { void Init_OpenCloseStructUnion (); // HeaderData_Fields - uint32_t CreateField (SvdField* field, uint32_t regSize, uint32_t offsCnt); + uint64_t CreateField (SvdField* field, uint32_t regSize, uint64_t offsCnt); bool CreateSortedFields (const FIELDMAPLIST& sortedFields, uint32_t regSize, std::string structName); bool AddFields (SvdItem* container, FIELDMAPLIST& sortedFields); bool CreateFields (SvdRegister* reg, std::string structName); diff --git a/tools/svdconv/SVDGenerator/include/HeaderGenerator.h b/tools/svdconv/SVDGenerator/include/HeaderGenerator.h index b723b0b6e..5e7c3c9f0 100644 --- a/tools/svdconv/SVDGenerator/include/HeaderGenerator.h +++ b/tools/svdconv/SVDGenerator/include/HeaderGenerator.h @@ -53,7 +53,7 @@ class HeaderGenerator : public CodeGenerator { void MakeDoxyCommentAddress (const std::string& text, uint32_t address); void MakeDoxyCommentNumber (const std::string& text, uint32_t number); void MakeDoxyComment (const std::string& text); - void MakeDoxyCommentBitField (const std::string& text, uint32_t firstBit, uint32_t lastBit); + void MakeDoxyCommentBitField (const std::string& text, uint64_t firstBit, uint64_t lastBit); void MakeDoxyCommentBitPos (const std::string& text, uint32_t pos); void MakeInterruptStruct (const std::string& text, int32_t number, bool lastEnum); void MakeDoxygenAddPeripheral (const std::string& text); @@ -62,7 +62,7 @@ class HeaderGenerator : public CodeGenerator { void MakeAnnonUnionCompiler (const std::string& text, bool bStart); void MakeCMSISConfig (const std::string& text, const std::string& MCUName, const CmsisCfg& cmsisCfg); void MakeRegisterStruct (const std::string& text, SvdTypes::Access, const std::string& dataType, uint32_t size); - void MakeFieldStruct (const std::string& text, SvdTypes::Access, const std::string& dataType, uint32_t size, uint32_t bitWidth); + void MakeFieldStruct (const std::string& text, SvdTypes::Access, const std::string& dataType, uint32_t size, uint64_t bitWidth); void MakeFieldUnion (const std::string& text, SvdTypes::Access accessType, const std::string& dataType, uint32_t size); void MakeEnumValue (const std::string& enumName, uint32_t val, const std::string& name, const std::string& descr); void MakeDefine (const std::string& name, uint32_t val, uint32_t base, uint32_t tabOffs, const std::string& descr); @@ -74,7 +74,7 @@ class HeaderGenerator : public CodeGenerator { void MakeEnumStructUnionEndArray (const std::string& textBuf, c_header::Index elementIndex, c_header::Special specialType, c_header::Additional langAdditional, uint32_t num); void MakeFieldPosMask (const std::string& field, const std::string& peri, const std::string& reg, uint32_t pos, uint32_t mask); void MakeFieldPosMask2 (const std::string& field, const std::string& peri, const std::string& reg, uint32_t pos, uint32_t mask); - void MakeFieldPosMask3 (const std::string& name, const std::string& field, uint32_t pos, uint32_t mask); + void MakeFieldPosMask3 (const std::string& name, const std::string& field, uint64_t pos, uint64_t mask); void CreateDoxyComment (const std::string& str); void CreateDoxyCommentStar (const std::string& str); void CreateDoxyCommentLineBreak (const std::string& str); diff --git a/tools/svdconv/SVDGenerator/include/SfdGenerator.h b/tools/svdconv/SVDGenerator/include/SfdGenerator.h index a5195220a..03170e9fe 100644 --- a/tools/svdconv/SVDGenerator/include/SfdGenerator.h +++ b/tools/svdconv/SVDGenerator/include/SfdGenerator.h @@ -10,6 +10,7 @@ #include "SfdGenAPI.h" #include "FileIo.h" +#include #include #include #include @@ -39,10 +40,10 @@ const std::string LOC_STRING_EDIT2 = "( (%s)((%s >> %i) & 0x%X), ((%s = (%s & (% // Write exp. : RegisterName, RegisterName, (cast), Mask, UpShift, (cast), guiEditSizeString, AND-Operation, UpShift //( (unsigned char)((NVIC_IP0>>16) & 0xFF), ((NVIC_IP0 = (NVIC_IP0 & ~(0xFFUL<<16 )) | ((unsigned int)(Gui_u8:GuiVal & 0xFF) << 16 ) ) )) -const std::string LOC_STRING_EDIT3 = "( (%s)((%s >> %i) & 0x%X), ((%s = (%s & ~(0x%XUL << %i )) | ((%s)(%s:GuiVal & 0x%X) << %i ) ) ))"; +const std::string LOC_STRING_EDIT3 = "( (%s)((%s >> %" PRIu64 ") & 0x%" PRIX64 "), ((%s = (%s & ~(0x%" PRIX64 "UL << %" PRIu64 " )) | ((%s)(%s:GuiVal & 0x%" PRIX64 ") << %" PRIu64 " ) ) ))"; //( (unsigned char)((NVIC_IP0>>16) & 0xFF) -const std::string LOC_STRING_EDIT3_RO = "( (%s)((%s >> %i) & 0x%X) )"; +const std::string LOC_STRING_EDIT3_RO = "( (%s)((%s >> %" PRIu64 ") & 0x%" PRIX64 ") )"; const std::string LOC_STRING_OBIT = "( (%s) %s )"; @@ -94,7 +95,7 @@ class SfdGenerator : public CodeGenerator { void Generate_OptionSelectText(const std::string& text, uint32_t value); // Location - void MakeLocationEdit (const std::string& regName, uint32_t firstBit, uint32_t lastBit, uint32_t accRead, uint32_t accWrite); + void MakeLocationEdit (const std::string& regName, uint64_t firstBit, uint64_t lastBit, uint64_t accRead, uint64_t accWrite); void MakeLocationObit (const std::string& regName, uint32_t bitWidth); // Interrupts @@ -113,15 +114,15 @@ class SfdGenerator : public CodeGenerator { void CreateCItemTextonly (const std::string& text, uint32_t num); void CreateCItem (const std::string& text, uint32_t num); void CreateCItemEnd (const std::string& text, sfd::Index elementIndex); - void CreateOBit (const std::string& text, uint32_t num); + void CreateOBit (const std::string& text, uint64_t num); void CreateOBitNoRange (const std::string& text, uint32_t num); - void CreateORange (const std::string& text, uint32_t num1, uint32_t num2); + void CreateORange (const std::string& text, uint64_t num1, uint64_t num2); void CreateIBit (const std::string& text, uint32_t num); void CreateIBitAddr (const std::string& text, uint32_t num1, uint32_t num2); - void CreateIBitAddrAcc (const std::string& text, uint32_t num1, uint32_t num2, SvdTypes::Access acc); + void CreateIBitAddrAcc (const std::string& text, uint64_t num1, uint32_t num2, SvdTypes::Access acc); void CreateIRange (const std::string& text, uint32_t num1, uint32_t num2); void CreateIRangeAddr (const std::string& text, uint32_t num1, uint32_t num2, uint32_t num3); - void CreateIRangeAddrAcc (const std::string& text, uint32_t num1, uint32_t num2, uint32_t num3, SvdTypes::Access acc); + void CreateIRangeAddrAcc (const std::string& text, uint64_t num1, uint64_t num2, uint32_t num3, SvdTypes::Access acc); void CreateInfoAddr (const std::string& text, uint32_t num1); void CreateTextonly (const std::string& text); diff --git a/tools/svdconv/SVDGenerator/src/HeaderData_Field.cpp b/tools/svdconv/SVDGenerator/src/HeaderData_Field.cpp index eec3b6c04..dad7a1812 100644 --- a/tools/svdconv/SVDGenerator/src/HeaderData_Field.cpp +++ b/tools/svdconv/SVDGenerator/src/HeaderData_Field.cpp @@ -40,7 +40,9 @@ bool HeaderData::CreateFields(SvdRegister* reg, string structName) FIELDMAPLIST sortedFields; AddFields(fieldCont, sortedFields); - CreateSortedFields(sortedFields, regSize, structName); + if(!CreateSortedFields(sortedFields, regSize, structName)) { + return false; + } } return true; @@ -76,16 +78,10 @@ bool HeaderData::AddFields(SvdItem* container, FIELDMAPLIST& sortedFields) const auto offset = field->GetOffset(); const auto bitWidth = field->GetBitWidth(); - if(offset >= 32 || bitWidth <= 0 || bitWidth > 32) { - continue; - } - - const auto position = static_cast(offset); - const auto width = static_cast(bitWidth); - if(width > 32 - position) { + if(offset >= 32 || bitWidth <= 0 || bitWidth > 32 || offset + bitWidth > 32) { continue; } - const auto mask = static_cast(SvdUtils::CreateBitMask(width, position)); + const auto mask = SvdUtils::CreateBitMask(bitWidth, offset); ONESTRUCT* oneStruct = nullptr; for(auto& oneStr : sortedFields) { @@ -101,7 +97,7 @@ bool HeaderData::AddFields(SvdItem* container, FIELDMAPLIST& sortedFields) oneStruct = &*sortedFields.rbegin(); } - oneStruct->fields.emplace(position, field); + oneStruct->fields.emplace(offset, field); oneStruct->mask |= mask; } @@ -112,20 +108,27 @@ bool HeaderData::AddFields(SvdItem* container, FIELDMAPLIST& sortedFields) bool HeaderData::CreateSortedFields(const FIELDMAPLIST& sortedFields, uint32_t regSize, string structName) { m_reservedFieldCnt = 0; + const auto registerBits = uint64_t{regSize} * 8; for(const auto& oneStruct : sortedFields) { - uint32_t offsCnt = 0; + uint64_t offsCnt = 0; m_gen->Generate(""); for(const auto& [newOffs, field] : oneStruct.fields) { - int32_t res = newOffs - offsCnt; - - GenerateReservedField(res, regSize); - offsCnt += res; + if(newOffs < offsCnt || newOffs > registerBits) { + m_gen->Generate("Reserved bits calculation error", -1); + return false; + } + GenerateReservedField(newOffs - offsCnt, regSize); + offsCnt = newOffs; offsCnt += CreateField(field, regSize, offsCnt); } - GenerateReservedField((regSize * 8) - offsCnt, regSize); // fill up (pad) bitfields + if(offsCnt > registerBits) { + m_gen->Generate("Reserved bits calculation error", -1); + return false; + } + GenerateReservedField(registerBits - offsCnt, regSize); // fill up (pad) bitfields m_gen->Generate("%s", structName.c_str()); } @@ -133,33 +136,30 @@ bool HeaderData::CreateSortedFields(const FIELDMAPLIST& sortedFields, uint32_t r return true; } -uint32_t HeaderData::CreateField(SvdField* field, uint32_t regSize, uint32_t offsCnt) +uint64_t HeaderData::CreateField(SvdField* field, uint32_t regSize, uint64_t offsCnt) { if(!field) { return false; } - uint32_t sizeNeeded = 0; - const auto& dataType = SvdUtils::GetDataTypeString(regSize); const auto name = field->GetNameCalculated(); const auto descr = field->GetDescriptionCalculated(); const auto accType = field->GetEffectiveAccess(); - uint32_t fieldWidth = field->GetBitWidth(); - uint32_t fieldPos = (uint32_t)field->GetOffset(); + const auto fieldWidth = field->GetBitWidth(); + const auto fieldPos = field->GetOffset(); + if(fieldPos >= 32 || fieldWidth <= 0 || fieldWidth > 32 || fieldPos + fieldWidth > 32) { + return 0; + } m_gen->Generate("%s", accType, dataType.c_str(), regSize, fieldWidth, name.c_str()); - uint32_t lastBit = fieldPos + fieldWidth -1; + const auto lastBit = fieldPos + fieldWidth -1; m_gen->Generate("%s", fieldPos, lastBit, !descr.empty()? descr.c_str() : name.c_str()); - if(sizeNeeded < fieldWidth) { - sizeNeeded = fieldWidth; - } - if(offsCnt != fieldPos) { m_gen->Generate("Reserved bits calculation error", -1); } - return sizeNeeded; + return fieldWidth; } diff --git a/tools/svdconv/SVDGenerator/src/HeaderData_PosMask.cpp b/tools/svdconv/SVDGenerator/src/HeaderData_PosMask.cpp index 1f9255b73..6e1ec5009 100644 --- a/tools/svdconv/SVDGenerator/src/HeaderData_PosMask.cpp +++ b/tools/svdconv/SVDGenerator/src/HeaderData_PosMask.cpp @@ -218,14 +218,13 @@ bool HeaderData::CreateFieldPosMask(SvdField* field, PosMaskNames *posMaskNames) const auto& alternateGroup = posMaskNames->alternate; const auto name = field->GetNameCalculated(); const auto fieldName = field->GetHierarchicalNameResulting(); - const auto offset = field->GetOffset(); + const auto firstBit = field->GetOffset(); uint32_t bitWidth = field->GetEffectiveBitWidth(); - const auto fieldMask = SvdUtils::CreateBitMask(bitWidth, offset); + const auto fieldMask = SvdUtils::CreateBitMask(bitWidth, firstBit); if(!fieldMask || fieldMask > UINT32_MAX) { return false; } - const auto firstBit = static_cast(offset); - const auto bitMaxNum = static_cast(SvdUtils::CreateBitMask(bitWidth)); + const auto bitMaxNum = SvdUtils::CreateBitMask(bitWidth); if(!alternateGroup.empty()) m_gen->Generate("%s_%s", name.c_str(), firstBit, bitMaxNum, fieldName.c_str(), alternateGroup.c_str()); diff --git a/tools/svdconv/SVDGenerator/src/HeaderData_RegStructure.cpp b/tools/svdconv/SVDGenerator/src/HeaderData_RegStructure.cpp index e82b7fb9f..4804bfe16 100644 --- a/tools/svdconv/SVDGenerator/src/HeaderData_RegStructure.cpp +++ b/tools/svdconv/SVDGenerator/src/HeaderData_RegStructure.cpp @@ -464,7 +464,7 @@ void HeaderData::GenerateReservedWidth(int32_t nMany, uint32_t width) } } -void HeaderData::GenerateReservedField(int32_t resBits, uint32_t regSize) +void HeaderData::GenerateReservedField(uint64_t resBits, uint32_t regSize) { if(!resBits) { return; @@ -474,10 +474,6 @@ void HeaderData::GenerateReservedField(int32_t resBits, uint32_t regSize) m_gen->Generate("", SvdTypes::Access::UNDEF, dataType.c_str(), regSize, resBits); // experimental: type and size only - if(resBits < 0) { - m_gen->Generate("Reserved bits calculation negative: %i", -1, resBits); - } - m_reservedFieldCnt++; return; diff --git a/tools/svdconv/SVDGenerator/src/HeaderGenerator.cpp b/tools/svdconv/SVDGenerator/src/HeaderGenerator.cpp index aefe67721..ccc8bf7b4 100644 --- a/tools/svdconv/SVDGenerator/src/HeaderGenerator.cpp +++ b/tools/svdconv/SVDGenerator/src/HeaderGenerator.cpp @@ -11,6 +11,7 @@ #include "ErrLog.h" +#include #include #include #include @@ -287,10 +288,10 @@ void HeaderGenerator::MakeRegisterStruct(const std::string& text, SvdTypes::Acce Generate("%s %s \r\t\t\t\t\t\t\t\t\t\t%s;", ioType, dataType, text.c_str()); } -void HeaderGenerator::MakeFieldStruct(const std::string& text, SvdTypes::Access accessType, const std::string& dataType, uint32_t size, uint32_t bitWidth) +void HeaderGenerator::MakeFieldStruct(const std::string& text, SvdTypes::Access accessType, const std::string& dataType, uint32_t size, uint64_t bitWidth) { const std::string& ioType = SvdTypes::GetAccessTypeIo(accessType); - Generate("%s %s \r\t\t\t\t\t\t\t\t\t\t%s \r\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t: %d;", ioType, dataType, text.c_str(), bitWidth); + Generate("%s %s \r\t\t\t\t\t\t\t\t\t\t%s \r\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t: %" PRIu64 ";", ioType, dataType, text.c_str(), bitWidth); } void HeaderGenerator::MakeFieldUnion(const std::string& text, SvdTypes::Access accessType, const std::string& dataType, uint32_t size) @@ -687,7 +688,7 @@ void HeaderGenerator::MakeDoxyComment(const std::string& text) Generate(""); } -void HeaderGenerator::MakeDoxyCommentBitField(const std::string& text, uint32_t firstBit, uint32_t lastBit) +void HeaderGenerator::MakeDoxyCommentBitField(const std::string& text, uint64_t firstBit, uint64_t lastBit) { uint32_t extraSpaces = m_extraSpaces; @@ -695,7 +696,7 @@ void HeaderGenerator::MakeDoxyCommentBitField(const std::string& text, uint32_t GenerateH_CRTabOffset(HEADER_INLINE_COMMENT_DISTANCE); if(m_debugHeaderfile) { - Generate("[%i..%i] %s", lastBit, firstBit, text.c_str()); + Generate("[%" PRIu64 "..%" PRIu64 "] %s", lastBit, firstBit, text.c_str()); } else { Generate("%s", text.c_str()); @@ -804,7 +805,7 @@ void HeaderGenerator::MakeFieldPosMask2(const std::string& field, const std::str } } -void HeaderGenerator::MakeFieldPosMask3(const std::string& pName, const std::string& field, uint32_t pos, uint32_t mask) +void HeaderGenerator::MakeFieldPosMask3(const std::string& pName, const std::string& field, uint64_t pos, uint64_t mask) { if(pos >= 32 || mask > (UINT32_MAX >> pos)) { LogMsg("M220", MSG("Field mask exceeds 32 bits")); @@ -813,22 +814,22 @@ void HeaderGenerator::MakeFieldPosMask3(const std::string& pName, const std::str Generate("#define %s%s ", pName, m_definePosStr); GenerateH_CRTabOffset(HEADER_POSMAK_VAL_OFFSET); - Generate("(%iUL) ", pos); + Generate("(%" PRIu64 "UL) ", pos); // Doxy Comment GenerateH_CRTabOffset(HEADER_POSMAK_COMMENT_OFFSET2); - Generate("%s (Bit %i)", field, pos); + Generate("%s (Bit %" PRIu64 ")", field, pos); GenerateH_CRTabOffset(HEADER_POSMAK_COMMENTEND_OFFSET2); Generate(""); const auto calcMask = mask << pos; Generate("#define %s%s ", pName, m_defineMaskStr); GenerateH_CRTabOffset(HEADER_POSMAK_VAL_OFFSET); - Generate("(0x%xUL) ", calcMask); + Generate("(0x%" PRIx64 "UL) ", calcMask); // Doxy Comment GenerateH_CRTabOffset(HEADER_POSMAK_COMMENT_OFFSET2); - Generate("%s (Bitfield-Mask: 0x%02x)", field, mask); + Generate("%s (Bitfield-Mask: 0x%02" PRIx64 ")", field, mask); GenerateH_CRTabOffset(HEADER_POSMAK_COMMENTEND_OFFSET2); Generate(""); diff --git a/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp b/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp index 9ef9de2a3..da696c5f7 100644 --- a/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp +++ b/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp @@ -748,8 +748,8 @@ bool SfdData::CreateRegisterItem(SvdRegister* reg) const auto address = (uint32_t)reg->GetAbsoluteAddress(); const auto accessType = reg->GetEffectiveAccess(); auto regWidth = reg->GetEffectiveBitWidth(); - const auto accRead = (uint32_t)reg->GetAccessMaskRead(); - const auto accWrite = (uint32_t)reg->GetAccessMaskWrite(); + const auto accRead = reg->GetAccessMaskRead(); + const auto accWrite = reg->GetAccessMaskWrite(); if(regWidth < 8) { regWidth = 8; @@ -768,7 +768,7 @@ bool SfdData::CreateRegisterItem(SvdRegister* reg) if(cont && cont->IsValid() && cont->GetChildCount()) { list fieldsList; CreateFields(cont, fieldsList); - const auto accMaskReadRtree = (uint32_t) reg->GetAccessMask(); + const auto accMaskReadRtree = reg->GetAccessMask(); CreateItemDescription(reg, "RTree"); m_gen->Generate("SFDITEM_REG__%s", itemName.c_str()); @@ -814,8 +814,8 @@ bool SfdData::CreateRegisterArrayItem(SvdRegister* reg) const auto address = (uint32_t)reg->GetAbsoluteAddress(); const auto accessType = reg->GetEffectiveAccess(); auto regWidth = reg->GetEffectiveBitWidth(); - const auto accRead = (uint32_t)reg->GetAccessMaskRead(); - const auto accWrite = (uint32_t)reg->GetAccessMaskWrite(); + const auto accRead = reg->GetAccessMaskRead(); + const auto accWrite = reg->GetAccessMaskWrite(); if(regWidth < 8) { regWidth = 8; @@ -873,17 +873,15 @@ bool SfdData::CreateFieldItem(SvdField* field) const auto dispName = field->GetDisplayNameCalculated(); const auto fieldDescr = SvdUtils::CheckTextGeneric_SfrCC2(field->GetDescriptionCalculated()); const auto regName = field->GetParentRegisterNameHierarchical(); - const auto offset = field->GetOffset(); - const auto bitWidth = (uint32_t)field->GetEffectiveBitWidth(); + const auto firstBit = field->GetOffset(); + const auto bitWidth = field->GetEffectiveBitWidth(); const auto accessType = field->GetEffectiveAccess(); const auto regAddress = (uint32_t)field->GetAbsoluteAddress(); - const auto fieldMask = SvdUtils::CreateBitMask(bitWidth, offset); + const auto fieldMask = SvdUtils::CreateBitMask(bitWidth, firstBit); if(!fieldMask || fieldMask > UINT32_MAX) { LogMsg("M226", MSG("SFD supports only bit ranges within 32 bits: '" + fieldName + "'"), field->GetLineNumber()); return false; } - const auto firstBit = static_cast(offset); - uint32_t regBitWidth = 32; const auto parent = field->GetParent(); if(parent) { @@ -900,14 +898,14 @@ bool SfdData::CreateFieldItem(SvdField* field) field->GetValuesDescriptionString(valuesDescr); valuesDescr = SvdUtils::CheckTextGeneric_SfrCC2(valuesDescr); - uint32_t lastBit = firstBit + bitWidth -1; - const auto bitMaxNum = static_cast(SvdUtils::CreateBitMask(bitWidth)); + const auto lastBit = firstBit + bitWidth -1; + const auto bitMaxNum = SvdUtils::CreateBitMask(bitWidth); CreateItemDescription(field, "Item"); m_gen->Generate("SFDITEM_FIELD__%s", itemName.c_str()); m_gen->Generate("%s", dispName.c_str()); - uint32_t accRead, accWrite; + uint64_t accRead, accWrite; switch(accessType) { case SvdTypes::Access::READONLY: accRead = bitMaxNum; @@ -1012,11 +1010,6 @@ bool SfdData::CreateEnumValues(const list& conts, SvdField* f bool SfdData::CreateEnumValueSet(SvdEnumContainer *cont, SvdField* field) { - const auto bitWidth = field->GetEffectiveBitWidth(); - if(bitWidth == 0 || bitWidth > MAX_BITWIDTH_FOR_COMBO) { - return false; - } - const auto& childs = cont->GetChildren(); if(childs.empty()) { return true; @@ -1039,6 +1032,7 @@ bool SfdData::CreateEnumValueSet(SvdEnumContainer *cont, SvdField* field) enumValues[val.u32] = enu; } + const auto bitWidth = field->GetEffectiveBitWidth(); const auto bitMaxNum = uint32_t{1} << bitWidth; for(uint32_t bitNum=0; bitNum(" ", items[elementIndex].c_str()); } -void SfdGenerator::CreateOBit(const std::string& text, uint32_t num) +void SfdGenerator::CreateOBit(const std::string& text, uint64_t num) { - Generate(" %s", num, num, text.c_str()); + Generate(" %s", num, num, text.c_str()); } void SfdGenerator::CreateOBitNoRange(const std::string& text, uint32_t num) @@ -440,9 +438,9 @@ void SfdGenerator::CreateOBitNoRange(const std::string& text, uint32_t num) Generate(" %s", num, text.c_str()); } -void SfdGenerator::CreateORange(const std::string& text, uint32_t num1, uint32_t num2) +void SfdGenerator::CreateORange(const std::string& text, uint64_t num1, uint64_t num2) { - Generate(" %s", num1, num2, text.c_str()); + Generate(" %s", num1, num2, text.c_str()); } void SfdGenerator::CreateIBit(const std::string& text, uint32_t num) @@ -455,10 +453,10 @@ void SfdGenerator::CreateIBitAddr(const std::string& text, uint32_t num1, uint32 Generate(" [Bit %i] (@ 0x%08X) %s", num1, num2, text.c_str()); } -void SfdGenerator::CreateIBitAddrAcc(const std::string& text, uint32_t num1, uint32_t num2, SvdTypes::Access acc) +void SfdGenerator::CreateIBitAddrAcc(const std::string& text, uint64_t num1, uint32_t num2, SvdTypes::Access acc) { const std::string &accStr = SvdTypes::GetAccessTypeSfd(acc); - Generate(" [Bit %i] %s (@ 0x%08X) %s", num1, accStr.c_str(), num2, text.c_str()); + Generate(" [Bit %" PRIu64 "] %s (@ 0x%08X) %s", num1, accStr.c_str(), num2, text.c_str()); } void SfdGenerator::CreateIRange(const std::string& text, uint32_t num1, uint32_t num2) @@ -471,10 +469,10 @@ void SfdGenerator::CreateIRangeAddr(const std::string& text, uint32_t num1, uint Generate(" [Bits %i..%i] (@ 0x%08X) %s", num1, num2, num3, text.c_str()); } -void SfdGenerator::CreateIRangeAddrAcc(const std::string& text, uint32_t num1, uint32_t num2, uint32_t num3, SvdTypes::Access acc) +void SfdGenerator::CreateIRangeAddrAcc(const std::string& text, uint64_t num1, uint64_t num2, uint32_t num3, SvdTypes::Access acc) { const std::string &accStr = SvdTypes::GetAccessTypeSfd(acc); - Generate(" [Bits %i..%i] %s (@ 0x%08X) %s", num1, num2, accStr.c_str(), num3, text.c_str()); + Generate(" [Bits %" PRIu64 "..%" PRIu64 "] %s (@ 0x%08X) %s", num1, num2, accStr.c_str(), num3, text.c_str()); } void SfdGenerator::CreateInfoAddr(const std::string& text, uint32_t num1) diff --git a/tools/svdconv/SVDModel/include/SvdUtils.h b/tools/svdconv/SVDModel/include/SvdUtils.h index 76dd3e3e8..437e312e8 100644 --- a/tools/svdconv/SVDModel/include/SvdUtils.h +++ b/tools/svdconv/SVDModel/include/SvdUtils.h @@ -57,7 +57,7 @@ class SvdUtils { static bool ConvertBitRange (const std::string &text, uint32_t& m_offset, uint32_t& m_size); // Returns zero when the bit range is empty or does not fit in 64 bits. - static uint64_t CreateBitMask (uint32_t width, uint64_t offset = 0); + static uint64_t CreateBitMask (int64_t width, uint64_t offset = 0); static bool DoConvertNumberXBin (const std::string &text, uint32_t value, std::set& numbers); static bool ConvertCpuRevision (const std::string &text, uint32_t& rev); static bool ConvertProtectionStringType (const std::string &text, SvdTypes::ProtectionType& protection, uint32_t lineNo); diff --git a/tools/svdconv/SVDModel/src/SvdRegister.cpp b/tools/svdconv/SVDModel/src/SvdRegister.cpp index 228f5b61b..5148db908 100644 --- a/tools/svdconv/SVDModel/src/SvdRegister.cpp +++ b/tools/svdconv/SVDModel/src/SvdRegister.cpp @@ -725,7 +725,11 @@ bool SvdRegister::CheckItem() } // Officially supported register size 1..32 bits. Unofficial "support" (and max value) for 64 bits. - const auto maxRegValue = SvdUtils::CreateBitMask(regWidth); + uint64_t maxRegValue = 0; + if(regWidth < 64) { + maxRegValue = (uint64_t)0x1 << regWidth; + } + maxRegValue -= 1; if(resetValue > maxRegValue) { LogMsg("M382", LEVEL("Register"), NAME(name), NAME2("Reset Value"), HEXNUM(resetValue), NUM(regWidth), lineNo); diff --git a/tools/svdconv/SVDModel/src/SvdUtils.cpp b/tools/svdconv/SVDModel/src/SvdUtils.cpp index d97fc9def..2e17df757 100644 --- a/tools/svdconv/SVDModel/src/SvdUtils.cpp +++ b/tools/svdconv/SVDModel/src/SvdUtils.cpp @@ -446,9 +446,9 @@ bool SvdUtils::ConvertNumber(const string &text, set &numbers) return true; } -uint64_t SvdUtils::CreateBitMask(uint32_t width, uint64_t offset) +uint64_t SvdUtils::CreateBitMask(int64_t width, uint64_t offset) { - if(width == 0 || width > 64 || offset >= 64 || width > 64 - offset) { + if(width <= 0 || width > 64 || offset >= 64 || width + offset > 64) { return 0; } diff --git a/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp b/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp index 5414035ef..3d47202d6 100644 --- a/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp +++ b/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp @@ -11,6 +11,7 @@ #include "SvdField.h" #include "SvdPeripheral.h" #include "SvdRegister.h" +#include "SvdUtils.h" #include "HeaderData.h" #include "SfdData.h" #include "ErrLog.h" @@ -37,6 +38,7 @@ class BitWidthHeaderData : public HeaderData { public: using HeaderData::HeaderData; using HeaderData::AddFields; + using HeaderData::CreateSortedFields; using HeaderData::CreateFieldPosMask; }; @@ -50,7 +52,6 @@ class BitWidthSfdData : public SfdData { public: using SfdData::SfdData; using SfdData::CreateFieldItem; - using SfdData::CreateEnumValueSet; }; class SvdBitWidthTest : public testing::Test, public IErrConsumer { @@ -189,55 +190,63 @@ class SvdBitWidthTest : public testing::Test, public IErrConsumer { int m_previousWarnings = 0; }; -// The model retains preliminary 64-bit support. These arithmetic checks do not -// imply support for 64-bit input or generated output. -TEST_F(SvdBitWidthTest, FullWidthFieldsPreserveEnumsAndAccessMasks) { - for(const auto width : {32, 64}) { +TEST_F(SvdBitWidthTest, BitMasksUseWideIntermediatesWithoutAcceptingInvalidRanges) { + EXPECT_EQ(uint64_t{UINT32_MAX}, SvdUtils::CreateBitMask(32)); + EXPECT_EQ(uint64_t{1} << 31, SvdUtils::CreateBitMask(1, 31)); + // Exercise the arithmetic helper's existing 64-bit preparation, not SVD input support. + EXPECT_EQ(UINT64_MAX, SvdUtils::CreateBitMask(64)); + EXPECT_EQ(uint64_t{1} << 63, SvdUtils::CreateBitMask(1, 63)); + EXPECT_EQ(0U, SvdUtils::CreateBitMask(2, 63)); + EXPECT_EQ(0U, SvdUtils::CreateBitMask(1, UINT64_MAX)); + const int64_t widths[] = {INT64_MIN, -1, 0, 65, int64_t{UINT32_MAX} + 2, INT64_MAX}; + for(const auto width : widths) { SCOPED_TRACE(width); - m_messages.clear(); - XMLTreeElement element(nullptr, "register"); - const auto fields = ConfigureRegister(element, to_string(width)); - const auto fieldElement = AddField(fields, "FULL", "0", to_string(width)); - fieldElement->CreateElement("access", "read-only"); - AddEnum(fieldElement, "4294967295"); - SvdRegister reg(nullptr); + EXPECT_EQ(0U, SvdUtils::CreateBitMask(width)); + } +} - ASSERT_TRUE(reg.Construct(&element)); - ASSERT_TRUE(reg.IsValid()); - ASSERT_NE(nullptr, reg.GetFieldContainer()); - ASSERT_EQ(1U, reg.GetFieldContainer()->GetChildCount()); - const auto field = dynamic_cast(reg.GetFieldContainer()->GetChildren().front()); - ASSERT_NE(nullptr, field); - EXPECT_TRUE(field->IsValid()); - ASSERT_EQ(1U, field->GetEnumContainer().size()); - const auto enumContainer = field->GetEnumContainer().front(); - ASSERT_EQ(1U, enumContainer->GetChildCount()); - EXPECT_TRUE(enumContainer->GetChildren().front()->IsValid()); - EXPECT_FALSE(HasMessage("M335")); +TEST_F(SvdBitWidthTest, FullWidthFieldsPreserveEnumsAndAccessMasks) { + constexpr int32_t width = 32; + XMLTreeElement element(nullptr, "register"); + const auto fields = ConfigureRegister(element, to_string(width)); + const auto fieldElement = AddField(fields, "FULL", "0", to_string(width)); + fieldElement->CreateElement("access", "read-only"); + AddEnum(fieldElement, "4294967295"); + SvdRegister reg(nullptr); - const uint64_t expectedMask = width == 64 ? UINT64_MAX : UINT32_MAX; - EXPECT_EQ(expectedMask, reg.GetAccessMask()); - EXPECT_EQ(expectedMask, reg.GetAccessMaskRead()); - EXPECT_EQ(0U, reg.GetAccessMaskWrite()); - EXPECT_EQ(SvdTypes::Access::READONLY, reg.GetAccessCalculated()); - } + ASSERT_TRUE(reg.Construct(&element)); + ASSERT_TRUE(reg.IsValid()); + ASSERT_NE(nullptr, reg.GetFieldContainer()); + ASSERT_EQ(1U, reg.GetFieldContainer()->GetChildCount()); + const auto field = dynamic_cast(reg.GetFieldContainer()->GetChildren().front()); + ASSERT_NE(nullptr, field); + EXPECT_TRUE(field->IsValid()); + ASSERT_EQ(1U, field->GetEnumContainer().size()); + const auto enumContainer = field->GetEnumContainer().front(); + ASSERT_EQ(1U, enumContainer->GetChildCount()); + EXPECT_TRUE(enumContainer->GetChildren().front()->IsValid()); + EXPECT_FALSE(HasMessage("M335")); + + const uint64_t expectedMask = UINT32_MAX; + EXPECT_EQ(expectedMask, reg.GetAccessMask()); + EXPECT_EQ(expectedMask, reg.GetAccessMaskRead()); + EXPECT_EQ(0U, reg.GetAccessMaskWrite()); + EXPECT_EQ(SvdTypes::Access::READONLY, reg.GetAccessCalculated()); } TEST_F(SvdBitWidthTest, UpperBoundaryFieldsProduceOnlyTheirOwnWriteBit) { - for(const auto width : {32, 64}) { - SCOPED_TRACE(width); - XMLTreeElement element(nullptr, "register"); - const auto fields = ConfigureRegister(element, to_string(width)); - AddField(fields, "TOP", to_string(width - 1), "1"); - SvdRegister reg(nullptr); + constexpr int32_t width = 32; + XMLTreeElement element(nullptr, "register"); + const auto fields = ConfigureRegister(element, to_string(width)); + AddField(fields, "TOP", to_string(width - 1), "1"); + SvdRegister reg(nullptr); - ASSERT_TRUE(reg.Construct(&element)); - ASSERT_TRUE(reg.IsValid()); - ASSERT_NE(nullptr, reg.GetFieldContainer()); - EXPECT_TRUE(reg.GetFieldContainer()->GetChildren().front()->IsValid()); - EXPECT_EQ(uint64_t{1} << (width - 1), reg.GetAccessMaskWrite()); - EXPECT_EQ(width == 64 ? UINT64_MAX : uint64_t{UINT32_MAX}, reg.GetAccessMaskRead()); - } + ASSERT_TRUE(reg.Construct(&element)); + ASSERT_TRUE(reg.IsValid()); + ASSERT_NE(nullptr, reg.GetFieldContainer()); + EXPECT_TRUE(reg.GetFieldContainer()->GetChildren().front()->IsValid()); + EXPECT_EQ(uint64_t{1} << (width - 1), reg.GetAccessMaskWrite()); + EXPECT_EQ(uint64_t{UINT32_MAX}, reg.GetAccessMaskRead()); } TEST_F(SvdBitWidthTest, FullWidth32BitWriteMaskDoesNotSignExtend) { @@ -264,7 +273,7 @@ TEST_F(SvdBitWidthTest, InvalidFieldWidthsAreRejectedBeforeEnumChecks) { SCOPED_TRACE(input.width); m_messages.clear(); XMLTreeElement element(nullptr, "register"); - const auto fields = ConfigureRegister(element, "64"); + const auto fields = ConfigureRegister(element, "32"); AddEnum(AddField(fields, "BAD", "0", input.width), "1"); AddField(fields, "VALID", "1", "1"); SvdRegister reg(nullptr); @@ -288,9 +297,8 @@ TEST_F(SvdBitWidthTest, FieldsOutsideOffsetAndRegisterBoundsAreRejected) { const char* width; const char* diagnostic; }; - for(const auto& input : {RangeCase{"64", "64", "1", "M309"}, - RangeCase{"64", "18446744073709551614", "1", "M309"}, - RangeCase{"64", "63", "2", "M324"}, + for(const auto& input : {RangeCase{"32", "64", "1", "M309"}, + RangeCase{"32", "18446744073709551614", "1", "M309"}, RangeCase{"32", "31", "2", "M324"}}) { SCOPED_TRACE(input.offset); SCOPED_TRACE(input.width); @@ -314,20 +322,20 @@ TEST_F(SvdBitWidthTest, FieldsOutsideOffsetAndRegisterBoundsAreRejected) { TEST_F(SvdBitWidthTest, AccessMaskSkipsInvalidAndUncheckedFieldRanges) { SvdRegister reg(nullptr); - reg.SetBitWidth(64); + reg.SetBitWidth(32); reg.SetAccess(SvdTypes::Access::READWRITE); - AddUncheckedField(reg, 63, 1); + AddUncheckedField(reg, 31, 1); AddUncheckedField(reg, 0, 0); AddUncheckedField(reg, 0, 65); AddUncheckedField(reg, 0, -2); AddUncheckedField(reg, 64, 1); AddUncheckedField(reg, UINT64_MAX - 1, 1); - AddUncheckedField(reg, 63, 2); + AddUncheckedField(reg, 31, 2); AddUncheckedField(reg, 0, 64)->Invalidate(); EXPECT_TRUE(reg.CalcAccessMask()); - EXPECT_EQ(uint64_t{1} << 63, reg.GetAccessMaskWrite()); - EXPECT_EQ(UINT64_MAX, reg.GetAccessMaskRead()); + EXPECT_EQ(uint64_t{1} << 31, reg.GetAccessMaskWrite()); + EXPECT_EQ(uint64_t{UINT32_MAX}, reg.GetAccessMaskRead()); EXPECT_EQ(SvdTypes::Access::READWRITE, reg.GetAccessCalculated()); } @@ -392,7 +400,8 @@ TEST_F(SvdBitWidthTest, FullWidth32BitRegisterWithoutFieldsPreservesAllAccessBit TEST_F(SvdBitWidthTest, FieldValidationHandlesMissingRegisterAncestors) { SvdFieldContainer fields(nullptr); SvdDimension dimension(nullptr); - for(SvdItem* parent : {static_cast(&fields), static_cast(&dimension)}) { + SvdItem* parents[] = {&fields, &dimension}; + for(const auto parent : parents) { SvdField field(parent); field.SetName("BIT"); field.SetOffset(0); @@ -424,6 +433,28 @@ TEST_F(SvdBitWidthTest, HeaderFieldGroupsDetectFullWidthAndUpperBitOverlaps) { EXPECT_EQ(upper, groups.back().fields.at(31)); } +TEST_F(SvdBitWidthTest, HeaderFieldPaddingRejectsOverlapsAndOutOfRangePositions) { + SvdOptions options; + FileHeaderInfo info; + BitWidthHeaderData header(info, options); + SvdRegister reg(nullptr); + reg.SetBitWidth(32); + + FIELDMAPLIST groups(1); + auto& fields = groups.front().fields; + fields.emplace(UINT64_C(0x100000000), AddUncheckedField(reg, UINT64_C(0x100000000), 1)); + EXPECT_FALSE(header.CreateSortedFields(groups, 4, "bits")); + + fields.clear(); + fields.emplace(0, AddUncheckedField(reg, 0, 2)); + fields.emplace(1, AddUncheckedField(reg, 1, 1)); + EXPECT_FALSE(header.CreateSortedFields(groups, 4, "bits")); + + fields.clear(); + fields.emplace(0, AddUncheckedField(reg, 0, 9)); + EXPECT_FALSE(header.CreateSortedFields(groups, 1, "bits")); +} + TEST_F(SvdBitWidthTest, HeaderMasksPreserve32BitBoundaryTextAndRejectWiderRanges) { CreateOutputDirectory(); FileIo output; @@ -681,7 +712,9 @@ TEST_F(SvdBitWidthTest, HeaderRejectsInvalidShiftRangesWithoutPartialMacros) { BitWidthHeaderGenerator generator(&output); generator.MakeFieldPosMask3("INVALID_POSITION", "Position outside register", 32, 1); generator.MakeFieldPosMask3("INVALID_MASK", "Mask crosses register boundary", 31, 3); - EXPECT_EQ(2, count(m_messages.begin(), m_messages.end(), "M220")); + generator.MakeFieldPosMask3("INVALID_WIDE_POSITION", "Position must not wrap", UINT64_C(0x10000001f), 1); + generator.MakeFieldPosMask3("INVALID_WIDE_MASK", "Mask must not wrap", 0, UINT64_C(0x100000000)); + EXPECT_EQ(4, count(m_messages.begin(), m_messages.end(), "M220")); generator.MakeFieldPosMask3("VALID", "Top 32-bit field", 31, 1); ASSERT_TRUE(output.Flush()); @@ -690,6 +723,8 @@ TEST_F(SvdBitWidthTest, HeaderRejectsInvalidShiftRangesWithoutPartialMacros) { const string text{istreambuf_iterator(input), istreambuf_iterator()}; EXPECT_EQ(string::npos, text.find("INVALID_POSITION")); EXPECT_EQ(string::npos, text.find("INVALID_MASK")); + EXPECT_EQ(string::npos, text.find("INVALID_WIDE_POSITION")); + EXPECT_EQ(string::npos, text.find("INVALID_WIDE_MASK")); EXPECT_EQ("(31UL)", DefineValue(text, "VALID_Pos")); EXPECT_EQ("(0x80000000UL)", DefineValue(text, "VALID_Msk")); } @@ -714,60 +749,34 @@ TEST_F(SvdBitWidthTest, SfdRejectsUnsupportedExpressionReferenceOutsideDeviceChi EXPECT_FALSE(filesystem::exists(output)); } -TEST_F(SvdBitWidthTest, SfdEnumSetsRejectWidthsOutsideComboRange) { - SvdOptions options; - FileHeaderInfo info; - BitWidthSfdData sfd(info, options); - SvdRegister reg(nullptr); - reg.SetBitWidth(64); - const auto field = AddUncheckedField(reg, 0, 6); - SvdEnumContainer values(field); - const auto entry = new SvdEnum(&values); - values.AddItem(entry); - entry->SetName("LAST"); - entry->SetValue(63); - EXPECT_TRUE(sfd.CreateEnumValueSet(&values, field)); - - for(const auto width : {0, 7, 64}) { - SCOPED_TRACE(width); - field->SetBitWidth(width); - EXPECT_FALSE(sfd.CreateEnumValueSet(&values, field)); - } - EXPECT_TRUE(entry->IsValid()); - EXPECT_EQ(63U, entry->GetValue().u32); -} - TEST_F(SvdBitWidthTest, FullWidthFieldsStillReportIncompatibleRegisterAccess) { - for(const auto width : {32, 64}) { - SCOPED_TRACE(width); - m_messages.clear(); - SvdRegister reg(nullptr); - reg.SetName("CTRL"); - reg.SetBitWidth(width); - reg.SetAccess(SvdTypes::Access::READONLY); - const auto field = AddUncheckedField(reg, 0, width); - - EXPECT_TRUE(field->CheckItem()); - EXPECT_TRUE(field->IsValid()); - EXPECT_TRUE(HasMessage("M367")); - EXPECT_EQ(SvdTypes::Access::READWRITE, field->GetAccess()); - } + constexpr int32_t width = 32; + SvdRegister reg(nullptr); + reg.SetName("CTRL"); + reg.SetBitWidth(width); + reg.SetAccess(SvdTypes::Access::READONLY); + const auto field = AddUncheckedField(reg, 0, width); + + EXPECT_TRUE(field->CheckItem()); + EXPECT_TRUE(field->IsValid()); + EXPECT_TRUE(HasMessage("M367")); + EXPECT_EQ(SvdTypes::Access::READWRITE, field->GetAccess()); } TEST_F(SvdBitWidthTest, RegisterSkipsRejectedRangesBeforeCheckingSiblingConflicts) { SvdRegister reg(nullptr); reg.SetName("CTRL"); - reg.SetBitWidth(64); - const auto invalid = AddUncheckedField(reg, 63, 2); - const auto valid = AddUncheckedField(reg, 63, 1); + reg.SetBitWidth(32); + const auto invalid = AddUncheckedField(reg, 31, 2); + const auto valid = AddUncheckedField(reg, 31, 1); - EXPECT_TRUE(reg.CheckFields(reg.GetFieldContainer(), 64, reg.GetName())); + EXPECT_TRUE(reg.CheckFields(reg.GetFieldContainer(), 32, reg.GetName())); EXPECT_FALSE(invalid->IsValid()); EXPECT_TRUE(valid->IsValid()); EXPECT_EQ(1, count(m_messages.begin(), m_messages.end(), "M345")); EXPECT_FALSE(HasMessage("M336")); EXPECT_FALSE(HasMessage("M338")); - EXPECT_EQ(uint64_t{1} << 63, reg.GetAccessMaskWrite()); + EXPECT_EQ(uint64_t{1} << 31, reg.GetAccessMaskWrite()); } } // namespace diff --git a/tools/svdconv/test/unittests/src/SvdDimensionBoundsTest.cpp b/tools/svdconv/test/unittests/src/SvdDimensionBoundsTest.cpp index bcccb61d2..0b626e4e2 100644 --- a/tools/svdconv/test/unittests/src/SvdDimensionBoundsTest.cpp +++ b/tools/svdconv/test/unittests/src/SvdDimensionBoundsTest.cpp @@ -399,7 +399,9 @@ TEST_F(SvdDimensionBoundsTest, FieldAndRegisterExpansionStopsWhenInheritedMetada SvdField field(&fields); field.SetOffset(0); field.SetBitWidth(1); - SvdItem& source = isField ? static_cast(field) : static_cast(reg); + SvdItem& fieldItem = field; + SvdItem& registerItem = reg; + SvdItem& source = isField ? fieldItem : registerItem; const auto dim = Dimension(source, 2, isField ? 1 : 4); source.SetDerivedFrom(new SvdDerivedFrom(&source)); // Two indices and one copied item fit; its inherited metadata does not. From 579576914705131f08b170dee1368ee8ee4d942d Mon Sep 17 00:00:00 2001 From: Thorsten de Buhr Date: Tue, 20 Oct 2026 14:04:28 +0200 Subject: [PATCH 12/26] refactor(svdconv): keep checked field generation within 32 bits --- .../svdconv/SVDGenerator/include/HeaderData.h | 8 +-- .../SVDGenerator/include/HeaderGenerator.h | 6 +- .../SVDGenerator/include/SfdGenerator.h | 15 +++-- .../SVDGenerator/src/HeaderData_Field.cpp | 22 +++---- .../SVDGenerator/src/HeaderData_PosMask.cpp | 8 +-- .../src/HeaderData_RegStructure.cpp | 6 +- .../SVDGenerator/src/HeaderGenerator.cpp | 26 ++++---- tools/svdconv/SVDGenerator/src/SfdData.cpp | 5 +- .../SVDGenerator/src/SfdData_SingleItems.cpp | 22 +++---- .../svdconv/SVDGenerator/src/SfdGenerator.cpp | 22 +++---- tools/svdconv/SVDModel/include/SvdUtils.h | 4 ++ tools/svdconv/SVDModel/src/SvdCExpression.cpp | 8 +-- tools/svdconv/SVDModel/src/SvdUtils.cpp | 20 +++++++ .../test/unittests/src/SvdBitWidthTest.cpp | 60 ++++++++++++------- 14 files changed, 134 insertions(+), 98 deletions(-) diff --git a/tools/svdconv/SVDGenerator/include/HeaderData.h b/tools/svdconv/SVDGenerator/include/HeaderData.h index 4c7e8d505..ea5702149 100644 --- a/tools/svdconv/SVDGenerator/include/HeaderData.h +++ b/tools/svdconv/SVDGenerator/include/HeaderData.h @@ -19,8 +19,8 @@ #include struct ONESTRUCT { - uint64_t mask; - std::map fields; + uint32_t mask; + std::map fields; ONESTRUCT() : mask(0) {} }; @@ -142,7 +142,7 @@ class HeaderData { void GenerateReserved (int32_t resBytes, uint32_t address, bool bGenerate = true); void GenerateReserved (); void GenerateReservedWidth (int32_t nMany, uint32_t width); - void GenerateReservedField (uint64_t resBits, uint32_t regSize); + void GenerateReservedField (int32_t resBits, uint32_t regSize); void AddReservedBytesLater (int32_t reservedBytes); uint32_t NodeHasChilds (RegTreeNode *node, uint32_t pos); bool NodeHasRegs (RegTreeNode *node); @@ -159,7 +159,7 @@ class HeaderData { void Init_OpenCloseStructUnion (); // HeaderData_Fields - uint64_t CreateField (SvdField* field, uint32_t regSize, uint64_t offsCnt); + uint32_t CreateField (SvdField* field, uint32_t regSize, uint32_t offsCnt); bool CreateSortedFields (const FIELDMAPLIST& sortedFields, uint32_t regSize, std::string structName); bool AddFields (SvdItem* container, FIELDMAPLIST& sortedFields); bool CreateFields (SvdRegister* reg, std::string structName); diff --git a/tools/svdconv/SVDGenerator/include/HeaderGenerator.h b/tools/svdconv/SVDGenerator/include/HeaderGenerator.h index 5e7c3c9f0..b723b0b6e 100644 --- a/tools/svdconv/SVDGenerator/include/HeaderGenerator.h +++ b/tools/svdconv/SVDGenerator/include/HeaderGenerator.h @@ -53,7 +53,7 @@ class HeaderGenerator : public CodeGenerator { void MakeDoxyCommentAddress (const std::string& text, uint32_t address); void MakeDoxyCommentNumber (const std::string& text, uint32_t number); void MakeDoxyComment (const std::string& text); - void MakeDoxyCommentBitField (const std::string& text, uint64_t firstBit, uint64_t lastBit); + void MakeDoxyCommentBitField (const std::string& text, uint32_t firstBit, uint32_t lastBit); void MakeDoxyCommentBitPos (const std::string& text, uint32_t pos); void MakeInterruptStruct (const std::string& text, int32_t number, bool lastEnum); void MakeDoxygenAddPeripheral (const std::string& text); @@ -62,7 +62,7 @@ class HeaderGenerator : public CodeGenerator { void MakeAnnonUnionCompiler (const std::string& text, bool bStart); void MakeCMSISConfig (const std::string& text, const std::string& MCUName, const CmsisCfg& cmsisCfg); void MakeRegisterStruct (const std::string& text, SvdTypes::Access, const std::string& dataType, uint32_t size); - void MakeFieldStruct (const std::string& text, SvdTypes::Access, const std::string& dataType, uint32_t size, uint64_t bitWidth); + void MakeFieldStruct (const std::string& text, SvdTypes::Access, const std::string& dataType, uint32_t size, uint32_t bitWidth); void MakeFieldUnion (const std::string& text, SvdTypes::Access accessType, const std::string& dataType, uint32_t size); void MakeEnumValue (const std::string& enumName, uint32_t val, const std::string& name, const std::string& descr); void MakeDefine (const std::string& name, uint32_t val, uint32_t base, uint32_t tabOffs, const std::string& descr); @@ -74,7 +74,7 @@ class HeaderGenerator : public CodeGenerator { void MakeEnumStructUnionEndArray (const std::string& textBuf, c_header::Index elementIndex, c_header::Special specialType, c_header::Additional langAdditional, uint32_t num); void MakeFieldPosMask (const std::string& field, const std::string& peri, const std::string& reg, uint32_t pos, uint32_t mask); void MakeFieldPosMask2 (const std::string& field, const std::string& peri, const std::string& reg, uint32_t pos, uint32_t mask); - void MakeFieldPosMask3 (const std::string& name, const std::string& field, uint64_t pos, uint64_t mask); + void MakeFieldPosMask3 (const std::string& name, const std::string& field, uint32_t pos, uint32_t mask); void CreateDoxyComment (const std::string& str); void CreateDoxyCommentStar (const std::string& str); void CreateDoxyCommentLineBreak (const std::string& str); diff --git a/tools/svdconv/SVDGenerator/include/SfdGenerator.h b/tools/svdconv/SVDGenerator/include/SfdGenerator.h index 03170e9fe..a5195220a 100644 --- a/tools/svdconv/SVDGenerator/include/SfdGenerator.h +++ b/tools/svdconv/SVDGenerator/include/SfdGenerator.h @@ -10,7 +10,6 @@ #include "SfdGenAPI.h" #include "FileIo.h" -#include #include #include #include @@ -40,10 +39,10 @@ const std::string LOC_STRING_EDIT2 = "( (%s)((%s >> %i) & 0x%X), ((%s = (%s & (% // Write exp. : RegisterName, RegisterName, (cast), Mask, UpShift, (cast), guiEditSizeString, AND-Operation, UpShift //( (unsigned char)((NVIC_IP0>>16) & 0xFF), ((NVIC_IP0 = (NVIC_IP0 & ~(0xFFUL<<16 )) | ((unsigned int)(Gui_u8:GuiVal & 0xFF) << 16 ) ) )) -const std::string LOC_STRING_EDIT3 = "( (%s)((%s >> %" PRIu64 ") & 0x%" PRIX64 "), ((%s = (%s & ~(0x%" PRIX64 "UL << %" PRIu64 " )) | ((%s)(%s:GuiVal & 0x%" PRIX64 ") << %" PRIu64 " ) ) ))"; +const std::string LOC_STRING_EDIT3 = "( (%s)((%s >> %i) & 0x%X), ((%s = (%s & ~(0x%XUL << %i )) | ((%s)(%s:GuiVal & 0x%X) << %i ) ) ))"; //( (unsigned char)((NVIC_IP0>>16) & 0xFF) -const std::string LOC_STRING_EDIT3_RO = "( (%s)((%s >> %" PRIu64 ") & 0x%" PRIX64 ") )"; +const std::string LOC_STRING_EDIT3_RO = "( (%s)((%s >> %i) & 0x%X) )"; const std::string LOC_STRING_OBIT = "( (%s) %s )"; @@ -95,7 +94,7 @@ class SfdGenerator : public CodeGenerator { void Generate_OptionSelectText(const std::string& text, uint32_t value); // Location - void MakeLocationEdit (const std::string& regName, uint64_t firstBit, uint64_t lastBit, uint64_t accRead, uint64_t accWrite); + void MakeLocationEdit (const std::string& regName, uint32_t firstBit, uint32_t lastBit, uint32_t accRead, uint32_t accWrite); void MakeLocationObit (const std::string& regName, uint32_t bitWidth); // Interrupts @@ -114,15 +113,15 @@ class SfdGenerator : public CodeGenerator { void CreateCItemTextonly (const std::string& text, uint32_t num); void CreateCItem (const std::string& text, uint32_t num); void CreateCItemEnd (const std::string& text, sfd::Index elementIndex); - void CreateOBit (const std::string& text, uint64_t num); + void CreateOBit (const std::string& text, uint32_t num); void CreateOBitNoRange (const std::string& text, uint32_t num); - void CreateORange (const std::string& text, uint64_t num1, uint64_t num2); + void CreateORange (const std::string& text, uint32_t num1, uint32_t num2); void CreateIBit (const std::string& text, uint32_t num); void CreateIBitAddr (const std::string& text, uint32_t num1, uint32_t num2); - void CreateIBitAddrAcc (const std::string& text, uint64_t num1, uint32_t num2, SvdTypes::Access acc); + void CreateIBitAddrAcc (const std::string& text, uint32_t num1, uint32_t num2, SvdTypes::Access acc); void CreateIRange (const std::string& text, uint32_t num1, uint32_t num2); void CreateIRangeAddr (const std::string& text, uint32_t num1, uint32_t num2, uint32_t num3); - void CreateIRangeAddrAcc (const std::string& text, uint64_t num1, uint64_t num2, uint32_t num3, SvdTypes::Access acc); + void CreateIRangeAddrAcc (const std::string& text, uint32_t num1, uint32_t num2, uint32_t num3, SvdTypes::Access acc); void CreateInfoAddr (const std::string& text, uint32_t num1); void CreateTextonly (const std::string& text); diff --git a/tools/svdconv/SVDGenerator/src/HeaderData_Field.cpp b/tools/svdconv/SVDGenerator/src/HeaderData_Field.cpp index dad7a1812..bc7731d02 100644 --- a/tools/svdconv/SVDGenerator/src/HeaderData_Field.cpp +++ b/tools/svdconv/SVDGenerator/src/HeaderData_Field.cpp @@ -76,12 +76,11 @@ bool HeaderData::AddFields(SvdItem* container, FIELDMAPLIST& sortedFields) } } - const auto offset = field->GetOffset(); - const auto bitWidth = field->GetBitWidth(); - if(offset >= 32 || bitWidth <= 0 || bitWidth > 32 || offset + bitWidth > 32) { + uint32_t offset, bitWidth; + if(!SvdUtils::GetBitRange32(field->GetOffset(), field->GetBitWidth(), offset, bitWidth)) { continue; } - const auto mask = SvdUtils::CreateBitMask(bitWidth, offset); + const auto mask = SvdUtils::CreateBitMask32(bitWidth, offset); ONESTRUCT* oneStruct = nullptr; for(auto& oneStr : sortedFields) { @@ -108,10 +107,14 @@ bool HeaderData::AddFields(SvdItem* container, FIELDMAPLIST& sortedFields) bool HeaderData::CreateSortedFields(const FIELDMAPLIST& sortedFields, uint32_t regSize, string structName) { m_reservedFieldCnt = 0; - const auto registerBits = uint64_t{regSize} * 8; + if(regSize > sizeof(uint32_t)) { + m_gen->Generate("Register bitfields exceed 32 bits", -1); + return false; + } + const auto registerBits = regSize * 8; for(const auto& oneStruct : sortedFields) { - uint64_t offsCnt = 0; + uint32_t offsCnt = 0; m_gen->Generate(""); for(const auto& [newOffs, field] : oneStruct.fields) { @@ -136,7 +139,7 @@ bool HeaderData::CreateSortedFields(const FIELDMAPLIST& sortedFields, uint32_t r return true; } -uint64_t HeaderData::CreateField(SvdField* field, uint32_t regSize, uint64_t offsCnt) +uint32_t HeaderData::CreateField(SvdField* field, uint32_t regSize, uint32_t offsCnt) { if(!field) { return false; @@ -146,9 +149,8 @@ uint64_t HeaderData::CreateField(SvdField* field, uint32_t regSize, uint64_t off const auto name = field->GetNameCalculated(); const auto descr = field->GetDescriptionCalculated(); const auto accType = field->GetEffectiveAccess(); - const auto fieldWidth = field->GetBitWidth(); - const auto fieldPos = field->GetOffset(); - if(fieldPos >= 32 || fieldWidth <= 0 || fieldWidth > 32 || fieldPos + fieldWidth > 32) { + uint32_t fieldPos, fieldWidth; + if(!SvdUtils::GetBitRange32(field->GetOffset(), field->GetBitWidth(), fieldPos, fieldWidth)) { return 0; } diff --git a/tools/svdconv/SVDGenerator/src/HeaderData_PosMask.cpp b/tools/svdconv/SVDGenerator/src/HeaderData_PosMask.cpp index 6e1ec5009..90f058f54 100644 --- a/tools/svdconv/SVDGenerator/src/HeaderData_PosMask.cpp +++ b/tools/svdconv/SVDGenerator/src/HeaderData_PosMask.cpp @@ -218,13 +218,11 @@ bool HeaderData::CreateFieldPosMask(SvdField* field, PosMaskNames *posMaskNames) const auto& alternateGroup = posMaskNames->alternate; const auto name = field->GetNameCalculated(); const auto fieldName = field->GetHierarchicalNameResulting(); - const auto firstBit = field->GetOffset(); - uint32_t bitWidth = field->GetEffectiveBitWidth(); - const auto fieldMask = SvdUtils::CreateBitMask(bitWidth, firstBit); - if(!fieldMask || fieldMask > UINT32_MAX) { + uint32_t firstBit, bitWidth; + if(!SvdUtils::GetBitRange32(field->GetOffset(), field->GetEffectiveBitWidth(), firstBit, bitWidth)) { return false; } - const auto bitMaxNum = SvdUtils::CreateBitMask(bitWidth); + const auto bitMaxNum = SvdUtils::CreateBitMask32(bitWidth); if(!alternateGroup.empty()) m_gen->Generate("%s_%s", name.c_str(), firstBit, bitMaxNum, fieldName.c_str(), alternateGroup.c_str()); diff --git a/tools/svdconv/SVDGenerator/src/HeaderData_RegStructure.cpp b/tools/svdconv/SVDGenerator/src/HeaderData_RegStructure.cpp index 4804bfe16..e82b7fb9f 100644 --- a/tools/svdconv/SVDGenerator/src/HeaderData_RegStructure.cpp +++ b/tools/svdconv/SVDGenerator/src/HeaderData_RegStructure.cpp @@ -464,7 +464,7 @@ void HeaderData::GenerateReservedWidth(int32_t nMany, uint32_t width) } } -void HeaderData::GenerateReservedField(uint64_t resBits, uint32_t regSize) +void HeaderData::GenerateReservedField(int32_t resBits, uint32_t regSize) { if(!resBits) { return; @@ -474,6 +474,10 @@ void HeaderData::GenerateReservedField(uint64_t resBits, uint32_t regSize) m_gen->Generate("", SvdTypes::Access::UNDEF, dataType.c_str(), regSize, resBits); // experimental: type and size only + if(resBits < 0) { + m_gen->Generate("Reserved bits calculation negative: %i", -1, resBits); + } + m_reservedFieldCnt++; return; diff --git a/tools/svdconv/SVDGenerator/src/HeaderGenerator.cpp b/tools/svdconv/SVDGenerator/src/HeaderGenerator.cpp index ccc8bf7b4..172a2ee9a 100644 --- a/tools/svdconv/SVDGenerator/src/HeaderGenerator.cpp +++ b/tools/svdconv/SVDGenerator/src/HeaderGenerator.cpp @@ -11,7 +11,6 @@ #include "ErrLog.h" -#include #include #include #include @@ -288,10 +287,10 @@ void HeaderGenerator::MakeRegisterStruct(const std::string& text, SvdTypes::Acce Generate("%s %s \r\t\t\t\t\t\t\t\t\t\t%s;", ioType, dataType, text.c_str()); } -void HeaderGenerator::MakeFieldStruct(const std::string& text, SvdTypes::Access accessType, const std::string& dataType, uint32_t size, uint64_t bitWidth) +void HeaderGenerator::MakeFieldStruct(const std::string& text, SvdTypes::Access accessType, const std::string& dataType, uint32_t size, uint32_t bitWidth) { const std::string& ioType = SvdTypes::GetAccessTypeIo(accessType); - Generate("%s %s \r\t\t\t\t\t\t\t\t\t\t%s \r\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t: %" PRIu64 ";", ioType, dataType, text.c_str(), bitWidth); + Generate("%s %s \r\t\t\t\t\t\t\t\t\t\t%s \r\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t\t: %d;", ioType, dataType, text.c_str(), bitWidth); } void HeaderGenerator::MakeFieldUnion(const std::string& text, SvdTypes::Access accessType, const std::string& dataType, uint32_t size) @@ -688,7 +687,7 @@ void HeaderGenerator::MakeDoxyComment(const std::string& text) Generate(""); } -void HeaderGenerator::MakeDoxyCommentBitField(const std::string& text, uint64_t firstBit, uint64_t lastBit) +void HeaderGenerator::MakeDoxyCommentBitField(const std::string& text, uint32_t firstBit, uint32_t lastBit) { uint32_t extraSpaces = m_extraSpaces; @@ -696,7 +695,7 @@ void HeaderGenerator::MakeDoxyCommentBitField(const std::string& text, uint64_t GenerateH_CRTabOffset(HEADER_INLINE_COMMENT_DISTANCE); if(m_debugHeaderfile) { - Generate("[%" PRIu64 "..%" PRIu64 "] %s", lastBit, firstBit, text.c_str()); + Generate("[%i..%i] %s", lastBit, firstBit, text.c_str()); } else { Generate("%s", text.c_str()); @@ -805,31 +804,28 @@ void HeaderGenerator::MakeFieldPosMask2(const std::string& field, const std::str } } -void HeaderGenerator::MakeFieldPosMask3(const std::string& pName, const std::string& field, uint64_t pos, uint64_t mask) +void HeaderGenerator::MakeFieldPosMask3(const std::string& pName, const std::string& field, uint32_t pos, uint32_t mask) { - if(pos >= 32 || mask > (UINT32_MAX >> pos)) { - LogMsg("M220", MSG("Field mask exceeds 32 bits")); - return; - } + uint32_t calcMask; Generate("#define %s%s ", pName, m_definePosStr); GenerateH_CRTabOffset(HEADER_POSMAK_VAL_OFFSET); - Generate("(%" PRIu64 "UL) ", pos); + Generate("(%iUL) ", pos); // Doxy Comment GenerateH_CRTabOffset(HEADER_POSMAK_COMMENT_OFFSET2); - Generate("%s (Bit %" PRIu64 ")", field, pos); + Generate("%s (Bit %i)", field, pos); GenerateH_CRTabOffset(HEADER_POSMAK_COMMENTEND_OFFSET2); Generate(""); - const auto calcMask = mask << pos; + calcMask = mask << pos; Generate("#define %s%s ", pName, m_defineMaskStr); GenerateH_CRTabOffset(HEADER_POSMAK_VAL_OFFSET); - Generate("(0x%" PRIx64 "UL) ", calcMask); + Generate("(0x%xUL) ", calcMask); // Doxy Comment GenerateH_CRTabOffset(HEADER_POSMAK_COMMENT_OFFSET2); - Generate("%s (Bitfield-Mask: 0x%02" PRIx64 ")", field, mask); + Generate("%s (Bitfield-Mask: 0x%02x)", field, mask); GenerateH_CRTabOffset(HEADER_POSMAK_COMMENTEND_OFFSET2); Generate(""); diff --git a/tools/svdconv/SVDGenerator/src/SfdData.cpp b/tools/svdconv/SVDGenerator/src/SfdData.cpp index 1705c5cf1..5cd5e15d2 100644 --- a/tools/svdconv/SVDGenerator/src/SfdData.cpp +++ b/tools/svdconv/SVDGenerator/src/SfdData.cpp @@ -26,9 +26,8 @@ bool CheckSfdBitRange(SvdItem* item) const auto reg = dynamic_cast(item); const auto field = dynamic_cast(item); if(reg || field) { - const auto width = item->GetEffectiveBitWidth(); - const auto mask = SvdUtils::CreateBitMask(width, field ? field->GetOffset() : 0); - if(!mask || mask > UINT32_MAX) { + uint32_t offset, width; + if(!SvdUtils::GetBitRange32(field ? field->GetOffset() : 0, item->GetEffectiveBitWidth(), offset, width)) { LogMsg("M226", MSG("SFD supports only bit ranges within 32 bits: '" + item->GetNameCalculated() + "'"), item->GetLineNumber()); return false; } diff --git a/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp b/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp index da696c5f7..2a61fe830 100644 --- a/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp +++ b/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp @@ -748,8 +748,8 @@ bool SfdData::CreateRegisterItem(SvdRegister* reg) const auto address = (uint32_t)reg->GetAbsoluteAddress(); const auto accessType = reg->GetEffectiveAccess(); auto regWidth = reg->GetEffectiveBitWidth(); - const auto accRead = reg->GetAccessMaskRead(); - const auto accWrite = reg->GetAccessMaskWrite(); + const auto accRead = (uint32_t)reg->GetAccessMaskRead(); + const auto accWrite = (uint32_t)reg->GetAccessMaskWrite(); if(regWidth < 8) { regWidth = 8; @@ -768,7 +768,7 @@ bool SfdData::CreateRegisterItem(SvdRegister* reg) if(cont && cont->IsValid() && cont->GetChildCount()) { list fieldsList; CreateFields(cont, fieldsList); - const auto accMaskReadRtree = reg->GetAccessMask(); + const auto accMaskReadRtree = (uint32_t) reg->GetAccessMask(); CreateItemDescription(reg, "RTree"); m_gen->Generate("SFDITEM_REG__%s", itemName.c_str()); @@ -814,8 +814,8 @@ bool SfdData::CreateRegisterArrayItem(SvdRegister* reg) const auto address = (uint32_t)reg->GetAbsoluteAddress(); const auto accessType = reg->GetEffectiveAccess(); auto regWidth = reg->GetEffectiveBitWidth(); - const auto accRead = reg->GetAccessMaskRead(); - const auto accWrite = reg->GetAccessMaskWrite(); + const auto accRead = (uint32_t)reg->GetAccessMaskRead(); + const auto accWrite = (uint32_t)reg->GetAccessMaskWrite(); if(regWidth < 8) { regWidth = 8; @@ -873,15 +873,15 @@ bool SfdData::CreateFieldItem(SvdField* field) const auto dispName = field->GetDisplayNameCalculated(); const auto fieldDescr = SvdUtils::CheckTextGeneric_SfrCC2(field->GetDescriptionCalculated()); const auto regName = field->GetParentRegisterNameHierarchical(); - const auto firstBit = field->GetOffset(); - const auto bitWidth = field->GetEffectiveBitWidth(); const auto accessType = field->GetEffectiveAccess(); const auto regAddress = (uint32_t)field->GetAbsoluteAddress(); - const auto fieldMask = SvdUtils::CreateBitMask(bitWidth, firstBit); - if(!fieldMask || fieldMask > UINT32_MAX) { + uint32_t firstBit = 0; + uint32_t bitWidth = 0; + if(!SvdUtils::GetBitRange32(field->GetOffset(), field->GetEffectiveBitWidth(), firstBit, bitWidth)) { LogMsg("M226", MSG("SFD supports only bit ranges within 32 bits: '" + fieldName + "'"), field->GetLineNumber()); return false; } + uint32_t regBitWidth = 32; const auto parent = field->GetParent(); if(parent) { @@ -899,13 +899,13 @@ bool SfdData::CreateFieldItem(SvdField* field) valuesDescr = SvdUtils::CheckTextGeneric_SfrCC2(valuesDescr); const auto lastBit = firstBit + bitWidth -1; - const auto bitMaxNum = SvdUtils::CreateBitMask(bitWidth); + const auto bitMaxNum = SvdUtils::CreateBitMask32(bitWidth); CreateItemDescription(field, "Item"); m_gen->Generate("SFDITEM_FIELD__%s", itemName.c_str()); m_gen->Generate("%s", dispName.c_str()); - uint64_t accRead, accWrite; + uint32_t accRead, accWrite; switch(accessType) { case SvdTypes::Access::READONLY: accRead = bitMaxNum; diff --git a/tools/svdconv/SVDGenerator/src/SfdGenerator.cpp b/tools/svdconv/SVDGenerator/src/SfdGenerator.cpp index 906471897..6bf664344 100644 --- a/tools/svdconv/SVDGenerator/src/SfdGenerator.cpp +++ b/tools/svdconv/SVDGenerator/src/SfdGenerator.cpp @@ -205,11 +205,13 @@ void SfdGenerator::Generate_OptionSelectText(const std::string& text, uint32_t v } -void SfdGenerator::MakeLocationEdit(const std::string& regName, uint64_t firstBit, uint64_t lastBit, uint64_t accRead, uint64_t accWrite) +void SfdGenerator::MakeLocationEdit(const std::string& regName, uint32_t firstBit, uint32_t lastBit, uint32_t accRead, uint32_t accWrite) { string guiEditSizeString; string castString; - const auto bitWidth = lastBit - firstBit + 1; + uint32_t bitWidth = 0; + + bitWidth = lastBit - firstBit + 1; if(bitWidth < 9) { guiEditSizeString = GuiEditItems[GUI_EDIT_ITEM_8]; castString = CastItems[CAST_CHAR]; } else if(bitWidth < 17) { guiEditSizeString = GuiEditItems[GUI_EDIT_ITEM_16]; castString = CastItems[CAST_SHORT]; } @@ -428,9 +430,9 @@ void SfdGenerator::CreateCItemEnd(const std::string& text, sfd::Index elementInd Generate(" ", items[elementIndex].c_str()); } -void SfdGenerator::CreateOBit(const std::string& text, uint64_t num) +void SfdGenerator::CreateOBit(const std::string& text, uint32_t num) { - Generate(" %s", num, num, text.c_str()); + Generate(" %s", num, num, text.c_str()); } void SfdGenerator::CreateOBitNoRange(const std::string& text, uint32_t num) @@ -438,9 +440,9 @@ void SfdGenerator::CreateOBitNoRange(const std::string& text, uint32_t num) Generate(" %s", num, text.c_str()); } -void SfdGenerator::CreateORange(const std::string& text, uint64_t num1, uint64_t num2) +void SfdGenerator::CreateORange(const std::string& text, uint32_t num1, uint32_t num2) { - Generate(" %s", num1, num2, text.c_str()); + Generate(" %s", num1, num2, text.c_str()); } void SfdGenerator::CreateIBit(const std::string& text, uint32_t num) @@ -453,10 +455,10 @@ void SfdGenerator::CreateIBitAddr(const std::string& text, uint32_t num1, uint32 Generate(" [Bit %i] (@ 0x%08X) %s", num1, num2, text.c_str()); } -void SfdGenerator::CreateIBitAddrAcc(const std::string& text, uint64_t num1, uint32_t num2, SvdTypes::Access acc) +void SfdGenerator::CreateIBitAddrAcc(const std::string& text, uint32_t num1, uint32_t num2, SvdTypes::Access acc) { const std::string &accStr = SvdTypes::GetAccessTypeSfd(acc); - Generate(" [Bit %" PRIu64 "] %s (@ 0x%08X) %s", num1, accStr.c_str(), num2, text.c_str()); + Generate(" [Bit %i] %s (@ 0x%08X) %s", num1, accStr.c_str(), num2, text.c_str()); } void SfdGenerator::CreateIRange(const std::string& text, uint32_t num1, uint32_t num2) @@ -469,10 +471,10 @@ void SfdGenerator::CreateIRangeAddr(const std::string& text, uint32_t num1, uint Generate(" [Bits %i..%i] (@ 0x%08X) %s", num1, num2, num3, text.c_str()); } -void SfdGenerator::CreateIRangeAddrAcc(const std::string& text, uint64_t num1, uint64_t num2, uint32_t num3, SvdTypes::Access acc) +void SfdGenerator::CreateIRangeAddrAcc(const std::string& text, uint32_t num1, uint32_t num2, uint32_t num3, SvdTypes::Access acc) { const std::string &accStr = SvdTypes::GetAccessTypeSfd(acc); - Generate(" [Bits %" PRIu64 "..%" PRIu64 "] %s (@ 0x%08X) %s", num1, num2, accStr.c_str(), num3, text.c_str()); + Generate(" [Bits %i..%i] %s (@ 0x%08X) %s", num1, num2, accStr.c_str(), num3, text.c_str()); } void SfdGenerator::CreateInfoAddr(const std::string& text, uint32_t num1) diff --git a/tools/svdconv/SVDModel/include/SvdUtils.h b/tools/svdconv/SVDModel/include/SvdUtils.h index 437e312e8..c162d515e 100644 --- a/tools/svdconv/SVDModel/include/SvdUtils.h +++ b/tools/svdconv/SVDModel/include/SvdUtils.h @@ -56,6 +56,10 @@ class SvdUtils { static bool ConvertNumberXBin (const std::string &text, std::set& numbers); static bool ConvertBitRange (const std::string &text, uint32_t& m_offset, uint32_t& m_size); + // Validate model values before converting them to a 32-bit output range. + static bool GetBitRange32 (uint64_t rawOffset, int64_t rawWidth, uint32_t& offset, uint32_t& width); + // Returns zero when the bit range is empty or does not fit in 32 bits. + static uint32_t CreateBitMask32 (uint32_t width, uint32_t offset = 0); // Returns zero when the bit range is empty or does not fit in 64 bits. static uint64_t CreateBitMask (int64_t width, uint64_t offset = 0); static bool DoConvertNumberXBin (const std::string &text, uint32_t value, std::set& numbers); diff --git a/tools/svdconv/SVDModel/src/SvdCExpression.cpp b/tools/svdconv/SVDModel/src/SvdCExpression.cpp index b995c6da9..634e79f67 100644 --- a/tools/svdconv/SVDModel/src/SvdCExpression.cpp +++ b/tools/svdconv/SVDModel/src/SvdCExpression.cpp @@ -178,13 +178,11 @@ string SvdCExpression::CreateFieldExpression(SvdItem* item) } const auto regName = field->GetParentRegisterNameHierarchical(); - const auto firstBit = field->GetOffset(); - const auto bitWidth = field->GetEffectiveBitWidth(); - const auto fieldMask = SvdUtils::CreateBitMask(bitWidth, firstBit); - if(!field->IsValid() || !fieldMask || fieldMask > UINT32_MAX) { + uint32_t firstBit, bitWidth; + if(!field->IsValid() || !SvdUtils::GetBitRange32(field->GetOffset(), field->GetEffectiveBitWidth(), firstBit, bitWidth)) { return SvdUtils::EMPTY_STRING; } - const auto bitMaxNum = SvdUtils::CreateBitMask(bitWidth); + const auto bitMaxNum = SvdUtils::CreateBitMask32(bitWidth); std::stringstream ss; ss << "0x" << std::hex << bitMaxNum; diff --git a/tools/svdconv/SVDModel/src/SvdUtils.cpp b/tools/svdconv/SVDModel/src/SvdUtils.cpp index 2e17df757..00062dba5 100644 --- a/tools/svdconv/SVDModel/src/SvdUtils.cpp +++ b/tools/svdconv/SVDModel/src/SvdUtils.cpp @@ -446,6 +446,26 @@ bool SvdUtils::ConvertNumber(const string &text, set &numbers) return true; } +bool SvdUtils::GetBitRange32(uint64_t rawOffset, int64_t rawWidth, uint32_t& offset, uint32_t& width) +{ + if(rawOffset >= 32 || rawWidth <= 0 || rawWidth > 32 || rawWidth > 32 - rawOffset) { + return false; + } + + offset = static_cast(rawOffset); + width = static_cast(rawWidth); + return true; +} + +uint32_t SvdUtils::CreateBitMask32(uint32_t width, uint32_t offset) +{ + if(width == 0 || width > 32 || offset >= 32 || width > 32 - offset) { + return 0; + } + + return (UINT32_MAX >> (32 - width)) << offset; +} + uint64_t SvdUtils::CreateBitMask(int64_t width, uint64_t offset) { if(width <= 0 || width > 64 || offset >= 64 || width + offset > 64) { diff --git a/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp b/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp index 3d47202d6..c1fdb9fd8 100644 --- a/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp +++ b/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp @@ -205,6 +205,19 @@ TEST_F(SvdBitWidthTest, BitMasksUseWideIntermediatesWithoutAcceptingInvalidRange } } +TEST_F(SvdBitWidthTest, OutputMasksStayWithin32Bits) { + EXPECT_EQ(UINT32_MAX, SvdUtils::CreateBitMask32(32)); + EXPECT_EQ(UINT32_C(0x7fffffff), SvdUtils::CreateBitMask32(31)); + EXPECT_EQ(UINT32_C(0x80000000), SvdUtils::CreateBitMask32(1, 31)); + EXPECT_EQ(UINT32_C(0xfffffffe), SvdUtils::CreateBitMask32(31, 1)); + EXPECT_EQ(0U, SvdUtils::CreateBitMask32(0)); + EXPECT_EQ(0U, SvdUtils::CreateBitMask32(33)); + EXPECT_EQ(0U, SvdUtils::CreateBitMask32(UINT32_MAX)); + EXPECT_EQ(0U, SvdUtils::CreateBitMask32(2, 31)); + EXPECT_EQ(0U, SvdUtils::CreateBitMask32(1, 32)); + EXPECT_EQ(0U, SvdUtils::CreateBitMask32(1, UINT32_MAX)); +} + TEST_F(SvdBitWidthTest, FullWidthFieldsPreserveEnumsAndAccessMasks) { constexpr int32_t width = 32; XMLTreeElement element(nullptr, "register"); @@ -442,7 +455,7 @@ TEST_F(SvdBitWidthTest, HeaderFieldPaddingRejectsOverlapsAndOutOfRangePositions) FIELDMAPLIST groups(1); auto& fields = groups.front().fields; - fields.emplace(UINT64_C(0x100000000), AddUncheckedField(reg, UINT64_C(0x100000000), 1)); + fields.emplace(UINT32_MAX, AddUncheckedField(reg, UINT32_MAX, 1)); EXPECT_FALSE(header.CreateSortedFields(groups, 4, "bits")); fields.clear(); @@ -453,6 +466,7 @@ TEST_F(SvdBitWidthTest, HeaderFieldPaddingRejectsOverlapsAndOutOfRangePositions) fields.clear(); fields.emplace(0, AddUncheckedField(reg, 0, 9)); EXPECT_FALSE(header.CreateSortedFields(groups, 1, "bits")); + EXPECT_FALSE(header.CreateSortedFields(groups, 8, "bits")); } TEST_F(SvdBitWidthTest, HeaderMasksPreserve32BitBoundaryTextAndRejectWiderRanges) { @@ -484,6 +498,8 @@ TEST_F(SvdBitWidthTest, HeaderMasksPreserve32BitBoundaryTextAndRejectWiderRanges EXPECT_FALSE(header.CreateFieldPosMask(AddUncheckedField(reg, 0, 33), &names)); EXPECT_FALSE(header.CreateFieldPosMask(AddUncheckedField(reg, 0, 64), &names)); EXPECT_FALSE(header.CreateFieldPosMask(AddUncheckedField(reg, UINT64_C(0x100000000), 1), &names)); + EXPECT_FALSE(header.CreateFieldPosMask(AddUncheckedField(reg, UINT64_C(0x10000001f), 1), &names)); + EXPECT_FALSE(header.CreateFieldPosMask(AddUncheckedField(reg, UINT64_MAX, 1), &names)); } TEST_F(SvdBitWidthTest, FieldExpressionsPreserve32BitBoundaryTextAndRejectWiderRanges) { @@ -705,28 +721,26 @@ TEST_F(SvdBitWidthTest, HeaderFieldGroupsSkipUncheckedInvalidRanges) { EXPECT_EQ(upper, groups.front().fields.at(31)); } -TEST_F(SvdBitWidthTest, HeaderRejectsInvalidShiftRangesWithoutPartialMacros) { - CreateOutputDirectory(); - FileIo output; - output.SetFileName((m_outputDirectory / "masks.h").string()); - BitWidthHeaderGenerator generator(&output); - generator.MakeFieldPosMask3("INVALID_POSITION", "Position outside register", 32, 1); - generator.MakeFieldPosMask3("INVALID_MASK", "Mask crosses register boundary", 31, 3); - generator.MakeFieldPosMask3("INVALID_WIDE_POSITION", "Position must not wrap", UINT64_C(0x10000001f), 1); - generator.MakeFieldPosMask3("INVALID_WIDE_MASK", "Mask must not wrap", 0, UINT64_C(0x100000000)); - EXPECT_EQ(4, count(m_messages.begin(), m_messages.end(), "M220")); - generator.MakeFieldPosMask3("VALID", "Top 32-bit field", 31, 1); - ASSERT_TRUE(output.Flush()); - - ifstream input(m_outputDirectory / "masks.h"); - ASSERT_TRUE(input.is_open()); - const string text{istreambuf_iterator(input), istreambuf_iterator()}; - EXPECT_EQ(string::npos, text.find("INVALID_POSITION")); - EXPECT_EQ(string::npos, text.find("INVALID_MASK")); - EXPECT_EQ(string::npos, text.find("INVALID_WIDE_POSITION")); - EXPECT_EQ(string::npos, text.find("INVALID_WIDE_MASK")); - EXPECT_EQ("(31UL)", DefineValue(text, "VALID_Pos")); - EXPECT_EQ("(0x80000000UL)", DefineValue(text, "VALID_Msk")); +TEST_F(SvdBitWidthTest, OutputRangesRejectInvalidModelValuesBeforeConversion) { + uint32_t offset = 0, width = 0; + ASSERT_TRUE(SvdUtils::GetBitRange32(0, 32, offset, width)); + EXPECT_EQ(0U, offset); + EXPECT_EQ(32U, width); + ASSERT_TRUE(SvdUtils::GetBitRange32(31, 1, offset, width)); + EXPECT_EQ(31U, offset); + EXPECT_EQ(1U, width); + + const pair invalidRanges[] = { + {0, 0}, {0, -1}, {0, INT64_MIN}, {0, 33}, {0, INT64_MAX}, + {0, int64_t{UINT32_MAX} + 2}, {31, 2}, {32, 1}, + {UINT64_C(0x100000000), 1}, {UINT64_C(0x10000001f), 1}, {UINT64_MAX, 1}, + }; + for(const auto& [rawOffset, rawWidth] : invalidRanges) { + SCOPED_TRACE(to_string(rawOffset) + ":" + to_string(rawWidth)); + EXPECT_FALSE(SvdUtils::GetBitRange32(rawOffset, rawWidth, offset, width)); + EXPECT_EQ(31U, offset); + EXPECT_EQ(1U, width); + } } TEST_F(SvdBitWidthTest, SfdRejectsUnsupportedExpressionReferenceOutsideDeviceChildren) { From 9e5e7eddab31adeda609b6a279d9384320013fdc Mon Sep 17 00:00:00 2001 From: Thorsten de Buhr Date: Tue, 20 Oct 2026 14:16:05 +0200 Subject: [PATCH 13/26] refactor(svdconv): reuse SFD result to guard SFR generation --- tools/svdconv/SVDConv/src/SVDConv.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tools/svdconv/SVDConv/src/SVDConv.cpp b/tools/svdconv/SVDConv/src/SVDConv.cpp index 188b315a2..aa030644f 100644 --- a/tools/svdconv/SVDConv/src/SVDConv.cpp +++ b/tools/svdconv/SVDConv/src/SVDConv.cpp @@ -339,15 +339,14 @@ SVD_ERR SvdConv::CheckSvdFile() } // ---------------------- Generate SFD File ---------------------- - bool sfdGenerated = false; if(m_svdOptions.IsGenerateSfd()) { t1 = CrossPlatformUtils::ClockInMsec(); + success = false; if(device) { generator->SetSvdFileName(path); generator->SetProgramInfo(version, descr, copyright); - sfdGenerated = generator->SfdFile(device, outDir); + success = generator->SfdFile(device, outDir); } - success = sfdGenerated; t2 = CrossPlatformUtils::ClockInMsec() - t1; if(success) { LogMsg("M040", NAME("Generate System Viewer SFD File"), TIME(t2)); } @@ -355,7 +354,7 @@ SVD_ERR SvdConv::CheckSvdFile() } // ---------------------- Generate SFR File ---------------------- - if(m_svdOptions.IsGenerateSfr() && sfdGenerated) { + if(m_svdOptions.IsGenerateSfr() && success) { t1 = CrossPlatformUtils::ClockInMsec(); if(device) { generator->SetSvdFileName(path); From 408afdfe7a743aeba064c2cfd773100edbf543cc Mon Sep 17 00:00:00 2001 From: Thorsten de Buhr Date: Tue, 20 Oct 2026 14:31:35 +0200 Subject: [PATCH 14/26] docs(svdconv): explain the checked bit-range boundary --- tools/svdconv/SVDModel/src/SvdUtils.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tools/svdconv/SVDModel/src/SvdUtils.cpp b/tools/svdconv/SVDModel/src/SvdUtils.cpp index 00062dba5..ae874d3db 100644 --- a/tools/svdconv/SVDModel/src/SvdUtils.cpp +++ b/tools/svdconv/SVDModel/src/SvdUtils.cpp @@ -446,6 +446,13 @@ bool SvdUtils::ConvertNumber(const string &text, set &numbers) return true; } +// SvdUtils::GetBitRange32() preserves the existing raw getter contracts: +// GetBitWidth() is already signed 32-bit with -1 meaning unset; SvdField::GetOffset() +// retains wide values and its unset sentinel for inheritance and diagnostics. +// Validate the complete range before narrowing: offset 31 with width 2 is invalid, +// and a large offset must not wrap to a valid position. A checked SvdField accessor +// could own this check; changing the raw getters instead requires coordinated +// changes to parsing, storage, setters, dimensions, inheritance and sentinels. bool SvdUtils::GetBitRange32(uint64_t rawOffset, int64_t rawWidth, uint32_t& offset, uint32_t& width) { if(rawOffset >= 32 || rawWidth <= 0 || rawWidth > 32 || rawWidth > 32 - rawOffset) { From e36333710886df3b7ea0caec004aa14842fc77f8 Mon Sep 17 00:00:00 2001 From: Thorsten de Buhr Date: Tue, 20 Oct 2026 14:34:36 +0200 Subject: [PATCH 15/26] docs(svdconv): track field-range validation follow-up --- tools/svdconv/docs/todo.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 tools/svdconv/docs/todo.md diff --git a/tools/svdconv/docs/todo.md b/tools/svdconv/docs/todo.md new file mode 100644 index 000000000..7b8cc1964 --- /dev/null +++ b/tools/svdconv/docs/todo.md @@ -0,0 +1,26 @@ +# SVDConv TODO + +## Field ranges and 32-bit output + +- [ ] Review whether a checked `SvdField::GetBitRange(uint32_t&, uint32_t&)` accessor should own the validation + currently provided by `SvdUtils::GetBitRange32()`. This is a possible model cleanup; the current helper + preserves the existing raw getter contracts. + +`SvdUtils::GetBitRange32()` in [SvdUtils.cpp](../SVDModel/src/SvdUtils.cpp) validates the complete field range +before converting model values to the 32-bit values used by the generators: + +- `GetBitWidth()` already returns `int32_t`; `-1` represents an unset value used during inheritance. +- `SvdField::GetOffset()` retains a `uint64_t` raw value, including its unset sentinel and invalid inputs needed + for inheritance and diagnostics. Narrowing this getter could turn an invalid offset such as `0x100000000` + into the valid position `0` before validation. +- Offset and width must be checked together: offset `31` and width `2` exceed the supported 32-bit range. + +The helper checks offset, width and their combined range before either conversion. It leaves the output +parameters unchanged on failure. Existing wide model values and intermediate arithmetic do not imply support +for 64-bit SVD input or output. + +Changing the raw getters or field storage requires coordinated changes to parsing, setters, dimension +calculations, inheritance and sentinel handling. Preserve invalid raw values until diagnostics can report them, +and retain the regression coverage in [SvdSentinelModelTest.cpp](../test/unittests/src/SvdSentinelModelTest.cpp) +and [SvdBitWidthTest.cpp](../test/unittests/src/SvdBitWidthTest.cpp), including full-width masks, bit 31, +negative widths and large offsets that would otherwise wrap. From 566a603e567d478a0a1a3ecee7419d4f3a3d2417 Mon Sep 17 00:00:00 2001 From: Thorsten de Buhr Date: Tue, 20 Oct 2026 14:38:16 +0200 Subject: [PATCH 16/26] refactor(svdconv): move SFD validation helpers into SfdData --- tools/svdconv/SVDGenerator/include/SfdData.h | 3 ++ tools/svdconv/SVDGenerator/src/SfdData.cpp | 20 ++++++------- .../test/unittests/src/GeneratorTest.cpp | 28 +++++++++---------- 3 files changed, 25 insertions(+), 26 deletions(-) diff --git a/tools/svdconv/SVDGenerator/include/SfdData.h b/tools/svdconv/SVDGenerator/include/SfdData.h index fabd2aa77..e5a9b9faa 100644 --- a/tools/svdconv/SVDGenerator/include/SfdData.h +++ b/tools/svdconv/SVDGenerator/include/SfdData.h @@ -73,6 +73,9 @@ class SfdData { bool IsValid (SvdItem* item); private: + static bool CheckBitRange (SvdItem* item); + static bool CheckBitRanges (SvdItem* item); + const SvdOptions& m_options; FileIo* m_fileIo; SfdGenerator* m_gen; diff --git a/tools/svdconv/SVDGenerator/src/SfdData.cpp b/tools/svdconv/SVDGenerator/src/SfdData.cpp index 5cd5e15d2..69f1f21f8 100644 --- a/tools/svdconv/SVDGenerator/src/SfdData.cpp +++ b/tools/svdconv/SVDGenerator/src/SfdData.cpp @@ -19,9 +19,7 @@ using namespace std; -namespace { - -bool CheckSfdBitRange(SvdItem* item) +bool SfdData::CheckBitRange(SvdItem* item) { const auto reg = dynamic_cast(item); const auto field = dynamic_cast(item); @@ -35,12 +33,12 @@ bool CheckSfdBitRange(SvdItem* item) return true; } -bool CheckSfdBitRanges(SvdItem* item) +bool SfdData::CheckBitRanges(SvdItem* item) { if(!item->IsValid()) { return true; } - if(!CheckSfdBitRange(item)) { + if(!CheckBitRange(item)) { return false; } @@ -56,23 +54,21 @@ bool CheckSfdBitRanges(SvdItem* item) LogMsg("M226", MSG("Invalid item referenced by an SFD expression: '" + condition->GetText() + "'"), condition->GetLineNumber()); return false; } - if(!CheckSfdBitRange(referenced)) { + if(!CheckBitRange(referenced)) { return false; } } } for(const auto child : item->GetChildren()) { - if(!CheckSfdBitRanges(child)) { + if(!CheckBitRanges(child)) { return false; } } const auto dim = item->GetDimension(); - return !dim || CheckSfdBitRanges(dim); + return !dim || CheckBitRanges(dim); } -} // namespace - SfdData::SfdData(const FileHeaderInfo &fileHeaderInfo, const SvdOptions& options): m_options(options), @@ -106,7 +102,7 @@ bool SfdData::Create(SvdItem *item, const string &fileName) } const auto device = dynamic_cast(item); - if(!device || !CheckSfdBitRanges(device)) { + if(!device || !CheckBitRanges(device)) { return false; } for(const auto& [name, reg] : device->GetExpressionRegistersList()) { @@ -114,7 +110,7 @@ bool SfdData::Create(SvdItem *item, const string &fileName) LogMsg("M226", MSG("Invalid register referenced by an SFD expression: '" + name + "'")); return false; } - if(!CheckSfdBitRanges(reg)) { + if(!CheckBitRanges(reg)) { return false; } } diff --git a/tools/svdconv/test/unittests/src/GeneratorTest.cpp b/tools/svdconv/test/unittests/src/GeneratorTest.cpp index fb0721b56..f9347144b 100644 --- a/tools/svdconv/test/unittests/src/GeneratorTest.cpp +++ b/tools/svdconv/test/unittests/src/GeneratorTest.cpp @@ -49,19 +49,19 @@ class TestHeaderData : public HeaderData { using HeaderData::CreateRegisterPosMask; }; -SvdField* AddField(SvdItem* parent, const string& name, uint32_t offset) { - const auto field = new SvdField(parent); - field->SetName(name); - field->SetOffset(offset); - field->SetBitWidth(1); - parent->AddItem(field); - return field; -} - } // namespace TEST(HeaderData, CreateRegisterPosMaskHandlesDimensionVariants) { + const auto addField = [](SvdItem* parent, const string& name, uint32_t offset) { + const auto field = new SvdField(parent); + field->SetName(name); + field->SetOffset(offset); + field->SetBitWidth(1); + parent->AddItem(field); + return field; + }; + SvdOptions options; FileHeaderInfo fileHeaderInfo; TestHeaderData headerData(fileHeaderInfo, options); @@ -73,20 +73,20 @@ TEST(HeaderData, CreateRegisterPosMaskHandlesDimensionVariants) { const auto fields = new SvdFieldContainer(reg); reg->AddItem(fields); - AddField(fields, "PLAIN", 0); + addField(fields, "PLAIN", 0); - const auto arrayField = AddField(fields, "ARRAY", 1); + const auto arrayField = addField(fields, "ARRAY", 1); const auto arrayDim = new SvdDimension(arrayField); arrayDim->GetExpression()->SetType(SvdTypes::Expression::ARRAY); arrayField->SetDimension(arrayDim); - const auto extendField = AddField(fields, "EXTEND%s", 2); + const auto extendField = addField(fields, "EXTEND%s", 2); const auto extendDim = new SvdDimension(extendField); extendDim->GetExpression()->SetType(SvdTypes::Expression::EXTEND); extendField->SetDimension(extendDim); - AddField(extendDim, "EXTEND0", 2); - const auto invalidField = AddField(extendDim, "EXTEND1", 3); + addField(extendDim, "EXTEND0", 2); + const auto invalidField = addField(extendDim, "EXTEND1", 3); invalidField->SetValid(false); extendDim->AddItem(new SvdItem(extendDim)); From 8b88163d249cfdee10e75642c5424c0c9603cea2 Mon Sep 17 00:00:00 2001 From: Thorsten de Buhr Date: Tue, 20 Oct 2026 14:48:41 +0200 Subject: [PATCH 17/26] refactor(svdconv): simplify unsigned shift literal --- tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp b/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp index 2a61fe830..d6b266a26 100644 --- a/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp +++ b/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp @@ -1033,7 +1033,7 @@ bool SfdData::CreateEnumValueSet(SvdEnumContainer *cont, SvdField* field) } const auto bitWidth = field->GetEffectiveBitWidth(); - const auto bitMaxNum = uint32_t{1} << bitWidth; + const uint32_t bitMaxNum = 1U << bitWidth; for(uint32_t bitNum=0; bitNum Date: Tue, 20 Oct 2026 15:02:02 +0200 Subject: [PATCH 18/26] fix(svdconv): bound enum lists and calculate counts in 64 bits --- .../SVDGenerator/src/SfdData_SingleItems.cpp | 8 ++++-- .../test/unittests/src/SvdBitWidthTest.cpp | 25 +++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp b/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp index d6b266a26..7448733a0 100644 --- a/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp +++ b/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp @@ -1015,6 +1015,11 @@ bool SfdData::CreateEnumValueSet(SvdEnumContainer *cont, SvdField* field) return true; } + const auto bitWidth = field->GetEffectiveBitWidth(); + if(bitWidth == 0 || bitWidth > MAX_BITWIDTH_FOR_COMBO) { + return false; + } + const auto defaultValue = cont->GetDefaultValue(); map enumValues; @@ -1032,8 +1037,7 @@ bool SfdData::CreateEnumValueSet(SvdEnumContainer *cont, SvdField* field) enumValues[val.u32] = enu; } - const auto bitWidth = field->GetEffectiveBitWidth(); - const uint32_t bitMaxNum = 1U << bitWidth; + const uint64_t bitMaxNum = 1ULL << bitWidth; for(uint32_t bitNum=0; bitNumSetValue(0); + for(const auto width : {0, 7, 31, 32, 64}) { + SCOPED_TRACE(width); + field.SetBitWidth(width); + ASSERT_FALSE(sfd.CreateEnumValueSet(&values, &field)); + } + for(const auto width : {1, 6}) { + SCOPED_TRACE(width); + field.SetBitWidth(width); + EXPECT_TRUE(sfd.CreateEnumValueSet(&values, &field)); + } +} + TEST_F(SvdBitWidthTest, SfdRejectsInvalidExpressionReferencesBeforeCreatingOutput) { CreateOutputDirectory(); SvdDevice device(nullptr); From 38fa7861755fb1cc9f6613f810c5a0e557d21f5c Mon Sep 17 00:00:00 2001 From: Thorsten de Buhr Date: Tue, 20 Oct 2026 15:06:28 +0200 Subject: [PATCH 19/26] refactor(svdconv): use 32-bit counts after enum width validation --- tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp b/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp index 7448733a0..88aa5c2e9 100644 --- a/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp +++ b/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp @@ -1037,7 +1037,7 @@ bool SfdData::CreateEnumValueSet(SvdEnumContainer *cont, SvdField* field) enumValues[val.u32] = enu; } - const uint64_t bitMaxNum = 1ULL << bitWidth; + const uint32_t bitMaxNum = 1U << bitWidth; for(uint32_t bitNum=0; bitNum Date: Tue, 20 Oct 2026 15:10:26 +0200 Subject: [PATCH 20/26] style(svdconv): expand newly added control-flow blocks --- tools/svdconv/SVDModel/src/SvdCluster.cpp | 8 +++-- tools/svdconv/SVDModel/src/SvdDerivedFrom.cpp | 8 +++-- tools/svdconv/SVDModel/src/SvdDimension.cpp | 8 +++-- tools/svdconv/SVDModel/src/SvdInterrupt.cpp | 20 +++++++++--- tools/svdconv/SVDModel/src/SvdItem.cpp | 32 ++++++++++++++----- tools/svdconv/SVDModel/src/SvdPeripheral.cpp | 24 ++++++++++---- .../test/integtests/src/SvdConvIntegTests.cpp | 20 +++++++++--- .../unittests/src/SvdDimensionBoundsTest.cpp | 12 +++++-- 8 files changed, 99 insertions(+), 33 deletions(-) diff --git a/tools/svdconv/SVDModel/src/SvdCluster.cpp b/tools/svdconv/SVDModel/src/SvdCluster.cpp index 25d4564d8..38d55020b 100644 --- a/tools/svdconv/SVDModel/src/SvdCluster.cpp +++ b/tools/svdconv/SVDModel/src/SvdCluster.cpp @@ -294,9 +294,13 @@ bool SvdCluster::CalculateDim() } const auto newClust = new SvdCluster(dim); dim->AddItem(newClust); - if(!CopyChilds(this, newClust)) { return false; } + if(!CopyChilds(this, newClust)) { + return false; + } newClust->CopyItem(this); - if(HasExpansionError()) { return false; } + if(HasExpansionError()) { + return false; + } newClust->SetName(dim->CreateName(dimIndexname)); newClust->SetDisplayName(dim->CreateDisplayName(dimIndexname)); newClust->SetDescription(dim->CreateDescription(dimIndexname)); diff --git a/tools/svdconv/SVDModel/src/SvdDerivedFrom.cpp b/tools/svdconv/SVDModel/src/SvdDerivedFrom.cpp index ac3829af6..add8a9616 100644 --- a/tools/svdconv/SVDModel/src/SvdDerivedFrom.cpp +++ b/tools/svdconv/SVDModel/src/SvdDerivedFrom.cpp @@ -116,11 +116,15 @@ bool SvdDerivedFrom::DeriveItem(SvdItem *from) SetDerivedFromItem(from); // Also copy all element's childs - if(!CopyChilds(from, parent)) { return false; } + if(!CopyChilds(from, parent)) { + return false; + } // Create a copy parent->CopyItem(from); - if(HasExpansionError()) { return false; } + if(HasExpansionError()) { + return false; + } const auto dim = parent->GetDimension(); if(dim) { diff --git a/tools/svdconv/SVDModel/src/SvdDimension.cpp b/tools/svdconv/SVDModel/src/SvdDimension.cpp index eb4a329c8..779204ead 100644 --- a/tools/svdconv/SVDModel/src/SvdDimension.cpp +++ b/tools/svdconv/SVDModel/src/SvdDimension.cpp @@ -379,7 +379,9 @@ bool SvdDimension::CalculateDim() LogMsg("M250", NAME(parent ? parent->GetName() : GetName()), GetLineNumber()); } Invalidate(); - if(parent) { parent->Invalidate(); } + if(parent) { + parent->Invalidate(); + } m_dimIndexList.clear(); return false; } @@ -394,7 +396,9 @@ bool SvdDimension::CheckExpansionRange(uint64_t start, uint64_t increment) const auto parent = GetParent(); LogMsg("M250", NAME(parent ? parent->GetName() : GetName()), GetLineNumber()); Invalidate(); - if(parent) { parent->Invalidate(); } + if(parent) { + parent->Invalidate(); + } return false; } return !HasExpansionError(); diff --git a/tools/svdconv/SVDModel/src/SvdInterrupt.cpp b/tools/svdconv/SVDModel/src/SvdInterrupt.cpp index f5eaa2b44..c8d7fda55 100644 --- a/tools/svdconv/SVDModel/src/SvdInterrupt.cpp +++ b/tools/svdconv/SVDModel/src/SvdInterrupt.cpp @@ -65,7 +65,9 @@ bool SvdInterrupt::CalculateDim() if(parent) { const auto parentDim = parent->GetDimension(); if(parentDim) { - if(!CopyDim(this, parent)) { return false; } + if(!CopyDim(this, parent)) { + return false; + } dim = GetDimension(); dim->SetDimIncrement(1); } @@ -83,7 +85,9 @@ bool SvdInterrupt::CalculateDim() dim->ClearChildren(); } - if(!dim->CalculateDim()) { return false; } + if(!dim->CalculateDim()) { + return false; + } const auto& dimIndexList = dim->GetDimIndexList(); uint32_t value = GetValue(); @@ -99,13 +103,19 @@ bool SvdInterrupt::CalculateDim() } for(const auto& dimIndex : dimIndexList) { - if(!dim->ReserveExpansion(1)) { return false; } + if(!dim->ReserveExpansion(1)) { + return false; + } const auto newIrq = new SvdInterrupt(dim); dim->AddItem(newIrq); - if(!CopyChilds(this, newIrq)) { return false; } + if(!CopyChilds(this, newIrq)) { + return false; + } newIrq->CopyItem (this); - if(HasExpansionError()) { return false; } + if(HasExpansionError()) { + return false; + } newIrq->SetName (dim->CreateName(dimIndex)); newIrq->SetDescription (dim->CreateDescription(dimIndex)); newIrq->SetValue (value); diff --git a/tools/svdconv/SVDModel/src/SvdItem.cpp b/tools/svdconv/SVDModel/src/SvdItem.cpp index ab8c7a65b..c06457cf1 100644 --- a/tools/svdconv/SVDModel/src/SvdItem.cpp +++ b/tools/svdconv/SVDModel/src/SvdItem.cpp @@ -985,7 +985,9 @@ bool SvdItem::CopyChilds(SvdItem *from, SvdItem *hook) if(lv == L_EnumeratedValues) { // more containers can be set! const auto nItem = new SvdEnumContainer(hook); hook->AddItem(nItem); - if(!CopyChilds(copy, nItem)) { return false; } + if(!CopyChilds(copy, nItem)) { + return false; + } nItem->CopyItem(copy); } else if(lv == L_Fields) { @@ -998,7 +1000,9 @@ bool SvdItem::CopyChilds(SvdItem *from, SvdItem *hook) else { nItem = (SvdFieldContainer *)*(hook->GetChildren().begin()); } - if(!CopyChilds(copy, nItem)) { return false; } + if(!CopyChilds(copy, nItem)) { + return false; + } } else if(lv == L_Registers) { SvdRegisterContainer *nItem; @@ -1010,38 +1014,50 @@ bool SvdItem::CopyChilds(SvdItem *from, SvdItem *hook) else { nItem = (SvdRegisterContainer *)*(hook->GetChildren().begin()); } - if(!CopyChilds(copy, nItem)) { return false; } + if(!CopyChilds(copy, nItem)) { + return false; + } } // Elements else if(lv == L_EnumeratedValue) { const auto nItem = new SvdEnum(hook); hook->AddItem(nItem); - if(!CopyChilds(copy, nItem)) { return false; } + if(!CopyChilds(copy, nItem)) { + return false; + } nItem->CopyItem(copy); } else if(lv == L_Field) { const auto nItem = new SvdField(hook); hook->AddItem(nItem); - if(!CopyChilds(copy, nItem)) { return false; } + if(!CopyChilds(copy, nItem)) { + return false; + } nItem->CopyItem(copy); } else if(lv == L_Register) { const auto nItem = new SvdRegister(hook); hook->AddItem(nItem); - if(!CopyChilds(copy, nItem)) { return false; } + if(!CopyChilds(copy, nItem)) { + return false; + } nItem->CopyItem(copy); } else if(lv == L_Peripheral) { const auto nItem = new SvdPeripheral(hook); hook->AddItem(nItem); - if(!CopyChilds(copy, nItem)) { return false; } + if(!CopyChilds(copy, nItem)) { + return false; + } nItem->CopyItem(copy); } else if(lv == L_Cluster) { const auto nItem = new SvdCluster(hook); hook->AddItem(nItem); - if(!CopyChilds(copy, nItem)) { return false; } + if(!CopyChilds(copy, nItem)) { + return false; + } nItem->CopyItem(copy); } if(hook->HasExpansionError()) { diff --git a/tools/svdconv/SVDModel/src/SvdPeripheral.cpp b/tools/svdconv/SVDModel/src/SvdPeripheral.cpp index 3bb1e2099..8c63f9d7f 100644 --- a/tools/svdconv/SVDModel/src/SvdPeripheral.cpp +++ b/tools/svdconv/SVDModel/src/SvdPeripheral.cpp @@ -373,7 +373,9 @@ bool SvdPeripheral::CopyItem(SvdItem *from) if(resetMask == 0 ) { SetResetMask (pFrom->GetResetMask ()); } if(access == SvdTypes::Access::UNDEF) { SetAccess (pFrom->GetAccess ()); } - if(!CopyAddressBlocks(pFrom) || HasExpansionError()) { return false; } + if(!CopyAddressBlocks(pFrom) || HasExpansionError()) { + return false; + } SvdItem::CopyItem(pFrom); CalculateDim(); @@ -391,7 +393,9 @@ bool SvdPeripheral::CopyAddressBlocks(SvdPeripheral *from) continue; } - if(!ReserveExpansion(1)) { return false; } + if(!ReserveExpansion(1)) { + return false; + } const auto addressBlock = new SvdAddressBlock(this); addressBlock->CopyItem(addrBlock); AddAddressBlock(addressBlock); @@ -434,7 +438,9 @@ bool SvdPeripheral::CopyDisableCondition(SvdCExpression* from) return true; } - if(!ReserveExpansion(1)) { return false; } + if(!ReserveExpansion(1)) { + return false; + } m_disableCondition = new SvdCExpression(*from); return true; @@ -477,10 +483,14 @@ bool SvdPeripheral::CalculateDim() } const auto newPeri = new SvdPeripheral(dim); dim->AddItem(newPeri); - if(!CopyDerivedFrom(newPeri, this) || !CopyChilds(parent, newPeri)) { return false; } + if(!CopyDerivedFrom(newPeri, this) || !CopyChilds(parent, newPeri)) { + return false; + } newPeri->CopyItem (parent); - if(HasExpansionError()) { return false; } + if(HasExpansionError()) { + return false; + } newPeri->SetName (dim->CreateName(dimIndexname)); newPeri->SetDisplayName (dim->CreateDisplayName(dimIndexname)); newPeri->SetDescription (dim->CreateDescription(dimIndexname)); @@ -1285,7 +1295,9 @@ bool SvdPeripheral::SortAddressBlocks(map& addrBlock continue; // only use "register" addressBlocks } - if(!ReserveExpansion(1)) { return false; } + if(!ReserveExpansion(1)) { + return false; + } const auto block = new SvdAddressBlock(this); block->CopyItem(addrBlock); diff --git a/tools/svdconv/test/integtests/src/SvdConvIntegTests.cpp b/tools/svdconv/test/integtests/src/SvdConvIntegTests.cpp index a305d67e6..41f0db9c4 100644 --- a/tools/svdconv/test/integtests/src/SvdConvIntegTests.cpp +++ b/tools/svdconv/test/integtests/src/SvdConvIntegTests.cpp @@ -320,7 +320,9 @@ TEST_F(SvdConvIntegTests, ExpansionLimitStopsAllGeneratorsAndNextInputStillWorks SvdConv converter; EXPECT_EQ(2, converter.Check(args, args, nullptr)); string messages; - for(const auto& message : ErrLog::Get()->GetLogMessages()) { messages += message; } + for(const auto& message : ErrLog::Get()->GetLogMessages()) { + messages += message; + } EXPECT_NE(string::npos, messages.find("M249")); for(const auto* suffix : {".h", ".sfd", "_MapFields.txt"}) { EXPECT_FALSE(RteFsUtils::Exists(output + "/PosMaskDim" + suffix)); @@ -363,7 +365,9 @@ TEST_F(SvdConvIntegTests, RejectedSfdDoesNotCompileStaleOutput) { SvdConv converter; EXPECT_EQ(2, converter.Check(args, args, nullptr)); string messages; - for(const auto& message : ErrLog::Get()->GetLogMessages()) { messages += message; } + for(const auto& message : ErrLog::Get()->GetLogMessages()) { + messages += message; + } EXPECT_NE(string::npos, messages.find("M226")); EXPECT_NE(string::npos, messages.find("M111")); EXPECT_EQ(string::npos, messages.find("Generate System Viewer SFR File")); @@ -407,7 +411,9 @@ TEST_F(SvdConvIntegTests, InvalidExpressionFieldDoesNotCompileStaleOutput) { SvdConv converter; EXPECT_EQ(2, converter.Check(args, args, nullptr)); string messages; - for(const auto& message : ErrLog::Get()->GetLogMessages()) { messages += message; } + for(const auto& message : ErrLog::Get()->GetLogMessages()) { + messages += message; + } EXPECT_NE(string::npos, messages.find("M310")); EXPECT_NE(string::npos, messages.find("M226")); EXPECT_NE(string::npos, messages.find("M111")); @@ -431,7 +437,9 @@ TEST_F(SvdConvIntegTests, FailedSfdCreationDoesNotCompileOutput) { SvdConv converter; EXPECT_EQ(2, converter.Check(args, args, nullptr)); string messages; - for(const auto& message : ErrLog::Get()->GetLogMessages()) { messages += message; } + for(const auto& message : ErrLog::Get()->GetLogMessages()) { + messages += message; + } EXPECT_NE(string::npos, messages.find("M130")); EXPECT_NE(string::npos, messages.find("M111")); EXPECT_EQ(string::npos, messages.find("Generate System Viewer SFR File")); @@ -469,7 +477,9 @@ TEST_F(SvdConvIntegTests, ReadOnlySfdDoesNotCompileStaleOutput) { EXPECT_TRUE(RteFsUtils::SetFileReadOnly(output, false)); EXPECT_EQ(2, result); string messages; - for(const auto& message : ErrLog::Get()->GetLogMessages()) { messages += message; } + for(const auto& message : ErrLog::Get()->GetLogMessages()) { + messages += message; + } EXPECT_NE(string::npos, messages.find("M130")); EXPECT_NE(string::npos, messages.find("M111")); EXPECT_EQ(string::npos, messages.find("Generate System Viewer SFR File")); diff --git a/tools/svdconv/test/unittests/src/SvdDimensionBoundsTest.cpp b/tools/svdconv/test/unittests/src/SvdDimensionBoundsTest.cpp index 0b626e4e2..0ce093b05 100644 --- a/tools/svdconv/test/unittests/src/SvdDimensionBoundsTest.cpp +++ b/tools/svdconv/test/unittests/src/SvdDimensionBoundsTest.cpp @@ -42,8 +42,12 @@ class SvdDimensionBoundsTest : public testing::Test, public IErrConsumer { log->SetErrConsumer(m_previousConsumer); log->SetQuietMode(m_previousQuiet); log->ResetMsgCount(); - for(int i = 0; i < m_previousErrors; i++) { log->IncErrCnt(); } - for(int i = 0; i < m_previousWarnings; i++) { log->IncWarnCnt(); } + for(int i = 0; i < m_previousErrors; i++) { + log->IncErrCnt(); + } + for(int i = 0; i < m_previousWarnings; i++) { + log->IncWarnCnt(); + } } bool Consume(const PdscMsg& msg, const string&) override { @@ -107,7 +111,9 @@ TEST_F(SvdDimensionBoundsTest, InvalidAndOversizedCountsDoNotAllocateIndices) { TEST_F(SvdDimensionBoundsTest, OversizedExplicitListIsStoppedBeforeExpansion) { SvdRegister reg(nullptr); string indices; - for(size_t i = 0; i <= SvdItem::MAX_EXPANSION_ITEMS; i++) { indices += "A,"; } + for(size_t i = 0; i <= SvdItem::MAX_EXPANSION_ITEMS; i++) { + indices += "A,"; + } const auto dim = Dimension(reg, 2, 4, indices); EXPECT_FALSE(reg.CalculateDim()); EXPECT_TRUE(reg.HasExpansionError()); From 2f189e08eb80aeebf13cb50ac716bb48b68c7401 Mon Sep 17 00:00:00 2001 From: Thorsten de Buhr Date: Tue, 20 Oct 2026 15:20:39 +0200 Subject: [PATCH 21/26] refactor(svdconv): reuse enum width limit in field diagnostics --- tools/svdconv/SVDModel/src/SvdField.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/svdconv/SVDModel/src/SvdField.cpp b/tools/svdconv/SVDModel/src/SvdField.cpp index 1c5cda5c8..cb60167a8 100644 --- a/tools/svdconv/SVDModel/src/SvdField.cpp +++ b/tools/svdconv/SVDModel/src/SvdField.cpp @@ -552,7 +552,7 @@ bool SvdField::CheckItem() const auto fieldMaxVal = SvdUtils::CreateBitMask(width); const auto& enumConts = GetEnumContainer(); - if(enumConts.empty() && width < 6) { // message "please add enum values" + if(enumConts.empty() && width < MAX_BITWIDTH_FOR_COMBO) { // message "please add enum values" LogMsg("M347", NAME(name), lineNo); } From e422da582d3f414b9013cb24fe0adf53bc0e7b53 Mon Sep 17 00:00:00 2001 From: Thorsten de Buhr Date: Tue, 20 Oct 2026 15:31:11 +0200 Subject: [PATCH 22/26] refactor(svdconv): assign validated bit ranges without explicit casts --- tools/svdconv/SVDModel/src/SvdUtils.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/svdconv/SVDModel/src/SvdUtils.cpp b/tools/svdconv/SVDModel/src/SvdUtils.cpp index ae874d3db..e817a4f85 100644 --- a/tools/svdconv/SVDModel/src/SvdUtils.cpp +++ b/tools/svdconv/SVDModel/src/SvdUtils.cpp @@ -459,8 +459,8 @@ bool SvdUtils::GetBitRange32(uint64_t rawOffset, int64_t rawWidth, uint32_t& off return false; } - offset = static_cast(rawOffset); - width = static_cast(rawWidth); + offset = rawOffset; + width = rawWidth; return true; } From 633bdd4e2e7f6f4f32c5a7dcda234081f14967e8 Mon Sep 17 00:00:00 2001 From: Thorsten de Buhr Date: Tue, 20 Oct 2026 15:57:06 +0200 Subject: [PATCH 23/26] test(svdconv): cover input-bound failure paths --- tools/svdconv/SVDModel/src/SvdInterrupt.cpp | 3 - tools/svdconv/SVDModel/src/SvdUtils.cpp | 2 +- .../test/unittests/src/SvdBitWidthTest.cpp | 78 +++++ .../unittests/src/SvdDimensionBoundsTest.cpp | 310 ++++++++++++++++++ 4 files changed, 389 insertions(+), 4 deletions(-) diff --git a/tools/svdconv/SVDModel/src/SvdInterrupt.cpp b/tools/svdconv/SVDModel/src/SvdInterrupt.cpp index c8d7fda55..740dcd31b 100644 --- a/tools/svdconv/SVDModel/src/SvdInterrupt.cpp +++ b/tools/svdconv/SVDModel/src/SvdInterrupt.cpp @@ -113,9 +113,6 @@ bool SvdInterrupt::CalculateDim() } newIrq->CopyItem (this); - if(HasExpansionError()) { - return false; - } newIrq->SetName (dim->CreateName(dimIndex)); newIrq->SetDescription (dim->CreateDescription(dimIndex)); newIrq->SetValue (value); diff --git a/tools/svdconv/SVDModel/src/SvdUtils.cpp b/tools/svdconv/SVDModel/src/SvdUtils.cpp index e817a4f85..40c5dd8e9 100644 --- a/tools/svdconv/SVDModel/src/SvdUtils.cpp +++ b/tools/svdconv/SVDModel/src/SvdUtils.cpp @@ -455,7 +455,7 @@ bool SvdUtils::ConvertNumber(const string &text, set &numbers) // changes to parsing, storage, setters, dimensions, inheritance and sentinels. bool SvdUtils::GetBitRange32(uint64_t rawOffset, int64_t rawWidth, uint32_t& offset, uint32_t& width) { - if(rawOffset >= 32 || rawWidth <= 0 || rawWidth > 32 || rawWidth > 32 - rawOffset) { + if(rawOffset >= 32 || rawWidth <= 0 || rawWidth > 32 || rawOffset + rawWidth > 32) { return false; } diff --git a/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp b/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp index 27cad18bb..c67696898 100644 --- a/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp +++ b/tools/svdconv/test/unittests/src/SvdBitWidthTest.cpp @@ -38,6 +38,8 @@ class BitWidthHeaderData : public HeaderData { public: using HeaderData::HeaderData; using HeaderData::AddFields; + using HeaderData::CreateField; + using HeaderData::CreateFields; using HeaderData::CreateSortedFields; using HeaderData::CreateFieldPosMask; }; @@ -470,6 +472,82 @@ TEST_F(SvdBitWidthTest, HeaderFieldPaddingRejectsOverlapsAndOutOfRangePositions) EXPECT_FALSE(header.CreateSortedFields(groups, 8, "bits")); } +TEST_F(SvdBitWidthTest, HeaderFieldsPropagateRejectedRegisterLayouts) { + SvdOptions options; + FileHeaderInfo info; + BitWidthHeaderData header(info, options); + SvdRegister reg(nullptr); + AddUncheckedField(reg, 7, 2); + + reg.SetBitWidth(8); + EXPECT_FALSE(header.CreateFields(®, "bits")); + reg.SetBitWidth(64); + EXPECT_FALSE(header.CreateFields(®, "bits")); + reg.SetBitWidth(32); + EXPECT_TRUE(header.CreateFields(®, "bits")); +} + +TEST_F(SvdBitWidthTest, HeaderFieldsEmitBoundedPaddingAndRejectInvalidFieldFragments) { + CreateOutputDirectory(); + const auto inputFile = m_outputDirectory / "input.svd"; + { + ofstream input(inputFile); + input << "\n"; + ASSERT_TRUE(input.good()); + } + SvdDevice device(nullptr); + device.SetName("TEST"); + SvdRegister reg(nullptr); + reg.SetBitWidth(32); + AddUncheckedField(reg, 2, 3)->SetName("ACTIVE"); + SvdOptions options; + FileHeaderInfo info; + info.svdFileName = inputFile.string(); + const auto output = m_outputDirectory / "masks.h"; + { + BitWidthHeaderData header(info, options); + ASSERT_TRUE(header.Create(&device, output.string())); + EXPECT_TRUE(header.CreateFields(®, "bits")); + + SvdField field(nullptr); + field.SetName("REJECTED"); + const pair invalidRanges[] = { + {0, 0}, {0, -1}, {0, 33}, {0, 64}, {31, 2}, {32, 1}, + {UINT64_C(0x10000001f), 1}, + }; + for(const auto& [offset, width] : invalidRanges) { + SCOPED_TRACE(to_string(offset) + ":" + to_string(width)); + field.SetOffset(offset); + field.SetBitWidth(width); + EXPECT_EQ(0U, header.CreateField(&field, 4, 0)); + } + field.SetName("UPPER"); + field.SetOffset(31); + field.SetBitWidth(1); + field.SetAccess(SvdTypes::Access::READWRITE); + EXPECT_EQ(1U, header.CreateField(&field, 4, 31)); + } + ifstream input(output); + ASSERT_TRUE(input.is_open()); + const string text{istreambuf_iterator(input), istreambuf_iterator()}; + istringstream tokens(text); + string token, compact; + while(tokens >> token) { + compact += token; + } + const auto leading = compact.find("uint32_t:2;"); + const auto active = compact.find("__IOMuint32_tACTIVE:3;"); + const auto trailing = compact.find("uint32_t:27;"); + ASSERT_NE(string::npos, leading); + ASSERT_NE(string::npos, active); + ASSERT_NE(string::npos, trailing); + EXPECT_LT(leading, active); + EXPECT_LT(active, trailing); + EXPECT_NE(string::npos, compact.find("__IOMuint32_tUPPER:1;")); + EXPECT_EQ(string::npos, text.find("REJECTED")); + EXPECT_EQ(string::npos, text.find("Reserved bits calculation error")); +} + TEST_F(SvdBitWidthTest, HeaderMasksPreserve32BitBoundaryTextAndRejectWiderRanges) { CreateOutputDirectory(); FileIo output; diff --git a/tools/svdconv/test/unittests/src/SvdDimensionBoundsTest.cpp b/tools/svdconv/test/unittests/src/SvdDimensionBoundsTest.cpp index 0ce093b05..cf4a3e876 100644 --- a/tools/svdconv/test/unittests/src/SvdDimensionBoundsTest.cpp +++ b/tools/svdconv/test/unittests/src/SvdDimensionBoundsTest.cpp @@ -4,10 +4,12 @@ * SPDX-License-Identifier: Apache-2.0 */ #include "SvdItem.h" +#include "SvdAddressBlock.h" #include "SvdCluster.h" #include "SvdDerivedFrom.h" #include "SvdDevice.h" #include "SvdDimension.h" +#include "SvdEnum.h" #include "SvdField.h" #include "SvdInterrupt.h" #include "SvdPeripheral.h" @@ -66,6 +68,15 @@ class SvdDimensionBoundsTest : public testing::Test, public IErrConsumer { return dim; } + static SvdAddressBlock* AddressBlock(SvdPeripheral& peripheral, uint32_t offset = 0) { + const auto block = new SvdAddressBlock(&peripheral); + block->SetOffset(offset); + block->SetSize(0x100); + block->SetUsage(SvdTypes::AddrBlockUsage::REGISTERS); + peripheral.AddAddressBlock(block); + return block; + } + bool HasMessage(const string& id) const { return find(m_messages.begin(), m_messages.end(), id) != m_messages.end(); } @@ -255,6 +266,112 @@ TEST_F(SvdDimensionBoundsTest, CopiedPeripheralDescendantsShareOneBudget) { } } +TEST_F(SvdDimensionBoundsTest, DerivationStopsAtChildAndDimensionMetadataBudgetBoundaries) { + SvdItem source(nullptr); + Dimension(source, 2)->SetDimName("SOURCE"); + const auto reg = new SvdRegister(&source); + source.AddItem(reg); + reg->SetName("DATA"); + reg->SetOffset(4); + reg->SetBitWidth(32); + + for(const size_t remaining : {size_t{0}, size_t{1}, size_t{2}}) { + SCOPED_TRACE(remaining); + SvdItem target(nullptr); + const auto derived = new SvdDerivedFrom(&target); + target.SetDerivedFrom(derived); + ASSERT_TRUE(target.ReserveExpansion(SvdItem::MAX_EXPANSION_ITEMS - remaining)); + + EXPECT_EQ(remaining == 2, derived->DeriveItem(&source)); + EXPECT_EQ(remaining < 2, target.HasExpansionError()); + EXPECT_EQ(&source, derived->GetDerivedFromItem()); + ASSERT_EQ(remaining > 0 ? 1U : 0U, target.GetChildCount()); + if(remaining == 2) { + ASSERT_NE(nullptr, target.GetDimension()); + EXPECT_EQ(2U, target.GetDimension()->GetDim()); + EXPECT_TRUE(target.GetDimension()->GetDimName().empty()); + EXPECT_EQ(&source, target.GetCopiedFrom()); + } + else { + EXPECT_EQ(nullptr, target.GetDimension()); + EXPECT_EQ(nullptr, target.GetCopiedFrom()); + } + if(remaining > 0) { + EXPECT_EQ("DATA", target.GetChildren().front()->GetName()); + EXPECT_EQ(reg, target.GetChildren().front()->GetCopiedFrom()); + } + } +} + +TEST_F(SvdDimensionBoundsTest, CopiedFieldsAndRegistersPropagateDescendantBudgetFailures) { + for(const bool isField : {false, true}) { + SCOPED_TRACE(isField); + SvdItem source(nullptr); + SvdItem* child = nullptr; + if(isField) { + child = new SvdField(&source); + child->AddItem(new SvdEnumContainer(child)); + } + else { + child = new SvdRegister(&source); + child->AddItem(new SvdFieldContainer(child)); + } + source.AddItem(child); + child->SetName("DATA"); + child->SetBitWidth(isField ? 1 : 32); + + for(const size_t remaining : {size_t{1}, size_t{2}}) { + SCOPED_TRACE(remaining); + SvdItem target(nullptr); + ASSERT_TRUE(target.ReserveExpansion(SvdItem::MAX_EXPANSION_ITEMS - remaining)); + + EXPECT_EQ(remaining == 2, target.CopyChilds(&source, &target)); + EXPECT_EQ(remaining == 1, target.HasExpansionError()); + ASSERT_EQ(1U, target.GetChildCount()); + const auto copy = target.GetChildren().front(); + EXPECT_EQ(child->GetSvdLevel(), copy->GetSvdLevel()); + ASSERT_EQ(remaining - 1, copy->GetChildCount()); + EXPECT_EQ(remaining == 2 ? child : nullptr, copy->GetCopiedFrom()); + if(remaining == 2) { + EXPECT_EQ("DATA", copy->GetName()); + EXPECT_EQ(child->GetChildren().front(), copy->GetChildren().front()->GetCopiedFrom()); + } + } + } +} + +TEST_F(SvdDimensionBoundsTest, DirectlyBuiltEnumLeafSubtreesStillRespectTheCopyBudget) { + // Direct model callers can attach children to leaf elements outside the XML schema. + SvdField sourceField(nullptr); + SvdEnumContainer source(&sourceField); + const auto value = new SvdEnum(&source); + source.AddItem(value); + value->SetName("OUTER"); + value->SetValue(1); + const auto nested = new SvdEnum(value); + value->AddItem(nested); + nested->SetName("INNER"); + nested->SetValue(2); + + for(const size_t remaining : {size_t{1}, size_t{2}}) { + SCOPED_TRACE(remaining); + SvdField targetField(nullptr); + SvdEnumContainer target(&targetField); + ASSERT_TRUE(target.ReserveExpansion(SvdItem::MAX_EXPANSION_ITEMS - remaining)); + + EXPECT_EQ(remaining == 2, target.CopyChilds(&source, &target)); + EXPECT_EQ(remaining == 1, target.HasExpansionError()); + ASSERT_EQ(1U, target.GetChildCount()); + const auto copy = target.GetChildren().front(); + ASSERT_EQ(remaining - 1, copy->GetChildCount()); + EXPECT_EQ(remaining == 2 ? value : nullptr, copy->GetCopiedFrom()); + if(remaining == 2) { + EXPECT_EQ("INNER", copy->GetChildren().front()->GetName()); + EXPECT_EQ(nested, copy->GetChildren().front()->GetCopiedFrom()); + } + } +} + TEST_F(SvdDimensionBoundsTest, IndexBudgetIsSharedBySiblingDimensions) { SvdItem root(nullptr); SvdRegister first(&root), second(&root), third(&root); @@ -421,6 +538,132 @@ TEST_F(SvdDimensionBoundsTest, FieldAndRegisterExpansionStopsWhenInheritedMetada } } +TEST_F(SvdDimensionBoundsTest, ClusterAndPeripheralExpansionCheckInheritedMetadataBudget) { + for(const size_t remaining : {size_t{3}, size_t{6}}) { + SCOPED_TRACE(remaining); + SvdCluster cluster(nullptr); + cluster.SetOffset(0); + SvdPeripheral peripheral(nullptr); + peripheral.SetAddress(0x40000000); + SvdItem* sources[] = {&cluster, &peripheral}; + + for(const auto source : sources) { + SCOPED_TRACE(source->GetSvdLevel()); + const auto dim = Dimension(*source, 2); + source->SetDerivedFrom(new SvdDerivedFrom(source)); + // Each copy needs one item and one inherited metadata object, after the two indices. + ASSERT_TRUE(source->ReserveExpansion(SvdItem::MAX_EXPANSION_ITEMS - remaining)); + + EXPECT_EQ(remaining == 6, source->CalculateDim()); + EXPECT_EQ(remaining == 3, source->HasExpansionError()); + ASSERT_EQ(remaining == 6 ? 2U : 1U, dim->GetChildCount()); + const auto first = dim->GetChildren().front(); + if(remaining == 6) { + for(const auto copy : dim->GetChildren()) { + ASSERT_NE(nullptr, copy->GetDerivedFrom()); + EXPECT_EQ(source, copy->GetDerivedFrom()->GetDerivedFromItem()); + EXPECT_EQ(source, copy->GetCopiedFrom()); + } + EXPECT_EQ("ITEM0", first->GetName()); + EXPECT_EQ("ITEM1", dim->GetChildren().back()->GetName()); + } + else { + EXPECT_EQ(nullptr, first->GetDerivedFrom()); + EXPECT_EQ(nullptr, first->GetCopiedFrom()); + } + } + } +} + +TEST_F(SvdDimensionBoundsTest, PeripheralDimensionStopsWhenAddressBlocksCannotBeCopied) { + for(const size_t remaining : {size_t{3}, size_t{6}}) { + SCOPED_TRACE(remaining); + SvdPeripheral peripheral(nullptr); + peripheral.SetAddress(0x40000000); + const auto block = AddressBlock(peripheral, 0x20); + const auto dim = Dimension(peripheral, 2, 0x1000); + ASSERT_TRUE(peripheral.ReserveExpansion(SvdItem::MAX_EXPANSION_ITEMS - remaining)); + + EXPECT_EQ(remaining == 6, peripheral.CalculateDim()); + EXPECT_EQ(remaining == 3, peripheral.HasExpansionError()); + ASSERT_EQ(remaining == 6 ? 2U : 1U, dim->GetChildCount()); + for(const auto child : dim->GetChildren()) { + const auto copy = dynamic_cast(child); + ASSERT_NE(nullptr, copy); + if(remaining == 6) { + ASSERT_EQ(1U, copy->GetAddressBlock().size()); + const auto copiedBlock = copy->GetAddressBlock().front(); + EXPECT_NE(block, copiedBlock); + EXPECT_EQ(0x20U, copiedBlock->GetOffset()); + EXPECT_EQ(0x100U, copiedBlock->GetSize()); + EXPECT_EQ(block, copiedBlock->GetCopiedFrom()); + EXPECT_EQ(&peripheral, copy->GetCopiedFrom()); + } + else { + EXPECT_TRUE(copy->GetAddressBlock().empty()); + EXPECT_EQ(nullptr, copy->GetCopiedFrom()); + } + } + EXPECT_EQ(1U, peripheral.GetAddressBlock().size()); + } +} + +TEST_F(SvdDimensionBoundsTest, PeripheralCopyPropagatesDisableConditionBudgetFailure) { + SvdCExpression expression; + expression.SetText("CONTROL.ENABLE == 0"); + SvdPeripheral source(nullptr); + source.SetName("PERIPHERAL"); + ASSERT_TRUE(source.CopyDisableCondition(&expression)); + + for(const size_t remaining : {size_t{0}, size_t{1}}) { + SCOPED_TRACE(remaining); + SvdPeripheral target(nullptr); + ASSERT_TRUE(target.ReserveExpansion(SvdItem::MAX_EXPANSION_ITEMS - remaining)); + + target.CopyItem(&source); + EXPECT_EQ(remaining == 0, target.HasExpansionError()); + if(remaining == 1) { + ASSERT_NE(nullptr, target.GetDisableCondition()); + EXPECT_NE(source.GetDisableCondition(), target.GetDisableCondition()); + EXPECT_EQ(expression.GetText(), target.GetDisableCondition()->GetText()); + EXPECT_EQ(&source, target.GetCopiedFrom()); + EXPECT_EQ("PERIPHERAL", target.GetName()); + } + else { + EXPECT_EQ(nullptr, target.GetDisableCondition()); + EXPECT_EQ(nullptr, target.GetCopiedFrom()); + } + } +} + +TEST_F(SvdDimensionBoundsTest, AddressBlockSortingStopsAtTheSharedBudgetBoundary) { + for(const size_t remaining : {size_t{1}, size_t{2}}) { + SCOPED_TRACE(remaining); + SvdPeripheral peripheral(nullptr); + const auto upper = AddressBlock(peripheral, 0x200); + const auto lower = AddressBlock(peripheral, 0); + ASSERT_TRUE(peripheral.ReserveExpansion(SvdItem::MAX_EXPANSION_ITEMS - remaining)); + map sorted; + + EXPECT_EQ(remaining == 2, peripheral.SortAddressBlocks(sorted)); + EXPECT_EQ(remaining == 1, peripheral.HasExpansionError()); + EXPECT_EQ(remaining, sorted.size()); + const auto upperCopy = sorted.find(0x200); + EXPECT_NE(sorted.end(), upperCopy); + if(upperCopy != sorted.end()) { + EXPECT_NE(upper, upperCopy->second); + EXPECT_EQ(upper, upperCopy->second->GetCopiedFrom()); + } + if(remaining == 2 && !sorted.empty()) { + EXPECT_EQ(0U, sorted.begin()->first); + EXPECT_EQ(lower, sorted.begin()->second->GetCopiedFrom()); + } + // The same cleanup used by MergeAddressBlocks releases unmerged temporary copies. + EXPECT_TRUE(peripheral.CopyMergedAddressBlocks(sorted)); + EXPECT_EQ(2U, peripheral.GetAddressBlock().size()); + } +} + TEST_F(SvdDimensionBoundsTest, PeripheralExpansionPreservesAddressesAndStopsAtTheBudgetBoundary) { for(const size_t remaining : {size_t{3}, size_t{4}}) { SCOPED_TRACE(remaining); @@ -468,6 +711,73 @@ TEST_F(SvdDimensionBoundsTest, InterruptDimensionPreservesTheLargestInitializedV EXPECT_FALSE(HasMessage("M250")); } +TEST_F(SvdDimensionBoundsTest, InterruptDimensionAccountsForCopiedMetadataIndicesAndItems) { + for(const size_t remaining : {size_t{0}, size_t{2}, size_t{3}, size_t{5}}) { + SCOPED_TRACE(remaining); + SvdPeripheral peripheral(nullptr); + Dimension(peripheral, 2); + SvdInterrupt irq(&peripheral); + irq.SetName("IRQ%s"); + irq.SetValue(16); + ASSERT_TRUE(peripheral.ReserveExpansion(SvdItem::MAX_EXPANSION_ITEMS - remaining)); + + EXPECT_EQ(remaining == 5, irq.CalculateDim()); + EXPECT_EQ(remaining < 5, peripheral.HasExpansionError()); + const auto dim = irq.GetDimension(); + if(remaining == 0) { + EXPECT_EQ(nullptr, dim); + continue; + } + ASSERT_NE(nullptr, dim); + EXPECT_EQ(remaining >= 3 ? 2U : 0U, dim->GetDimIndexList().size()); + ASSERT_EQ(remaining == 5 ? 2U : 0U, dim->GetChildCount()); + if(remaining == 5) { + const auto last = dynamic_cast(dim->GetChildren().back()); + ASSERT_NE(nullptr, last); + EXPECT_EQ("IRQ1", last->GetName()); + EXPECT_EQ(17U, last->GetValue()); + } + } +} + +TEST_F(SvdDimensionBoundsTest, DirectlyBuiltInterruptLeafSubtreesStillRespectTheCopyBudget) { + // The generic model API permits descendants even though interrupts are XML leaf elements. + for(const size_t remaining : {size_t{3}, size_t{6}}) { + SCOPED_TRACE(remaining); + SvdInterrupt irq(nullptr); + irq.SetValue(16); + const auto dim = Dimension(irq, 2, 1); + const auto value = new SvdEnum(&irq); + irq.AddItem(value); + value->SetName("CHILD"); + value->SetValue(1); + ASSERT_TRUE(irq.ReserveExpansion(SvdItem::MAX_EXPANSION_ITEMS - remaining)); + + EXPECT_EQ(remaining == 6, irq.CalculateDim()); + EXPECT_EQ(remaining == 3, irq.HasExpansionError()); + ASSERT_EQ(remaining == 6 ? 2U : 1U, dim->GetChildCount()); + uint32_t index = 0; + for(const auto copy : dim->GetChildren()) { + const auto copiedIrq = dynamic_cast(copy); + ASSERT_NE(nullptr, copiedIrq); + ASSERT_EQ(remaining == 6 ? 1U : 0U, copy->GetChildCount()); + if(remaining == 6) { + EXPECT_EQ("CHILD", copy->GetChildren().front()->GetName()); + EXPECT_EQ(value, copy->GetChildren().front()->GetCopiedFrom()); + EXPECT_EQ("ITEM" + to_string(index), copiedIrq->GetName()); + EXPECT_EQ(16U + index, copiedIrq->GetValue()); + EXPECT_EQ(index, copiedIrq->GetDimElementIndex()); + } + else { + EXPECT_TRUE(copiedIrq->GetName().empty()); + EXPECT_EQ(SvdItem::VALUE32_NOT_INIT, copiedIrq->GetValue()); + EXPECT_EQ(SvdItem::VALUE32_NOT_INIT, copiedIrq->GetDimElementIndex()); + } + index++; + } + } +} + TEST_F(SvdDimensionBoundsTest, InterruptDimensionCannotWrapItsValue) { SvdPeripheral peripheral(nullptr); Dimension(peripheral, 3, 4); From 50a6cba71ee26abded9968d205dc22d130d26029 Mon Sep 17 00:00:00 2001 From: Thorsten de Buhr Date: Tue, 20 Oct 2026 16:31:53 +0200 Subject: [PATCH 24/26] fix(svdconv): allow one million expansion operations --- tools/svdconv/SVDConv/README.md | 2 +- tools/svdconv/SVDModel/include/SvdItem.h | 2 +- tools/svdconv/test/integtests/src/SvdConvIntegTests.cpp | 4 +++- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/tools/svdconv/SVDConv/README.md b/tools/svdconv/SVDConv/README.md index 2c2abb2c9..1b10a9737 100644 --- a/tools/svdconv/SVDConv/README.md +++ b/tools/svdconv/SVDConv/README.md @@ -52,7 +52,7 @@ SVDConv supports Cortex-M devices with register and field ranges within 32 bits. prevents overflow; it does not add support for 64-bit input or output. SFD/SFR rejects wider ranges with M226 before SFD file creation. SFR compilation is skipped when SFD generation fails. -Dimension expansion has a cumulative limit of 100,000 generated indices and model copies per input file. +Dimension expansion has a cumulative limit of 1,000,000 generated indices and model copies per input file. Nested arrays, inherited content, and repeated expansion share this limit. Exceeding it reports M249 and stops processing that input before any output generator runs. This bounds expansion work, not the XML file size or total process memory. Invalid dimension index ranges and overflowing expansion addresses report M250. diff --git a/tools/svdconv/SVDModel/include/SvdItem.h b/tools/svdconv/SVDModel/include/SvdItem.h index 6410a65ab..3882e2d7c 100644 --- a/tools/svdconv/SVDModel/include/SvdItem.h +++ b/tools/svdconv/SVDModel/include/SvdItem.h @@ -122,7 +122,7 @@ class SvdItem : public SvdElement { static const uint64_t VALUE64_NOT_INIT; // Cumulative expansion work per input tree, including indices and copied items. - static constexpr size_t MAX_EXPANSION_ITEMS = 100000; + static constexpr size_t MAX_EXPANSION_ITEMS = 1000000; bool ReserveExpansion (size_t count); bool HasExpansionError () const; diff --git a/tools/svdconv/test/integtests/src/SvdConvIntegTests.cpp b/tools/svdconv/test/integtests/src/SvdConvIntegTests.cpp index 41f0db9c4..5ae3879cc 100644 --- a/tools/svdconv/test/integtests/src/SvdConvIntegTests.cpp +++ b/tools/svdconv/test/integtests/src/SvdConvIntegTests.cpp @@ -7,6 +7,7 @@ #include "SvdConvTestUtils.h" #include "SVDConv.h" +#include "SvdItem.h" #include "SfrccInterface.h" #include "ErrLog.h" @@ -301,7 +302,8 @@ TEST_F(SvdConvIntegTests, ExpansionLimitStopsAllGeneratorsAndNextInputStillWorks const auto position = xml.find(marker); ASSERT_NE(string::npos, position); - for(const auto* dimension : {"100001", "50000"}) { + for(const auto count : {SvdItem::MAX_EXPANSION_ITEMS + 1, SvdItem::MAX_EXPANSION_ITEMS / 2}) { + const auto dimension = to_string(count); SCOPED_TRACE(dimension); const string directory = SvdConvIntegTestEnv::testoutput_dir + "/expansionLimit/" + dimension; ASSERT_TRUE(RteFsUtils::CreateDirectories(directory)); From 35b0cac3617c8fe544830ddba8588b6684ae7efc Mon Sep 17 00:00:00 2001 From: Thorsten de Buhr Date: Tue, 20 Oct 2026 16:53:21 +0200 Subject: [PATCH 25/26] fix(svdconv): validate field width before advancing padding --- tools/svdconv/SVDGenerator/src/HeaderData_Field.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/tools/svdconv/SVDGenerator/src/HeaderData_Field.cpp b/tools/svdconv/SVDGenerator/src/HeaderData_Field.cpp index bc7731d02..219b3912c 100644 --- a/tools/svdconv/SVDGenerator/src/HeaderData_Field.cpp +++ b/tools/svdconv/SVDGenerator/src/HeaderData_Field.cpp @@ -123,13 +123,12 @@ bool HeaderData::CreateSortedFields(const FIELDMAPLIST& sortedFields, uint32_t r return false; } GenerateReservedField(newOffs - offsCnt, regSize); - offsCnt = newOffs; - offsCnt += CreateField(field, regSize, offsCnt); - } - - if(offsCnt > registerBits) { - m_gen->Generate("Reserved bits calculation error", -1); - return false; + const auto fieldWidth = CreateField(field, regSize, newOffs); + if(fieldWidth > registerBits - newOffs) { + m_gen->Generate("Reserved bits calculation error", -1); + return false; + } + offsCnt = newOffs + fieldWidth; } GenerateReservedField(registerBits - offsCnt, regSize); // fill up (pad) bitfields From 43c3f60d2c45643ccc1b65e405c39f4f264f5049 Mon Sep 17 00:00:00 2001 From: Thorsten de Buhr Date: Wed, 21 Oct 2026 14:13:50 +0200 Subject: [PATCH 26/26] refactor(svdconv): return a 32-bit register access mask --- tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp | 2 +- tools/svdconv/SVDModel/include/SvdRegister.h | 2 +- tools/svdconv/SVDModel/src/SvdRegister.cpp | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp b/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp index 88aa5c2e9..b7c610ccb 100644 --- a/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp +++ b/tools/svdconv/SVDGenerator/src/SfdData_SingleItems.cpp @@ -768,7 +768,7 @@ bool SfdData::CreateRegisterItem(SvdRegister* reg) if(cont && cont->IsValid() && cont->GetChildCount()) { list fieldsList; CreateFields(cont, fieldsList); - const auto accMaskReadRtree = (uint32_t) reg->GetAccessMask(); + const auto accMaskReadRtree = reg->GetAccessMask(); CreateItemDescription(reg, "RTree"); m_gen->Generate("SFDITEM_REG__%s", itemName.c_str()); diff --git a/tools/svdconv/SVDModel/include/SvdRegister.h b/tools/svdconv/SVDModel/include/SvdRegister.h index 7fe478737..bec5d7e77 100644 --- a/tools/svdconv/SVDModel/include/SvdRegister.h +++ b/tools/svdconv/SVDModel/include/SvdRegister.h @@ -84,7 +84,7 @@ class SvdRegister : public SvdItem bool CalcAccessMask (); uint64_t GetAccessMaskRead (); uint64_t GetAccessMaskWrite (); - uint64_t GetAccessMask (); + uint32_t GetAccessMask (); SvdTypes::Access GetAccessCalculated (); bool CheckEnumeratedValues (); bool AddToMap (SvdEnum *enu, std::map &map); diff --git a/tools/svdconv/SVDModel/src/SvdRegister.cpp b/tools/svdconv/SVDModel/src/SvdRegister.cpp index 5148db908..97e1aa5b8 100644 --- a/tools/svdconv/SVDModel/src/SvdRegister.cpp +++ b/tools/svdconv/SVDModel/src/SvdRegister.cpp @@ -508,9 +508,9 @@ uint64_t SvdRegister::GetAccessMaskWrite() return m_accessMaskWrite; } -uint64_t SvdRegister::GetAccessMask() +uint32_t SvdRegister::GetAccessMask() { - return SvdUtils::CreateBitMask(GetEffectiveBitWidth()); + return static_cast(SvdUtils::CreateBitMask(GetEffectiveBitWidth())); } bool SvdRegister::AddToMap(SvdField *field, map &map)