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

[Eugene Leviant via llvm-commits]

пятница, 16 сентября 2016 г. пользователь Rui Ueyama написал:

> On Thu, Sep 15, 2016 at 3:17 PM, Eugene Leviant <evgeny.leviant at gmail.com
> <javascript:_e(%7B%7D,'cvml','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?
>

Well, may be. What spec are you talking about?

>
>
>>
>>
>>> 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/20160916/fc71db33/attachment.html>