[PATCH] D19464: ELF: Create .gnu.version and .gnu.version_r sections when linking against versioned DSOs.

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 25 11:10:32 PDT 2016


ruiu added inline comments.

================
Comment at: ELF/InputFiles.h:243
@@ -240,2 +242,3 @@
   typedef typename ELFT::Sym Elf_Sym;
+  typedef typename ELFT::Half Elf_Half;
   typedef typename ELFT::Word Elf_Word;
----------------
It's maybe because I'm not familiar enough with ELF, but I'd use uint16_t instead of ELF "Half" type as I don't memorize the ELF integral types. (It is always 16 bit right?)

================
Comment at: ELF/InputFiles.h:251-253
@@ -245,3 +250,5 @@
   std::vector<StringRef> Undefs;
   StringRef SoName;
+  const Elf_Shdr *VersymSec = nullptr;
+  const Elf_Shdr *VerdefSec = nullptr;
 
----------------
A versioned symbol is identified only by its filename (soname) and version string, so I think it is better to add the version string field to this class, instead of adding raw VerdefSec and VerdefSec fields. It is I think easier to understand.

================
Comment at: ELF/InputFiles.h:282
@@ +281,3 @@
+  // data structures in the output file.
+  std::map<const Elf_Verdef *, NeededVer> Verdefs;
+
----------------
Why don't you use DenseMap?


http://reviews.llvm.org/D19464





More information about the llvm-commits mailing list