<div dir="ltr">Not intentional. I'll revert (again) and look into it. Apologies for the thrash.</div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Nov 7, 2017 at 5:20 PM, Rafael Avila de Espindola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Sorry, the test still fails, but it is not a crash, it is lld producing<br>
a different error message.<br>
<br>
This shows that yaml2obj is producing a different output on that input<br>
now. Is that intentional?<br>
<br>
Cheers,<br>
Rafael<br>
<div class="HOEnZb"><div class="h5"><br>
Rafael Avila de Espindola <<a href="mailto:rafael.espindola@gmail.com">rafael.espindola@gmail.com</a>> writes:<br>
<br>
> The same test still crashes.<br>
><br>
><br>
> Dave Lee via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> writes:<br>
><br>
>> Author: kastiglione<br>
>> Date: Tue Nov 7 16:58:50 2017<br>
>> New Revision: 317646<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=317646&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=317646&view=rev</a><br>
>> Log:<br>
>> Reapply: Allow yaml2obj to order implicit sections for ELF<br>
>><br>
>> Summary:<br>
>> This change allows yaml input to control the order of implicitly added sections<br>
>> (`.symtab`, `.strtab`, `.shstrtab`). The order is controlled by adding a<br>
>> placeholder section of the given name to the Sections field.<br>
>><br>
>> This change is to support changes in D39582, where it is desirable to control<br>
>> the location of the `.dynsym` section.<br>
>><br>
>> This reapplied version fixes:<br>
>> 1. use of a function call within an assert<br>
>> 2. failing lld test which has an unnamed section<br>
>><br>
>> Additionally, one more test to cover the unnamed section failure.<br>
>><br>
>> Reviewers: compnerd, jakehehrlich<br>
>><br>
>> Reviewed By: jakehehrlich<br>
>><br>
>> Subscribers: llvm-commits<br>
>><br>
>> Differential Revision: <a href="https://reviews.llvm.org/D39749" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D39749</a><br>
>><br>
>> Added:<br>
>> llvm/trunk/test/tools/<wbr>yaml2obj/section-ordering.yaml<br>
>> llvm/trunk/test/tools/<wbr>yaml2obj/unnamed-section.yaml<br>
>> Modified:<br>
>> llvm/trunk/lib/ObjectYAML/<wbr>ELFYAML.cpp<br>
>> llvm/trunk/tools/yaml2obj/<wbr>yaml2elf.cpp<br>
>><br>
>> Modified: llvm/trunk/lib/ObjectYAML/<wbr>ELFYAML.cpp<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ObjectYAML/ELFYAML.cpp?rev=317646&r1=317645&r2=317646&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/<wbr>ObjectYAML/ELFYAML.cpp?rev=<wbr>317646&r1=317645&r2=317646&<wbr>view=diff</a><br>
>> ==============================<wbr>==============================<wbr>==================<br>
>> --- llvm/trunk/lib/ObjectYAML/<wbr>ELFYAML.cpp (original)<br>
>> +++ llvm/trunk/lib/ObjectYAML/<wbr>ELFYAML.cpp Tue Nov 7 16:58:50 2017<br>
>> @@ -388,7 +388,7 @@ void ScalarEnumerationTraits<<wbr>ELFYAML::EL<br>
>> #define ECase(X) IO.enumCase(Value, #X, ELF::X)<br>
>> ECase(SHT_NULL);<br>
>> ECase(SHT_PROGBITS);<br>
>> - // No SHT_SYMTAB. Use the top-level `Symbols` key instead.<br>
>> + ECase(SHT_SYMTAB);<br>
>> // FIXME: Issue a diagnostic with this information.<br>
>> ECase(SHT_STRTAB);<br>
>> ECase(SHT_RELA);<br>
>><br>
>> Added: llvm/trunk/test/tools/<wbr>yaml2obj/section-ordering.yaml<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/yaml2obj/section-ordering.yaml?rev=317646&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/test/tools/<wbr>yaml2obj/section-ordering.<wbr>yaml?rev=317646&view=auto</a><br>
>> ==============================<wbr>==============================<wbr>==================<br>
>> --- llvm/trunk/test/tools/<wbr>yaml2obj/section-ordering.yaml (added)<br>
>> +++ llvm/trunk/test/tools/<wbr>yaml2obj/section-ordering.yaml Tue Nov 7 16:58:50 2017<br>
>> @@ -0,0 +1,29 @@<br>
>> +# Ensures that implicitly added sections can be ordered within Sections.<br>
>> +# RUN: yaml2obj %s -o %t<br>
>> +# RUN: llvm-readobj -sections %t | FileCheck %s<br>
>> +<br>
>> +!ELF<br>
>> +FileHeader:<br>
>> + Class: ELFCLASS64<br>
>> + Data: ELFDATA2LSB<br>
>> + Type: ET_EXEC<br>
>> + Machine: EM_X86_64<br>
>> +Sections:<br>
>> + - Name: .text<br>
>> + Type: SHT_PROGBITS<br>
>> + Flags: [ SHF_ALLOC, SHF_EXECINSTR ]<br>
>> + - Name: .symtab<br>
>> + Type: SHT_SYMTAB<br>
>> + - Name: .data<br>
>> + Type: SHT_PROGBITS<br>
>> + Flags: [ SHF_ALLOC, SHF_WRITE ]<br>
>> + - Name: .shstrtab<br>
>> + Type: SHT_STRTAB<br>
>> + - Name: .strtab<br>
>> + Type: SHT_STRTAB<br>
>> +<br>
>> +# CHECK: Name: .text<br>
>> +# CHECK: Name: .symtab<br>
>> +# CHECK: Name: .data<br>
>> +# CHECK: Name: .shstrtab<br>
>> +# CHECK: Name: .strtab<br>
>><br>
>> Added: llvm/trunk/test/tools/<wbr>yaml2obj/unnamed-section.yaml<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/yaml2obj/unnamed-section.yaml?rev=317646&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/test/tools/<wbr>yaml2obj/unnamed-section.yaml?<wbr>rev=317646&view=auto</a><br>
>> ==============================<wbr>==============================<wbr>==================<br>
>> --- llvm/trunk/test/tools/<wbr>yaml2obj/unnamed-section.yaml (added)<br>
>> +++ llvm/trunk/test/tools/<wbr>yaml2obj/unnamed-section.yaml Tue Nov 7 16:58:50 2017<br>
>> @@ -0,0 +1,14 @@<br>
>> +# Ensure yaml2obj doesn't crash on unnamed sections<br>
>> +# RUN: yaml2obj %s<br>
>> +<br>
>> +!ELF<br>
>> +FileHeader:<br>
>> + Class: ELFCLASS64<br>
>> + Data: ELFDATA2LSB<br>
>> + Type: ET_EXEC<br>
>> + Machine: EM_X86_64<br>
>> +Sections:<br>
>> + - Type: SHT_PROGBITS<br>
>> + - Name: .text<br>
>> + Type: SHT_PROGBITS<br>
>> + Flags: [ SHF_ALLOC, SHF_EXECINSTR ]<br>
>><br>
>> Modified: llvm/trunk/tools/yaml2obj/<wbr>yaml2elf.cpp<br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/yaml2obj/yaml2elf.cpp?rev=317646&r1=317645&r2=317646&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/tools/<wbr>yaml2obj/yaml2elf.cpp?rev=<wbr>317646&r1=317645&r2=317646&<wbr>view=diff</a><br>
>> ==============================<wbr>==============================<wbr>==================<br>
>> --- llvm/trunk/tools/yaml2obj/<wbr>yaml2elf.cpp (original)<br>
>> +++ llvm/trunk/tools/yaml2obj/<wbr>yaml2elf.cpp Tue Nov 7 16:58:50 2017<br>
>> @@ -74,6 +74,14 @@ public:<br>
>> Idx = I->getValue();<br>
>> return false;<br>
>> }<br>
>> + /// asserts if name is not present in the map<br>
>> + unsigned get(StringRef Name) const {<br>
>> + unsigned Idx = 0;<br>
>> + auto missing = lookup(Name, Idx);<br>
>> + assert(!missing && "Expected section not found in index");<br>
>> + return Idx;<br>
>> + }<br>
>> + unsigned size() const { return Map.size(); }<br>
>> };<br>
>> } // end anonymous namespace<br>
>><br>
>> @@ -144,19 +152,21 @@ class ELFState {<br>
>> ContiguousBlobAccumulator &CBA);<br>
>><br>
>> // - SHT_NULL entry (placed first, i.e. 0'th entry)<br>
>> - // - symbol table (.symtab) (placed third to last)<br>
>> - // - string table (.strtab) (placed second to last)<br>
>> - // - section header string table (.shstrtab) (placed last)<br>
>> - unsigned getDotSymTabSecNo() const { return Doc.Sections.size() + 1; }<br>
>> - unsigned getDotStrTabSecNo() const { return Doc.Sections.size() + 2; }<br>
>> - unsigned getDotShStrTabSecNo() const { return Doc.Sections.size() + 3; }<br>
>> - unsigned getSectionCount() const { return Doc.Sections.size() + 4; }<br>
>> + // - symbol table (.symtab) (defaults to third to last)<br>
>> + // - string table (.strtab) (defaults to second to last)<br>
>> + // - section header string table (.shstrtab) (defaults to last)<br>
>> + unsigned getDotSymTabSecNo() const { return SN2I.get(".symtab"); }<br>
>> + unsigned getDotStrTabSecNo() const { return SN2I.get(".strtab"); }<br>
>> + unsigned getDotShStrTabSecNo() const { return SN2I.get(".shstrtab"); }<br>
>> + unsigned getSectionCount() const { return SN2I.size() + 1; }<br>
>><br>
>> ELFState(const ELFYAML::Object &D) : Doc(D) {}<br>
>><br>
>> public:<br>
>> static int writeELF(raw_ostream &OS, const ELFYAML::Object &Doc);<br>
>> };<br>
>> +<br>
>> +static const char * const ImplicitSecNames[] = {".symtab", ".strtab", ".shstrtab"};<br>
>> } // end anonymous namespace<br>
>><br>
>> template <class ELFT><br>
>> @@ -211,10 +221,6 @@ bool ELFState<ELFT>::<wbr>initSectionHeaders(<br>
>> zero(SHeader);<br>
>> SHeaders.push_back(SHeader);<br>
>><br>
>> - for (const auto &Sec : Doc.Sections)<br>
>> - DotShStrtab.add(Sec->Name);<br>
>> - DotShStrtab.finalize();<br>
>> -<br>
>> for (const auto &Sec : Doc.Sections) {<br>
>> zero(SHeader);<br>
>> SHeader.sh_name = DotShStrtab.getOffset(Sec-><wbr>Name);<br>
>> @@ -547,12 +553,9 @@ bool ELFState<ELFT>::<wbr>writeSectionContent<br>
>> }<br>
>><br>
>> template <class ELFT> bool ELFState<ELFT>::<wbr>buildSectionIndex() {<br>
>> - SN2I.addName(".symtab", getDotSymTabSecNo());<br>
>> - SN2I.addName(".strtab", getDotStrTabSecNo());<br>
>> - SN2I.addName(".shstrtab", getDotShStrTabSecNo());<br>
>> -<br>
>> for (unsigned i = 0, e = Doc.Sections.size(); i != e; ++i) {<br>
>> StringRef Name = Doc.Sections[i]->Name;<br>
>> + DotShStrtab.add(Name);<br>
>> if (Name.empty())<br>
>> continue;<br>
>> // "+ 1" to take into account the SHT_NULL entry.<br>
>> @@ -562,6 +565,17 @@ template <class ELFT> bool ELFState<ELFT<br>
>> return false;<br>
>> }<br>
>> }<br>
>> +<br>
>> + auto SecNo = 1 + Doc.Sections.size();<br>
>> + // Add special sections after input sections, if necessary.<br>
>> + for (const auto &Name : ImplicitSecNames)<br>
>> + if (!SN2I.addName(Name, SecNo)) {<br>
>> + // Account for this section, since it wasn't in the Doc<br>
>> + ++SecNo;<br>
>> + DotShStrtab.add(Name);<br>
>> + }<br>
>> +<br>
>> + DotShStrtab.finalize();<br>
>> return true;<br>
>> }<br>
>><br>
>> @@ -608,32 +622,23 @@ int ELFState<ELFT>::writeELF(raw_<wbr>ostream<br>
>> Header.e_shentsize * Header.e_shnum;<br>
>> ContiguousBlobAccumulator CBA(SectionContentBeginOffset)<wbr>;<br>
>><br>
>> - // Doc might not contain .symtab, .strtab and .shstrtab sections,<br>
>> - // but we will emit them, so make sure to add them to ShStrTabSHeader.<br>
>> - State.DotShStrtab.add(".<wbr>symtab");<br>
>> - State.DotShStrtab.add(".<wbr>strtab");<br>
>> - State.DotShStrtab.add(".<wbr>shstrtab");<br>
>> -<br>
>> std::vector<Elf_Shdr> SHeaders;<br>
>> + SHeaders.reserve(State.SN2I.<wbr>size());<br>
>> if(!State.initSectionHeaders(<wbr>SHeaders, CBA))<br>
>> return 1;<br>
>><br>
>> - // .symtab section.<br>
>> - Elf_Shdr SymtabSHeader;<br>
>> - State.initSymtabSectionHeader(<wbr>SymtabSHeader, CBA);<br>
>> - SHeaders.push_back(<wbr>SymtabSHeader);<br>
>> -<br>
>> - // .strtab string table header.<br>
>> - Elf_Shdr DotStrTabSHeader;<br>
>> - State.initStrtabSectionHeader(<wbr>DotStrTabSHeader, ".strtab", State.DotStrtab,<br>
>> - CBA);<br>
>> - SHeaders.push_back(<wbr>DotStrTabSHeader);<br>
>> -<br>
>> - // .shstrtab string table header.<br>
>> - Elf_Shdr ShStrTabSHeader;<br>
>> - State.initStrtabSectionHeader(<wbr>ShStrTabSHeader, ".shstrtab", State.DotShStrtab,<br>
>> - CBA);<br>
>> - SHeaders.push_back(<wbr>ShStrTabSHeader);<br>
>> + // Populate SHeaders with implicit sections not present in the Doc<br>
>> + for (const auto &Name : ImplicitSecNames)<br>
>> + if (State.SN2I.get(Name) >= SHeaders.size())<br>
>> + SHeaders.push_back({});<br>
>> +<br>
>> + // Initialize the implicit sections<br>
>> + auto Index = State.SN2I.get(".symtab");<br>
>> + State.initSymtabSectionHeader(<wbr>SHeaders[Index], CBA);<br>
>> + Index = State.SN2I.get(".strtab");<br>
>> + State.initStrtabSectionHeader(<wbr>SHeaders[Index], ".strtab", State.DotStrtab, CBA);<br>
>> + Index = State.SN2I.get(".shstrtab");<br>
>> + State.initStrtabSectionHeader(<wbr>SHeaders[Index], ".shstrtab", State.DotShStrtab, CBA);<br>
>><br>
>> // Now we can decide segment offsets<br>
>> State.setProgramHeaderLayout(<wbr>PHeaders, SHeaders);<br>
>><br>
>><br>
>> ______________________________<wbr>_________________<br>
>> llvm-commits mailing list<br>
>> <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div>