Hi Nick,

New patch attached.
I noticed that in the patch I also did not include the hash table free
function. Now it is correct.

On 08/19/2016 09:53 AM, Nick Clifton wrote:
> One minor point and one question:
>
>> +/* Create an X86-64 ELF linker hash table.  */
> Cut and paste typo - I assume that you meant 'Create an ARC ELF linker hash table'.
Indeed, cut and paste typo.
>> @@ -2020,17 +2148,21 @@ elf_arc_finish_dynamic_symbol (bfd * output_bfd,
>>  
>>    if (h->needs_copy)
>>      {
>> +      struct elf_arc_link_hash_table *arc_htab = elf_arc_hash_table (info);
>> +
>> +      if (h->dynindx == -1
>> +	  || (h->root.type != bfd_link_hash_defined
>> +	      && h->root.type != bfd_link_hash_defweak)
>> +	  || arc_htab->srelbss == NULL)
>> +	abort ();
> I am not sure if an abort is the correct function to call here.  Abort should
> only be called if there is an internal error in the library.  If it is possible
> for bad user input to trigger the situation then an error message should be
> generated instead.  I have not followed through all the logic, so maybe the
> conditions being tested should all be satisfied by earlier parts in the linking
> process, (in which case a BFD_ASSERT would be better than a direct call to abort),
> but it seems to me that it might be possible for a dynamic symbol to become 
> undefined somehow, and in that case an error message would be more appropriate.
IMHO, the abort is really just a failsafe detection to validate that we
can indeed do the copying, if we do not abort we will segm fault or
generate bad code.
To be honest, I would not bet on the correctness of this needs_copy
conditioning, but a priori it should verify that all the requirements
for the copy are met. If not it should be verified and reported before
this point.

Looking forward for your comments.

Cheers,
Cupertino