From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 42786 invoked by alias); 30 Mar 2016 09:11:20 -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 42776 invoked by uid 89); 30 Mar 2016 09:11:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.3 required=5.0 tests=AWL,BAYES_50,KAM_STOCKGEN,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=no version=3.3.2 spammy=SECTION, sym_name, sk:error_r, STUB_ENTRY_NAME X-HELO: mail-qg0-f53.google.com Received: from mail-qg0-f53.google.com (HELO mail-qg0-f53.google.com) (209.85.192.53) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-GCM-SHA256 encrypted) ESMTPS; Wed, 30 Mar 2016 09:11:09 +0000 Received: by mail-qg0-f53.google.com with SMTP id y89so33609528qge.2 for ; Wed, 30 Mar 2016 02:11:09 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-transfer-encoding; bh=Oe0SjH/v/QsAYkRxXFKM7H1xLbCprpDah83zkFYa4Ak=; b=ROJctOt8cyVOYbR7gREHU3UfXnDi50+Pn8zMvAuYL2YvCbDIjSTI69wO0urHmOn6+C kQn/f7xM2bg+cKJnZ1UjOeRnILoC9K85/eJATfbNt/wN3k2bLv1/AALB8gbNM8fHFPwg ulbdATb3rVz+hyt6im+lgKg/qJ4yRktoSIR5cPvXhOgyRpRIpVqIEFQUBSc99VbRN6rt FFazYhK239cbc54xmL5MDr5M3fv0F4abExAOI1xziv6IvzO+NTk0k5xWzEC8TvRvN44g ZRDD11IUQP8A8YJ2RZflZB0T1lHVGvbcPP+GzhDJow3Tlceux9JzQlDMkBDsl1HPQmIS bOZA== X-Gm-Message-State: AD7BkJLcv2HAkJ5Ms7gJldmQA8t1C6i6+Be+OFmaLonq7mOOh+m53YiSGbtYW4DPc8NzLGqfFLFNgVAzlED0xN0q MIME-Version: 1.0 X-Received: by 10.140.233.135 with SMTP id e129mr8262644qhc.86.1459329067663; Wed, 30 Mar 2016 02:11:07 -0700 (PDT) Received: by 10.140.22.164 with HTTP; Wed, 30 Mar 2016 02:11:01 -0700 (PDT) In-Reply-To: <2632328.d9AUHOZn5J@e108577-lin> References: <004801d13d57$4a13b670$de3b2350$@foss.arm.com> <2632328.d9AUHOZn5J@e108577-lin> Date: Wed, 30 Mar 2016 09:11:00 -0000 Message-ID: Subject: Re: [RFC PATCH, binutils, ARM 2/11, ping] Factor our stub creation in ARM backend From: Christophe Lyon To: Thomas Preudhomme Cc: binutils@sourceware.org Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2016-03/txt/msg00414.txt.bz2 On 29 March 2016 at 16:33, Thomas Preudhomme wrote: > Ping? > > On Wednesday 23 December 2015 15:55:26 Thomas Preud'homme wrote: >> Hi, >> >> [Posting patch series as RFC] >> >> This patch is part of a patch series to add support for ARMv8-M security >> extension[1] to GNU ld. This specific patch factors out the code to crea= te >> a stub in ARM backend to avoid code duplication in subsequent patches >> adding new code for creating Secure Gateway veneers. >> >> >> [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 Sec= urity >> Extensions: Requirements on Development Tools >> >> ChangeLog entries is as follows: >> >> >> *** bfd/ChangeLog *** >> >> 2015-12-16 Thomas Preud'homme >> >> * elf32-arm.c (elf32_arm_create_stub): New function. >> (elf32_arm_size_stubs): Use elf32_arm_create_stub for stub creat= ion. >> >> >> diff --git a/bfd/elf32-arm.c b/bfd/elf32-arm.c >> index >> 639fe0f125688ccf218b449467c746686841386f..e6afb99512524ac26c0e3842e8dbe2= 93c >> 58bacad 100644 --- a/bfd/elf32-arm.c >> +++ b/bfd/elf32-arm.c >> @@ -5031,6 +5031,103 @@ cortex_a8_erratum_scan (bfd *input_bfd, >> return FALSE; >> } >> >> +/* Create or update a stub entry depending on whether the stub can alre= ady >> be + found in HTAB. The stub is identified by: >> + - its type STUB_TYPE >> + - its source branch (note that several can share the same stub) whose >> + section and relocation (if any) are given by SECTION and IRELA >> + respectively >> + - its target symbol whose input section, hash, name, value and branch >> type + are given in SYM_SEC, HASH, SYM_NAME, SYM_VALUE and BRANCH_TY= PE >> + 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 *secti= on, >> + Elf_Internal_Rela *irela, asection *sym_sec, >> + struct elf32_arm_link_hash_entry *hash, char *sym_n= ame, >> + bfd_vma sym_value, enum arm_st_branch_type branch_t= ype, >> + 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_nam= e, >> FALSE, + FALSE); >> + /* The proper stub has already been created, just update its value. = */ >> + 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_na= me); >> + 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_na= me); >> + else >> + sprintf (stub_entry->output_name, STUB_ENTRY_NAME, sym_name); >> + >> + *new_stub =3D TRUE; >> + return TRUE; >> +} >> + >> /* Determine and set the size of the stub section for a final link. >> >> The basic idea here is to examine all the relocations looking for >> @@ -5174,14 +5271,11 @@ elf32_arm_size_stubs (bfd *output_bfd, >> { >> unsigned int r_type, r_indx; >> enum elf32_arm_stub_type stub_type; >> - struct elf32_arm_stub_hash_entry *stub_entry; >> asection *sym_sec; >> bfd_vma sym_value; >> bfd_vma destination; >> struct elf32_arm_link_hash_entry *hash; >> const char *sym_name; >> - char *stub_name; >> - const asection *id_sec; >> unsigned char st_type; >> enum arm_st_branch_type branch_type; >> bfd_boolean created_stub =3D FALSE; >> @@ -5364,6 +5458,8 @@ elf32_arm_size_stubs (bfd *output_bfd, >> >> do >> { >> + bfd_boolean new_stub; >> + >> /* Determine what (if any) linker stub is needed. */ >> stub_type =3D arm_type_of_stub (info, section, irela, >> st_type, &branch_type, >> @@ -5372,74 +5468,21 @@ elf32_arm_size_stubs (bfd *output_bfd, >> if (stub_type =3D=3D arm_stub_none) >> break; >> >> - /* 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; >> - >> /* We've either created a stub for this reloc alread= y, >> or we are about to. */ >> - 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; >> - } >> - >> - stub_entry =3D elf32_arm_add_stub (stub_name, sectio= n, >> - 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, i= rela, >> + sym_sec, hash, >> + (char *) sym_name, sym_valu= e, >> + branch_type, &new_stub); >> >> - 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) I'm not sure to understand why you check both? >> { >> - free (stub_name); >> - goto error_ret_free_internal; >> + if (!created_stub) >> + goto error_ret_free_internal; >> } >> - >> - /* For historical reasons, use the existing names for >> - ARM-to-Thumb and Thumb-to-ARM stubs. */ >> - 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_JU= MP19) >> - && 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_JUMP2= 4) >> - && 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); >> - >> - stub_changed =3D TRUE; >> + stub_changed =3D TRUE; >> } >> while (0); >> >> >> The patch doesn't show any regression when running the binutils-gdb >> testsuite for the arm-none-eabi target. >> >> Any comments? >> >> Best regards, >> >> Thomas >