[PATCH] D95505: [yaml2obj] Initial support for 32-bit XCOFF in yaml2obj.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

I've taken another look and think you need a fair bit more testing.

1. You need to show that all the fields can be specified explicitly. Currently, you only do it for a subset of the fields.
2. You probably should show that all the flags are supported.
3. You need to test your error paths, to show that the errors are reported properly.
4. If you don't need the string table support in the initial patch (and by my understanding you don't), you can move all the related logic into another indepenedent patch, and then ensure it is properly tested there.
5. I think you need testing for undef and abs symbols too,
6. I think you need testing for non data/text/bss sections (with their zero addresses).



================
Comment at: llvm/test/tools/yaml2obj/XCOFF/basic-doc.yaml:1
+## Check that yaml2obj automatically assigns ommited fields with values.
+# RUN: yaml2obj %s -o %t
----------------



================
Comment at: llvm/test/tools/yaml2obj/XCOFF/basic-doc.yaml:3-4
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-readobj --headers  %t | FileCheck %s --check-prefix=CHECK-HEADERS
+# RUN: llvm-readobj --symbols  %t | FileCheck %s --check-prefix=CHECK-SYMBOLS
+
----------------
You can do this all at once.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95505