[PATCH] D63267: [yaml2obj] - Allow setting custom section types for implicit sections.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: test/tools/llvm-readobj/elf-wrong-shstrtab-type.test:1-2
-## wrong-shstrtab-type.elf-x86-64 contains .shstrtab section which has SHT_PROGBITS type.
+## .shstrtab section has SHT_PROGBITS type.
 ## Check we do not fail to dump the section headers in this case.
 
----------------
Change this comment to:
"Check we do not fail to dump the section headers when a .shstrtab section does not have a SHT_STRTAB type."


================
Comment at: test/tools/yaml2obj/elf-symtab-shinfo.yaml:27-37
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_X86_64
 Sections:
-  - Name:          .symtab
-    Info:          42
-    Type:          SHT_SYMTAB
-  - Name:          .dynsym
-    Info:          26
-    Type:          SHT_SYMTAB
+  - Name: .symtab
+    Info: 42
----------------
This change doesn't seem related?


================
Comment at: test/tools/yaml2obj/explicit-dynsym-no-dynstr.yaml:22
   - Name: .dynsym
-    Type: SHT_SYMTAB
+    Type: SHT_DYNSYM
----------------
Unrelated?


================
Comment at: test/tools/yaml2obj/implicit-sections-types.test:4
+
+## Check the flags set by default in case sections were not
+## described in the YAML, but implicitly added.
----------------
flags -> types


================
Comment at: test/tools/yaml2obj/implicit-sections-types.test:4-5
+
+## Check the flags set by default in case sections were not
+## described in the YAML, but implicitly added.
+
----------------
jhenderson wrote:
> flags -> types
restructure the second half of this sentence to:

"in case sections were implicitly added and not described in the YAML"


================
Comment at: test/tools/yaml2obj/implicit-sections-types.test:32
+
+## Check we can set any arbitrary flags when describing sections
+## that are usually implicit.
----------------
flags -> types


================
Comment at: test/tools/yaml2obj/implicit-sections-types.test:59
+  - Name: .strtab
+    Type: SHT_PROGBITS
+  - Name: .shstrtab
----------------
It might be nice if this picked a range of section types (e.g. SHT_RELA, SHT_DYNAMIC etc).


================
Comment at: tools/yaml2obj/yaml2elf.cpp:471
   SHeader.sh_name = DotShStrtab.getOffset(Name);
-  SHeader.sh_type = ELF::SHT_STRTAB;
+  SHeader.sh_type = YAMLSec ? (uint32_t)YAMLSec->Type : ELF::SHT_STRTAB;
   SHeader.sh_addralign = YAMLSec ? (uint64_t)YAMLSec->AddressAlign : 1;
----------------
Is the cast needed? It isn't above.


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

https://reviews.llvm.org/D63267