[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.


More information about the llvm-commits mailing list