[yaml2obj][XCOFF] add the SectionIndex field for symbol.

[Esme]

Summary

This is found during testing the error paths in D98003.
See the error case 2 in llvm/test/tools/obj2yaml/XCOFF/invalid.yaml of D98003.

[jhenderson]

Hmm, I think maybe this should be a yaml2obj error rather than just writing some arbitrary value. If a symbol references a non-existent section by name, it's just dodgy YAML. (OTOH, it would be perfectly reasonable to allow specification by index and for an invalid index to be allowed without error).

[Esme]

Add the SectionIndex field for symbol.
1: a symbol can reference a section by SectionName or SectionIndex.
2: a symbol can reference a section by both SectionName and SectionIndex.
3: if both Section and SectionIndex are specified, but the two values refer to different sections, an error will be reported.
4: an invalid SectionIndex is allowed.
5: if a symbol references a non-existent section by SectionName, an error will be reported.

[Higuoxing]

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

325

llvm/test/tools/yaml2obj/XCOFF/symbol-section.yaml

74

It might be good to report the invalid section index in the error message. e.g.,

the SectionIndex 2 specified in the symbol is invalid.

[Esme]

Address comment

[jhenderson]

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

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

327

Perhaps "does not exist" instead of "is invalid"? Up to you though.

332

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.

llvm/test/tools/yaml2obj/XCOFF/symbol-section.yaml

37

73

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.

[Esme]

Thanks!
Address comment.

llvm/test/tools/yaml2obj/XCOFF/symbol-section.yaml

73

Leave a TODO in this patch.

[jhenderson]

LGTM; please give @Higuoxing a chance to comment again.

[Higuoxing]

LGTM, thanks!

llvm/include/llvm/ObjectYAML/XCOFFYAML.h

struct Section {
   uint32_t Flags;
   yaml::BinaryRef SectionData;
   std::vector<Relocation> Relocations;
 };
 
 struct Symbol {
   StringRef SymbolName;
   llvm::yaml::Hex64 Value; // Symbol value; storage class-dependent.
-  StringRef SectionName;
+  Optional<StringRef> SectionName;
+  Optional<uint16_t> SectionIndex;
   llvm::yaml::Hex16 Type;
   XCOFF::StorageClass StorageClass;
   uint8_t NumberOfAuxEntries;
 };
 
 struct Object {
   FileHeader Header;
   std::vector<Section> Sections;

llvm/lib/Object/XCOFFObjectFile.cpp

 }
 
 uint16_t XCOFFObjectFile::getMagic() const {
   return is64Bit() ? fileHeader64()->Magic : fileHeader32()->Magic;
 }
 
 Expected<DataRefImpl> XCOFFObjectFile::getSectionByNum(int16_t Num) const {
   if (Num <= 0 || Num > getNumberOfSections())
-    return errorCodeToError(object_error::invalid_section_index);
+    return createStringError(object_error::invalid_section_index,
+                             "the section index (" + Twine(Num) +
+                                 ") is invalid");
 
   DataRefImpl DRI;
   DRI.p = getWithOffset(getSectionHeaderTableAddress(),
                         getSectionHeaderSize() * (Num - 1));
   return DRI;
 }
 
 Expected<StringRef>

llvm/lib/ObjectYAML/XCOFFEmitter.cpp

if (Is64Bit) {
         // string table.
         W.write<int32_t>(0);
         W.write<uint32_t>(Strings.getOffset(YamlSym.SymbolName));
       } else {
         writeName(YamlSym.SymbolName, W);
       }
       W.write<uint32_t>(YamlSym.Value);
     }
-    W.write<int16_t>(
-        YamlSym.SectionName.size() ? SectionIndexMap[YamlSym.SectionName] : 0);
+    if (YamlSym.SectionName) {
+      if (!SectionIndexMap.count(*YamlSym.SectionName)) {
+        ErrHandler("the SectionName " + *YamlSym.SectionName +
+                   " specified in the symbol is invalid");
+        return false;
+      }
+      if (YamlSym.SectionIndex &&
+          SectionIndexMap[*YamlSym.SectionName] != *YamlSym.SectionIndex) {
+        ErrHandler("SectionName and SectionIndex refer to different sections");
+        return false;
+      }
+      W.write<int16_t>(SectionIndexMap[*YamlSym.SectionName]);
+    } else {
+      W.write<int16_t>(YamlSym.SectionIndex ? *YamlSym.SectionIndex : 0);
+    }
     W.write<uint16_t>(YamlSym.Type);
     W.write<uint8_t>(YamlSym.StorageClass);
     W.write<uint8_t>(YamlSym.NumberOfAuxEntries);
 
     // Now output the auxiliary entry.
     for (uint8_t I = 0, E = YamlSym.NumberOfAuxEntries; I < E; ++I) {
       // TODO: Auxiliary entry is not supported yet.
       // The auxiliary entries for a symbol follow its symbol table entry. The

llvm/lib/ObjectYAML/XCOFFYAML.cpp

void MappingTraits<XCOFFYAML::Section>::mapping(IO &IO,
   IO.mapOptional("SectionData", Sec.SectionData);
   IO.mapOptional("Relocations", Sec.Relocations);
 }
 
 void MappingTraits<XCOFFYAML::Symbol>::mapping(IO &IO, XCOFFYAML::Symbol &S) {
   IO.mapOptional("Name", S.SymbolName);
   IO.mapOptional("Value", S.Value);
   IO.mapOptional("Section", S.SectionName);
+  IO.mapOptional("SectionIndex", S.SectionIndex);
   IO.mapOptional("Type", S.Type);
   IO.mapOptional("StorageClass", S.StorageClass);
   IO.mapOptional("NumberOfAuxEntries", S.NumberOfAuxEntries);
 }
 
 void MappingTraits<XCOFFYAML::Object>::mapping(IO &IO, XCOFFYAML::Object &Obj) {
   IO.mapTag("!XCOFF", true);
   IO.mapRequired("FileHeader", Obj.Header);
   IO.mapOptional("Sections", Obj.Sections);
   IO.mapOptional("Symbols", Obj.Symbols);
 }
 
 } // namespace yaml
 } // namespace llvm

