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

Message ID CAKdteOZ6vBOMvpayKSGme=LiBVZ3Xu9Fo16sAjdFTL3sc5sfyg@mail.gmail.com
State New
Headers show

Commit Message

Christophe Lyon Aug. 29, 2016, 2:56 p.m.
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?

Thanks,

Christophe

> Best regards,

>

> Thomas
2016-08-29  Christophe Lyon  <chritophe.lyon@linaro.org>

	bfd/
	* elf32-arm.c (cmse_entry_fct_p): Read first instruction in an
	endianness independent way.

Comments

Christophe Lyon Aug. 30, 2016, 8:56 a.m. | #1
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.

Thanks

Christophe

> Best regards,

>

> Thomas

Patch

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;
 }