Re: [RFC PATCH, binutils, ARM 11/11, ping] Add support for stable secure gateway veneers addresses

[Thomas Preudhomme <thomas.preudhomme@foss.arm.com>]

On 30/08/16 09:56, Christophe Lyon wrote:
> On 30 August 2016 at 10:10, Thomas Preudhomme
> <thomas.preudhomme@foss.arm.com> wrote:
>> Hi Christophe,
>>
>>
>> On 29/08/16 15:56, Christophe Lyon wrote:
>>>
>>> On 26 August 2016 at 16:18, Thomas Preudhomme
>>> <thomas.preudhomme@foss.arm.com> wrote:
>>>>
>>>> On 26/08/16 13:36, Thomas Preudhomme wrote:
>>>>>
>>>>>
>>>>> Hi Christophe,
>>>>>
>>>>> On 26/08/16 12:55, Christophe Lyon wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> I've noticed that the new tests fail on armeb. I didn't look at the
>>>>>> detailed logs yet, but I guess you can a look?
>>>>>>
>>>>>> ./ld/ld.sum:FAIL: Input secure gateway import library
>>>>>> ./ld/ld.sum:FAIL: Input secure gateway import library: no output import
>>>>>> library
>>>>>> ./ld/ld.sum:FAIL: Input secure gateway import library: earlier stub
>>>>>> section base
>>>>>> ./ld/ld.sum:FAIL: Input secure gateway import library: later stub
>>>>>> section
>>>>>> base
>>>>>> ./ld/ld.sum:FAIL: Input secure gateway import library: veneer comeback
>>>>>> ./ld/ld.sum:FAIL: Input secure gateway import library: entry function
>>>>>> change
>>>>>
>>>>>
>>>>>
>>>>> I can reproduce indeed. I'll have a look, thanks for the notice. Note
>>>>> that
>>>>> Monday is a bank holidays here so might only answer after that if I'm
>>>>> too
>>>>> slow
>>>>> to find the root cause.
>>>>
>>>>
>>>>
>>>> Doh, the code checks for a SG instruction by comparing the 4 bytes in the
>>>> code against its litteral value. The problem of course is that the read
>>>> puts
>>>> the 4 bytes of the instruction in memory order but these will then be
>>>> interpreted according to the endianness.
>>>>
>>>> The fix should be easy.
>>>>
>>>
>>> Indeed, thanks to your analysis it was quick enough.
>>>
>>> Here is a patch, maybe there is a simpler way?
>>
>>
>> diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c
>> index 9ff418a..5c04b9b 100644
>> --- a/bfd/elf32-arm.c
>> +++ b/bfd/elf32-arm.c
>> @@ -5791,11 +5791,19 @@ cmse_entry_fct_p (struct elf32_arm_link_hash_entry
>> *hash)
>>    section = hash->root.root.u.def.section;
>>    abfd = section->owner;
>>    offset = hash->root.root.u.def.value - section->vma;
>> -  if (!bfd_get_section_contents (abfd, section, &first_insn, offset,
>> -                                sizeof (first_insn)))
>> -    return FALSE;
>>
>>    /* Start by SG instruction.  */
>> +  bfd_byte * contents;
>> +  /* Get cached copy if it exists.  */
>> +  if (elf_section_data (section)->this_hdr.contents != NULL)
>> +    contents = elf_section_data (section)->this_hdr.contents;
>> +  else
>> +    {
>> +      /* Go get them off disk.  */
>> +      if (! bfd_malloc_and_get_section (abfd, section, &contents))
>> +       return FALSE;
>> +    }
>> +  first_insn = bfd_get_32 (abfd, contents + offset);
>>    return first_insn == 0xe97fe97f;
>>  }
>>
>> I have a similar patch but I'm still using bfd_get_section_contents with a
>> 4byte buffer named contents. It avoids reading the entire section to just
>> analyze 4 bytes. I did it on Friday so wanted to test it a bit more today.
>>
>
> OK that's fine, it sounds better than mine. I posted mine because
> I wasn't sure you were actually working on a fix.

I am, will post the patch later today. Thanks for providing a fix.

Best regards,

Thomas