[PATCH] D81258: [yaml2obj] - Introduce a 10 Mb limit of the output by default and a --max-size option.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/yaml2obj/ELF/output-limit.yaml:10
+# RUN: yaml2obj %s -DSIZE=0x9FFEC0 --docnum=1 -o /dev/null 2>&1
+# RUN: not yaml2obj %s -DSIZE=0x9FFEC1 --docnum=1 -o /dev/null 2>&1 | \
+# RUN:  FileCheck %s --check-prefix=ERROR
----------------
grimar wrote:
> jhenderson wrote:
> > grimar wrote:
> > > jhenderson wrote:
> > > > I think it would be worthwhile testing a case where there is no section header table at all. This is because the `CBA` is not used to write the elf header and program header table.
> > > I'd leave it for a follow-up:
> > > 
> > > we have an issue currently. Even when all headers are ommitted, we still write them:
> > > 
> > > ```
> > >   State.writeELFHeader(OS, SHOff);
> > >   writeArrayData(OS, makeArrayRef(PHeaders));
> > >   CBA.writeBlobToStream(OS);
> > >   writeArrayData(OS, makeArrayRef(SHeaders));
> > >   return true;
> > > ```
> > > 
> > > Instead we should write the number of headers specified in  `e_shnum`. Actually the logic should be even a bit more complex.
> > > E.g. when the `e_shnum` (e.g 3) is overriden with the use of `SHNum` (e.g. 1), we should write the number of headers that would be written normally, i.e. 3, I think.
> > > 
> > > For now the condition I`ve used is intentionally simple and matches the current logic:
> > > 
> > > ```
> > > SHOff + arrayDataSize(makeArrayRef(SHeaders)) > MaxSize
> > > ```
> > > 
> > > This patch should allow to fix this issue nicely (will simplify writing a test).
> > Doesn't `Sections: []` result in no sections (and therefore no section headers) at all?
> If you mean just:
> 
> ```
> --- !ELF
> FileHeader:
>   Class:   ELFCLASS64
>   Data:    ELFDATA2LSB
>   Type:    ET_EXEC
>   Machine: EM_X86_64
> Sections: []
> ```
> 
> Then - no, the output will contain 2 implicit sections (.strtab and .shstrtab) + the SHF_UNDEF section at index 0.
> 
> Sections headers can be omitted with:
> 
> ```
> SectionHeaderTable:
>   Sections: []
> ```
> 
> But because of the bug I've described, this only sets the `e_shoff` and `e_shnum` to 0,
> but still writes the section header table which consumes the file size.
I was thinking of the former. Sorry, I thought that did the job (why are we emitting the string tables in that situation...?). Thanks for the clarification.


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

https://reviews.llvm.org/D81258