This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [2/16][binutils][AARCH64]Add relocation support for large memory model. [LD]Add BFD_RELOC_AARCH64_LD64_GOTOFF_LO15 Support.
- From: Marcus Shawcroft <marcus dot shawcroft at gmail dot com>
- To: Nick Clifton <nickc at redhat dot com>
- Cc: Renlin Li <renlin dot li at arm dot com>, "binutils at sourceware dot org" <binutils at sourceware dot org>, Marcus Shawcroft <Marcus dot Shawcroft at arm dot com>, Jiong Wang <jiong dot wang at arm dot com>
- Date: Tue, 8 Sep 2015 19:03:01 +0100
- Subject: Re: [2/16][binutils][AARCH64]Add relocation support for large memory model. [LD]Add BFD_RELOC_AARCH64_LD64_GOTOFF_LO15 Support.
- Authentication-results: sourceware.org; auth=none
- References: <55EF0938 dot 9060404 at arm dot com> <55EF0D86 dot 6030904 at redhat dot com>
On 8 September 2015 at 17:32, Nick Clifton <nickc@redhat.com> wrote:
> Hi Renlin,
>
> + int howto_index = bfd_r_type - BFD_RELOC_AARCH64_RELOC_START;
> + (*_bfd_error_handler)
> + (_("%B: Local symbol descriptor table be NULL when applying
> "
> + "relocation %s against local symbol"),
> + input_bfd, elfNN_aarch64_howto_table[howto_index].name);
> + abort ();
>
> I don't like having calls to abort() inside library functions. You have
> already emitted an error message, so I think that it would be better to set
> the error code and return FALSE.
The text of that error message doesn't read properly, can we fix that to please?
>
>
> + /* For local symbol, we have done absolute relocation in
> static
> + linking stage. While for share library, we need to
> update
> + the content of GOT entry according to the share objects
> + loading base address. So we need to generate a
> + R_AARCH64_RELATIVE reloc for dynamic linker. */
> + s = globals->root.srelgot;
> + if (s == NULL)
> + abort ();
This is checking internal consistency, the existing practice both here
and in _bfd_final_link_relocate which it wraps is to handle internal
inconsistency with an abort(), so this part of the proposed patch
seems to me to be consistent with existing practice.
If we move away from the use of abort() for such internal consistency
issues I wonder which bfd_error_* code should be used, none of them
look like a good fit?
Cheers
/Marcus