From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9365 invoked by alias); 4 Apr 2016 14:01:18 -0000 Mailing-List: contact binutils-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: binutils-owner@sourceware.org Received: (qmail 9356 invoked by uid 89); 4 Apr 2016 14:01:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.4 required=5.0 tests=BAYES_00,KAM_LAZY_DOMAIN_SECURITY,KAM_STOCKGEN,RP_MATCHES_RCVD autolearn=no version=3.3.2 spammy=wednesday, Developer, Wednesday, UD:arm.com X-HELO: foss.arm.com Received: from foss.arm.com (HELO foss.arm.com) (217.140.101.70) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 04 Apr 2016 14:01:07 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 1A74C424; Mon, 4 Apr 2016 06:59:56 -0700 (PDT) Received: from e108577-lin.localnet (usa-sjc-imap-foss1.foss.arm.com [10.72.51.249]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 7E2EF3F459; Mon, 4 Apr 2016 07:01:05 -0700 (PDT) From: Thomas Preudhomme To: Christophe Lyon Cc: binutils@sourceware.org Subject: Re: [RFC PATCH, binutils, ARM 2/11, ping] Factor our stub creation in ARM backend Date: Mon, 04 Apr 2016 14:01:00 -0000 Message-ID: <2440053.bcko6yOCN4@e108577-lin> User-Agent: KMail/4.13.3 (Linux/3.13.0-79-generic; KDE/4.13.3; x86_64; ; ) In-Reply-To: References: <004801d13d57$4a13b670$de3b2350$@foss.arm.com> <2632328.d9AUHOZn5J@e108577-lin> MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" X-IsSubscribed: yes X-SW-Source: 2016-04/txt/msg00053.txt.bz2 On Wednesday 30 March 2016 11:11:01 Christophe Lyon wrote: > On 29 March 2016 at 16:33, Thomas Preudhomme >=20 > wrote: > > Ping? > >=20 > > On Wednesday 23 December 2015 15:55:26 Thomas Preud'homme wrote: > >> Hi, > >>=20 > >> [Posting patch series as RFC] > >>=20 > >> This patch is part of a patch series to add support for ARMv8-M securi= ty > >> extension[1] to GNU ld. This specific patch factors out the code to > >> create > >> a stub in ARM backend to avoid code duplication in subsequent patches > >> adding new code for creating Secure Gateway veneers. > >>=20 > >>=20 > >> [1] Software requirements for ARMv8-M security extension are described= in > >> document ARM-ECM-0359818 [2] [2] Available on http://infocenter.arm.com > >> in > >> Developer guides and articles > Software development > ARM=C2=AEv8-M S= ecurity > >> Extensions: Requirements on Development Tools > >>=20 > >> ChangeLog entries is as follows: > >>=20 > >>=20 > >> *** bfd/ChangeLog *** > >>=20 > >> 2015-12-16 Thomas Preud'homme > >>=20 > >> * elf32-arm.c (elf32_arm_create_stub): New function. > >> (elf32_arm_size_stubs): Use elf32_arm_create_stub for stub > >> creation. > >>=20 > >> diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c > >> index > >> 639fe0f125688ccf218b449467c746686841386f..e6afb99512524ac26c0e3842e8db= e29 > >> 3c > >> 58bacad 100644 --- a/bfd/elf32-arm.c > >> +++ b/bfd/elf32-arm.c > >> @@ -5031,6 +5031,103 @@ cortex_a8_erratum_scan (bfd *input_bfd, > >>=20 > >> return FALSE; > >>=20=20 > >> } > >>=20 > >> +/* Create or update a stub entry depending on whether the stub can > >> already > >> be + found in HTAB. The stub is identified by: > >> + - its type STUB_TYPE > >> + - its source branch (note that several can share the same stub) wh= ose > >> + section and relocation (if any) are given by SECTION and IRELA > >> + respectively > >> + - its target symbol whose input section, hash, name, value and bra= nch > >> type + are given in SYM_SEC, HASH, SYM_NAME, SYM_VALUE and > >> BRANCH_TYPE > >> + respectively > >> + > >> + If found, the value of the stub's target symbol is updated from > >> SYM_VALUE + and *NEW_STUB is set to FALSE. Otherwise, *NEW_STUB is = set > >> to + TRUE and the stub entry is initialized. > >> + > >> + Returns whether the stub could be successfully created or updated,= or > >> FALSE + if an error occured. */ > >> + > >> +static bfd_boolean > >> +elf32_arm_create_stub (struct elf32_arm_link_hash_table *htab, > >> + enum elf32_arm_stub_type stub_type, asection > >> *section, > >> + Elf_Internal_Rela *irela, asection *sym_sec, > >> + struct elf32_arm_link_hash_entry *hash, char > >> *sym_name, + bfd_vma sym_value, enum > >> arm_st_branch_type branch_type, + bfd_boolean > >> *new_stub) > >> +{ > >> + const asection *id_sec; > >> + char *stub_name; > >> + struct elf32_arm_stub_hash_entry *stub_entry; > >> + unsigned int r_type; > >> + > >> + BFD_ASSERT (stub_type !=3D arm_stub_none); > >> + *new_stub =3D FALSE; > >> + > >> + BFD_ASSERT (irela); > >> + BFD_ASSERT (section); > >> + > >> + /* Support for grouping stub sections. */ > >> + id_sec =3D htab->stub_group[section->id].link_sec; > >> + > >> + /* Get the name of this stub. */ > >> + stub_name =3D elf32_arm_stub_name (id_sec, sym_sec, hash, irela, > >> stub_type); + if (!stub_name) > >> + return FALSE; > >> + > >> + stub_entry =3D arm_stub_hash_lookup (&htab->stub_hash_table, stub_n= ame, > >> FALSE, + FALSE); > >> + /* The proper stub has already been created, just update its value.= =20 > >> */ > >> + if (stub_entry !=3D NULL) > >> + { > >> + free (stub_name); > >> + stub_entry->target_value =3D sym_value; > >> + return TRUE; > >> + } > >> + > >> + stub_entry =3D elf32_arm_add_stub (stub_name, section, htab); > >> + if (stub_entry =3D=3D NULL) > >> + { > >> + free (stub_name); > >> + return FALSE; > >> + } > >> + > >> + stub_entry->target_value =3D sym_value; > >> + stub_entry->target_section =3D sym_sec; > >> + stub_entry->stub_type =3D stub_type; > >> + stub_entry->h =3D hash; > >> + stub_entry->branch_type =3D branch_type; > >> + > >> + if (sym_name =3D=3D NULL) > >> + sym_name =3D "unnamed"; > >> + stub_entry->output_name =3D (char *) > >> + bfd_alloc (htab->stub_bfd, sizeof (THUMB2ARM_GLUE_ENTRY_NAME) > >> + + strlen (sym_name)); > >> + if (stub_entry->output_name =3D=3D NULL) > >> + { > >> + free (stub_name); > >> + return FALSE; > >> + } > >> + > >> + /* For historical reasons, use the existing names for ARM-to-Thumb = and > >> + Thumb-to-ARM stubs. */ > >> + r_type =3D ELF32_R_TYPE (irela->r_info); > >> + if ((r_type =3D=3D (unsigned int) R_ARM_THM_CALL > >> + || r_type =3D=3D (unsigned int) R_ARM_THM_JUMP24 > >> + || r_type =3D=3D (unsigned int) R_ARM_THM_JUMP19) > >> + && branch_type =3D=3D ST_BRANCH_TO_ARM) > >> + sprintf (stub_entry->output_name, THUMB2ARM_GLUE_ENTRY_NAME, > >> sym_name); + else if ((r_type =3D=3D (unsigned int) R_ARM_CALL > >> + || r_type =3D=3D (unsigned int) R_ARM_JUMP24) > >> + && branch_type =3D=3D ST_BRANCH_TO_THUMB) > >> + sprintf (stub_entry->output_name, ARM2THUMB_GLUE_ENTRY_NAME, > >> sym_name); + else > >> + sprintf (stub_entry->output_name, STUB_ENTRY_NAME, sym_name); > >> + > >> + *new_stub =3D TRUE; > >> + return TRUE; > >> +} > >> + > >>=20 > >> /* Determine and set the size of the stub section for a final link. > >>=20=20 > >> The basic idea here is to examine all the relocations looking for > >>=20 > >> @@ -5174,14 +5271,11 @@ elf32_arm_size_stubs (bfd *output_bfd, > >>=20 > >> { > >>=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20 > >> unsigned int r_type, r_indx; > >> enum elf32_arm_stub_type stub_type; > >>=20 > >> - struct elf32_arm_stub_hash_entry *stub_entry; > >>=20 > >> asection *sym_sec; > >> bfd_vma sym_value; > >> bfd_vma destination; > >> struct elf32_arm_link_hash_entry *hash; > >> const char *sym_name; > >>=20 > >> - char *stub_name; > >> - const asection *id_sec; > >>=20 > >> unsigned char st_type; > >> enum arm_st_branch_type branch_type; > >> bfd_boolean created_stub =3D FALSE; > >>=20 > >> @@ -5364,6 +5458,8 @@ elf32_arm_size_stubs (bfd *output_bfd, > >>=20 > >> do > >>=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20 > >> { > >>=20 > >> + bfd_boolean new_stub; > >> + > >>=20 > >> /* Determine what (if any) linker stub is needed. = */ > >> stub_type =3D arm_type_of_stub (info, section, ire= la, > >>=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20 > >> st_type, &branch_typ= e, > >>=20 > >> @@ -5372,74 +5468,21 @@ elf32_arm_size_stubs (bfd *output_bfd, > >>=20 > >> if (stub_type =3D=3D arm_stub_none) > >>=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20 > >> break; > >>=20 > >> - /* Support for grouping stub sections. */ > >> - id_sec =3D htab->stub_group[section->id].link_sec; > >> - > >> - /* Get the name of this stub. */ > >> - stub_name =3D elf32_arm_stub_name (id_sec, sym_sec, > >> hash, > >> - irela, stub_type); > >> - if (!stub_name) > >> - goto error_ret_free_internal; > >> - > >>=20 > >> /* We've either created a stub for this reloc > >> already, > >>=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20 > >> or we are about to. */ > >>=20 > >> - created_stub =3D TRUE; > >> - > >> - stub_entry =3D arm_stub_hash_lookup > >> - (&htab->stub_hash_table, stub_name, > >> - FALSE, FALSE); > >> - if (stub_entry !=3D NULL) > >> - { > >> - /* The proper stub has already been created. = */ > >> - free (stub_name); > >> - stub_entry->target_value =3D sym_value; > >> - break; > >> - } Please take note of this lookup + conditional block. > >> - > >> - stub_entry =3D elf32_arm_add_stub (stub_name, sect= ion, > >> - htab); > >> - if (stub_entry =3D=3D NULL) > >> - { > >> - free (stub_name); > >> - goto error_ret_free_internal; > >> - } > >> + created_stub =3D > >> + elf32_arm_create_stub (htab, stub_type, section, > >> irela, + sym_sec, hash, > >> + (char *) sym_name, > >> sym_value, > >> + branch_type, &new_stub); > >>=20 > >> - stub_entry->target_value =3D sym_value; > >> - stub_entry->target_section =3D sym_sec; > >> - stub_entry->stub_type =3D stub_type; > >> - stub_entry->h =3D hash; > >> - stub_entry->branch_type =3D branch_type; > >> - > >> - if (sym_name =3D=3D NULL) > >> - sym_name =3D "unnamed"; > >> - stub_entry->output_name =3D (char *) > >> - bfd_alloc (htab->stub_bfd, > >> - sizeof (THUMB2ARM_GLUE_ENTRY_NAME) > >> - + strlen (sym_name)); > >> - if (stub_entry->output_name =3D=3D NULL) > >> + if (!created_stub || !new_stub) >=20 > I'm not sure to understand why you check both? Good catch! That's a result of the iterative process that for the create_st= ub=20 function. It should be a break for the !new_stub case, as in the code I=20 hilighted above. I'll respin the patch. Best regards, Thomas