[PATCH] D68983: [yaml2obj, obj2yaml] - Add support for SHT_NOTE sections.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: include/llvm/Support/YAMLTraits.h:652
     return QuotingType::Single;
-  if (isspace(S.front()) || isspace(S.back()))
+  if (isspace(static_cast<unsigned char>(S.front())) ||
+      isspace(static_cast<unsigned char>(S.back())))
----------------
Interesting. Is there a test covering the fixed bug?


================
Comment at: test/tools/obj2yaml/elf-sht-note.yaml:3
+
+## Check how we dump the broken case when the content is less than 12 bytes in size.
+## (12 is the size of n_namesz, n_descsz and n_type 4-bytes fields that must always present).
----------------
Check we dump hex digit pairs if the note section is invalid.

Then, group invalid cases together, the first of which is when the size is less than 12 bytes.

It may be clearer if we place valid cases first.


================
Comment at: test/tools/obj2yaml/elf-sht-note.yaml:62
+
+## We can't dump the Name, Desc and Type fields when SHT_NOTE has data of
+## size < 12 + size of name, desc and their paddings.
----------------
when the content is shorter than the computed size


================
Comment at: test/tools/yaml2obj/elf-sht-note.yaml:26
+# NOTE-NEXT:   SectionData (
+## namesz == (0x03000000) == sizeof("AB") + null terminator.
+## descsz == (0x00000000) for an empty description.
----------------
NUL terminator.

NUL != null.


================
Comment at: test/tools/yaml2obj/elf-sht-note.yaml:56
+
+## Check that 'Tag' field is mandatory. 
+
----------------
What is 'Tag'


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

https://reviews.llvm.org/D68983