ELF support for both little and big endian targets

[Michele Scandale]

Hi all,

I just started working on MCLinker to develop a linker for the OpenRISC OR1K architecture. The current implementation of the OR1K processor is big endian, thus I would need to support big endian ELFs. The current implementation of mclinker does not support big endian ELFs. Because of this, I deciced to implement such support both in reading and writing, and I would like this contribution to be a part of the official mclinker trunk.
To simplify the review process (the overall patch is quite big) I shared my work on https://github.com/michele-scandale/mclinker/tree/endianness-support. The patch is composed by the last three commits in the 'endianness-support' branch:
- 4dde7c2b85799c01a24654ce6580eda9fd1f81de : this commit is a preliminary refactor on the target hooks to read/write relocations.
- d911273db810fd4fa768836e09dcad920a527270 : this commit adds the new component ELFReaderWriter. This class is based on ELFReaderIf and ELFReader and it is extedend with methods to implements the writing of an ELF object. There is the base class named GenericELFReaderWriter, plus the template specialization for 32/64 bitsize and little/big endianness.
- 551b4b01ebd69ed1122652adc528b48a594cb960 : this commit integrates the new component in the GNULDBackend and other parts and removes the old ELFReader objects.

From a functional point of view nothing changed: i.e. there are no regression in the test suite. Few methods have been refactored.

Any feedback is appreciated.

Regards,
-Michele

[Diana Chen]

Hi Michele, 

Thanks a lot for the patch. It looks good. While it's quite a big change that we need some more time to look through it and do some further testing. Besides, there are some coding style differences, for example, a pointer/reference operator in a variable declaration, we expect it to be 'Module& pModule' instead of 'Module &pModule'. Could you please help to refine them?

Thanks,

Diana

--
You received this message because you are subscribed to the Google Groups "MCLinker" group.
To unsubscribe from this group and stop receiving emails from it, send an email to mclinker+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--
柏聿

[Michele Scandale]

On Friday, August 29, 2014 5:02:57 AM UTC+1, Diana Chen wrote:

Hi Michele, 

Thanks a lot for the patch. It looks good. While it's quite a big change that we need some more time to look through it and do some further testing. Besides, there are some coding style differences, for example, a pointer/reference operator in a variable declaration, we expect it to be 'Module& pModule' instead of 'Module &pModule'. Could you please help to refine them?

Sure, is there a document discussing the coding style required so that I can fix all other style issues?

Thanks again.

Regards,
-Michele
 

[Diana Chen]

Hi Michele,

Currently there is no strict coding style in MCLinker source, but we'd like to keep the source uniform in the same file. For those enhanced or extended code for existed code, use the existed coding style in that file. We're considering using LLVM coding standard in the future. While for now, we just keep the style the same in a file.

Thanks,

Diana

[Michele Scandale]

Hi Diana,

in https://github.com/michele-scandale/mclinker/tree/endianness-support you can find the commit that fixes the style inconsistencies.
Please let me know if the whole patch is fine.

Thanks in advance.

Best regards,
-Michele

[Diana Chen]

Hi Michele,

Thanks for the fixes. We're currently working on some big style fix and it has mostly been done. We decide to follow Google C++ Style Guide except the include style. For the include style, we follow LLVM's. Also we provide the style checker in MCLinker to check the coding style. (which is "make cpplint". Please find it in Wiki getting start page). So it's very appreciated if you can help to rebase upstream to your change and sorry for the inconvenient.

Another question is that is there any benefit of having ELFReader and ELFWriter merged into one class? While we thought that it would be more clear to separate them.

Thank you.

Best regards,

Diana

--

You received this message because you are subscribed to the Google Groups "MCLinker" group.

[Michele Scandale]

On Wednesday, October 1, 2014 9:28:06 AM UTC+1, Diana Chen wrote:

Hi Michele,

Thanks for the fixes. We're currently working on some big style fix and it has mostly been done. We decide to follow Google C++ Style Guide except the include style. For the include style, we follow LLVM's. Also we provide the style checker in MCLinker to check the coding style. (which is "make cpplint". Please find it in Wiki getting start page). So it's very appreciated if you can help to rebase upstream to your change and sorry for the inconvenient.

No problem for this. To better understand, should the naming convention be the one reported in the Google C++ Style Guide? Or you decided to adopt only the formatting guidelines?
 

Another question is that is there any benefit of having ELFReader and ELFWriter merged into one class? While we thought that it would be more clear to separate them

My idea was to have  a unique template class to have helper methods for manipulating an ELF given the architecture bitsize and the endianness. In this way the real elf reader and writer can use the unique instance create in the backend object to implement the functionality.
Probably the names GenericELFReaderWriter and ELFReaderWriter are not the perfect choice, but I haven't found a better name. I am open to suggestions :-)

Thanks.

Best regards,
-Michele
 

[Diana Chen]

On Thu, Oct 16, 2014 at 5:56 AM, Michele Scandale <michele....@gmail.com> wrote:

On Wednesday, October 1, 2014 9:28:06 AM UTC+1, Diana Chen wrote:

Hi Michele,

Thanks for the fixes. We're currently working on some big style fix and it has mostly been done. We decide to follow Google C++ Style Guide except the include style. For the include style, we follow LLVM's. Also we provide the style checker in MCLinker to check the coding style. (which is "make cpplint". Please find it in Wiki getting start page). So it's very appreciated if you can help to rebase upstream to your change and sorry for the inconvenient.

No problem for this. To better understand, should the naming convention be the one reported in the Google C++ Style Guide? Or you decided to adopt only the formatting guidelines?

Currently just the formatting guidelines. Thank you. 

 

 

Another question is that is there any benefit of having ELFReader and ELFWriter merged into one class? While we thought that it would be more clear to separate them

My idea was to have  a unique template class to have helper methods for manipulating an ELF given the architecture bitsize and the endianness. In this way the real elf reader and writer can use the unique instance create in the backend object to implement the functionality.
Probably the names GenericELFReaderWriter and ELFReaderWriter are not the perfect choice, but I haven't found a better name. I am open to suggestions :-)

Thanks.

Best regards,
-Michele
 

Thank you.

Best regards,

Diana

On Tue, Sep 30, 2014 at 6:38 AM, Michele Scandale <michele....@gmail.com> wrote:

Hi Diana,

in https://github.com/michele-scandale/mclinker/tree/endianness-support you can find the commit that fixes the style inconsistencies.
Please let me know if the whole patch is fine.

Thanks in advance.

Best regards,
-Michele

--
You received this message because you are subscribed to the Google Groups "MCLinker" group.

To unsubscribe from this group and stop receiving emails from it, send an email to mclinker+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "MCLinker" group.
To unsubscribe from this group and stop receiving emails from it, send an email to mclinker+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--
Diana