Reland "[yaml2obj] - Don't allow setting StOther and Other/Visibility at the same...

This relands this commit, I mistakenly reverted the original change
thinking it was the cause of the observed MSan failures but it was not.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@370206 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/ObjectYAML/ELFYAML.h b/include/llvm/ObjectYAML/ELFYAML.h
index a282f3299453d3cfdbf935fc459e03bd6758177f..91a279bd7aa4ad197c21c9fd8cf49869e465f325 100644 (file)
--- a/include/llvm/ObjectYAML/ELFYAML.h
+++ b/include/llvm/ObjectYAML/ELFYAML.h
@@ -107,7 +107,12 @@ struct Symbol {
   ELF_STB Binding;
   llvm::yaml::Hex64 Value;
   llvm::yaml::Hex64 Size;
-  uint8_t Other;
+  Optional<uint8_t> Other;
+
+  // This can be used to set any custom value for the st_other field
+  // when it is not possible to do so using the "Other" field, which only takes
+  // specific named constants.
+  Optional<uint8_t> StOther;
 };
 
 struct SectionOrType {

diff --git a/lib/ObjectYAML/ELFEmitter.cpp b/lib/ObjectYAML/ELFEmitter.cpp
index 6a2d28e16e2fe45038ab6d0374244d0f7db7a1b7..96b13e4e530312c5145c2b8d6769453aef12dabe 100644 (file)
--- a/lib/ObjectYAML/ELFEmitter.cpp
+++ b/lib/ObjectYAML/ELFEmitter.cpp
@@ -464,7 +464,12 @@ toELFSymbols(NameToIdxMap &SN2I, ArrayRef<ELFYAML::Symbol> Symbols,
     }
     // else Symbol.st_shndex == SHN_UNDEF (== 0), since it was zero'd earlier.
     Symbol.st_value = Sym.Value;
-    Symbol.st_other = Sym.Other;
+
+    if (Sym.Other)
+      Symbol.st_other = *Sym.Other;
+    else if (Sym.StOther)
+      Symbol.st_other = *Sym.StOther;
+
     Symbol.st_size = Sym.Size;
   }
 

diff --git a/lib/ObjectYAML/ELFYAML.cpp b/lib/ObjectYAML/ELFYAML.cpp
index e7db190eb0a64ab5f82104c71efd40cd4788c791..fe6be47d8fdb22a311a59ccf3beeb50a044d50bb 100644 (file)
--- a/lib/ObjectYAML/ELFYAML.cpp
+++ b/lib/ObjectYAML/ELFYAML.cpp
@@ -866,15 +866,28 @@ void MappingTraits<ELFYAML::ProgramHeader>::mapping(
 namespace {
 
 struct NormalizedOther {
-  NormalizedOther(IO &)
-      : Visibility(ELFYAML::ELF_STV(0)), Other(ELFYAML::ELF_STO(0)) {}
-  NormalizedOther(IO &, uint8_t Original)
-      : Visibility(Original & 0x3), Other(Original & ~0x3) {}
+  NormalizedOther(IO &) {}
+  NormalizedOther(IO &, Optional<uint8_t> Original) {
+    if (uint8_t Val = *Original & 0x3)
+      Visibility = Val;
+    if (uint8_t Val = *Original & ~0x3)
+      Other = Val;
+  }
+
+  Optional<uint8_t> denormalize(IO &) {
+    if (!Visibility && !Other)
+      return None;
 
-  uint8_t denormalize(IO &) { return Visibility | Other; }
+    uint8_t Ret = 0;
+    if (Visibility)
+      Ret |= *Visibility;
+    if (Other)
+      Ret |= *Other;
+    return Ret;
+  }
 
-  ELFYAML::ELF_STV Visibility;
-  ELFYAML::ELF_STO Other;
+  Optional<ELFYAML::ELF_STV> Visibility;
+  Optional<ELFYAML::ELF_STO> Other;
 };
 
 } // end anonymous namespace
@@ -896,17 +909,16 @@ void MappingTraits<ELFYAML::Symbol>::mapping(IO &IO, ELFYAML::Symbol &Symbol) {
   // targets (e.g. MIPS) may use it to specify the named bits to set (e.g.
   // STO_MIPS_OPTIONAL). For producing broken objects we want to allow writing
   // any value to st_other. To do this we allow one more field called "StOther".
-  // If it is present in a YAML document, we set st_other to that integer,
-  // ignoring the other fields.
-  Optional<llvm::yaml::Hex64> Other;
-  IO.mapOptional("StOther", Other);
-  if (Other) {
-    Symbol.Other = *Other;
-  } else {
-    MappingNormalization<NormalizedOther, uint8_t> Keys(IO, Symbol.Other);
-    IO.mapOptional("Visibility", Keys->Visibility, ELFYAML::ELF_STV(0));
-    IO.mapOptional("Other", Keys->Other, ELFYAML::ELF_STO(0));
-  }
+  // If it is present in a YAML document, we set st_other to its integer value
+  // whatever it is.
+  // obj2yaml should not print 'StOther', it should print 'Visibility' and
+  // 'Other' fields instead.
+  assert(!IO.outputting() || !Symbol.StOther.hasValue());
+  IO.mapOptional("StOther", Symbol.StOther);
+  MappingNormalization<NormalizedOther, Optional<uint8_t>> Keys(IO,
+                                                                Symbol.Other);
+  IO.mapOptional("Visibility", Keys->Visibility);
+  IO.mapOptional("Other", Keys->Other);
 }
 
 StringRef MappingTraits<ELFYAML::Symbol>::validate(IO &IO,
@@ -915,6 +927,8 @@ StringRef MappingTraits<ELFYAML::Symbol>::validate(IO &IO,
     return "Index and Section cannot both be specified for Symbol";
   if (Symbol.NameIndex && !Symbol.Name.empty())
     return "Name and NameIndex cannot both be specified for Symbol";
+  if (Symbol.StOther && Symbol.Other)
+    return "StOther cannot be specified for Symbol with either Visibility or Other";
   return StringRef();
 }
 

diff --git a/test/tools/yaml2obj/elf-symbol-stother.yaml b/test/tools/yaml2obj/elf-symbol-stother.yaml
index c867b9ec8981203de65fe54a1a7a93a99b7fa516..32f392f39862638c9f3137bfa499ce6a2ceeabd0 100644 (file)
--- a/test/tools/yaml2obj/elf-symbol-stother.yaml
+++ b/test/tools/yaml2obj/elf-symbol-stother.yaml
@@ -77,3 +77,32 @@ Symbols:
     StOther: 4
   - Name:    bar
     StOther: 0xff
+
+## Check we can't set StOther for a symbol if Visibility or Other is also specified.
+
+# RUN: not yaml2obj --docnum=5 2>&1 %s | FileCheck %s --check-prefix=ERR2
+# RUN: not yaml2obj --docnum=6 2>&1 %s | FileCheck %s --check-prefix=ERR2
+
+# ERR2: error: StOther cannot be specified for Symbol with either Visibility or Other
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_MIPS
+Symbols:
+  - Name:    foo
+    StOther: 0
+    Other: [ STO_MIPS_OPTIONAL ]
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS32
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_MIPS
+Symbols:
+  - Name:    foo
+    StOther: 0
+    Visibility: STV_DEFAULT