[PATCH] D26012: Don't create a dummy ELF to process a binary file

Rui Ueyama via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 26 14:24:10 PDT 2016


ruiu added inline comments.


================
Comment at: ELF/Config.h:72
 struct Configuration {
   InputFile *FirstElf = nullptr;
+  uint8_t OSABI = 0;
----------------
Do we still need this `FirstELF`?


================
Comment at: ELF/InputFiles.cpp:797-800
+  if (!Config->FirstElf) {
+    Config->FirstElf = Obj;
+    Config->OSABI = Obj->getOSABI();
+  }
----------------
Do you think you can move the logic to inferMachineType in Driver.cpp?


================
Comment at: ELF/InputFiles.cpp:824
 
-// Wraps a binary blob with an ELF header and footer
-// so that we can link it as a regular ELF file.
-template <class ELFT> InputFile *BinaryFile::createELF() {
-  ArrayRef<uint8_t> Blob((uint8_t *)MB.getBufferStart(), MB.getBufferSize());
-  StringRef Filename = MB.getBufferIdentifier();
-  Buffer = wrapBinaryWithElfHeader<ELFT>(Blob, Filename);
+template <class ELFT> void BinaryFile::parse() {
+  StringRef Buf = MB.getBuffer();
----------------
Nice!


================
Comment at: ELF/InputSection.h:239-240
 public:
   InputSection(uintX_t Flags, uint32_t Type, uintX_t Addralign,
                ArrayRef<uint8_t> Data);
+  InputSection(uintX_t Flags, uint32_t Type, uintX_t Addralign,
----------------
If you add a new constructor, can you remove this instead?


================
Comment at: ELF/SymbolTable.h:77-78
   Symbol *addRegular(StringRef Name, uint8_t Binding, uint8_t StOther);
+  Symbol *addRegular(StringRef Name, InputSectionBase<ELFT> *Section,
+                     uint8_t Binding, uint8_t Type, uintX_t Value);
   Symbol *addSynthetic(StringRef N, OutputSectionBase<ELFT> *Section,
----------------
Is there any way to reduce the number of overloaded functions here? I want to avoid having a bunch of functions that do basically the same thing.


https://reviews.llvm.org/D26012





More information about the llvm-commits mailing list