[PATCH] D59082: [yaml2obj] - Allow producing ELFDATANONE ELFs

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

What about support for other invalid data encodings e.g. 3, 4 etc?



================
Comment at: lib/ObjectYAML/ELFYAML.cpp:235-236
 #define ECase(X) IO.enumCase(Value, #X, ELF::X)
-  // Since the semantics of ELFDATANONE is "invalid", just don't accept it
-  // here.
+  // The Semantics of ELFDATANONE is "invalid", but we accept it because
+  // want to be able to produce "invalid" binaries for the tests.
+  ECase(ELFDATANONE);
----------------
I'd completely rewrite the first part of the sentence, up to the comma as "ELFDATANONE is an invalid data encoding, ..."
"because want" -> "because we want"
Remove the quotes around "invalid"



================
Comment at: test/tools/yaml2obj/elf-header-data.yaml:1
+# We have a YAML describing an invalid data type.
+# Check we are able to produce the invalid binary.
----------------
Please name this test after ELFDATANONE, e.g. elfdatanone.yaml or elf-header-elfdatanone.aml etc?


================
Comment at: test/tools/yaml2obj/elf-header-data.yaml:6
+
+# Both llvm-readobj and obj2yaml currently recognize that the object is invalid.
+# RUN: not llvm-readobj -file-headers %t.o 2>&1 | FileCheck %s
----------------
What about llvm-objdump? I'm guessing they all rely on the same Object library, so all get the error from the same place?

Perhaps a better test is to show using something like `od` that the byte at EI_DATA is 0?


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

https://reviews.llvm.org/D59082