[PATCH] D114434: [NFC][XCOFF] [llvm-readobj] replace binaries with YAMLs (only tests for Symbols)

[Esme Yi via Phabricator via llvm-commits]

Esme added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/XCOFF/symbols-invalid.test:4-5
+
+# RUN: yaml2obj %s -o %t5
+# RUN: llvm-readobj --syms %t5 | FileCheck %s
+
----------------
jhenderson wrote:
> I'm not necessarily opposed to this being a separate file, although I think it's more common in newer tests to keep invalid behaviour checks in the same files as their corresponding regular behaviour.
> 
> Also the `%t5` needs changing to just `%t`.
Thanks for your comments!
I prefer to separate invalid behavior checks, because it's convenient to add other invalid checks in follow-up works.


================
Comment at: llvm/test/tools/llvm-readobj/XCOFF/symbols.test:30
+        FileStringType:   XFT_CV
+## The C_STAT symbol with a SECT auxiliary entry.
+  - Name:               .text
----------------
jhenderson wrote:
> Should this be `CSECT` rather than `SECT`?
It should be `Section auxiliary entry`,  and abbreviated as `SECT auxiliary entry`.


================
Comment at: llvm/test/tools/llvm-readobj/XCOFF/symbols.test:90
+
+# SYMBOL32:      AddressSize: 32bit
+# SYMBOL32-NEXT: Symbols [
----------------
jhenderson wrote:
> The addition of the `#` characters has made this a little hard to review. Are there any actual changes in the checks performed, or is the output semantically identical?
I removed duplicated checks for a given symbol type and added a check for C_WEAKEXT that were not tested.


================
Comment at: llvm/test/tools/llvm-readobj/XCOFF/symbols64.test:115
+# SYMBOL64-NEXT:       Index: 5
+# SYMBOL64-NEXT:       SectionLen: 21474836484
+# SYMBOL64-NEXT:       ParameterHashIndex: 0x2
----------------
shchenz wrote:
> The length seems not right to me.
The length value is correct in fact. 
The section length value in the CSECT Auxiliary Entry for XCOFF64 is composed of two fields, i.e. x_scnlen_lo and x_scnlen_hi, that are SectionOrLengthLo and SectionOrLengthHi in YAML.
I set `SectionOrLengthLo:      4` and `SectionOrLengthHi:      5` in YAML,  and 21474836484 equals to 0x500000004, so the value is right.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114434