[yaml2obj] - Allow placing local symbols after globals.

[grimar]

Summary

This allows us to produce broken binaries with local
symbols placed after global in '.dynsym'/'.symtab'

Also, simplifies the code.

[MaskRay]

test/tools/yaml2obj/elf-symbols-binding-order.yaml

5 ↗

(On Diff #217377)

--symbols dump both .symtab and .dynsym.

I think you can change the CHECK lines for .dynsym so that you can:

1. test --dyn-symbols on .dynsym
2. test --symbols on both .symtab and .dynsym

[jhenderson]

lib/ObjectYAML/ELFEmitter.cpp

998 ↗

(On Diff #217377)

Maybe: for (size_t I = 0, S = V.size(); I < S; ++I)

test/tools/yaml2obj/elf-symbols-binding-order.yaml

5 ↗

(On Diff #217377)

As this is just a yaml2obj test, I think we can just remove "--dyn-symbols". There would be no need to change the CHECK lines then.

[grimar]

• Addressed review comments.

lib/ObjectYAML/ELFEmitter.cpp

998 ↗

(On Diff #217377)

Ok.

test/tools/yaml2obj/elf-symbols-binding-order.yaml

5 ↗

(On Diff #217377)

I removed --dyn-symbols.

[jhenderson]

LGTM.

llvm/lib/ObjectYAML/ELFEmitter.cpp

for (unsigned I = 0, E = Doc.Sections.size(); I != E; ++I) {
     }
   }
 
   DotShStrtab.finalize();
   return true;
 }
 
 static bool buildSymbolsMap(ArrayRef<ELFYAML::Symbol> V, NameToIdxMap &Map) {
-  bool GlobalSymbolSeen = false;
-  std::size_t I = 0;
-  for (const ELFYAML::Symbol &Sym : V) {
-    ++I;
-
-    StringRef Name = Sym.Name;
-    if (Sym.Binding.value == ELF::STB_LOCAL && GlobalSymbolSeen) {
-      WithColor::error() << "Local symbol '" + Name +
-                                "' after global in Symbols list.\n";
-      return false;
-    }
-    if (Sym.Binding.value != ELF::STB_LOCAL)
-      GlobalSymbolSeen = true;
-
-    if (!Name.empty() && !Map.addName(Name, I)) {
-      WithColor::error() << "Repeated symbol name: '" << Name << "'.\n";
-      return false;
-    }
-  }
+  for (size_t I = 0, S = V.size(); I < S; ++I) {
+    const ELFYAML::Symbol &Sym = V[I];
+    if (Sym.Name.empty() || Map.addName(Sym.Name, I + 1))
+      continue;
+    WithColor::error() << "Repeated symbol name: '" << Sym.Name << "'.\n";
   return true;
 }
 
 template <class ELFT> bool ELFState<ELFT>::buildSymbolIndexes() {
   return buildSymbolsMap(Doc.Symbols, SymN2I) &&
          buildSymbolsMap(Doc.DynamicSymbols, DynSymN2I);
 }
 

llvm/test/tools/yaml2obj/elf-symbols-binding-order.yaml

-## Check we restrict placing local symbols after global in .symtab
-## We might want to change it later to allow doing that
-## for producing broken outputs.
+## Check we allow placing local symbols after global to
+## .symtab and .dynsym. This allows us to produce broken outputs.
 
-# RUN: not yaml2obj %s -o %t 2>&1 | FileCheck %s
-# CHECK: error: Local symbol 'bar' after global in Symbols list.
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-readelf --symbols %t | FileCheck %s
+
+# CHECK:      Symbol table '.dynsym' contains 3 entries:
+# CHECK-NEXT:    Num:    Value          Size Type    Bind   Vis      Ndx Name
+# CHECK-NEXT:      0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND
+# CHECK-NEXT:      1: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND dynamicGlobal
+# CHECK-NEXT:      2: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND dynamicLocal
+
+# CHECK:      Symbol table '.symtab' contains 3 entries:
+# CHECK-NEXT:    Num:    Value          Size Type    Bind   Vis      Ndx Name
+# CHECK-NEXT:      0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND
+# CHECK-NEXT:      1: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND staticGlobal
+# CHECK-NEXT:      2: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND staticLocal
 
 --- !ELF
 FileHeader:
-  Class:           ELFCLASS64
-  Data:            ELFDATA2LSB
-  Type:            ET_REL
-  Machine:         EM_X86_64
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_X86_64
 Symbols:
-  - Name:    foo
+  - Name:    staticGlobal
+    Binding: STB_GLOBAL
+  - Name:    staticLocal
+    Binding: STB_LOCAL
+DynamicSymbols:
+  - Name:    dynamicGlobal
     Binding: STB_GLOBAL
-  - Name:    bar
+  - Name:    dynamicLocal
     Binding: STB_LOCAL