Re: [PATCH] [BINUTILS] [MICROBLAZE] PIC Data Text Relative

[Andrew Sadek <andrew dot sadek dot se at gmail dot com>]

On Mon, Mar 26, 2018 at 1:48 AM, Michael Eager <eager@eagercon.com> wrote:
>
>>>>       (microblaze_bfd_write_branch_absolute_value_64): replace relative
>>>> branch with absolute in case 'adjust_insn_abs_refs' is true
>>>>       (microblaze_bfd_revert_base_reg_value_64): revert base register
>>>> from r20 to r0 in case 'adjust_insn_abs_refs' is true
>>>
>>>
>>>
>>> These two functions are short and only referenced in
>>> microblaze_elf_relocate_section().  It might be clearer to put them
>>> inline in this function.
>>>
>>> I'm unclear what is going on in these functions or why.  Why are you
>>> replacing PCREL or TEXTREL reloc with a generic reloc?
>>>
>>> I'm not comfortable with editing the instructions to replace opcodes or
>>> registers.  Shouldn't this be handled in GCC by generating the correct
>>> instruction in the first place?  What this means is that the source
>>> assembly generated by GCC will be different from that generated by
>>> objdump.
>>>
>>
>> In fact, the idea behind 'adjust-insn-abs-refs' option is when making
>> static linking
>> to a position independent executable with the base program using -R,
>> the relocate function
>> checks whether the symbol is coming from the external file or not then
>> re-adjust the instruction
>> to use absolute addressing instead since the reference location in
>> memory is fixed and shall be
>> excluded from PIC/PIE.
>> - For function call: brlid => bralid and accordingly PCREL => Generic
>> Reloc
>> - For data reference => replace base register  r0 => r20 and TEXTREL =>
>> Generic
>>
>> I understand it's not the best clean way. I agree with you that it
>> would be better to handle it in GCC.
>> So, I came up with this solution: Make a variable/function declaration
>> attribute in microblaze.c called for example
>> 'absolute_reference' which then excludes the symbol reference from PIC
>> and generate the corresponding
>> instruction. In addition, replace 'adjust-insn-abs-refs' in linker
>> with something like 'validate-insn-abs-refs'
>> that only checks if symbol references coming from the external file
>> are invoked with absolute addressing
>> and generate error/warning if not. What do you think ?
>
>
> I'll have to give this some thought.
>
> Do any other targets support a text relative addressing mode?

I could find it in ARM but the instructions that retrieve the data
support pc-relative addressing also could not find the feature of
switching between relative and absolute addressing in PIC.
I found this discussion in gnu archive:
https://lists.gnu.org/archive/html/help-gplusplus/2012-03/msg00000.html

>
>> I get this error:
>>
>> *Hi. This is the qmail-send program at sourceware.org.
>> *I'm afraid I wasn't able to deliver your message to the following
>> addresses.
>> *This is a permanent error; I've given up. Sorry it didn't work out.
>>
>> *<gcc-patches@gcc.gnu.org>:
>> *Invalid mime type "text/html" detected in message text or
>> *attachment.  Please send plain text messages only.
>> *See http://sourceware.org/lists.html#sourceware-list-info for more
>> information.
>>
>> I checked the website, it's stated that only plain text is allowed.
>
> Set your email client to send text only.  If you are using gmail, click
> the arrow at the bottom right of the compose window and select "Plain
> Text Mode".

Thanks. It's working now. I m just attaching the same patch.

> --
> Michael Eager    eager@eagercon.com
>
> 1960 Park Blvd., Palo Alto, CA 94306



-- 

Andrew

Attachment: binutils.patch.txt
Description: Text document