grimar added a comment. I have not looked what is needed to fix yaml2obj testcases yet, but below there are few first comments.
Also, 2 LLVM side tests added should live in "llvm/test/tools/yaml2obj" I think (not in llvm/test/ObjectYAML/ELF/), because this is actually the place where we test "yaml2obj" usually. ================ Comment at: llvm/lib/ObjectYAML/ELFEmitter.cpp:513 bool IsStatic = STType == SymtabType::Static; - const auto &Symbols = IsStatic ? Doc.Symbols : Doc.DynamicSymbols; + const auto &Symbols = (IsStatic && Doc.Symbols) ? *Doc.Symbols : Doc.DynamicSymbols; ---------------- It doesn't look correct. We should never use dynamic symbols instead of static symbols I believe. ================ Comment at: llvm/test/ObjectYAML/ELF/no-symtab-generated.yaml:14 +Sections: + - Name: .debug_line + Type: SHT_PROGBITS ---------------- You do not need any sections. ================ Comment at: llvm/test/ObjectYAML/ELF/no-symtab-generated.yaml:18 + +# CHECK-NOT: Name: .symtab +# CHECK-NOT: Type: SHT_SYMTAB ---------------- We often use the following order: 1) Run lines 2) Check lines 3) YAML I.e. the order you have used in symtab-generated.yaml. I'd suggest to be consistent and stick to it. Also, probably would be better to have a single test case file, i.e. to merge them. You can see other our tests which do that. ================ Comment at: llvm/test/ObjectYAML/ELF/symtab-generated.yaml:1 +# 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 usually use ## for comments in yaml2obj tests. ================ Comment at: llvm/test/ObjectYAML/ELF/symtab-generated.yaml:31 + Machine: EM_X86_64 + Entry: 0x00000000004004C0 +Sections: ---------------- You do not need Entry. ================ Comment at: llvm/test/ObjectYAML/ELF/symtab-generated.yaml:33 +Sections: +- Name: .text + Type: SHT_PROGBITS ---------------- You do not need any sections it seems. ================ Comment at: llvm/test/ObjectYAML/ELF/symtab-generated.yaml:41 +- Name: main + Type: STT_FUNC + Section: .text ---------------- Having only a "Name" should be enough. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68943/new/ https://reviews.llvm.org/D68943 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits