[PATCH] D15610: [ELF] - Fixed handling relocations against zero sized .eh_frame section.
George Rimar via llvm-commits
llvm-commits at lists.llvm.org
Mon Dec 21 03:32:19 PST 2015
grimar added inline comments.
================
Comment at: ELF/OutputSections.cpp:856-860
@@ -855,3 +855,7 @@
}
- return VA + cast<MergeInputSection<ELFT>>(Section)->getOffset(Offset) +
- Addend;
+ uintX_t SecOff;
+ if (isa<EHInputSection<ELFT>>(Section))
+ SecOff = cast<EHInputSection<ELFT>>(Section)->getOffset(Offset);
+ else
+ SecOff = cast<MergeInputSection<ELFT>>(Section)->getOffset(Offset);
+
----------------
ruiu wrote:
> Is there any reason to not make getOffset a virtual function?
That would require some design change. For some reason approach that I used is seems to be as standart in new lld, for example there is such method:
```
template <class ELFT>
typename ELFFile<ELFT>::uintX_t
InputSectionBase<ELFT>::getOffset(uintX_t Offset) {
switch (SectionKind) {
case Regular:
return cast<InputSection<ELFT>>(this)->OutSecOff + Offset;
case EHFrame:
return cast<EHInputSection<ELFT>>(this)->getOffset(Offset);
case Merge:
return cast<MergeInputSection<ELFT>>(this)->getOffset(Offset);
case MipsReginfo:
return cast<MipsReginfoInputSection<ELFT>>(this)->getOffset(Offset);
}
llvm_unreachable("Invalid section kind");
}
```
I can only guess why it was done in that way. But answering your question: to make it virtual all such places should be reimplemented first, there will be casting problem if I make it virtual only in SplitInputSection class (which is base for EHInputSection an MergeInputSection).
To use cast<SplitInputSection<ELFT>> I would need to add something to next enum for classof() implementation to recognize the type of SplitInputSection:
```
enum Kind { Regular, EHFrame, Merge, MipsReginfo };
```
That looks very inconsistent with what we already have and if requires change then for all at once I think.
http://reviews.llvm.org/D15610
More information about the llvm-commits
mailing list