llvm/test/tools/yaml2obj/XCOFF/symbol-section.yaml

+## Test the behavior of the symbol reference section.
+
+## Case1: a symbol can reference a section by SectionName or SectionIndex.
+# RUN: yaml2obj --docnum=1 %s -o %t1
+# RUN: llvm-readobj %t1 -s | FileCheck %s --check-prefix=CASE1
+
+# CASE1:      Symbols [
+# CASE1-NEXT:   Symbol {
+# CASE1-NEXT:     Index: 0
+# CASE1-NEXT:     Name: 
+# CASE1-NEXT:     Value: 0x0
+# CASE1-NEXT:     Section: .text
+# CASE1-NEXT:     Type: 0x0
+# CASE1-NEXT:     StorageClass: C_NULL (0x0)
+# CASE1-NEXT:     NumberOfAuxEntries: 0
+# CASE1-NEXT:   }
+# CASE1-NEXT:   Symbol {
+# CASE1-NEXT:     Index: 1
+# CASE1-NEXT:     Name: 
+# CASE1-NEXT:     Value: 0x0
+# CASE1-NEXT:     Section: .text
+# CASE1-NEXT:     Type: 0x0
+# CASE1-NEXT:     StorageClass: C_NULL (0x0)
+# CASE1-NEXT:     NumberOfAuxEntries: 0
+# CASE1-NEXT:   }
+# CASE1-NEXT: ]
+
+--- !XCOFF
+FileHeader:
+  MagicNumber: 0x1DF
+Sections:
+  - Name: .text
+Symbols:
+  - Section: .text
+  - SectionIndex: 1
+
+## 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
+# RUN: llvm-readobj %t2 -s | FileCheck %s --check-prefix=CASE2
+
+# CASE2:      Symbols [
+# CASE2-NEXT:   Symbol {
+# CASE2-NEXT:     Index: 0
+# CASE2-NEXT:     Name: 
+# CASE2-NEXT:     Value: 0x0
+# CASE2-NEXT:     Section: .text
+# CASE2-NEXT:     Type: 0x0
+# CASE2-NEXT:     StorageClass: C_NULL (0x0)
+# CASE2-NEXT:     NumberOfAuxEntries: 0
+# CASE2-NEXT:   }
+# CASE2-NEXT: ]
+
+--- !XCOFF
+FileHeader:
+  MagicNumber: 0x1DF
+Sections:
+  - Name: .text
+Symbols:
+  - Section:      [[SECNAME=<none>]]
+    SectionIndex: [[SECINDEX=<none>]]
+
+## Case3: if both Section and SectionIndex are specified, but the two
+##        values refer to different sections, an error will be reported.
+# RUN: not yaml2obj --docnum=2 -DSECNAME='.text' -DSECINDEX=0 %s -o %t3 2>&1 \
+# RUN:   | FileCheck %s --check-prefix=CASE3
+
+# CASE3: SectionName and SectionIndex refer to different sections
+
+## Case4: yaml2obj allows the specification of an invalid SectionIndex.
+# RUN: yaml2obj --docnum=2 -DSECINDEX=2 %s -o %t4
+# RUN: not llvm-readobj %t4 -s 2>&1 | FileCheck %s --check-prefix=CASE4
+
+# CASE4: the section index (2) is invalid 
+
+## Case 5: if a symbol references a non-existent section by SectionName,
+## an error will be reported.
+# RUN: not yaml2obj --docnum=2 -DSECNAME='.data' %s -o %t5 2>&1 \
+# RUN:   | FileCheck %s --check-prefix=CASE5
+
+# CASE5: the SectionName .data specified in the symbol is invalid