[PATCH] D83482: [yaml2obj] - Add a syntax to override e_phoff, e_phentsize and e_phnum fields.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/lib/ObjectYAML/ELFYAML.cpp:873-875
+  IO.mapOptional("PhOff", FileHdr.PhOff);
+  IO.mapOptional("PhEntSize", FileHdr.PhEntSize);
+  IO.mapOptional("PhNum", FileHdr.PhNum);
----------------
The section header table related fields use "SH" not "Sh". It probably makes sense for us to standardise on one form or the other. In other words, I think we should probably use "PH*" here.


================
Comment at: llvm/test/tools/yaml2obj/ELF/header-sh-fields.yaml:4-15
 ## First we check the default values.
 
-# RUN: yaml2obj --docnum=1 %s -o %t1
-# RUN: llvm-readelf --file-headers %t1 | FileCheck %s --check-prefix=DEFAULT
+# RUN: yaml2obj %s -o %t-default
+# RUN: llvm-readelf --file-headers %t-default | FileCheck %s --check-prefix=DEFAULT
 
-# DEFAULT:   Start of section headers:          88 (bytes into file)
-# DEFAULT:   Size of section headers:           64 (bytes)
-# DEFAULT:   Number of section headers:         3
-# DEFAULT:   Section header string table index: 2
-
---- !ELF
-FileHeader:
-  Class:   ELFCLASS64
-  Data:    ELFDATA2LSB
-  Type:    ET_REL
-  Machine: EM_X86_64
-
-## Override 3 fields: e_shoff, e_shnum and e_shstrndx. Check the output.
-
-# RUN: yaml2obj --docnum=2 %s -o %t2
-# RUN: llvm-readelf --file-headers %t2 | FileCheck %s --check-prefix=CUSTOM
-
-# CUSTOM: Start of section headers:          2 (bytes into file)
-# CUSTOM: Size of section headers:           64 (bytes)
-# CUSTOM: Number of section headers:         3
-# CUSTOM: Section header string table index: 4
+# DEFAULT: Start of program headers:          64 (bytes into file)
+# DEFAULT: Start of section headers:          88 (bytes into file)
----------------
This case isn't really checking the default values any more. It's checking that the default variable values as specified in the YAML are followed, but the purpose of the check seems to me to be to show what happens when the fields aren't specified.


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

https://reviews.llvm.org/D83482