Merge pull request #13 from pfnet/use-more-strict-compile-option

Use more strict compile options
pfnet · Jul 18, 2023 · f7ea7ea · f7ea7ea
2 parents 6f0d686 + 5ac192f
commit f7ea7ea
Show file tree

Hide file tree

Showing 11 changed files with 39 additions and 25 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -1,13 +1,15 @@
 cmake_minimum_required(VERSION 3.4)
 project(sold LANGUAGES CXX)
 
-set(CMAKE_CXX_STANDARD 14)
+set(CMAKE_CXX_STANDARD 17)
 set(CMAKE_CXX_STANDARD_REQUIRED ON)
 
 if(NOT CMAKE_BUILD_TYPE)
     set(CMAKE_BUILD_TYPE "RelWithDebInfo")
 endif()
 
+add_compile_options("$<$<COMPILE_LANGUAGE:CXX>:-Wall;-Werror;-Wextra;-Wno-unused-parameter;-Wno-missing-braces;-Wno-sign-compare;-Wimplicit-fallthrough;-Wno-error=deprecated-declarations>")
+
 set(CMAKE_MODULE_PATH ${CMAKE_CURRENT_SOURCE_DIR}/cmake)
 
 include(third-party)

diff --git a/elf_binary.cc b/elf_binary.cc
@@ -29,12 +29,12 @@
 #include "version_builder.h"
 
 ELFBinary::ELFBinary(const std::string& filename, int fd, char* head, size_t mapped_size, size_t filesize)
