From 1e964828b4ed4022378f94390e7cfb74fed0fe52 Mon Sep 17 00:00:00 2001 From: Stephen Anthony Date: Mon, 15 Jul 2024 13:21:26 -0230 Subject: [PATCH] Fix some warnings/suggestions from clang-tidy. @DirtyHairy, hope you don't mind. --- src/emucore/CartELF.cxx | 76 +++++++++++++++++------------------ src/emucore/CartELF.hxx | 2 +- src/emucore/elf/ElfLinker.cxx | 50 +++++++++-------------- src/emucore/elf/ElfLinker.hxx | 18 +++++++-- src/emucore/elf/ElfParser.cxx | 14 +++---- src/emucore/elf/ElfParser.hxx | 4 +- src/emucore/elf/ElfUtil.cxx | 16 ++++---- 7 files changed, 87 insertions(+), 93 deletions(-) diff --git a/src/emucore/CartELF.cxx b/src/emucore/CartELF.cxx index 4370f5ca6..cfcd5d8df 100644 --- a/src/emucore/CartELF.cxx +++ b/src/emucore/CartELF.cxx @@ -35,32 +35,32 @@ namespace { #ifdef DUMP_ELF void dumpElf(const ElfFile& elf) { - cout << std::endl << "ELF sections:" << std::endl << std::endl; + cout << "\nELF sections:\n\n"; size_t i = 0; - for (auto& section: elf.getSections()) { - if (section.type != 0x00) cout << i << " " << section << std::endl; + for (const auto& section: elf.getSections()) { + if (section.type != 0x00) cout << i << " " << section << '\n'; i++; } auto symbols = elf.getSymbols(); - cout << std::endl << "ELF symbols:" << std::endl << std::endl; - if (symbols.size() > 0) { + cout << "\nELF symbols:\n\n"; + if (!symbols.empty()) { i = 0; for (auto& symbol: symbols) - cout << (i++) << " " << symbol << std::endl; + cout << (i++) << " " << symbol << '\n'; } i = 0; - for (auto& section: elf.getSections()) { + for (const auto& section: elf.getSections()) { auto rels = elf.getRelocations(i++); if (!rels) continue; cout - << std::endl << "ELF relocations for section " - << section.name << ":" << std::endl << std::endl; + << "\nELF relocations for section " + << section.name << ":\n\n"; - for (auto& rel: *rels) cout << rel << std::endl; + for (auto& rel: *rels) cout << rel << '\n'; } } @@ -69,15 +69,12 @@ namespace { cout << std::hex << std::setfill('0'); cout - << std::endl - << "text segment size: 0x" << std::setw(8) << linker.getSegmentSize(ElfLinker::SegmentType::text) - << std::endl - << "data segment size: 0x" << std::setw(8) << linker.getSegmentSize(ElfLinker::SegmentType::data) - << std::endl - << "rodata segment size: 0x" << std::setw(8) << linker.getSegmentSize(ElfLinker::SegmentType::rodata) - << std::endl; + << "\ntext segment size: 0x" << std::setw(8) << linker.getSegmentSize(ElfLinker::SegmentType::text) + << "\ndata segment size: 0x" << std::setw(8) << linker.getSegmentSize(ElfLinker::SegmentType::data) + << "\nrodata segment size: 0x" << std::setw(8) << linker.getSegmentSize(ElfLinker::SegmentType::rodata) + << '\n'; - cout << std::endl << "relocated sections:" << std::endl << std::endl; + cout << "\nrelocated sections:\n\n"; const auto& sections = parser.getSections(); const auto& relocatedSections = linker.getRelocatedSections(); @@ -89,10 +86,10 @@ namespace { << sections[i].name << " @ 0x"<< std::setw(8) << (relocatedSections[i]->offset + linker.getSegmentBase(relocatedSections[i]->segment)) - << " size 0x" << std::setw(8) << sections[i].size << std::endl; + << " size 0x" << std::setw(8) << sections[i].size << '\n'; } - cout << std::endl << "relocated symbols:" << std::endl << std::endl; + cout << "\nrelocated symbols:\n\n"; const auto& symbols = parser.getSymbols(); const auto& relocatedSymbols = linker.getRelocatedSymbols(); @@ -122,30 +119,30 @@ namespace { cout << " (abs)"; } - cout << std::endl; + cout << '\n'; } const auto& initArray = linker.getInitArray(); const auto& preinitArray = linker.getPreinitArray(); - if (initArray.size()) { - cout << std::endl << "init array:" << std::endl << std::endl; + if (!initArray.empty()) { + cout << "\ninit array:\n\n"; - for (uInt32 address: initArray) - cout << " * 0x" << std::setw(8) << address << std::endl; + for (const uInt32 address: initArray) + cout << " * 0x" << std::setw(8) << address << '\n'; } - if (preinitArray.size()) { - cout << std::endl << "preinit array:" << std::endl << std::endl; + if (!preinitArray.empty()) { + cout << "\npreinit array:\n\n"; - for (uInt32 address: preinitArray) - cout << " * 0x" << std::setw(8) << address << std::endl; + for (const uInt32 address: preinitArray) + cout << " * 0x" << std::setw(8) << address << '\n'; } cout << std::dec; } #endif -} +} // namespace // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - CartridgeELF::CartridgeELF(const ByteBuffer& image, size_t size, string_view md5, @@ -189,16 +186,15 @@ CartridgeELF::CartridgeELF(const ByteBuffer& image, size_t size, string_view md5 dumpLinkage(elfParser, elfLinker); cout - << std::endl - << "ARM entrypoint: 0x" + << "\nARM entrypoint: 0x" << std::hex << std::setw(8) << std::setfill('0') << myArmEntrypoint << std::dec - << std::endl; + << '\n'; #endif } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -CartridgeELF::~CartridgeELF() {} +CartridgeELF::~CartridgeELF() = default; // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void CartridgeELF::reset() @@ -221,7 +217,7 @@ void CartridgeELF::install(System& system) { mySystem = &system; - for (size_t addr = 0; addr < 0x1000; addr += System::PAGE_SIZE) { + for (uInt16 addr = 0; addr < 0x1000; addr += System::PAGE_SIZE) { System::PageAccess access(this, System::PageAccessType::READ); access.romPeekCounter = &myRomAccessCounter[addr]; access.romPokeCounter = &myRomAccessCounter[addr]; @@ -308,7 +304,7 @@ void CartridgeELF::vcsWrite5(uInt8 zpAddress, uInt8 value) // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void CartridgeELF::vcsCopyOverblankToRiotRam() { - for (size_t i = 0; i < OVERBLANK_PROGRAM_SIZE; i++) + for (uInt8 i = 0; i < OVERBLANK_PROGRAM_SIZE; i++) vcsWrite5(0x80 + i, OVERBLANK_PROGRAM[i]); } @@ -337,13 +333,13 @@ CartridgeELF::BusTransaction CartridgeELF::BusTransaction::transactionDrive(uInt // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -void CartridgeELF::BusTransaction::setBusState(bool& drive, uInt8& value) +void CartridgeELF::BusTransaction::setBusState(bool& bs_drive, uInt8& bs_value) const { if (yield) { - drive = false; + bs_drive = false; } else { - drive = true; - value = this->value; + bs_drive = true; + bs_value = this->value; } } diff --git a/src/emucore/CartELF.hxx b/src/emucore/CartELF.hxx index 288b83a37..7a2e3abd8 100644 --- a/src/emucore/CartELF.hxx +++ b/src/emucore/CartELF.hxx @@ -70,7 +70,7 @@ class CartridgeELF: public Cartridge { static BusTransaction transactionYield(uInt16 address); static BusTransaction transactionDrive(uInt16 address, uInt8 value); - void setBusState(bool& drive, uInt8& value); + void setBusState(bool& drive, uInt8& value) const; uInt16 address; uInt8 value; diff --git a/src/emucore/elf/ElfLinker.cxx b/src/emucore/elf/ElfLinker.cxx index 9937a9ef1..1f8472b40 100644 --- a/src/emucore/elf/ElfLinker.cxx +++ b/src/emucore/elf/ElfLinker.cxx @@ -43,10 +43,10 @@ namespace { } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - bool checkSegmentOverlap(uInt32 segmentBase1, uInt32 segmentSize1, uInt32 segmentBase2, uInt32 segmentSize2) { + constexpr bool checkSegmentOverlap(uInt32 segmentBase1, uInt32 segmentSize1, uInt32 segmentBase2, uInt32 segmentSize2) { return !(segmentBase1 + segmentSize1 <= segmentBase2 || segmentBase2 + segmentSize2 <= segmentBase1); } -} +} // namespace // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - ElfLinker::ElfLinker(uInt32 textBase, uInt32 dataBase, uInt32 rodataBase, const ElfFile& elf) @@ -212,7 +212,7 @@ unique_ptr& ElfLinker::getSegmentDataRef(SegmentType type) // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void ElfLinker::relocateSections() { - auto& sections = myElf.getSections(); + const auto& sections = myElf.getSections(); myRelocatedSections.resize(sections.size(), std::nullopt); // relocate everything that is not .bss @@ -253,7 +253,7 @@ void ElfLinker::relocateSections() ElfLinkError::raise("segments overlap"); // allocate segment data - for (SegmentType segmentType: {SegmentType::text, SegmentType::data, SegmentType::rodata}) { + for (const SegmentType segmentType: {SegmentType::text, SegmentType::data, SegmentType::rodata}) { const uInt32 segmentSize = getSegmentSize(segmentType); if (segmentSize == 0) continue; @@ -293,7 +293,7 @@ void ElfLinker::relocateInitArrays() std::unordered_map relocatedPreinitArrays; // Relocate init arrays - for (size_t i = 0; i < sections.size(); i++) { + for (uInt32 i = 0; i < sections.size(); i++) { const auto& section = sections[i]; switch (section.type) { @@ -312,6 +312,9 @@ void ElfLinker::relocateInitArrays() preinitArraySize += section.size; break; + + default: + break; } } @@ -369,7 +372,7 @@ void ElfLinker::relocateSymbols(const vector& externalSymbols) // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void ElfLinker::applyRelocationsToSections() { - auto& sections = myElf.getSections(); + const auto& sections = myElf.getSections(); // apply relocations for (size_t iSection = 0; iSection < sections.size(); iSection++) { @@ -383,7 +386,7 @@ void ElfLinker::applyRelocationsToSections() } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -void ElfLinker::copyInitArrays(vector& initArray, std::unordered_map relocatedInitArrays) +void ElfLinker::copyInitArrays(vector& initArray, const std::unordered_map& relocatedInitArrays) { const uInt8* elfData = myElf.getData(); const auto& sections = myElf.getSections(); @@ -436,7 +439,7 @@ void ElfLinker::applyRelocationToSection(const ElfFile::Relocation& relocation, { const uInt32 op = read32(target); - Int32 offset = relocatedSymbol->value + relocation.addend.value_or(elfUtil::decode_B_BL(op)) - + const Int32 offset = relocatedSymbol->value + relocation.addend.value_or(elfUtil::decode_B_BL(op)) - getSegmentBase(targetSectionRelocated.segment) - targetSectionRelocated.offset - relocation.offset - 4; @@ -446,6 +449,9 @@ void ElfLinker::applyRelocationToSection(const ElfFile::Relocation& relocation, write32(target, elfUtil::encode_B_BL(offset, relocation.type == ElfFile::R_ARM_THM_CALL)); } + + default: + break; } } @@ -456,7 +462,7 @@ void ElfLinker::applyRelocationsToInitArrays(uInt8 initArrayType, vector const auto& symbols = myElf.getSymbols(); const auto& sections = myElf.getSections(); - for (size_t iSection = 0; iSection < sections.size(); iSection++) { + for (uInt32 iSection = 0; iSection < sections.size(); iSection++) { const auto& section = sections[iSection]; if (section.type != initArrayType) continue; @@ -479,29 +485,9 @@ void ElfLinker::applyRelocationsToInitArrays(uInt8 initArrayType, vector if (relocation.offset + 4 > section.size) ElfLinkError::raise("unable relocate init array: symbol " + relocation.symbolName + " out of range"); - const uInt32 index = (relocatedInitArrays.at(iSection) + relocation.offset) >> 2; - const uInt32 value = relocatedSymbol->value + relocation.addend.value_or(initArray[index]); - initArray[index] = value | (symbols[relocation.symbol].type == ElfFile::STT_FUNC ? 1 : 0); + const uInt32 index = (relocatedInitArrays.at(iSection) + relocation.offset) >> 2; + const uInt32 value = relocatedSymbol->value + relocation.addend.value_or(initArray[index]); + initArray[index] = value | (symbols[relocation.symbol].type == ElfFile::STT_FUNC ? 1 : 0); } } } - -// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -uInt32 ElfLinker::read32(const uInt8* address) -{ - uInt32 value = *(address++); - value |= *(address++) << 8; - value |= *(address++) << 16; - value |= *(address++) << 24; - - return value; -} - -// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -void ElfLinker::write32(uInt8* address, uInt32 value) -{ - *(address++) = value; - *(address++) = value >> 8; - *(address++) = value >> 16; - *(address++) = value >> 24; -} diff --git a/src/emucore/elf/ElfLinker.hxx b/src/emucore/elf/ElfLinker.hxx index bf7f53f3a..a6ca61175 100644 --- a/src/emucore/elf/ElfLinker.hxx +++ b/src/emucore/elf/ElfLinker.hxx @@ -101,14 +101,26 @@ class ElfLinker { void relocateInitArrays(); void relocateSymbols(const vector& externalSymbols); void applyRelocationsToSections(); - void copyInitArrays(vector& initArray, std::unordered_map relocatedInitArrays); + void copyInitArrays(vector& initArray, const std::unordered_map& relocatedInitArrays); void applyRelocationToSection(const ElfFile::Relocation& relocation, size_t iSection); void applyRelocationsToInitArrays(uInt8 initArrayType, vector& initArray, const std::unordered_map& relocatedInitArrays); - uInt32 read32(const uInt8* address); - void write32(uInt8* address, uInt32 value); + static constexpr uInt32 read32(const uInt8* address) { + uInt32 value = *(address++); + value |= *(address++) << 8; + value |= *(address++) << 16; + value |= *(address++) << 24; + + return value; + } + static constexpr void write32(uInt8* address, uInt32 value) { + *(address++) = value; + *(address++) = value >> 8; + *(address++) = value >> 16; + *(address++) = value >> 24; + } private: std::optional undefinedSymbolDefault; diff --git a/src/emucore/elf/ElfParser.cxx b/src/emucore/elf/ElfParser.cxx index 06bc3edf9..5415abc39 100644 --- a/src/emucore/elf/ElfParser.cxx +++ b/src/emucore/elf/ElfParser.cxx @@ -263,7 +263,7 @@ const char* ElfParser::getName(const Section& section, uInt32 offset) const // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - const ElfParser::Section* ElfParser::getSymtab() const { - for (auto& section: mySections) + for (const auto& section: mySections) if (section.type == SHT_SYMTAB) return §ion; return nullptr; @@ -312,7 +312,7 @@ ostream& operator<<(ostream& os, const ElfParser::Section& section) } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -ostream& operator<<(ostream& os, const ElfParser::Symbol symbol) +ostream& operator<<(ostream& os, const ElfParser::Symbol& symbol) { std::ios reset(nullptr); reset.copyfmt(os); @@ -324,8 +324,8 @@ ostream& operator<<(ostream& os, const ElfParser::Symbol symbol) << " value=0x" << std::setw(8) << symbol.value << " size=0x" << std::setw(8) << symbol.size << std::setw(1) - << " bind=0x" << std::setw(2) << (int)symbol.bind - << " type=0x" << std::setw(2) << (int)symbol.type; + << " bind=0x" << std::setw(2) << static_cast(symbol.bind) + << " type=0x" << std::setw(2) << static_cast(symbol.type); os.copyfmt(reset); @@ -335,7 +335,7 @@ ostream& operator<<(ostream& os, const ElfParser::Symbol symbol) } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -ostream& operator<<(ostream& os, const ElfParser::Relocation rel) +ostream& operator<<(ostream& os, const ElfParser::Relocation& rel) { std::ios reset(nullptr); reset.copyfmt(os); @@ -345,14 +345,14 @@ ostream& operator<<(ostream& os, const ElfParser::Relocation rel) << std::hex << std::setfill('0') << " offset=0x" << std::setw(8) << rel.offset << " info=0x" << std::setw(8) << rel.info - << " type=0x" << std::setw(2) << (int)rel.type; + << " type=0x" << std::setw(2) << static_cast(rel.type); if (rel.addend.has_value()) os << " addend=0x" << std::setw(8) << *rel.addend; os.copyfmt(reset); - os << " symbol=" << (int)rel.symbol; + os << " symbol=" << static_cast(rel.symbol); return os; } diff --git a/src/emucore/elf/ElfParser.hxx b/src/emucore/elf/ElfParser.hxx index 2e8900f1f..f73969a61 100644 --- a/src/emucore/elf/ElfParser.hxx +++ b/src/emucore/elf/ElfParser.hxx @@ -98,7 +98,7 @@ class ElfParser : public ElfFile { }; ostream& operator<<(ostream& os, const ElfParser::Section& section); -ostream& operator<<(ostream& os, const ElfParser::Symbol symbol); -ostream& operator<<(ostream& os, const ElfParser::Relocation relocation); +ostream& operator<<(ostream& os, const ElfParser::Symbol& symbol); +ostream& operator<<(ostream& os, const ElfParser::Relocation& relocation); #endif // ELF_PARSER diff --git a/src/emucore/elf/ElfUtil.cxx b/src/emucore/elf/ElfUtil.cxx index daf0d4824..4b5a6c5ef 100644 --- a/src/emucore/elf/ElfUtil.cxx +++ b/src/emucore/elf/ElfUtil.cxx @@ -21,8 +21,8 @@ Int32 elfUtil::decode_B_BL(uInt32 opcode) { // nomenclature follows Thumb32 BL / B.W encoding in Arm Architecture Reference - uInt16 hw1 = opcode; - uInt16 hw2 = opcode >> 16; + const uInt16 hw1 = opcode; + const uInt16 hw2 = opcode >> 16; const uInt8 s = (hw1 >> 10) & 0x01; const uInt8 i1 = ~((hw2 >> 13) ^ s) & 0x01; @@ -44,13 +44,13 @@ uInt32 elfUtil::encode_B_BL(Int32 offset, bool link) offset >>= 1; - uInt8 s = (offset >> 23) & 0x01; - uInt8 j2 = ((~offset >> 21) ^ s) & 0x01; - uInt8 j1 = ((~offset >> 22) ^ s) & 0x01; - uInt32 imm11 = offset & 0x7ff; - uInt32 imm10 = (offset >> 11) & 0x3ff; + const uInt8 s = (offset >> 23) & 0x01; + const uInt8 j2 = ((~offset >> 21) ^ s) & 0x01; + const uInt8 j1 = ((~offset >> 22) ^ s) & 0x01; + const uInt32 imm11 = offset & 0x7ff; + const uInt32 imm10 = (offset >> 11) & 0x3ff; - uInt16 hw1 = 0xf000 | (s << 10) | imm10; + const uInt16 hw1 = 0xf000 | (s << 10) | imm10; uInt16 hw2 = 0x9000 | (j1 << 13) | (j2 << 11) | imm11; if (link) hw2 |= 0x4000;