[PATCH] D28481: Respect section groups in GC
Evgeniy Stepanov via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 10 16:58:09 PST 2017
eugenis added inline comments.
================
Comment at: ELF/InputFiles.cpp:295-303
+ if (ComdatGroups
+ .insert(
+ CachedHashStringRef(getShtGroupSignature(ObjSections, Sec)))
.second)
continue;
for (uint32_t SecIndex : getShtGroupEntries(Sec)) {
if (SecIndex >= Size)
----------------
ruiu wrote:
> Revert these cosmetic changes.
Done, but this file is not clang-format clean.
================
Comment at: ELF/InputFiles.cpp:342-344
+ if (SecIndex >= Size)
+ fatal(toString(this) +
+ ": invalid section index in group: " + Twine(SecIndex));
----------------
ruiu wrote:
> It shouldn't fail this time because we have the same error check above, so you don't need this check. Then this can be written as
>
> for (uint32_t SecIndex : GroupSections)
> if (Sections[SecIndex] != &InputSection<ELFT>::Discarded)
> Sections[SecIndex]->GroupSections = GroupSections;
The check above runs for discarded groups only.
================
Comment at: ELF/InputFiles.cpp:345-346
+ ": invalid section index in group: " + Twine(SecIndex));
+ if (!Sections[SecIndex] ||
+ Sections[SecIndex] == &InputSection<ELFT>::Discarded)
+ continue;
----------------
ruiu wrote:
> It cannot be nullptr, no? I think this can only be Discarded or a regular section.
These cases above don't seem to set the section pointer:
case SHT_SYMTAB:
case SHT_SYMTAB_SHNDX:
case SHT_STRTAB:
case SHT_NULL:
Repository:
rL LLVM
https://reviews.llvm.org/D28481
More information about the llvm-commits
mailing list