From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1324 invoked by alias); 31 Jul 2019 14:44:47 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 1316 invoked by uid 89); 31 Jul 2019 14:44:47 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-15.8 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=gun, pointing X-HELO: mail-lj1-f169.google.com Received: from mail-lj1-f169.google.com (HELO mail-lj1-f169.google.com) (209.85.208.169) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 31 Jul 2019 14:44:45 +0000 Received: by mail-lj1-f169.google.com with SMTP id y17so41316553ljk.10 for ; Wed, 31 Jul 2019 07:44:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=W7AJmDNGAVGcNB2JwfuhMNaZjrL20YVWKkoTOgKagvU=; b=AKpqsKj2Y6UquS1rqVlsGkaVN+tS14oWSNbpEMX55BOcgCnpq1rflKhtVOPUtfTYEf 19381wN0xBoeUzZn6aQSZc4ogSAEDqTcLOIq4lEW3+lFsu04COnEK3vRUppSMsXbhJuT sLWCx2d8DelZCUZhDCVky2MTm2Tn7uuPil4TyXR4ll9HLoU4iwjFHmIdCQlCu83MWH4r mq9P/C5Gy2xfkpljfc8YaRyoVgLDGZgA/b0J50tqjcSyJgSMCtE2XuyjyWQ/VpmSIkfk VFGSuKDB9dCbyX6jC9EhAVVB8a8exS4c5IHnyQPppA2A22RST0uUj1B7jR4uqCeafN2Q EEtw== MIME-Version: 1.0 References: <20190515124006.25840-1-christophe.lyon@st.com> <20190515124006.25840-10-christophe.lyon@st.com> <21a7e3bb-40ab-8b7e-1c66-b991231eb808@foss.arm.com> In-Reply-To: From: Christophe Lyon Date: Wed, 31 Jul 2019 14:48:00 -0000 Message-ID: Subject: Re: [ARM/FDPIC v5 09/21] [ARM] FDPIC: Add support for taking address of nested function To: Kyrill Tkachov Cc: Christophe Lyon , "gcc-patches@gcc.gnu.org" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2019-07/txt/msg01886.txt.bz2 On Tue, 16 Jul 2019 at 14:42, Kyrill Tkachov wrote: > > > On 7/16/19 12:18 PM, Kyrill Tkachov wrote: > > Hi Christophe > > > > On 5/15/19 1:39 PM, Christophe Lyon wrote: > > > In FDPIC mode, the trampoline generated to support pointers to nested > > > functions looks like: > > > > > > .word trampoline address > > > .word trampoline GOT address > > > ldr r12, [pc, #8] > > > ldr r9, [pc, #8] > > > ldr pc, [pc, #8] > > > .word static chain value > > > .word GOT address > > > .word function's address > > > > > > because in FDPIC function pointers are actually pointers to function > > > descriptors, we have to actually generate a function descriptor for > > > the trampoline. > > > > > > 2019-XX-XX Christophe Lyon > > > Micka=C3=ABl Gu=C3=AAn=C3=A9 > > > > > > gcc/ > > > * config/arm/arm.c (arm_asm_trampoline_template): Add FDPIC > > > support. > > > (arm_trampoline_init): Likewise. > > > (arm_trampoline_init): Likewise. > > > * config/arm/arm.h (TRAMPOLINE_SIZE): Likewise. > > > > > > Change-Id: Idc4d5f629ae4f8d79bdf9623517481d524a0c144 > > > > > > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > > > index 40e3f3b..99d13bf 100644 > > > --- a/gcc/config/arm/arm.c > > > +++ b/gcc/config/arm/arm.c > > > @@ -3976,13 +3976,50 @@ arm_warn_func_return (tree decl) > > > .word static chain value > > > .word function's address > > > XXX FIXME: When the trampoline returns, r8 will be clobbered. */ > > > +/* In FDPIC mode, the trampoline looks like: > > > + .word trampoline address > > > + .word trampoline GOT address > > > + ldr r12, [pc, #8] ; #4 for Thumb2 > > > + ldr r9, [pc, #8] ; #4 for Thumb2 > > > + ldr pc, [pc, #8] ; #4 for Thumb2 > > > + .word static chain value > > > + .word GOT address > > > + .word function's address > > > +*/ > > > > > > I think this comment is not right for Thumb2. > > > > These load instructionshave 32-bit encodings, even in Thumb2 (they use > > high registers). > > Andre and Wilco pointed out to me offline that the offset should be #4 > for Arm mode. > > The Arm ARM at E1.2.3 says: > > PC, the program counter > > * When executing an A32 instruction, PC reads as the address of the > current instruction plus 8. > > * When executing a T32 instruction, PC reads as the address of the > current instruction plus 4. > Yes, it looks like the code is right, and the comment is wrong: - offset 8 for thumb2 mode - offset 4 for arm mode Thanks, Christophe > Thanks, > > Kyrill > > > > > > Also, please merge this comment with the one above (no separate /**/) > > > > > > > > static void > > > arm_asm_trampoline_template (FILE *f) > > > { > > > fprintf (f, "\t.syntax unified\n"); > > > > > > - if (TARGET_ARM) > > > + if (TARGET_FDPIC) > > > + { > > > + /* The first two words are a function descriptor pointing to t= he > > > + trampoline code just below. */ > > > + if (TARGET_ARM) > > > + fprintf (f, "\t.arm\n"); > > > + else if (TARGET_THUMB2) > > > + fprintf (f, "\t.thumb\n"); > > > + else > > > + /* Only ARM and Thumb-2 are supported. */ > > > + gcc_unreachable (); > > > + > > > + assemble_aligned_integer (UNITS_PER_WORD, const0_rtx); > > > + assemble_aligned_integer (UNITS_PER_WORD, const0_rtx); > > > + /* Trampoline code which sets the static chain register but al= so > > > + PIC register before jumping into real code. */ > > > + asm_fprintf (f, "\tldr\t%r, [%r, #%d]\n", > > > + STATIC_CHAIN_REGNUM, PC_REGNUM, > > > + TARGET_THUMB2 ? 8 : 4); > > > + asm_fprintf (f, "\tldr\t%r, [%r, #%d]\n", > > > + PIC_OFFSET_TABLE_REGNUM, PC_REGNUM, > > > + TARGET_THUMB2 ? 8 : 4); > > > + asm_fprintf (f, "\tldr\t%r, [%r, #%d]\n", > > > + PC_REGNUM, PC_REGNUM, > > > + TARGET_THUMB2 ? 8 : 4); > > > > > > As above, I think the offset should be 8 for both Arm and Thumb2. > > > > Thanks, > > > > Kyrill > > > > > > > + assemble_aligned_integer (UNITS_PER_WORD, const0_rtx); > > > + } > > > + else if (TARGET_ARM) > > > { > > > fprintf (f, "\t.arm\n"); > > > asm_fprintf (f, "\tldr\t%r, [%r, #0]\n", STATIC_CHAIN_REGNUM, > > > PC_REGNUM); > > > @@ -4023,12 +4060,40 @@ arm_trampoline_init (rtx m_tramp, tree fndecl, > > > rtx chain_value) > > > emit_block_move (m_tramp, assemble_trampoline_template (), > > > GEN_INT (TRAMPOLINE_SIZE), BLOCK_OP_NORMAL); > > > > > > - mem =3D adjust_address (m_tramp, SImode, TARGET_32BIT ? 8 : 12); > > > - emit_move_insn (mem, chain_value); > > > + if (TARGET_FDPIC) > > > + { > > > + rtx funcdesc =3D XEXP (DECL_RTL (fndecl), 0); > > > + rtx fnaddr =3D gen_rtx_MEM (Pmode, funcdesc); > > > + rtx gotaddr =3D gen_rtx_MEM (Pmode, plus_constant (Pmode, > > > funcdesc, 4)); > > > + /* The function start address is at offset 8, but in Thumb mode > > > + we want bit 0 set to 1 to indicate Thumb-ness, hence 9 > > > + below. */ > > > + rtx trampoline_code_start > > > + =3D plus_constant (Pmode, XEXP (m_tramp, 0), TARGET_THUMB2 ? 9 > > : 8); > > > + > > > + /* Write initial funcdesc which points to the trampoline. */ > > > + mem =3D adjust_address (m_tramp, SImode, 0); > > > + emit_move_insn (mem, trampoline_code_start); > > > + mem =3D adjust_address (m_tramp, SImode, 4); > > > + emit_move_insn (mem, gen_rtx_REG (Pmode, > > PIC_OFFSET_TABLE_REGNUM)); > > > + /* Setup static chain. */ > > > + mem =3D adjust_address (m_tramp, SImode, 20); > > > + emit_move_insn (mem, chain_value); > > > + /* GOT + real function entry point. */ > > > + mem =3D adjust_address (m_tramp, SImode, 24); > > > + emit_move_insn (mem, gotaddr); > > > + mem =3D adjust_address (m_tramp, SImode, 28); > > > + emit_move_insn (mem, fnaddr); > > > + } > > > + else > > > + { > > > + mem =3D adjust_address (m_tramp, SImode, TARGET_32BIT ? 8 : 12= ); > > > + emit_move_insn (mem, chain_value); > > > > > > - mem =3D adjust_address (m_tramp, SImode, TARGET_32BIT ? 12 : 16); > > > - fnaddr =3D XEXP (DECL_RTL (fndecl), 0); > > > - emit_move_insn (mem, fnaddr); > > > + mem =3D adjust_address (m_tramp, SImode, TARGET_32BIT ? 12 : 1= 6); > > > + fnaddr =3D XEXP (DECL_RTL (fndecl), 0); > > > + emit_move_insn (mem, fnaddr); > > > + } > > > > > > a_tramp =3D XEXP (m_tramp, 0); > > > emit_library_call (gen_rtx_SYMBOL_REF (Pmode, "__clear_cache"), > > > @@ -4042,7 +4107,9 @@ arm_trampoline_init (rtx m_tramp, tree fndecl, > > > rtx chain_value) > > > static rtx > > > arm_trampoline_adjust_address (rtx addr) > > > { > > > - if (TARGET_THUMB) > > > + /* For FDPIC don't fix trampoline address since it's a function > > > + descriptor and not a function address. */ > > > + if (TARGET_THUMB && !TARGET_FDPIC) > > > addr =3D expand_simple_binop (Pmode, IOR, addr, const1_rtx, > > > NULL, 0, OPTAB_LIB_WIDEN); > > > return addr; > > > diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h > > > index 45c0e2b..f80df63 100644 > > > --- a/gcc/config/arm/arm.h > > > +++ b/gcc/config/arm/arm.h > > > @@ -1602,7 +1602,7 @@ typedef struct > > > #define INIT_EXPANDERS arm_init_expanders () > > > > > > /* Length in units of the trampoline for entering a nested > > function. */ > > > -#define TRAMPOLINE_SIZE (TARGET_32BIT ? 16 : 20) > > > +#define TRAMPOLINE_SIZE (TARGET_FDPIC ? 32 : (TARGET_32BIT ? 16 : 2= 0)) > > > > > > /* Alignment required for a trampoline in bits. */ > > > #define TRAMPOLINE_ALIGNMENT 32 > > > -- > > > 2.6.3 > > >