Hi, Thanks for the explanation. If I understand you properly you suggest to move relocation parsing to the class with the following interface. Right? Who will be user of this class? If it is still only ELFFile class, what benefits will we get from separation of this logic? template <class ELFT> class ELFRelocationReader { public: ELFRelocationReader(.....); // Returns all created references. ReferenceRangeT getAllReferences(); // Returns references for specified section/symbol. ReferenceRangeT getReferences(StringRef sectionName, Elf_Sym *symbol, ArrayRef<uint8_t> content); protected: // Target can override these methods in the inherited class. virtual ELFReference<ELFT> *createReference(Elf_Rela &rel, Elf_Sym *symbol); virtual ELFReference<ELFT> *createReference(Elf_Rel &rel, Elf_Sym *symbol); }; On Wed, Feb 26, 2014 at 9:42 PM, Shankar Easwaran <shankare at codeaurora.org> wrote:> I was thinking of having a separate class that would return a vector of > ELFReferences when you the reader looks at a section and symbol. > > The class would be constructed with > _relocationAddendReferences/_relocationReferences. > > Each subtarget could make use of the functionality and create a different > type of ELFReference on a need basis.[...]>> What do you mean by removing relocation reading from the >> ELFObjectFile? I considered to customize the relocation reading for >> MIPS targets and my first idea was to factor out ELFReference creation >> into a couple of virtual functions. The first one is for Elf_Rel, the >> second one is for Elf_Rela. Then I planned to override these functions >> in the MipsELFFile class. But it looks like you have more profound >> idea. Could you share it?-- Simon
Hi Simon, The subclasses for ELFFile would use it I thought. For example if a target wants the basic logic for converting an input file to atoms, the target could just override the relocation handler to get all the references. I would use ErrorOr<ReferenceRangeT> to return any errors from relocation processing. Thanks Shankar Easwaran On 2/26/2014 3:16 PM, Simon Atanasyan wrote:> Hi, > > Thanks for the explanation. If I understand you properly you suggest > to move relocation parsing to the class with the following interface. > Right? > > Who will be user of this class? If it is still only ELFFile class, > what benefits will we get from separation of this logic? > > template <class ELFT> class ELFRelocationReader { > public: > ELFRelocationReader(.....); > > // Returns all created references. > ReferenceRangeT getAllReferences(); > > // Returns references for specified section/symbol. > ReferenceRangeT getReferences(StringRef sectionName, > Elf_Sym *symbol, > ArrayRef<uint8_t> content); > > protected: > // Target can override these methods in the inherited class. > virtual ELFReference<ELFT> *createReference(Elf_Rela &rel, Elf_Sym *symbol); > virtual ELFReference<ELFT> *createReference(Elf_Rel &rel, Elf_Sym *symbol); > }; > > On Wed, Feb 26, 2014 at 9:42 PM, Shankar Easwaran > <shankare at codeaurora.org> wrote: >> I was thinking of having a separate class that would return a vector of >> ELFReferences when you the reader looks at a section and symbol. >> >> The class would be constructed with >> _relocationAddendReferences/_relocationReferences. >> >> Each subtarget could make use of the functionality and create a different >> type of ELFReference on a need basis. > [...] > >>> What do you mean by removing relocation reading from the >>> ELFObjectFile? I considered to customize the relocation reading for >>> MIPS targets and my first idea was to factor out ELFReference creation >>> into a couple of virtual functions. The first one is for Elf_Rel, the >>> second one is for Elf_Rela. Then I planned to override these functions >>> in the MipsELFFile class. But it looks like you have more profound >>> idea. Could you share it?-- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by the Linux Foundation
Hi Shankar, I almost implement ELFRelocationReader but still not completely sure that this is a right direction. Suppose somebody wants to override creation of the `ELFReference` object from the `Elf_Rela` or `Elf_Rel` record. Let's consider two implementations A and B: A: ====1. Factor out `ELFReference` creation from `createDefinedAtomAndAssignRelocations` into a couple of virtual functions with the following signature: ELFReference<ELFT> * createRelocationReference(const Elf_Sym &symbol, const Elf_Rela &rai); ELFReference<ELFT> * createRelocationReference(const Elf_Sym &symbol, const Elf_Rel &ri, ArrayRef<uint8_t> content); 2. Override one or both these methods in the <target name>ELFFile class. B: ====1. Create new class `ELFRelocationReader` and factor out `ELFReference` creation into the methods of this class. 2. Add new virtual function `createRelocationReader` to the `ELFFile` to create an instance of the `ELFRelocationReader`. 3. Subclass `ELFRelocationReader` and implement a target specific relocation reading. 4. Override `ELFFile::createRelocationReader` to create `ELFRelocationReader` subclass. In both implementations we just move some portion of code from one place to another. We do not need to reuse the relocation reading functionality somewhere else. But the implementation "A" requires much less modifications and does not introduce any new entities. So from my point of view both approaches provide the same result but the implementation "B" much more wordy. Will we get any benefits if implement option "B"? Thanks. On Thu, Feb 27, 2014 at 2:33 AM, Shankar Easwaran <shankare at codeaurora.org> wrote:> The subclasses for ELFFile would use it I thought. > > For example if a target wants the basic logic for converting an input file > to atoms, the target could just override the relocation handler to get all > the references.[...]> On 2/26/2014 3:16 PM, Simon Atanasyan wrote:[...]>> Who will be user of this class? If it is still only ELFFile class, >> what benefits will we get from separation of this logic? >> >> template <class ELFT> class ELFRelocationReader { >> public: >> ELFRelocationReader(.....); >> >> // Returns all created references. >> ReferenceRangeT getAllReferences(); >> >> // Returns references for specified section/symbol. >> ReferenceRangeT getReferences(StringRef sectionName, >> Elf_Sym *symbol, >> ArrayRef<uint8_t> content); >> >> protected: >> // Target can override these methods in the inherited class. >> virtual ELFReference<ELFT> *createReference(Elf_Rela &rel, Elf_Sym >> *symbol); >> virtual ELFReference<ELFT> *createReference(Elf_Rel &rel, Elf_Sym >> *symbol); >> }; >> >> On Wed, Feb 26, 2014 at 9:42 PM, Shankar Easwaran >> <shankare at codeaurora.org> wrote: >>> >>> I was thinking of having a separate class that would return a vector of >>> ELFReferences when you the reader looks at a section and symbol. >>> >>> The class would be constructed with >>> _relocationAddendReferences/_relocationReferences. >>> >>> Each subtarget could make use of the functionality and create a different >>> type of ELFReference on a need basis. >> >> [...] >> >>>> What do you mean by removing relocation reading from the >>>> ELFObjectFile? I considered to customize the relocation reading for >>>> MIPS targets and my first idea was to factor out ELFReference creation >>>> into a couple of virtual functions. The first one is for Elf_Rel, the >>>> second one is for Elf_Rela. Then I planned to override these functions >>>> in the MipsELFFile class. But it looks like you have more profound >>>> idea. Could you share it?-- Simon