[PATCH] D36256: [ELF] Don't output headers into a segment if there's no space for them

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

I'd like to see something like this for embedded systems as the ELF file is often not directly run on the target (PT_LOAD regions are extracted into binary or hex and burned into flash), having the ELF headers mapped in the first PT_LOAD is often detrimental. I suggest you add Rafael as a reviewer as well as he has touched this area fairly recently.



================
Comment at: ELF/LinkerScript.cpp:704
   uint64_t HeaderSize = getHeaderSize();
-  if (HeaderSize <= Min || Script->hasPhdrsCommands()) {
-    Min = alignDown(Min - HeaderSize, Config->MaxPageSize);
-    Out::ElfHeader->Addr = Min;
-    Out::ProgramHeaders->Addr = Min + Out::ElfHeader->Size;
-    return;
+  if (Opt.HasSections) {
+    uint64_t Base = alignDown(Min, Config->MaxPageSize);
----------------
I found the combined if/else quite hard to follow, especially trying to work out what the differences between the two were as the logic is quite similar. Would it make sense to rewrite the combined if/else as:

```
uint64_t Base = (Opt.HasSections) ? alignDown(Min, Config->MaxPageSize) : 0;
  if (HeaderSize <= Min - Base || Script->hasPhdrsCommands()) {
    Min = (Opt.HasSections) ? Base : alignDown(Min - HeaderSize,
                                               Config->MaxPageSize);
    Out::ElfHeader->Addr = Min;
    Out::ProgramHeaders->Addr = Min + Out::ElfHeader->Size;
    return;
  }
```  
A comment to explain what we are trying to do would help here as it isn't immediately obvious from the code why we are doing this.


Repository:
  rL LLVM

https://reviews.llvm.org/D36256