[PATCH] D55839: [elfabi] Add support for writing ELF header for binary stubs

Jake Ehrlich via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 26 16:38:39 PDT 2019


jakehehrlich commandeered this revision.
jakehehrlich edited reviewers, added: amontanez; removed: jakehehrlich.
jakehehrlich added a comment.
Herald added a project: LLVM.

I'm picking this up. Looking for review on this again. I'm going to drive this to completion enough to use on Fuchsia and then maintain it. After it has the features we want for Fuchsia I'll work on adding features that other people want but at a slower pace (like I did after a while with llvm-objcopy) eventually I'll ramp down but honestly, unlike llvm-objcopy, I could see this tool stabilizing.



================
Comment at: llvm/tools/llvm-elfabi/ELFObjHandler.cpp:220
+/// @return Size (in bytes) of the final binary stub.
+template <class ELFT> static size_t getBinarySize(const ELFStub &Stub) {
+  using Elf_Ehdr = typename ELFT::Ehdr;
----------------
jakehehrlich wrote:
> Calculating the size and writing can be a bit fragile. That said you can't write until you have enough space allocated and because we want to use a FileBuffer and avoid copying, we only want to write once. A trick I've seen used is to think about 'WriteCommands' write commands know the maximum index that they write to and can perform the write themselves. So rather than performing size calculation and writes separately, you construct a list of write commands. From this list you then traverse it to get the maximum index written to, and then traverse it once more to write into the now allocated buffer. This way you don't have parallel code but you avoid reallocating a mapped buffer.
I went ahead and implemented my own advice here. Seems to solve the problem of separating calculation of size and writing of data. The alternative is to have a type that mirrors the format of the stub but contains layout information and then to perform layout and then writing individually like we do in llvm-objcopy. 


================
Comment at: llvm/tools/llvm-elfabi/llvm-elfabi.cpp:151-152
     if (TBEWriteError) {
       WithColor::error() << TBEWriteError << "\n";
       exit(1);
     }
----------------
jhenderson wrote:
> You might want to consider factoring these out into a single function that takes an `Error`.
Do you remember what you meant by this? I'm starting this back up again.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55839/new/

https://reviews.llvm.org/D55839





More information about the llvm-commits mailing list