[PATCH] D62809: [yaml2obj] - Change how we handle implicit sections.

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: tools/yaml2obj/yaml2elf.cpp:428
+        makeArrayRef(Syms));
+    SHeader.sh_size = arrayDataSize(makeArrayRef(Syms));
+  }
----------------
jhenderson wrote:
> grimar wrote:
> > jhenderson wrote:
> > > What about a user who explicitly specifies sh_size, but not Content? Is that possible currently for symbol table sections?
> > It was possible before this patch to do:
> > 
> > ```
> > !ELF
> > FileHeader:
> >   Class:   ELFCLASS64
> >   Data:    ELFDATA2LSB
> >   Type:    ET_DYN
> >   Machine: EM_X86_64
> > Sections:
> >   - Name:    .dynsym
> >     Type:    SHT_DYNSYM
> >     Address: 0x150
> >     Size:    0x200
> > ```
> > 
> > And get the .dynsym' of size 0x200. With this patch the default symbol table will be created
> > (because there is no `Content`) in this case and `Size` will be ignored.
> > 
> > Again, it seems to me that for correct implementation `Size` need to be `Optional`.
> > Otherwise we can't distinguish "No Size was set" and "Size was set to zero" cases.
> > 
> > 
> > 
> Hmmm... I'm not comfortable about this behaviour change, since this could test failures/spurious passes. If there are no uses of this approach currently, I guess it's okay though.
I'll work on a follow up for this in the first place after landing this patch.


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

https://reviews.llvm.org/D62809