kwk updated this revision to Diff 224968.
kwk marked 6 inline comments as done.
kwk added a comment.

- Remove unnecessary section
- Have consitent ordering of run, check and YAML lines
- use ## for comments in YAML
- Remove not needed Entry and Sections
- Only have Name for a symbol
- Moved test files from llvm/test/ObjectYAML/ELF/ to llvm/test/tools/yaml2obj/
- Fixup for test files


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D68943

Files:
  lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml
  lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml
  lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml
  llvm/include/llvm/ObjectYAML/ELFYAML.h
  llvm/lib/ObjectYAML/ELFEmitter.cpp
  llvm/test/tools/yaml2obj/symtab-generated.yaml
  llvm/test/tools/yaml2obj/symtab-not-generated.yaml
  llvm/tools/obj2yaml/elf2yaml.cpp

Index: llvm/tools/obj2yaml/elf2yaml.cpp
===================================================================
--- llvm/tools/obj2yaml/elf2yaml.cpp
+++ llvm/tools/obj2yaml/elf2yaml.cpp
@@ -200,8 +200,8 @@
       return TableOrErr.takeError();
     ShndxTable = *TableOrErr;
   }
-  if (SymTab)
-    if (Error E = dumpSymbols(SymTab, Y->Symbols))
+  if (SymTab && Y->Symbols)
+    if (Error E = dumpSymbols(SymTab, *Y->Symbols))
       return std::move(E);
   if (DynSymTab)
     if (Error E = dumpSymbols(DynSymTab, Y->DynamicSymbols))
Index: llvm/test/tools/yaml2obj/symtab-not-generated.yaml
===================================================================
--- /dev/null
+++ llvm/test/tools/yaml2obj/symtab-not-generated.yaml
@@ -0,0 +1,15 @@
+## In this test we don't explicitly define a "Symbols" entry in the YAML and
+## expect no .symtab section to be generated.
+
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-readobj --sections %t | FileCheck %s
+
+# CHECK-NOT: Name: .symtab
+# CHECK-NOT: Type: SHT_SYMTAB
+
+--- !ELF
+FileHeader:
+  Class:           ELFCLASS32
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_386
Index: llvm/test/tools/yaml2obj/symtab-generated.yaml
===================================================================
--- /dev/null
+++ llvm/test/tools/yaml2obj/symtab-generated.yaml
@@ -0,0 +1,33 @@
+## In this test we define a "Symbols" entry in the YAML and expect a .symtab
+## section to be generated. Notice that this is automatically added even though
+## we don't have .symtab listed in the "Sections" entry.
+
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-readobj --sections %t | FileCheck -v --color --dump-input=always %s
+
+# CHECK:      Sections [
+# CHECK:        Section {
+# CHECK:          Name: .symtab (19)
+# CHECK-NEXT:     Type: SHT_SYMTAB (0x2)
+# CHECK-NEXT:     Flags [ (0x0)
+# CHECK-NEXT:     ]
+# CHECK-NEXT:     Address: 0x0
+# CHECK-NEXT:     Offset: 0x68
+# CHECK-NEXT:     Size: 48
+# CHECK-NEXT:     Link: 1
+# CHECK-NEXT:     Info: 2
+# CHECK-NEXT:     AddressAlignment: 8
+# CHECK-NEXT:     EntrySize: 24
+# CHECK-NEXT:  }
+# CHECK-NEXT:]
+# CHECK-EMPTY:
+
+--- !ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_EXEC
+  Machine:         EM_X86_64
+Symbols:
+- Name:            main
+...
Index: llvm/lib/ObjectYAML/ELFEmitter.cpp
===================================================================
--- llvm/lib/ObjectYAML/ELFEmitter.cpp
+++ llvm/lib/ObjectYAML/ELFEmitter.cpp
@@ -210,8 +210,10 @@
         Doc.Sections.begin(),
         std::make_unique<ELFYAML::Section>(
             ELFYAML::Section::SectionKind::RawContent, /*IsImplicit=*/true));
-
-  std::vector<StringRef> ImplicitSections = {".symtab", ".strtab", ".shstrtab"};
+  
+  std::vector<StringRef> ImplicitSections = {".strtab", ".shstrtab"};
+  if (Doc.Symbols) 
+    ImplicitSections.push_back(".symtab");
   if (!Doc.DynamicSymbols.empty())
     ImplicitSections.insert(ImplicitSections.end(), {".dynsym", ".dynstr"});
 
