From: Christophe Lyon <christophe.lyon@linaro.org>
To: Richard Earnshaw <Richard.Earnshaw@arm.com>
Cc: christophe lyon St <christophe.lyon@st.com>,
gcc Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [ARM/FDPIC v3 04/21] [ARM] FDPIC: Add support for FDPIC for arm architecture
Date: Mon, 29 Oct 2018 10:31:00 -0000 [thread overview]
Message-ID: <CAKdteOYox66tEdR-eNKtjHz01-3PKXh_Xuh17tPPEk5Hx-zCKg@mail.gmail.com> (raw)
In-Reply-To: <eb0db64d-842e-2f88-d0ef-a6bbab6ad2fe@arm.com>
On Fri, 26 Oct 2018 at 17:42, Richard Earnshaw (lists)
<Richard.Earnshaw@arm.com> wrote:
>
> On 26/10/2018 16:25, Christophe Lyon wrote:
> > On Tue, 23 Oct 2018 at 16:07, Richard Earnshaw (lists)
> > <Richard.Earnshaw@arm.com> wrote:
> >>
> >> On 19/10/2018 14:40, Christophe Lyon wrote:
> >>> On 12/10/2018 12:45, Richard Earnshaw (lists) wrote:
> >>>> On 11/10/18 14:34, Christophe Lyon wrote:
> >>>>> The FDPIC register is hard-coded to r9, as defined in the ABI.
> >>>>>
> >>>>> We have to disable tailcall optimizations if we don't know if the
> >>>>> target function is in the same module. If not, we have to set r9 to
> >>>>> the value associated with the target module.
> >>>>>
> >>>>> When generating a symbol address, we have to take into account whether
> >>>>> it is a pointer to data or to a function, because different
> >>>>> relocations are needed.
> >>>>>
> >>>>> 2018-XX-XX Christophe Lyon <christophe.lyon@st.com>
> >>>>> Mickaël Guêné <mickael.guene@st.com>
> >>>>>
> >>>>> * config/arm/arm-c.c (__FDPIC__): Define new pre-processor macro
> >>>>> in FDPIC mode.
> >>>>> * config/arm/arm-protos.h (arm_load_function_descriptor): Declare
> >>>>> new function.
> >>>>> * config/arm/arm.c (arm_option_override): Define pic register to
> >>>>> FDPIC_REGNUM.
> >>>>> (arm_function_ok_for_sibcall) Disable sibcall optimization if we
> >>>>
> >>>> Missing colon.
> >>>>
> >>>>> have no decl or go through PLT.
> >>>>> (arm_load_pic_register): Handle TARGET_FDPIC.
> >>>>> (arm_is_segment_info_known): New function.
> >>>>> (arm_pic_static_addr): Add support for FDPIC.
> >>>>> (arm_load_function_descriptor): New function.
> >>>>> (arm_assemble_integer): Add support for FDPIC.
> >>>>> * config/arm/arm.h (PIC_OFFSET_TABLE_REG_CALL_CLOBBERED):
> >>>>> Define. (FDPIC_REGNUM): New define.
> >>>>> * config/arm/arm.md (call): Add support for FDPIC.
> >>>>> (call_value): Likewise.
> >>>>> (*restore_pic_register_after_call): New pattern.
> >>>>> (untyped_call): Disable if FDPIC.
> >>>>> (untyped_return): Likewise.
> >>>>> * config/arm/unspecs.md (UNSPEC_PIC_RESTORE): New.
> >>>>>
> >>>>
> >>>> Other comments inline.
> >>>>
> >>>>> diff --git a/gcc/config/arm/arm-c.c b/gcc/config/arm/arm-c.c
> >>>>> index 4471f79..90733cc 100644
> >>>>> --- a/gcc/config/arm/arm-c.c
> >>>>> +++ b/gcc/config/arm/arm-c.c
> >>>>> @@ -202,6 +202,8 @@ arm_cpu_builtins (struct cpp_reader* pfile)
> >>>>> builtin_define ("__ARM_EABI__");
> >>>>> }
> >>>>> + def_or_undef_macro (pfile, "__FDPIC__", TARGET_FDPIC);
> >>>>> +
> >>>>> def_or_undef_macro (pfile, "__ARM_ARCH_EXT_IDIV__", TARGET_IDIV);
> >>>>> def_or_undef_macro (pfile, "__ARM_FEATURE_IDIV", TARGET_IDIV);
> >>>>> diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h
> >>>>> index 0dfb3ac..28cafa8 100644
> >>>>> --- a/gcc/config/arm/arm-protos.h
> >>>>> +++ b/gcc/config/arm/arm-protos.h
> >>>>> @@ -136,6 +136,7 @@ extern int arm_max_const_double_inline_cost (void);
> >>>>> extern int arm_const_double_inline_cost (rtx);
> >>>>> extern bool arm_const_double_by_parts (rtx);
> >>>>> extern bool arm_const_double_by_immediates (rtx);
> >>>>> +extern rtx arm_load_function_descriptor (rtx funcdesc);
> >>>>> extern void arm_emit_call_insn (rtx, rtx, bool);
> >>>>> bool detect_cmse_nonsecure_call (tree);
> >>>>> extern const char *output_call (rtx *);
> >>>>> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> >>>>> index 8810df5..92ae24b 100644
> >>>>> --- a/gcc/config/arm/arm.c
> >>>>> +++ b/gcc/config/arm/arm.c
> >>>>> @@ -3470,6 +3470,14 @@ arm_option_override (void)
> >>>>> if (flag_pic && TARGET_VXWORKS_RTP)
> >>>>> arm_pic_register = 9;
> >>>>> + /* If in FDPIC mode then force arm_pic_register to be r9. */
> >>>>> + if (TARGET_FDPIC)
> >>>>> + {
> >>>>> + arm_pic_register = FDPIC_REGNUM;
> >>>>> + if (TARGET_ARM_ARCH < 7)
> >>>>> + error ("FDPIC mode is not supported on architectures older than
> >>>>> Armv7");
> >>>>
> >>>> What properties of FDPIC impose this requirement? Does it also apply to
> >>>> Armv8-m.baseline?
> >>>>
> >>> In fact, there was miscommunication on my side, resulting in a
> >>> misunderstanding between Kyrill and myself, which I badly translated
> >>> into this condition.
> >>>
> >>> My initial plan was to submit a patch series tested on v7, and send the
> >>> patches needed to support older architectures as a follow-up. The proper
> >>> restriction is actually "CPUs that do not support ARM or Thumb2". As you
> >>> may have noticed during the iterations of this patch series, I had
> >>> failed to remove partial Thumb1 support hunks.
> >>>
> >>> So really this should be rephrased, and rewritten as "FDPIC mode is
> >>> supported on architecture versions that support ARM or Thumb-2", if that
> >>> suits you. And the condition should thus be:
> >>> if (! TARGET_ARM && ! TARGET_THUMB2)
> >>> error ("...")
> >>>
> >>> This would also exclude Armv8-m.baseline, since it doesn't support Thumb2.
> >>
> >> When we get to v8-m.baseline the thumb1/2 distinction starts to become a
> >> lot more blurred. A lot of thumb2 features needed for stand-alone
> >> systems are then available. So what feature is it that you require in
> >> order to make fdpic work in (traditional) thumb2 that isn't in
> >> (traditional) thumb1?
> >>
> > At the moment I'm not sure about what feature is missing. It's rather
> > that we haven't made it work it although there were preliminary attempts.
> >
> > Since building GCC --with-cpu=cortex-m{0,23} --target arm-linux-gnueabi
> > currently fails, I tried using a fdpic toolchain built --with-cpu=cortex-m4,
> > forcing -mcpu=cortex-m{0,23} while building uClibc-ng. I noticed two kinds
> > of failures:
> > - parts of assembly files do not support Thumb-1, so they need porting at least
> > (ldso/ldso/arm/dl-startup.h)
> > - ICEs for lack of .md patterns (cortex-m4 uses pic_load_addr_32bit which is
> > missing for m{0,23}
> >
> > There are probably other problems that would be discovered at runtime.
> >
> > So it can probably be made to work, but I think that would be an enhancement
> > for later (not sure there's a real need: can we reasonably think about
> > running Linux on such small cores?)
>
> So would a sorry() call be more appropriate?
>
Yes, you are right. I'll do that.
> R.
>
> >
> >>
> >>
> >>> As a side note, I tried to build GCC master (without my patches)
> >>> --with-cpu=cortex-m23, and both targets arm-eabi and arm-linux-gnueabi
> >>> failed to buid.
> >>>
> >>> For arm-eabi, there are problems in newlib:
> >>> newlib/libc/sys/arm/crt0.S:145: Error: lo register required -- `add
> >>> sl,r2,#256'
> >>> newlib/libc/sys/arm/trap.S:88: Error: lo register required -- `sub
> >>> ip,sp,ip'
> >>>
> >>
> >> These all sound like basic CPU detection issues in newlib and need to be
> >> fixed at some point (it's probably still using some pre-ACLE macros to
> >> detect system capabilities).
> >>
> >> R.
> >>
> >>> For arm-linux-gnueabi, the failure happens while building libgcc:
> >>> /home/christophe.lyon/src/GCC/sources/newlib/newlib/libc/machine/arm/setjmp.S:169:
> >>> Error: selected processor does not support ARM opcodes
> >>> /newlib/newlib/libc/machine/arm/setjmp.S:176: Error: attempt to use an
> >>> ARM instruction on a Thumb-only processor -- `stmea a1!,{
> >>> v1-v7,fp,ip,sp,lr }'
> >>> /newlib/newlib/libc/machine/arm/setjmp.S:186: Error: attempt to use an
> >>> ARM instruction on a Thumb-only processor -- `mov a1,#0'
> >>> /newlib/newlib/libc/machine/arm/setjmp.S:188: Error: attempt to use an
> >>> ARM instruction on a Thumb-only processor -- `tst lr,#1'
> >>> /newlib/newlib/libc/machine/arm/setjmp.S:188: Error: attempt to use an
> >>> ARM instruction on a Thumb-only processor -- `moveq pc,lr'
> >>> /newlib/newlib/libc/machine/arm/setjmp.S:194: Error: selected processor
> >>> does not support ARM opcodes
> >>> /newlib/newlib/libc/machine/arm/setjmp.S:203: Error: attempt to use an
> >>> ARM instruction on a Thumb-only processor -- `ldmfd a1!,{
> >>> v1-v7,fp,ip,sp,lr }'
> >>> /newlib/newlib/libc/machine/arm/setjmp.S:214: Error: attempt to use an
> >>> ARM instruction on a Thumb-only processor -- `movs a1,a2'
> >>> /newlib/newlib/libc/machine/arm/setjmp.S:218: Error: attempt to use an
> >>> ARM instruction on a Thumb-only processor -- `moveq a1,#1'
> >>> /newlib/newlib/libc/machine/arm/setjmp.S:220: Error: attempt to use an
> >>> ARM instruction on a Thumb-only processor -- `tst lr,#1'
> >>> /newlib/newlib/libc/machine/arm/setjmp.S:220: Error: attempt to use an
> >>> ARM instruction on a Thumb-only processor -- `moveq pc,lr'
> >>>
> >>>
> >>>>> + }
> >>>>> +
> >>>>> if (arm_pic_register_string != NULL)
> >>>>> {
> >>>>> int pic_register = decode_reg_name (arm_pic_register_string);
> >>>>> @@ -7251,6 +7259,21 @@ arm_function_ok_for_sibcall (tree decl, tree exp)
> >>>>> if (cfun->machine->sibcall_blocked)
> >>>>> return false;
> >>>>> + if (TARGET_FDPIC)
> >>>>> + {
> >>>>> + /* In FDPIC, never tailcall something for which we have no decl:
> >>>>> + the target function could be in a different module, requiring
> >>>>> + a different FDPIC register value. */
> >>>>> + if (decl == NULL)
> >>>>> + return false;
> >>>>> +
> >>>>> + /* Don't tailcall if we go through the PLT since the FDPIC
> >>>>> + register is then corrupted and we don't restore it after
> >>>>> + static function calls. */
> >>>>> + if (!targetm.binds_local_p (decl))
> >>>>> + return false;
> >>>>> + }
> >>>>> +
> >>>>> /* Never tailcall something if we are generating code for
> >>>>> Thumb-1. */
> >>>>> if (TARGET_THUMB1)
> >>>>> return false;
> >>>>> @@ -7629,7 +7652,9 @@ arm_load_pic_register (unsigned long saved_regs
> >>>>> ATTRIBUTE_UNUSED)
> >>>>> {
> >>>>> rtx l1, labelno, pic_tmp, pic_rtx, pic_reg;
> >>>>> - if (crtl->uses_pic_offset_table == 0 || TARGET_SINGLE_PIC_BASE)
> >>>>> + if (crtl->uses_pic_offset_table == 0
> >>>>> + || TARGET_SINGLE_PIC_BASE
> >>>>> + || TARGET_FDPIC)
> >>>>> return;
> >>>>> gcc_assert (flag_pic);
> >>>>> @@ -7697,28 +7722,140 @@ arm_load_pic_register (unsigned long
> >>>>> saved_regs ATTRIBUTE_UNUSED)
> >>>>> emit_use (pic_reg);
> >>>>> }
> >>>>> +/* Try to know if the object will go in text or data segment. This is
> >>>>
> >>>> "Try to determine whether an object, referenced via ORIG, will be placed
> >>>> in the text or data segment."
> >>>>> + used in FDPIC mode, to decide which relocations to use when
> >>>>> + accessing ORIG. IS_READONLY is set to true if ORIG is a read-only
> >>>>
> >>>> Two spaces after a period.
> >>>>
> >>>>> + location, false otherwise. */
> >>>>
> >>>> You've missed the documentation of the return value: does returning true
> >>>> mean text vs data, or does it mean we know which it will go in, but
> >>>> don't have to return that information here.
> >>>>
> >>>> Generally, won't this break big time if users compile with
> >>>> -ffunction-sections or -fdata-sections? Is it sufficient to match
> >>>> .text.* as being text and .data.* for data?
> >>>>
> >>>
> >>> I compiled a small testcase with -ffunction-sections and -fdata-sections
> >>> and noticed no problem.
> >>> The code below does not attempt to match section names, I'm not sure to
> >>> understand your question?
> >>>
> >>>>
> >>>>> +static bool
> >>>>> +arm_is_segment_info_known (rtx orig, bool *is_readonly)
> >>>>> +{
> >>>>> + bool res = false;
> >>>>> +
> >>>>> + *is_readonly = false;
> >>>>> +
> >>>>> + if (GET_CODE (orig) == LABEL_REF)
> >>>>> + {
> >>>>> + res = true;
> >>>>> + *is_readonly = true;
> >>>>> + }
> >>>>> + else if (SYMBOL_REF_P (orig))
> >>>>> + {
> >>>>> + if (CONSTANT_POOL_ADDRESS_P (orig))
> >>>>> + {
> >>>>> + res = true;
> >>>>> + *is_readonly = true;
> >>>>> + }
> >>>>> + else if (SYMBOL_REF_LOCAL_P (orig)
> >>>>> + && !SYMBOL_REF_EXTERNAL_P (orig)
> >>>>> + && SYMBOL_REF_DECL (orig)
> >>>>> + && (!DECL_P (SYMBOL_REF_DECL (orig))
> >>>>> + || !DECL_COMMON (SYMBOL_REF_DECL (orig))))
> >>>>> + {
> >>>>> + tree decl = SYMBOL_REF_DECL (orig);
> >>>>> + tree init = (TREE_CODE (decl) == VAR_DECL)
> >>>>> + ? DECL_INITIAL (decl) : (TREE_CODE (decl) == CONSTRUCTOR)
> >>>>> + ? decl : 0;
> >>>>> + int reloc = 0;
> >>>>> + bool named_section, readonly;
> >>>>> +
> >>>>> + if (init && init != error_mark_node)
> >>>>> + reloc = compute_reloc_for_constant (init);
> >>>>> +
> >>>>> + named_section = TREE_CODE (decl) == VAR_DECL
> >>>>> + && lookup_attribute ("section", DECL_ATTRIBUTES (decl));
> >>>>> + readonly = decl_readonly_section (decl, reloc);
> >>>>> +
> >>>>> + /* We don't know where the link script will put a named
> >>>>> + section, so return false in such a case. */
> >>>>> + res = !named_section;
> >>>>> +
> >>>>> + if (!named_section)
> >>>>> + *is_readonly = readonly;
> >>>>> + }
> >>>>> + else
> >>>>> + {
> >>>>> + /* We don't know. */
> >>>>> + res = false;
> >>>>> + }
> >>>>> + }
> >>>>> + else
> >>>>> + gcc_unreachable ();
> >>>>> +
> >>>>> + return res;
> >>>>> +}
> >>>>> +
> >>>>> /* Generate code to load the address of a static var when flag_pic
> >>>>> is set. */
> >>>>> static rtx_insn *
> >>>>> arm_pic_static_addr (rtx orig, rtx reg)
> >>>>> {
> >>>>> rtx l1, labelno, offset_rtx;
> >>>>> + rtx_insn *insn;
> >>>>> gcc_assert (flag_pic);
> >>>>> - /* We use an UNSPEC rather than a LABEL_REF because this label
> >>>>> - never appears in the code stream. */
> >>>>> - labelno = GEN_INT (pic_labelno++);
> >>>>> - l1 = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, labelno),
> >>>>> UNSPEC_PIC_LABEL);
> >>>>> - l1 = gen_rtx_CONST (VOIDmode, l1);
> >>>>> + bool is_readonly = false;
> >>>>> + bool info_known = false;
> >>>>> - /* On the ARM the PC register contains 'dot + 8' at the time of the
> >>>>> - addition, on the Thumb it is 'dot + 4'. */
> >>>>> - offset_rtx = plus_constant (Pmode, l1, TARGET_ARM ? 8 : 4);
> >>>>> - offset_rtx = gen_rtx_UNSPEC (Pmode, gen_rtvec (2, orig, offset_rtx),
> >>>>> - UNSPEC_SYMBOL_OFFSET);
> >>>>> - offset_rtx = gen_rtx_CONST (Pmode, offset_rtx);
> >>>>> + if (TARGET_FDPIC
> >>>>> + && SYMBOL_REF_P (orig)
> >>>>> + && !SYMBOL_REF_FUNCTION_P (orig))
> >>>>> + info_known = arm_is_segment_info_known (orig, &is_readonly);
> >>>>> - return emit_insn (gen_pic_load_addr_unified (reg, offset_rtx,
> >>>>> labelno));
> >>>>> + if (TARGET_FDPIC
> >>>>> + && SYMBOL_REF_P (orig)
> >>>>> + && !SYMBOL_REF_FUNCTION_P (orig)
> >>>>> + && !info_known)
> >>>>> + {
> >>>>> + /* We don't know where orig is stored, so we have be
> >>>>> + pessimistic and use a GOT relocation. */
> >>>>> + rtx pat;
> >>>>> + rtx mem;
> >>>>> + rtx pic_reg = gen_rtx_REG (Pmode, FDPIC_REGNUM);
> >>>>> +
> >>>>> + pat = gen_calculate_pic_address (reg, pic_reg, orig);
> >>>>> +
> >>>>> + /* Make the MEM as close to a constant as possible. */
> >>>>> + mem = SET_SRC (pat);
> >>>>> + gcc_assert (MEM_P (mem) && !MEM_VOLATILE_P (mem));
> >>>>> + MEM_READONLY_P (mem) = 1;
> >>>>> + MEM_NOTRAP_P (mem) = 1;
> >>>>> +
> >>>>> + insn = emit_insn (pat);
> >>>>> + }
> >>>>> + else if (TARGET_FDPIC
> >>>>> + && SYMBOL_REF_P (orig)
> >>>>> + && (SYMBOL_REF_FUNCTION_P (orig)
> >>>>> + || (info_known && !is_readonly)))
> >>>>> + {
> >>>>> + /* We use the GOTOFF relocation. */
> >>>>> + rtx pic_reg = gen_rtx_REG (Pmode, FDPIC_REGNUM);
> >>>>> +
> >>>>> + rtx l1 = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, orig),
> >>>>> UNSPEC_PIC_SYM);
> >>>>> + emit_insn (gen_movsi (reg, l1));
> >>>>> + insn = emit_insn (gen_addsi3 (reg, reg, pic_reg));
> >>>>> + }
> >>>>> + else
> >>>>> + {
> >>>>> + /* Not FDPIC, not SYMBOL_REF_P or readonly: we can use
> >>>>> + PC-relative access. */
> >>>>> + /* We use an UNSPEC rather than a LABEL_REF because this label
> >>>>> + never appears in the code stream. */
> >>>>> + labelno = GEN_INT (pic_labelno++);
> >>>>> + l1 = gen_rtx_UNSPEC (Pmode, gen_rtvec (1, labelno),
> >>>>> UNSPEC_PIC_LABEL);
> >>>>> + l1 = gen_rtx_CONST (VOIDmode, l1);
> >>>>> +
> >>>>> + /* On the ARM the PC register contains 'dot + 8' at the time
> >>>>> of the
> >>>>> + addition, on the Thumb it is 'dot + 4'. */
> >>>>> + offset_rtx = plus_constant (Pmode, l1, TARGET_ARM ? 8 : 4);
> >>>>> + offset_rtx = gen_rtx_UNSPEC (Pmode, gen_rtvec (2, orig,
> >>>>> offset_rtx),
> >>>>> + UNSPEC_SYMBOL_OFFSET);
> >>>>> + offset_rtx = gen_rtx_CONST (Pmode, offset_rtx);
> >>>>> +
> >>>>> + insn = emit_insn (gen_pic_load_addr_unified (reg, offset_rtx,
> >>>>> + labelno));
> >>>>> + }
> >>>>> +
> >>>>> + return insn;
> >>>>> }
> >>>>> /* Return nonzero if X is valid as an ARM state addressing
> >>>>> register. */
> >>>>> @@ -15933,9 +16070,36 @@ get_jump_table_size (rtx_jump_table_data *insn)
> >>>>> return 0;
> >>>>> }
> >>>>> +/* Emit insns to load the function address from FUNCDESC (an FDPIC
> >>>>> + function descriptor) into a register and the GOT address into the
> >>>>> + FDPIC register, returning an rtx for the register holding the
> >>>>> + function address. */
> >>>>> +
> >>>>> +rtx
> >>>>> +arm_load_function_descriptor (rtx funcdesc)
> >>>>> +{
> >>>>> + rtx fnaddr_reg = gen_reg_rtx (Pmode);
> >>>>> + rtx pic_reg = gen_rtx_REG (Pmode, FDPIC_REGNUM);
> >>>>> + rtx fnaddr = gen_rtx_MEM (Pmode, funcdesc);
> >>>>> + rtx gotaddr = gen_rtx_MEM (Pmode, plus_constant (Pmode, funcdesc,
> >>>>> 4));
> >>>>> + rtx par = gen_rtx_PARALLEL (VOIDmode, rtvec_alloc (3));
> >>>>> +
> >>>>> + emit_move_insn (fnaddr_reg, fnaddr);
> >>>>> + /* The ABI requires the entry point address to be loaded first, so
> >>>>> + prevent the load from being moved after that of the GOT
> >>>>> + address. */
> >>>>> + XVECEXP (par, 0, 0) = gen_rtx_UNSPEC (VOIDmode,
> >>>>> + gen_rtvec (2, pic_reg, gotaddr),
> >>>>> + UNSPEC_PIC_RESTORE);
> >>>>> + XVECEXP (par, 0, 1) = gen_rtx_USE (VOIDmode, gen_rtx_REG (Pmode,
> >>>>> FDPIC_REGNUM))
> >>>>> + XVECEXP (par, 0, 2) = gen_rtx_CLOBBER (VOIDmode, gen_rtx_REG
> >>>>> (Pmode, FDPIC_REGNUM));
> >>>>
> >>>> Shouldn't one of these be fnaddr_reg and the other pic_reg?
> >>> I think the USE should be gotaddr, and CLOBBER should be pic_reg, thanks.
> >>>
> >>>>
> >>>>> + emit_insn (par);
> >>>>> +
> >>>>> + return fnaddr_reg;
> >>>>> +}
> >>>>> +
> >>>>> /* Return the maximum amount of padding that will be inserted before
> >>>>> label LABEL. */
> >>>>> -
> >>>>> static HOST_WIDE_INT
> >>>>> get_label_padding (rtx label)
> >>>>> {
> >>>>> @@ -22890,9 +23054,37 @@ arm_assemble_integer (rtx x, unsigned int
> >>>>> size, int aligned_p)
> >>>>> && (!SYMBOL_REF_LOCAL_P (x)
> >>>>> || (SYMBOL_REF_DECL (x)
> >>>>> ? DECL_WEAK (SYMBOL_REF_DECL (x)) : 0))))
> >>>>> - fputs ("(GOT)", asm_out_file);
> >>>>> + {
> >>>>> + if (TARGET_FDPIC && SYMBOL_REF_FUNCTION_P (x))
> >>>>> + fputs ("(GOTFUNCDESC)", asm_out_file);
> >>>>> + else
> >>>>> + fputs ("(GOT)", asm_out_file);
> >>>>> + }
> >>>>> else
> >>>>> - fputs ("(GOTOFF)", asm_out_file);
> >>>>> + {
> >>>>> + if (TARGET_FDPIC && SYMBOL_REF_FUNCTION_P (x))
> >>>>> + fputs ("(GOTOFFFUNCDESC)", asm_out_file);
> >>>>> + else
> >>>>> + {
> >>>>> + bool is_readonly;
> >>>>> +
> >>>>> + if (arm_is_segment_info_known (x, &is_readonly))
> >>>>> + fputs ("(GOTOFF)", asm_out_file);
> >>>>> + else
> >>>>> + fputs ("(GOT)", asm_out_file);
> >>>>> + }
> >>>>> + }
> >>>>> + }
> >>>>> +
> >>>>> + /* For FDPIC we also have to mark symbol for .data section. */
> >>>>> + if (TARGET_FDPIC
> >>>>> + && NEED_GOT_RELOC
> >>>>> + && flag_pic
> >>>>> + && !making_const_table
> >>>>> + && SYMBOL_REF_P (x))
> >>>>> + {
> >>>>> + if (SYMBOL_REF_FUNCTION_P (x))
> >>>>> + fputs ("(FUNCDESC)", asm_out_file);
> >>>>> }
> >>>>> fputc ('\n', asm_out_file);
> >>>>> return true;
> >>>>> diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
> >>>>> index 34894c0..4671d64 100644
> >>>>> --- a/gcc/config/arm/arm.h
> >>>>> +++ b/gcc/config/arm/arm.h
> >>>>> @@ -871,6 +871,9 @@ extern int arm_arch_cmse;
> >>>>> Pascal), so the following is not true. */
> >>>>> #define STATIC_CHAIN_REGNUM 12
> >>>>> +/* r9 is the FDPIC register (base register for GOT and FUNCDESC
> >>>>> accesses). */
> >>>>> +#define FDPIC_REGNUM 9
> >>>>> +
> >>>>> /* Define this to be where the real frame pointer is if it is not
> >>>>> possible to
> >>>>> work out the offset between the frame pointer and the automatic
> >>>>> variables
> >>>>> until after register allocation has taken place.
> >>>>> FRAME_POINTER_REGNUM
> >>>>> @@ -1927,6 +1930,10 @@ extern unsigned arm_pic_register;
> >>>>> data addresses in memory. */
> >>>>> #define PIC_OFFSET_TABLE_REGNUM arm_pic_register
> >>>>> +/* For FDPIC, the FDPIC register is call-clobbered (otherwise PLT
> >>>>> + entries would need to handle saving and restoring it). */
> >>>>> +#define PIC_OFFSET_TABLE_REG_CALL_CLOBBERED TARGET_FDPIC
> >>>>> +
> >>>>> /* We can't directly access anything that contains a symbol,
> >>>>> nor can we indirect via the constant pool. One exception is
> >>>>> UNSPEC_TLS, which is always PIC. */
> >>>>> diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
> >>>>> index 270b8e4..09a0701 100644
> >>>>> --- a/gcc/config/arm/arm.md
> >>>>> +++ b/gcc/config/arm/arm.md
> >>>>> @@ -8031,6 +8031,23 @@
> >>>>> rtx callee, pat;
> >>>>> tree addr = MEM_EXPR (operands[0]);
> >>>>> + /* Force FDPIC register (r9) before call. */
> >>>>> + if (TARGET_FDPIC)
> >>>>> + {
> >>>>> + /* No need to update r9 if calling a static function.
> >>>>> + In other words: set r9 for indirect or non-local calls. */
> >>>>> + callee = XEXP (operands[0], 0);
> >>>>> + if (!SYMBOL_REF_P (callee)
> >>>>> + || !SYMBOL_REF_LOCAL_P (callee)
> >>>>> + || arm_is_long_call_p (SYMBOL_REF_DECL (callee)))
> >>>>> + {
> >>>>> + emit_insn (gen_blockage ());
> >>>>> + rtx pic_reg = gen_rtx_REG (Pmode, FDPIC_REGNUM);
> >>>>> + emit_move_insn (pic_reg, get_hard_reg_initial_val (Pmode,
> >>>>> FDPIC_REGNUM));
> >>>>> + emit_insn (gen_rtx_USE (VOIDmode, pic_reg));
> >>>>> + }
> >>>>> + }
> >>>>> +
> >>>>> /* In an untyped call, we can get NULL for operand 2. */
> >>>>> if (operands[2] == NULL_RTX)
> >>>>> operands[2] = const0_rtx;
> >>>>> @@ -8044,6 +8061,13 @@
> >>>>> : !REG_P (callee))
> >>>>> XEXP (operands[0], 0) = force_reg (Pmode, callee);
> >>>>> + if (TARGET_FDPIC && !SYMBOL_REF_P (XEXP (operands[0], 0)))
> >>>>> + {
> >>>>> + /* Indirect call: set r9 with FDPIC value of callee. */
> >>>>> + XEXP (operands[0], 0)
> >>>>> + = arm_load_function_descriptor (XEXP (operands[0], 0));
> >>>>> + }
> >>>>> +
> >>>>> if (detect_cmse_nonsecure_call (addr))
> >>>>> {
> >>>>> pat = gen_nonsecure_call_internal (operands[0], operands[1],
> >>>>> @@ -8055,10 +8079,38 @@
> >>>>> pat = gen_call_internal (operands[0], operands[1], operands[2]);
> >>>>> arm_emit_call_insn (pat, XEXP (operands[0], 0), false);
> >>>>> }
> >>>>> +
> >>>>> + /* Restore FDPIC register (r9) after call. */
> >>>>> + if (TARGET_FDPIC)
> >>>>> + {
> >>>>> + /* No need to update r9 if calling a static function. */
> >>>>> + if (!SYMBOL_REF_P (callee)
> >>>>> + || !SYMBOL_REF_LOCAL_P (callee)
> >>>>> + || arm_is_long_call_p (SYMBOL_REF_DECL (callee)))
> >>>>> + {
> >>>>> + rtx pic_reg = gen_rtx_REG (Pmode, FDPIC_REGNUM);
> >>>>> + emit_move_insn (pic_reg, get_hard_reg_initial_val (Pmode,
> >>>>> FDPIC_REGNUM));
> >>>>> + emit_insn (gen_rtx_USE (VOIDmode, pic_reg));
> >>>>> + emit_insn (gen_blockage ());
> >>>>> + }
> >>>>> + }
> >>>>> DONE;
> >>>>> }"
> >>>>> )
> >>>>> +(define_insn "*restore_pic_register_after_call"
> >>>>> + [(parallel [(unspec [(match_operand:SI 0 "s_register_operand" "=r,r")
> >>>>> + (match_operand:SI 1 "nonimmediate_operand" "r,m")]
> >>>>> + UNSPEC_PIC_RESTORE)
> >>>>> + (use (match_dup 0))
> >>>>> + (clobber (match_dup 0))])
> >>>>> + ]
> >>>>> + ""
> >>>>> + "@
> >>>>> + mov\t%0, %1
> >>>>> + ldr\t%0, %1"
> >>>>> +)
> >>>>> +
> >>>>> (define_expand "call_internal"
> >>>>> [(parallel [(call (match_operand 0 "memory_operand" "")
> >>>>> (match_operand 1 "general_operand" ""))
> >>>>> @@ -8119,6 +8171,30 @@
> >>>>> rtx pat, callee;
> >>>>> tree addr = MEM_EXPR (operands[1]);
> >>>>> + /* Force FDPIC register (r9) before call. */
> >>>>> + if (TARGET_FDPIC)
> >>>>> + {
> >>>>> + /* No need to update the FDPIC register (r9) if calling a static
> >>>>> function.
> >>>>> + In other words: set r9 for indirect or non-local calls. */
> >>>>> + callee = XEXP (operands[1], 0);
> >>>>> + if (!SYMBOL_REF_P (callee)
> >>>>> + || !SYMBOL_REF_LOCAL_P (callee)
> >>>>> + || arm_is_long_call_p (SYMBOL_REF_DECL (callee)))
> >>>>> + {
> >>>>> + rtx par = gen_rtx_PARALLEL (VOIDmode, rtvec_alloc (3));
> >>>>> +
> >>>>> + XVECEXP (par, 0, 0) = gen_rtx_UNSPEC (VOIDmode,
> >>>>> + gen_rtvec (2, gen_rtx_REG (Pmode, FDPIC_REGNUM),
> >>>>> + get_hard_reg_initial_val (Pmode, FDPIC_REGNUM)),
> >>>>> + UNSPEC_PIC_RESTORE);
> >>>>> + XVECEXP (par, 0, 1)
> >>>>> + = gen_rtx_USE (VOIDmode, gen_rtx_REG (Pmode, FDPIC_REGNUM));
> >>>>> + XVECEXP (par, 0, 2)
> >>>>> + = gen_rtx_CLOBBER (VOIDmode, gen_rtx_REG (Pmode,
> >>>>> FDPIC_REGNUM));
> >>>>
> >>>> Again, this looks suspicious.
> >>>>
> >>> Yes, fixed for follow-up patch, with
> >>> USE for get_hard_reg_initial_val (Pmode, FDPIC_REGNUM)
> >>> CLOBBER for gen_rtx_REG (Pmode, FDPIC_REGNUM)
> >>>
> >>>>> + emit_insn (par);
> >>>>> + }
> >>>>> + }
> >>>>> +
> >>>>> /* In an untyped call, we can get NULL for operand 2. */
> >>>>> if (operands[3] == 0)
> >>>>> operands[3] = const0_rtx;
> >>>>> @@ -8132,6 +8208,14 @@
> >>>>> : !REG_P (callee))
> >>>>> XEXP (operands[1], 0) = force_reg (Pmode, callee);
> >>>>> + if (TARGET_FDPIC
> >>>>> + && !SYMBOL_REF_P (XEXP (operands[1], 0)))
> >>>>> + {
> >>>>> + /* Indirect call: set r9 with FDPIC value of callee. */
> >>>>> + XEXP (operands[1], 0)
> >>>>> + = arm_load_function_descriptor (XEXP (operands[1], 0));
> >>>>> + }
> >>>>> +
> >>>>> if (detect_cmse_nonsecure_call (addr))
> >>>>> {
> >>>>> pat = gen_nonsecure_call_value_internal (operands[0], operands[1],
> >>>>> @@ -8144,6 +8228,28 @@
> >>>>> operands[2], operands[3]);
> >>>>> arm_emit_call_insn (pat, XEXP (operands[1], 0), false);
> >>>>> }
> >>>>> + /* Restore FDPIC register (r9) after call. */
> >>>>> + if (TARGET_FDPIC)
> >>>>> + {
> >>>>> + /* No need to update r9 if calling a static function. */
> >>>>> + if (!SYMBOL_REF_P (callee)
> >>>>> + || !SYMBOL_REF_LOCAL_P (callee)
> >>>>> + || arm_is_long_call_p (SYMBOL_REF_DECL (callee)))
> >>>>> + {
> >>>>> + rtx par = gen_rtx_PARALLEL (VOIDmode, rtvec_alloc (3));
> >>>>> +
> >>>>> + XVECEXP (par, 0, 0) = gen_rtx_UNSPEC (VOIDmode,
> >>>>> + gen_rtvec (2, gen_rtx_REG (Pmode, FDPIC_REGNUM),
> >>>>> + get_hard_reg_initial_val (Pmode, FDPIC_REGNUM)),
> >>>>> + UNSPEC_PIC_RESTORE);
> >>>>> + XVECEXP (par, 0, 1)
> >>>>> + = gen_rtx_USE (VOIDmode, gen_rtx_REG (Pmode, FDPIC_REGNUM));
> >>>>> + XVECEXP (par, 0, 2)
> >>>>> + = gen_rtx_CLOBBER (VOIDmode, gen_rtx_REG (Pmode,
> >>>>> FDPIC_REGNUM));
> >>>>
> >>>> And again.
> >>> Yes
> >>>
> >>>>
> >>>>> + emit_insn (par);
> >>>>> + }
> >>>>> + }
> >>>>> +
> >>>>> DONE;
> >>>>> }"
> >>>>> )
> >>>>> @@ -8486,7 +8592,7 @@
> >>>>> (const_int 0))
> >>>>> (match_operand 1 "" "")
> >>>>> (match_operand 2 "" "")])]
> >>>>> - "TARGET_EITHER"
> >>>>> + "TARGET_EITHER && !TARGET_FDPIC"
> >>>>> "
> >>>>> {
> >>>>> int i;
> >>>>> @@ -8553,7 +8659,7 @@
> >>>>> (define_expand "untyped_return"
> >>>>> [(match_operand:BLK 0 "memory_operand" "")
> >>>>> (match_operand 1 "" "")]
> >>>>> - "TARGET_EITHER"
> >>>>> + "TARGET_EITHER && !TARGET_FDPIC"
> >>>>> "
> >>>>> {
> >>>>> int i;
> >>>>> diff --git a/gcc/config/arm/unspecs.md b/gcc/config/arm/unspecs.md
> >>>>> index 1941673..349ae0e 100644
> >>>>> --- a/gcc/config/arm/unspecs.md
> >>>>> +++ b/gcc/config/arm/unspecs.md
> >>>>> @@ -86,6 +86,7 @@
> >>>>> UNSPEC_PROBE_STACK ; Probe stack memory reference
> >>>>> UNSPEC_NONSECURE_MEM ; Represent non-secure memory in ARMv8-M
> >>>>> with
> >>>>> ; security extension
> >>>>> + UNSPEC_PIC_RESTORE ; Use to restore fdpic register
> >>>>> ])
> >>>>> (define_c_enum "unspec" [
> >>>>>
> >>>>
> >>>> .
> >>>>
> >>>
> >>
>
next prev parent reply other threads:[~2018-10-29 8:16 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-11 13:36 [ARM/FDPIC v3 00/21] FDPIC ABI for ARM Christophe Lyon
2018-10-11 13:36 ` [ARM/FDPIC v3 02/21] [ARM] FDPIC: Handle arm*-*-uclinuxfdpiceabi in configure scripts Christophe Lyon
2018-10-12 10:14 ` Richard Earnshaw (lists)
2018-10-15 8:46 ` Christophe Lyon
2018-11-09 12:31 ` Christophe Lyon
2018-10-11 13:36 ` [ARM/FDPIC v3 01/21] [ARM] FDPIC: Add -mfdpic option support Christophe Lyon
2018-10-12 10:01 ` Richard Earnshaw (lists)
2018-10-17 8:13 ` Sandra Loosemore
2018-10-11 13:37 ` [ARM/FDPIC v3 04/21] [ARM] FDPIC: Add support for FDPIC for arm architecture Christophe Lyon
2018-10-12 11:02 ` Richard Earnshaw (lists)
2018-10-19 13:57 ` Christophe Lyon
2018-10-23 15:12 ` Richard Earnshaw (lists)
2018-10-26 16:34 ` Christophe Lyon
2018-10-26 16:41 ` Richard Earnshaw (lists)
2018-10-29 10:31 ` Christophe Lyon [this message]
2018-10-11 13:37 ` [ARM/FDPIC v3 05/21] [ARM] FDPIC: Fix __do_global_dtors_aux and frame_dummy generation Christophe Lyon
2018-10-12 11:29 ` Richard Earnshaw (lists)
2018-10-11 13:37 ` [ARM/FDPIC v3 03/21] [ARM] FDPIC: Force FDPIC related options unless -mno-fdpic is provided Christophe Lyon
2018-10-12 10:46 ` Richard Earnshaw (lists)
2018-10-15 10:35 ` Christophe Lyon
2018-10-23 15:01 ` Richard Earnshaw (lists)
2018-10-23 15:42 ` Segher Boessenkool
2018-10-26 15:33 ` Christophe Lyon
2018-10-29 14:25 ` Christophe Lyon
2018-10-29 23:15 ` Segher Boessenkool
2018-10-11 13:38 ` [ARM/FDPIC v3 07/21] [ARM] FDPIC: Avoid saving/restoring r9 on stack since it is RO Christophe Lyon
2018-10-12 12:21 ` Richard Earnshaw (lists)
2018-10-15 14:30 ` Christophe Lyon
2018-10-11 13:38 ` [ARM/FDPIC v3 06/21] [ARM] FDPIC: Add support for c++ exceptions Christophe Lyon
2018-10-12 11:54 ` Richard Earnshaw (lists)
2018-10-15 14:22 ` Christophe Lyon
2018-10-11 13:38 ` [ARM/FDPIC v3 08/21] [ARM] FDPIC: Ensure local/global binding for function descriptors Christophe Lyon
2018-10-12 12:39 ` Richard Earnshaw (lists)
2018-10-11 13:39 ` [ARM/FDPIC v3 11/21] [ARM] FDPIC: Add support to unwind FDPIC signal frame Christophe Lyon
2018-10-11 13:39 ` [ARM/FDPIC v3 09/21] [ARM] FDPIC: Add support for taking address of nested function Christophe Lyon
2018-10-11 13:39 ` [ARM/FDPIC v3 10/21] [ARM] FDPIC: Implement TLS support Christophe Lyon
2018-10-11 13:40 ` [ARM/FDPIC v3 12/21] [ARM] FDPIC: Restore r9 after we call __aeabi_read_tp Christophe Lyon
2018-10-19 14:02 ` Christophe Lyon
2018-10-11 13:40 ` [ARM/FDPIC v3 13/21] [ARM] FDPIC: Force LSB bit for PC in Cortex-M architecture Christophe Lyon
2018-10-11 13:41 ` [ARM/FDPIC v3 16/21] [ARM][testsuite] FDPIC: Skip v8-m and v6-m tests that currently produce an ICE Christophe Lyon
2018-10-11 13:41 ` [ARM/FDPIC v3 14/21] [ARM][testsuite] FDPIC: Skip unsupported tests Christophe Lyon
2018-10-11 13:41 ` [ARM/FDPIC v3 15/21] [ARM][testsuite] FDPIC: Adjust scan-assembler patterns Christophe Lyon
2018-10-11 13:42 ` [ARM/FDPIC v3 18/21] [ARM][testsuite] FDPIC: Handle *-*-uclinux* Christophe Lyon
2018-10-11 13:42 ` [ARM/FDPIC v3 17/21] [ARM][testsuite] FDPIC: Skip tests that don't work in PIC mode Christophe Lyon
2018-10-11 13:42 ` [ARM/FDPIC v3 19/21] [ARM][testsuite] FDPIC: Enable tests on pie_enabled targets Christophe Lyon
2018-10-11 13:43 ` [ARM/FDPIC v3 20/21] [ARM][testsuite] FDPIC: Adjust pr43698.c to avoid clash with uclibc Christophe Lyon
2018-10-11 13:56 ` [ARM/FDPIC v3 21/21] [ARM][testsuite] FDPIC: Skip tests using architecture older than v7 Christophe Lyon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAKdteOYox66tEdR-eNKtjHz01-3PKXh_Xuh17tPPEk5Hx-zCKg@mail.gmail.com \
--to=christophe.lyon@linaro.org \
--cc=Richard.Earnshaw@arm.com \
--cc=christophe.lyon@st.com \
--cc=gcc-patches@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).