[PATCH] D28481: Respect section groups in GC
Evgenii Stepanov via llvm-commits
llvm-commits at lists.llvm.org
Wed Jan 11 12:01:34 PST 2017
Thanks for the pointer. I've may have sent a message to generic-abi
(if you get two, ignore the first one).
On Wed, Jan 11, 2017 at 11:21 AM, Rafael Avila de Espindola
<rafael.espindola at gmail.com> wrote:
>
> Please don't commit it just yet. There are important undecided semantic
> issues to resolve.
>
> Rui Ueyama via Phabricator via llvm-commits
> <llvm-commits at lists.llvm.org> writes:
>
>> ruiu accepted this revision.
>> ruiu added a comment.
>> This revision is now accepted and ready to land.
>>
>> LGTM with nits.
>>
>>
>>
>> ================
>> Comment at: ELF/MarkLive.cpp:83
>> std::function<void(ResolvedReloc<ELFT>)> Fn) {
>> if (Sec.AreRelocsRela) {
>> for (const typename ELFT::Rela &Rel : Sec.relas())
>> ----------------
>> ruiu wrote:
>>> // Visit relocations.
>> This belongs to this entire `if { ~ } else { ~ }` and not to this `if` clause, so it should be written before `if`.
>>
>>
>> ================
>> Comment at: ELF/MarkLive.cpp:95
>> + // Visit SHT_GROUP members if this section is in a group.
>> + for (auto SecIdx : Sec.GroupSections) {
>> + InputSectionBase<ELFT> *SameGroupSection =
>> ----------------
>> auto -> uint32_t
>>
>>
>> ================
>> Comment at: ELF/MarkLive.cpp:187
>> + // https://github.com/google/sanitizers/issues/260 for more context.
>> + if (Sec->GroupSections.size() > 0)
>> + return false;
>> ----------------
>> nit: use `empty()` if ArrayRef has that function.
>>
>>
>> Repository:
>> rL LLVM
>>
>> https://reviews.llvm.org/D28481
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list