@@ -508,7 +510,7 @@
                                              ELFYAML::Section *YAMLSec) {
 
   bool IsStatic = STType == SymtabType::Static;
-  const auto &Symbols = IsStatic ? Doc.Symbols : Doc.DynamicSymbols;
+  const auto &Symbols = (IsStatic && Doc.Symbols) ? *Doc.Symbols : Doc.DynamicSymbols;
 
   ELFYAML::RawContentSection *RawSec =
       dyn_cast_or_null<ELFYAML::RawContentSection>(YAMLSec);
@@ -1044,14 +1046,16 @@
     }
   };
 
-  Build(Doc.Symbols, SymN2I);
+  if (Doc.Symbols)
+    Build(*Doc.Symbols, SymN2I);
   Build(Doc.DynamicSymbols, DynSymN2I);
 }
 
 template <class ELFT> void ELFState<ELFT>::finalizeStrings() {
   // Add the regular symbol names to .strtab section.
-  for (const ELFYAML::Symbol &Sym : Doc.Symbols)
-    DotStrtab.add(ELFYAML::dropUniqueSuffix(Sym.Name));
+  if (Doc.Symbols)
+    for (const ELFYAML::Symbol &Sym : *Doc.Symbols)
+      DotStrtab.add(ELFYAML::dropUniqueSuffix(Sym.Name));
   DotStrtab.finalize();
 
   // Add the dynamic symbol names to .dynstr section.
Index: llvm/include/llvm/ObjectYAML/ELFYAML.h
===================================================================
--- llvm/include/llvm/ObjectYAML/ELFYAML.h
+++ llvm/include/llvm/ObjectYAML/ELFYAML.h
@@ -376,7 +376,7 @@
   // cleaner and nicer if we read them from the YAML as a separate
   // top-level key, which automatically ensures that invariants like there
   // being a single SHT_SYMTAB section are upheld.
-  std::vector<Symbol> Symbols;
+  Optional<std::vector<Symbol>> Symbols;
   std::vector<Symbol> DynamicSymbols;
 };
 
Index: lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml
===================================================================
--- lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml
+++ lldb/test/Shell/ObjectFile/ELF/minidebuginfo-no-lzma.yaml
@@ -5,11 +5,6 @@
 
 # RUN: yaml2obj %s > %t.obj
 
-# TODO(kwk): once yaml2obj doesn't auto-generate a .symtab section
-# when there's none in YAML, remove the following line:
-
-# RUN: llvm-objcopy --remove-section=.symtab %t.obj
-
 # RUN: %lldb -b -o 'image dump symtab' %t.obj 2>&1 | FileCheck %s
 
 # CHECK: warning: (x86_64) {{.*}}.obj No LZMA support found for reading .gnu_debugdata section
Index: lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml
===================================================================
--- lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml
+++ lldb/test/Shell/ObjectFile/ELF/minidebuginfo-find-symbols.yaml
@@ -2,11 +2,6 @@
 
 # RUN: yaml2obj %s > %t.obj
 
-# TODO(kwk): once yaml2obj doesn't auto-generate a .symtab section
-# when there's none in YAML, remove the following line:
-
-# RUN: llvm-objcopy --remove-section=.symtab %t.obj
-
 # RUN: %lldb -b -o 'image dump symtab' %t.obj | FileCheck %s
 
 # CHECK: [ 0] 1 X Code 0x00000000004005b0 0x000000000000000f 0x00000012 multiplyByFour
Index: lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml
===================================================================
--- lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml
+++ lldb/test/Shell/ObjectFile/ELF/minidebuginfo-corrupt-xz.yaml
@@ -5,11 +5,6 @@
 
 # RUN: yaml2obj %s > %t.obj
 
-# TODO(kwk): once yaml2obj doesn't auto-generate a .symtab section
-# when there's none in YAML, remove the following line:
-
-# RUN: llvm-objcopy --remove-section=.symtab %t.obj
-
 # RUN: %lldb -b -o 'image dump symtab' %t.obj 2>&1 | FileCheck %s
 
 # CHECK: warning: (x86_64) {{.*}}.obj An error occurred while decompression the section .gnu_debugdata: lzma_stream_buffer_decode()=lzma error: LZMA_DATA_ERROR
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to