[PATCH] D109566: [yaml2obj][XCOFF] add the SectionIndex field for symbol.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

Some small points, but otherwise looks pretty good to me.



================
Comment at: llvm/lib/ObjectYAML/XCOFFEmitter.cpp:327
+        ErrHandler("the SectionName " + *YamlSym.SectionName +
+                   " specified in the symbol is invalid");
+        return false;
----------------
Perhaps "does not exist" instead of "is invalid"? Up to you though.


================
Comment at: llvm/lib/ObjectYAML/XCOFFEmitter.cpp:332
+          SectionIndexMap[*YamlSym.SectionName] != *YamlSym.SectionIndex) {
+        ErrHandler("SectionName and SectionIndex refer to different sections");
+        return false;
----------------
You should specify the name and index in the error message, as otherwise a user won't be able to see which symbol is causing the problem.


================
Comment at: llvm/test/tools/yaml2obj/XCOFF/symbol-section.yaml:37
+
+## Case 2: a symbol can reference a section by both of SectionName and SectionIndex.
+# RUN: yaml2obj --docnum=2 -DSECNAME='.text' -DSECINDEX=1 %s -o %t2
----------------



================
Comment at: llvm/test/tools/yaml2obj/XCOFF/symbol-section.yaml:73
+
+# CASE4: the section index (2) is invalid 
+
----------------
Aside: this error message from llvm-readobj should ideally be updated to include the symbol name or index, so that a user can see which symbol is the problem. Fix in a different patch though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109566