-    : filename_(filename), fd_(fd), head_(head), mapped_size_(mapped_size), filesize_(filesize) {
+    : filename_(filename), fd_(fd), head_(head), filesize_(filesize), mapped_size_(mapped_size) {
     ehdr_ = reinterpret_cast<Elf_Ehdr*>(head);
 
     CHECK_EQ(ehdr_->e_type, ET_DYN);
     CHECK_EQ(ehdr_->e_ident[EI_DATA], ELFDATA2LSB);
-    CHECK((ehdr_->e_machine, EM_X86_64) || (ehdr_->e_machine, EM_AARCH64));
+    CHECK((ehdr_->e_machine == EM_X86_64) || (ehdr_->e_machine == EM_AARCH64));
 
     {
         size_t found = filename.rfind('/');
@@ -241,7 +241,7 @@ std::pair<std::string, std::string> ELFBinary::GetVersion(int index, const std::
                 Elf_Vernaux* vna = (Elf_Vernaux*)((char*)vn + vn->vn_aux);
                 for (int j = 0; j < vn->vn_cnt; ++j) {
                     VLOG(3) << "Elf_Vernaux: " << SOLD_LOG_KEY(vna->vna_hash) << SOLD_LOG_KEY(vna->vna_flags)
-                              << SOLD_LOG_KEY(vna->vna_other) << SOLD_LOG_KEY(strtab_ + vna->vna_name) << SOLD_LOG_KEY(vna->vna_next);
+                            << SOLD_LOG_KEY(vna->vna_other) << SOLD_LOG_KEY(strtab_ + vna->vna_name) << SOLD_LOG_KEY(vna->vna_next);
 
                     if (vna->vna_other == versym_[index]) {
                         LOG(INFO) << "Find Elf_Vernaux corresponds to " << versym_[index] << SOLD_LOG_KEY(strtab_ + vn->vn_file)
@@ -403,8 +403,8 @@ void ELFBinary::ParseEHFrameHeader(size_t off, size_t size) {
         eh_frame_header_.table.emplace_back(e);
 
         VLOG(3) << SOLD_LOG_32BITS(e.initial_loc) << SOLD_LOG_32BITS(e.fde_ptr) << SOLD_LOG_32BITS(off + e.fde_ptr)
-                  << SOLD_LOG_32BITS(AddrFromOffset(off)) << SOLD_LOG_32BITS(AddrFromOffset(off) + e.fde_ptr)
-                  << SOLD_LOG_32BITS(OffsetFromAddr(AddrFromOffset(off) + e.fde_ptr));
+                << SOLD_LOG_32BITS(AddrFromOffset(off)) << SOLD_LOG_32BITS(AddrFromOffset(off) + e.fde_ptr)
+                << SOLD_LOG_32BITS(OffsetFromAddr(AddrFromOffset(off) + e.fde_ptr));
 
         EHFrameHeader::FDE fde = {};
         EHFrameHeader::CIE cie = {};
@@ -478,10 +478,10 @@ void ELFBinary::ParseEHFrameHeader(size_t off, size_t size) {
         fde_read(&fde.initial_loc);
 
         VLOG(3) << "ParseEHFrameHeader table[" << i << "] = {" << SOLD_LOG_32BITS(e.initial_loc) << SOLD_LOG_32BITS(e.fde_ptr)
-                  << "} FDE = {" << SOLD_LOG_32BITS(fde.length) << SOLD_LOG_64BITS(fde.extended_length) << SOLD_LOG_32BITS(fde.CIE_delta)
-                  << SOLD_LOG_32BITS(fde.initial_loc) << "} CIE = {" << SOLD_LOG_32BITS(cie.length) << SOLD_LOG_32BITS(cie.CIE_id)
-                  << SOLD_LOG_8BITS(cie.version) << SOLD_LOG_KEY(cie.aug_str) << SOLD_LOG_DWEHPE(cie.FDE_encoding)
-                  << SOLD_LOG_DWEHPE(cie.LSDA_encoding) << "}";
+                << "} FDE = {" << SOLD_LOG_32BITS(fde.length) << SOLD_LOG_64BITS(fde.extended_length) << SOLD_LOG_32BITS(fde.CIE_delta)
+                << SOLD_LOG_32BITS(fde.initial_loc) << "} CIE = {" << SOLD_LOG_32BITS(cie.length) << SOLD_LOG_32BITS(cie.CIE_id)
+                << SOLD_LOG_8BITS(cie.version) << SOLD_LOG_KEY(cie.aug_str) << SOLD_LOG_DWEHPE(cie.FDE_encoding)
+                << SOLD_LOG_DWEHPE(cie.LSDA_encoding) << "}";
 
         CHECK(cie.FDE_encoding == (DW_EH_PE_sdata4 | DW_EH_PE_pcrel));
         CHECK(cie.LSDA_encoding == (DW_EH_PE_sdata4 | DW_EH_PE_pcrel) || cie.LSDA_encoding == DW_EH_PE_SOLD_DUMMY);

diff --git a/elf_binary.h b/elf_binary.h
@@ -53,15 +53,15 @@ class ELFBinary {
     const Elf_Verneed* verneed() const { return verneed_; }
     Elf_Verneed* verneed_mut() const { return verneed_; }
     const Elf_Versym* versym() const { return versym_; }
-    const Elf_Xword verneednum() const { return verneednum_; }
+    Elf_Xword verneednum() const { return verneednum_; }
     const Elf_Rel* rel() const { return rel_; }
     size_t num_rels() const { return num_rels_; }
     const Elf_Rel* plt_rel() const { return plt_rel_; }
     size_t num_plt_rels() const { return num_plt_rels_; }
     const EHFrameHeader* eh_frame_header() const { return &eh_frame_header_; }
     const char* strtab() const { return strtab_; }
-    const size_t dt_strtab() const { return dt_strtab_; }
-    const size_t strsz() const { return strsz_; }
+    size_t dt_strtab() const { return dt_strtab_; }
+    size_t strsz() const { return strsz_; }
 
     const char* head() const { return head_; }
     char* head_mut() const { return head_; }
@@ -72,8 +72,8 @@ class ELFBinary {
 
     uintptr_t init() const { return init_; }
     uintptr_t fini() const { return fini_; }
-    const uintptr_t init_array_addr() const { return init_array_addr_; };
-    const uintptr_t fini_array_addr() const { return fini_array_addr_; };
+    uintptr_t init_array_addr() const { return init_array_addr_; };
+    uintptr_t fini_array_addr() const { return fini_array_addr_; };
     const std::vector<uintptr_t>& init_array() const { return init_array_; }
     const std::vector<uintptr_t>& fini_array() const { return fini_array_; }
 

diff --git a/pybind_test/object.cc b/pybind_test/object.cc
@@ -12,7 +12,7 @@ struct Pet {
     void set(const std::string& name_) { name = name_; }
     void setName(const std::string& name_) { name = name_; }
     const std::string& getName() const { return name; }
-    const int getAge() const { return age; }
+    int getAge() const { return age; }
 
     int age;
     std::string name;

diff --git a/shdr_builder.cc b/shdr_builder.cc
@@ -99,7 +99,7 @@ void ShdrBuilder::Freeze() {
 }
 
 void ShdrBuilder::RegisterShdr(Elf_Off offset, uint64_t size, ShdrType type, uint64_t entsize, Elf_Word info) {
-    Elf_Shdr shdr = {0};
+    Elf_Shdr shdr = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
     shdr.sh_name = GetShName(type);
     shdr.sh_offset = offset;
     shdr.sh_addr = offset;

diff --git a/shdr_builder.h b/shdr_builder.h
@@ -40,7 +40,18 @@ class ShdrBuilder {
         {TLS, ".tls"}};
 
     // The first section header must be NULL.
-    std::vector<Elf_Shdr> shdrs = {Elf_Shdr{0}};
+    std::vector<Elf_Shdr> shdrs = {Elf_Shdr{
+        0,
+        0,
+        0,
+        0,
+        0,
+        0,
+        0,
+        0,
+        0,
+        0,
+    }};
     uint32_t GetShName(ShdrType type) const;
     uint32_t GetIndex(ShdrType type) const;
 };
diff --git a/sold.cc b/sold.cc
@@ -198,9 +198,9 @@ std::string Sold::BuildRunpath() {
     std::string runpath;
     for (const ELFBinary* b : link_binaries_) {
         std::vector<std::string> runpaths = SplitString(b->runpath(), ":");
-        for (const auto rp : runpaths) {
+        for (const auto &rp : runpaths) {
             bool matched = false;
-            for (const auto pattern : exclude_runpath_pattern_) {
+            for (const auto &pattern : exclude_runpath_pattern_) {
                 if (rp.find(pattern) != std::string::npos) matched = true;
             }
             if (!matched) {
@@ -587,7 +587,6 @@ void Sold::RelocateSymbol_x86_64(ELFBinary* bin, const Elf_Rel* rel, uintptr_t o
     std::tie(soname, version_name) = bin->GetVersion(ELF_R_SYM(rel->r_info), filename_to_soname_);
 
     int type = ELF_R_TYPE(rel->r_info);
-    const uintptr_t addend = rel->r_addend;
     std::vector<Elf_Rel> newrels;
 
     if (bin->IsVaddrInTLSData(rel->r_offset)) {
@@ -781,7 +780,6 @@ void Sold::RelocateSymbol_aarch64(ELFBinary* bin, const Elf_Rel* rel, uintptr_t
     std::tie(soname, version_name) = bin->GetVersion(ELF_R_SYM(rel->r_info), filename_to_soname_);
 
     int type = ELF_R_TYPE(rel->r_info);
-    const uintptr_t addend = rel->r_addend;
     std::vector<Elf_Rel> newrels;
 
     if (bin->IsVaddrInTLSData(rel->r_offset)) {

diff --git a/sold.h b/sold.h
@@ -107,7 +107,6 @@ class Sold {
     uintptr_t CodeSize() {
         uintptr_t p = 0;
         for (const Load& load : loads_) {
-            ELFBinary* bin = load.bin;
             Elf_Phdr* phdr = load.orig;
             p += (load.emit.p_offset + phdr->p_filesz);
         }

diff --git a/tools/search-symbol.sh b/tools/search-symbol.sh
@@ -15,7 +15,7 @@ for s in ${sos}
 do
     PRE_IFS=$IFS
     IFS=$'\n'
-    for l in `readelf -s $s | grep -e ${sym} | grep -v UND`
+    for l in `readelf -s $s --wide | grep -e ${sym} | grep -v UND`
     do
         echo ${l} in ${s}
     done

diff --git a/users/akawashiro/build_with_cuda.sh b/users/akawashiro/build_with_cuda.sh
@@ -0,0 +1,4 @@
+#!/bin/bash -eux
+
+cd "$(git rev-parse --show-toplevel)" 
+cmake -DCMAKE_PREFIX_PATH=$(realpath ~/tmp/pytorch-install/share/cmake/Torch) -DSOLD_LIBTORCH_TEST=ON -DSOLD_PYBIND_TEST=ON -GNinja -B build -S . -DCUDA_TOOLKIT_ROOT_DIR=/usr/local/cuda-11.7 -DCMAKE_CUDA_COMPILER=/usr/local/cuda-11.7/bin/nvcc
diff --git a/version_builder.cc b/version_builder.cc
@@ -64,7 +64,7 @@ uintptr_t VersionBuilder::SizeVerneed() const {
     uintptr_t s = 0;
     for (const auto& m1 : data) {
         s += sizeof(Elf_Verneed);
-        for (const auto& m2 : m1.second) {
+        for (size_t i = 0; i < m1.second.size(); i++) {
             s += sizeof(Elf_Vernaux);
         }
     }