[PATCH] D24616: [ELF] Improve section GC for comdat groups

[Rui Ueyama via llvm-commits]

On Thu, Sep 15, 2016 at 3:17 PM, Eugene Leviant <evgeny.leviant at gmail.com>
wrote:

>
>
> пятница, 16 сентября 2016 г. пользователь Rui Ueyama написал:
>
>> On Thu, Sep 15, 2016 at 1:02 PM, Eugene Leviant <evgeny.leviant at gmail.com
>> > wrote:
>>
>>> First of all, I need to identify this section, because, like I said,
>>> it's being added only once to a single object file. When some group is seen
>>> next time, all member sections are discarded. This means that relocations
>>> in some object file may point to discarded sections. When you see this
>>> discarded section in GC, you should (IMHO) find it counterpart and mark it
>>> live.
>>>
>>
>> No, I don't think relocations could point to sections that were removed
>> because of comdat group deduplication.
>>
>
> I'm sorry but this does happen. I can't share the real world example, but
> you can try the test case which is a part of this patch (comdat-gc.s).
> You'll get a crash in forEachSuccessor.
>

But isn't it out of the spec?


>
>
>> All relocations to sections in comdat groups should be through undefined
>> symbols. So, when the control reaches this part of code, all symbols should
>> have already been resolved, or it will end up with a link error. You
>> shouldn't have to "resolve" comdat groups by name again here.
>>
> What I'm doing now is:
>>>
>>> a) Get group signature, given object file and some input section index.
>>> This is done using SectionGroupSig hash map.
>>> b) Store all group signatures in hash set
>>> c) Iterate object files and fetch all input sections, which are members
>>> of comdat groups with signatures in this hash set.
>>>
>>> Now I'm trying to understand your suggestion. You suggest to keep list
>>> of member sections in each member of the group, correct?
>>> You cannot do so in discarded section, so you still need to find real
>>> section, having some object file and section number in it, right?
>>>
>>>
>>> 2016-09-15 22:45 GMT+03:00 Rui Ueyama <ruiu at google.com>:
>>>
>>>> Imagine any section that is in some comdat group have a GroupMembers
>>>> vector, so that starting from any comdat group member section, you can
>>>> reach all siblings in the same group. With that, all you have to do for a
>>>> section S to make all its siblings live is to do `for (InputSectionData
>>>> *Succ : S->GroupMember) Enque({Succ, 0});`. Doesn't it work?
>>>>
>>>> On Thu, Sep 15, 2016 at 12:32 PM, Eugene Leviant <
>>>> evgeny.leviant at gmail.com> wrote:
>>>>
>>>>> evgeny777 added a comment.
>>>>>
>>>>> Let's elaborate the idea. The main problem is that symbol 'D' inside
>>>>> resolveReloc() may point to InputSectionBase<ELFT>::Discarded. This
>>>>> happens because comdat group is added to only one object file and causes
>>>>> crash in GC, because forEachSuccessor implicitly casts Discarded to
>>>>> InputSection<ELFT> and tries to fetch relocs from it. How this
>>>>> 'GroupMembers' vector would help?
>>>>>
>>>>>
>>>>> Repository:
>>>>>   rL LLVM
>>>>>
>>>>> https://reviews.llvm.org/D24616
>>>>>
>>>>>
>>>>>
>>>>>
>>>>
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160915/4d6d8835/attachment.html>