[PATCH] D98003: [obj2yaml][XCOFF] Dump sections

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/obj2yaml/XCOFF/aix.yaml:136-141
+  - Name:         TestforXcoff
+    Value:        0x0
+    Section:      N_UNDEF
+    Type:         0x0
+    StorageClass: C_EXT
+    NumberOfAuxEntries: 1
----------------
Strictly, this should be like the inline edit (and same immediately below), but I'm not too fussed by that.


================
Comment at: llvm/test/tools/obj2yaml/XCOFF/invalid.yaml:1
+## Test that obj2yaml reports a suitable error when it encounters invalid content.
+
----------------
You should consider splitting this test into separate files, e.g. `invalid-section` and `invalid-symbol`, as I suspect it will grow as obj2yaml support for XCOFF improves.


================
Comment at: llvm/test/tools/obj2yaml/XCOFF/invalid.yaml:13-20
+  - Name:        .text
+    Flags:       [ STYP_TEXT ]
+    SectionData: '00007400'
+    FileOffsetToData: [[OFFSETTODATA=0x00]]
+Symbols:
+  - Name:         .longSymName
+    Value:        0x70
----------------



================
Comment at: llvm/test/tools/obj2yaml/XCOFF/invalid.yaml:28
+
+# ERROR2: Error reading file: [[FILE]]: Invalid section index
+
----------------
Not for this patch, but it would be nice if we could get the actual useful error message, rather than the error code converted to an error message. That would ensure the message actually provides useful context (here the question could be raised "which section index? Where was it and what was its value?" 


================
Comment at: llvm/tools/obj2yaml/xcoff2yaml.cpp:93-94
+      auto RelRefOrErr = Obj.relocations<Shdr, Reloc>(S);
+      if (!RelRefOrErr)
+        return RelRefOrErr.takeError();
+      for (const Reloc &R : RelRefOrErr.get()) {
----------------
This error path appears to be untested?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98003