[PATCH] D64401: [yaml2obj] - Allow overriding the sh_size field.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In D64401#1575613 <https://reviews.llvm.org/D64401#1575613>, @grimar wrote:

> May be it worth to split this into two patches? I am not sure.


Seems to me like there's nothing relating the sh_size overriding to the broken entry size? So  yes, they should be different patches.



================
Comment at: include/llvm/ObjectYAML/ELFYAML.h:148
+  // This can be used to override the sh_size field. It does not affect the
+  // content written anyhow.
+  Optional<llvm::yaml::Hex64> ShSize;
----------------
Delete the word "anyhow".


================
Comment at: lib/ObjectYAML/ELFYAML.cpp:916
+  // obj2yaml does not dump these fields. They are expected to be empty when we
+  // are producing YAML, because yaml2obj sets an appropriate values for
+  // sh_offset and sh_size automatically when they are not explicitly defined.
----------------
Delete "an"


================
Comment at: test/tools/yaml2obj/elf-override-shsize.yaml:1
+## Check we are able to set custom sh_size field
+## for different sections.
----------------
I wonder whether it's worth a test case with both Size and ShSize being set? It would show that the appropriate number of zeroes is written, but the sh_size field is changed. You could show the right number of zeroes are written by having non-zero section data from other sections before and after the broken section.


================
Comment at: test/tools/yaml2obj/elf-override-shsize.yaml:77
+# RUN: od -t x8 -v %t3 >> %t.txt
+# RUN: FileCheck %s --input-file=%t.txt --check-prefix=CASE2
+
----------------
What about a case where the content is smaller than the ShSize?


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

https://reviews.llvm.org/D64401