<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, May 27, 2015 at 6:33 PM, Rafael Espíndola <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:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Joining late to thread, but here is my opinion on the binary file issue:<br>
<br>
Split the set of all possible inputs in two<br>
<br>
* Simple inputs. These are valid files with no requirements about some<br>
implementation detail. For example, a file with an undefined reference<br>
to "foo", but doesn't care what is the symbol number of foo.<br>
* Complex inputs. These include all invalid/corrupted objects but also<br>
cases that depend on specific implementation details, like the<br>
particular layout of strings in a .strtab.<br>
<br>
For the first case, one can just use llvm-mc. An assembly input is far<br>
easier to read and maintain than yaml.<br>
<br>
For the second case, I think checking in the binary is a superior<br>
option. It makes sure we are testing the actual feature of the linker<br>
that was broken before. Maintenance is not an issue. We want to keep<br>
lld working with files that have multiple sections with the same name<br>
no meter what. Even if we decide those files are a bad idea and nuke<br>
generation from llvm, they are valid ELF and we should keep supporting<br>
them. Given how specific these files are, understanding them is also<br>
not a problem. Taking an example from the bitcode reader, can anyone<br>
guess what is the property of invalid-global-var-comdat-id.bc? :-)<br></blockquote><div><br></div><div>I agree that if a test can be written with llvm-mc, it probably makes sense to write it with llvm-mc. I bet that quite a few LLD tests that currently are using yaml could be switched over.<br></div><div><br></div><div>But there are relatively few easy ways to create object files that an assembler/compiler wouldn't create. yaml2obj is probably the easiest and least-error-prone for certain things. For example, to test error handling of certain things related to relocations, you can't use an assembler, but it is also requires creating object files that reference specific relocations with specific values of fields. 010 editor with the ELF binary template is the only tool that I know of that would let you moderately conveniently create files like lld/test/elf/AArch64/rel-abs16-overflow.test, whereas the yaml is pretty straightforward and is just a simple textual edit from the "regular case" test of the relocation, as you would expect for a test which is meant to differ from the working case in just a single number.</div><div><br></div><div>So from a developer's perspective it retains the feeling of "ok, here's a test for the usual case" and "ok, here's a test for the error case"; whereas the alternatives I can think of using llvm-mc + editing the binary make testing the error case seem like a huge chore by comparison, and likely the error case would just be untested.</div><div><br></div><div>So I think there's a gray area between your classification of "Simple inputs" and "Complex inputs" where yaml2obj is actually a pretty decent fit.</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
Cheers,<br>
Rafael<br>
<div class=""><div class="h5"><br>
<br>
On 26 May 2015 at 19:26, Sean Silva <<a href="mailto:chisophugis@gmail.com">chisophugis@gmail.com</a>> wrote:<br>
><br>
><br>
> On Tue, May 26, 2015 at 2:18 PM, Rui Ueyama <<a href="mailto:ruiu@google.com">ruiu@google.com</a>> wrote:<br>
>><br>
>> On Tue, May 26, 2015 at 2:08 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
>>><br>
>>><br>
>>><br>
>>> On Sun, May 24, 2015 at 9:19 AM, Simon Atanasyan <<a href="mailto:simon@atanasyan.com">simon@atanasyan.com</a>><br>
>>> wrote:<br>
>>>><br>
>>>> Author: atanasyan<br>
>>>> Date: Sun May 24 11:19:27 2015<br>
>>>> New Revision: 238115<br>
>>>><br>
>>>> URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D238115-26view-3Drev&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=G5quwkuzyg1-XX704dYH1NMxt5KZYVOgoCMbHn9m2d8&s=bvg0pKGWQY2lfAroax_YjdJ0C1oqqNcMF3lHz8W7xRo&e=" target="_blank">http://llvm.org/viewvc/llvm-project?rev=238115&view=rev</a><br>
>>>> Log:<br>
>>>> [ELF] Fix lld when no unique sections is used<br>
>>>><br>
>>>> Original patch of Shankar Easwaran with additional test case.<br>
>>>> The yaml2obj does not allow to create an object file with non-unique<br>
>>>> sections names<br>
>>><br>
>>><br>
>>> Should we fix yaml2obj?<br>
>><br>
>><br>
>> Yes we should fix it if it can't handle valid objects, but I think we<br>
>> probably start considering checking in object files (as opposed to checking<br>
>> in yaml files) is the right thing, as IIRC Rafael said before. Object files<br>
>> are the input for the linker, so in order to get a precise control over the<br>
>> input, it's better to use object files themselves.<br>
><br>
><br>
> Rafael was referring to using a binary as essentially an integration test<br>
> "to test that lld can work with a binary produced by a current clang". For<br>
> the bulk of testing that is definitely the wrong thing to do.<br>
><br>
> The idea that we need "precise control" in general is patently false.<br>
><br>
> For a given test input, we expect the same behavior on a class of equivalent<br>
> object files. What is considered equivalent is determined by what<br>
> transformations can be made to an object file that will result in the same<br>
> interpretation by the code being tested. For example, we can rearrange the<br>
> string table with no effect on the interpretation of the object file.<br>
><br>
> When testing a particular piece of code by feeding an input, the only<br>
> important thing is that we provide an input that falls into a class of<br>
> equivalent object files that exercise the code in the way we want. It is in<br>
> that sense that the output of yaml2obj is useful for testing. It is the same<br>
> reason that the .ll format is useful for testing. The .ll format cannot<br>
> represent all .bc files, nor can it represent all in-memory Module's, but<br>
> that is a *good thing*, because for most of the things that we want to test,<br>
> it *is* able to be transformed into a member of the equivalence class of<br>
> in-memory Module's that we need for testing, while being substantially more<br>
> readable (and other good properties).<br>
><br>
> -- Sean Silva<br>
><br>
>><br>
>><br>
>>><br>
>>>><br>
>>>> so the fix uses a binary input object file in the test<br>
>>>> case.<br>
>>>><br>
>>>> Added:<br>
>>>>     lld/trunk/test/elf/Inputs/no-unique-section-names.x86-64<br>
>>>>     lld/trunk/test/elf/no-unique-section-names.test<br>
>>>> Modified:<br>
>>>>     lld/trunk/lib/ReaderWriter/ELF/ELFFile.cpp<br>
>>>>     lld/trunk/lib/ReaderWriter/ELF/ELFFile.h<br>
>>>><br>
>>>> Modified: lld/trunk/lib/ReaderWriter/ELF/ELFFile.cpp<br>
>>>> URL:<br>
>>>> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_lld_trunk_lib_ReaderWriter_ELF_ELFFile.cpp-3Frev-3D238115-26r1-3D238114-26r2-3D238115-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=G5quwkuzyg1-XX704dYH1NMxt5KZYVOgoCMbHn9m2d8&s=Dqtu-FvSihAL9eRJjK9W7plh2aof46H7Qm9NS_kBSnE&e=" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/ELFFile.cpp?rev=238115&r1=238114&r2=238115&view=diff</a><br>
>>>><br>
>>>> ==============================================================================<br>
>>>> --- lld/trunk/lib/ReaderWriter/ELF/ELFFile.cpp (original)<br>
>>>> +++ lld/trunk/lib/ReaderWriter/ELF/ELFFile.cpp Sun May 24 11:19:27 2015<br>
>>>> @@ -143,7 +143,7 @@ std::error_code ELFFile<ELFT>::createAto<br>
>>>>        auto rai(_objFile->begin_rela(&section));<br>
>>>>        auto rae(_objFile->end_rela(&section));<br>
>>>><br>
>>>> -      _relocationAddendReferences[*sectionName] = make_range(rai, rae);<br>
>>>> +      _relocationAddendReferences[sHdr] = make_range(rai, rae);<br>
>>>>        totalRelocs += std::distance(rai, rae);<br>
>>>>      } else if (section.sh_type == llvm::ELF::SHT_REL) {<br>
>>>>        auto sHdr = _objFile->getSection(section.sh_info);<br>
>>>> @@ -155,7 +155,7 @@ std::error_code ELFFile<ELFT>::createAto<br>
>>>>        auto ri(_objFile->begin_rel(&section));<br>
>>>>        auto re(_objFile->end_rel(&section));<br>
>>>><br>
>>>> -      _relocationReferences[*sectionName] = make_range(ri, re);<br>
>>>> +      _relocationReferences[sHdr] = make_range(ri, re);<br>
>>>>        totalRelocs += std::distance(ri, re);<br>
>>>>      } else {<br>
>>>>        _sectionSymbols[&section];<br>
>>>> @@ -554,12 +554,12 @@ ELFDefinedAtom<ELFT> *ELFFile<ELFT>::cre<br>
>>>>    unsigned int referenceStart = _references.size();<br>
>>>><br>
>>>>    // Add Rela (those with r_addend) references:<br>
>>>> -  auto rari = _relocationAddendReferences.find(sectionName);<br>
>>>> +  auto rari = _relocationAddendReferences.find(section);<br>
>>>>    if (rari != _relocationAddendReferences.end())<br>
>>>>      createRelocationReferences(symbol, symContent, rari->second);<br>
>>>><br>
>>>>    // Add Rel references.<br>
>>>> -  auto rri = _relocationReferences.find(sectionName);<br>
>>>> +  auto rri = _relocationReferences.find(section);<br>
>>>>    if (rri != _relocationReferences.end())<br>
>>>>      createRelocationReferences(symbol, symContent, secContent,<br>
>>>> rri->second);<br>
>>>><br>
>>>><br>
>>>> Modified: lld/trunk/lib/ReaderWriter/ELF/ELFFile.h<br>
>>>> URL:<br>
>>>> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_lld_trunk_lib_ReaderWriter_ELF_ELFFile.h-3Frev-3D238115-26r1-3D238114-26r2-3D238115-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=G5quwkuzyg1-XX704dYH1NMxt5KZYVOgoCMbHn9m2d8&s=3CbOiQ3MtVHFJ750Q9wqVTxGRf-U445or3G40wICbto&e=" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/lib/ReaderWriter/ELF/ELFFile.h?rev=238115&r1=238114&r2=238115&view=diff</a><br>
>>>><br>
>>>> ==============================================================================<br>
>>>> --- lld/trunk/lib/ReaderWriter/ELF/ELFFile.h (original)<br>
>>>> +++ lld/trunk/lib/ReaderWriter/ELF/ELFFile.h Sun May 24 11:19:27 2015<br>
>>>> @@ -345,10 +345,10 @@ protected:<br>
>>>>    /// list of relocations references.  In ELF, if a section named,<br>
>>>> ".text" has<br>
>>>>    /// relocations will also have a section named ".rel.text" or<br>
>>>> ".rela.text"<br>
>>>>    /// which will hold the entries.<br>
>>>> -  std::unordered_map<StringRef, range<Elf_Rela_Iter>><br>
>>>> +  std::unordered_map<const Elf_Shdr *, range<Elf_Rela_Iter>><br>
>>>>    _relocationAddendReferences;<br>
>>>>    MergedSectionMapT _mergedSectionMap;<br>
>>>> -  std::unordered_map<StringRef, range<Elf_Rel_Iter>><br>
>>>> _relocationReferences;<br>
>>>> +  std::unordered_map<const Elf_Shdr *, range<Elf_Rel_Iter>><br>
>>>> _relocationReferences;<br>
>>>>    std::vector<ELFReference<ELFT> *> _references;<br>
>>>>    llvm::DenseMap<const Elf_Sym *, Atom *> _symbolToAtomMapping;<br>
>>>>    llvm::DenseMap<const ELFReference<ELFT> *, const Elf_Sym *><br>
>>>><br>
>>>> Added: lld/trunk/test/elf/Inputs/no-unique-section-names.x86-64<br>
>>>> URL:<br>
>>>> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_lld_trunk_test_elf_Inputs_no-2Dunique-2Dsection-2Dnames.x86-2D64-3Frev-3D238115-26view-3Dauto&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=G5quwkuzyg1-XX704dYH1NMxt5KZYVOgoCMbHn9m2d8&s=XidC4uVLZ0WuNoRIV7UNR0q49PP5SL8jwLfSCpYpNQg&e=" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/test/elf/Inputs/no-unique-section-names.x86-64?rev=238115&view=auto</a><br>
>>>><br>
>>>> ==============================================================================<br>
>>>> Binary files lld/trunk/test/elf/Inputs/no-unique-section-names.x86-64<br>
>>>> (added) and lld/trunk/test/elf/Inputs/no-unique-section-names.x86-64 Sun May<br>
>>>> 24 11:19:27 2015 differ<br>
>>>><br>
>>>> Added: lld/trunk/test/elf/no-unique-section-names.test<br>
>>>> URL:<br>
>>>> <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_lld_trunk_test_elf_no-2Dunique-2Dsection-2Dnames.test-3Frev-3D238115-26view-3Dauto&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=G5quwkuzyg1-XX704dYH1NMxt5KZYVOgoCMbHn9m2d8&s=40d653dYLBksgWE8uBulh5zEQ8dJhW4rJHTlszDWp9o&e=" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/test/elf/no-unique-section-names.test?rev=238115&view=auto</a><br>
>>>><br>
>>>> ==============================================================================<br>
>>>> --- lld/trunk/test/elf/no-unique-section-names.test (added)<br>
>>>> +++ lld/trunk/test/elf/no-unique-section-names.test Sun May 24 11:19:27<br>
>>>> 2015<br>
>>>> @@ -0,0 +1,19 @@<br>
>>>> +# Check handling object files with non-unique named sections.<br>
>>>> +<br>
>>>> +RUN: lld -flavor gnu -target x86_64-linux -shared -o %t \<br>
>>>> +RUN:     %p/Inputs/no-unique-section-names.x86-64<br>
>>>> +RUN: llvm-objdump -s %p/Inputs/no-unique-section-names.x86-64 %t \<br>
>>>> +RUN:   | FileCheck %s<br>
>>>> +<br>
>>>> +CHECK:      Contents of section .group:<br>
>>>> +CHECK-NEXT:  0000 01000000 08000000<br>
>>>> +CHECK-NEXT: Contents of section .text:<br>
>>>> +CHECK-NEXT:  0000 [[A1:[0-9a-f]+]] [[A2:[0-9a-f]+]] [[A3:[0-9a-f]+]]<br>
>>>> +CHECK-NEXT: Contents of section .group:<br>
>>>> +CHECK-NEXT:  0000 01000000 0a000000<br>
>>>> +CHECK-NEXT: Contents of section .text:<br>
>>>> +CHECK-NEXT:  0000 [[B1:[0-9a-f]+]] [[B2:[0-9a-f]+]] [[B3:[0-9a-f]+]]<br>
>>>> +<br>
>>>> +CHECK:      Contents of section .text:<br>
>>>> +CHECK:       {{[0-9a-f]+}} [[A1]] [[A2]] [[A3]]<br>
>>>> +CHECK-NEXT:  {{[0-9a-f]+}} [[B1]] [[B2]] [[B3]]<br>
>>>><br>
>>>><br>
>>>> _______________________________________________<br>
>>>> llvm-commits mailing list<br>
>>>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>>>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
>>><br>
>>><br>
>><br>
>><br>
>> _______________________________________________<br>
>> llvm-commits mailing list<br>
>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
>><br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
</div></div></blockquote></div><br></div></div>