On Fri, 12 Oct 2018 at 13:37, Richard Earnshaw (lists) <
richard.earns...@arm.com> wrote:
> On 11/10/18 14:34, Christophe Lyon wrote:
> > The main difference with existing support is that function addresses
> > are function descriptor addresses instead. This means that all code
> > dealing with function pointers now has to cope with function
> > descriptors instead.
> >
> > For the same reason, Linux kernel helpers can no longer be called by
> > dereferencing their address, so we implement the same functionality as
> > a regular function here.
> >
> > When restoring a function address, we also have to restore the FDPIC
> > register value (r9).
> >
> > 2018-XX-XX Christophe Lyon <christophe.l...@st.com>
> > Mickaël Guêné <mickael.gu...@st.com>
> >
> > gcc/
> > * ginclude/unwind-arm-common.h (unwinder_cache): Add reserved5
> > field.
> > (FDPIC_REGNUM): New define.
> >
> > libgcc/
> > * config/arm/linux-atomic.c (__kernel_cmpxchg): Add FDPIC support.
> > (__kernel_dmb): Likewise.
> > (__fdpic_cmpxchg): New function.
> > (__fdpic_dmb): New function.
> > * config/arm/unwind-arm.h (gnu_Unwind_Find_got): New function.
> > (_Unwind_decode_typeinfo_ptr): Add FDPIC support.
> > * unwindo-arm-common.inc (UCB_PR_GOT): New.
> > (funcdesc_t): New struct.
> > (get_eit_entry): Add FDPIC support.
> > (unwind_phase2): Likewise.
> > (unwind_phase2_forced): Likewise.
> > (__gnu_Unwind_RaiseException): Likewise.
> > (__gnu_Unwind_Resume): Likewise.
> > (__gnu_Unwind_Backtrace): Likewise.
> > * unwind-pe.h (read_encoded_value_with_base): Likewise.
> >
> > libstdc++/
> > * libsupc++/eh_personality.cc (get_ttype_entry): Add FDPIC
> > support.
> >
> > Change-Id: I517a49ff18fae21c686cd1c6008ea7974515b347
> >
> > diff --git a/gcc/ginclude/unwind-arm-common.h
> b/gcc/ginclude/unwind-arm-common.h
> > index 8a1a919..f663891 100644
> > --- a/gcc/ginclude/unwind-arm-common.h
> > +++ b/gcc/ginclude/unwind-arm-common.h
> > @@ -91,7 +91,7 @@ extern "C" {
> > _uw reserved2; /* Personality routine address */
> > _uw reserved3; /* Saved callsite address */
> > _uw reserved4; /* Forced unwind stop arg */
> > - _uw reserved5;
> > + _uw reserved5; /* Personality routine GOT value in FDPIC mode.
> */
> > }
> > unwinder_cache;
> > /* Propagation barrier cache (valid after phase 1): */
> > @@ -247,4 +247,6 @@ typedef unsigned long _uleb128_t;
> > } /* extern "C" */
> > #endif
> >
> > +#define FDPIC_REGNUM 9
>
> Looking at the rest of this file, I think it can end up being included
> in user code. So you have to put predefines into the reserved
> namespace. Why do you need this here anyway?
>
> That was to address a comment I got on v2 of this patch: I was requested
to avoid hardcoding "r9" in unwind-arm.h, and to use FDPIC_REGNUM.
I needed the definition to be visible in both unwind-arm.h and
unwind-arm-common.inc, but I moved it at a too high level. I'll fix this.
> +
> > #endif /* defined UNWIND_ARM_COMMON_H */
> > diff --git a/libgcc/config/arm/linux-atomic.c
> b/libgcc/config/arm/linux-atomic.c
> > index d334c58..161d1ce 100644
> > --- a/libgcc/config/arm/linux-atomic.c
> > +++ b/libgcc/config/arm/linux-atomic.c
> > @@ -25,11 +25,49 @@ see the files COPYING3 and COPYING.RUNTIME
> respectively. If not, see
> >
> > /* Kernel helper for compare-and-exchange. */
> > typedef int (__kernel_cmpxchg_t) (int oldval, int newval, int *ptr);
> > +#if __FDPIC__
> > +/* Non-FDPIC ABIs call __kernel_cmpxchg directly by dereferencing its
> > + address, but under FDPIC we would generate a broken call
> > + sequence. That's why we have to implement __kernel_cmpxchg and
> > + __kernel_dmb here: this way, the FDPIC call sequence works. */
> > +#define __kernel_cmpxchg __fdpic_cmpxchg
> > +#else
> > #define __kernel_cmpxchg (*(__kernel_cmpxchg_t *) 0xffff0fc0)
> > +#endif
> >
> > /* Kernel helper for memory barrier. */
> > typedef void (__kernel_dmb_t) (void);
> > +#if __FDPIC__
> > +#define __kernel_dmb __fdpic_dmb
> > +#else
> > #define __kernel_dmb (*(__kernel_dmb_t *) 0xffff0fa0)
> > +#endif
> > +
> > +#if __FDPIC__
> > +static int __fdpic_cmpxchg (int oldval, int newval, int *ptr)
> > +{
> > + int result;
> > +
> > + asm volatile ("1: ldrex r3, [%[ptr]]\n\t"
> > + "subs r3, r3, %[oldval]\n\t"
> > + "itt eq\n\t"
> > + "strexeq r3, %[newval], [%[ptr]]\n\t"
> > + "teqeq r3, #1\n\t"
> > + "it eq\n\t"
> > + "beq 1b\n\t"
> > + "rsbs %[result], r3, #0\n\t"
> > + : [result] "=r" (result)
> > + : [oldval] "r" (oldval) , [newval] "r" (newval), [ptr] "r"
> (ptr)
> > + : "r3");
> > + return result;
> > +}
> > +
> > +static void __fdpic_dmb ()
> > +{
> > + asm volatile ("dmb\n\t");
> > +}
> > +
>
> The whole point of __kernel_dmb and __kernel_cmpxchg is that the kernel
> can map in a code sequence specific to the runtime CPU. This ensures
> that the right sequence is used for that CPU. This breaks that
> principle entirely.
>
> Right, I guess we had v7-only in mind.
I'll check how we can fix that.
> > +#endif
> >
> > /* Note: we implement byte, short and int versions of atomic operations
> using
> > the above kernel helpers; see linux-atomic-64bit.c for "long long"
> (64-bit)
> > diff --git a/libgcc/config/arm/unwind-arm.h
> b/libgcc/config/arm/unwind-arm.h
> > index 9f7d3f2..db5dfa2 100644
> > --- a/libgcc/config/arm/unwind-arm.h
> > +++ b/libgcc/config/arm/unwind-arm.h
> > @@ -33,9 +33,31 @@
> > /* Use IP as a scratch register within the personality routine. */
> > #define UNWIND_POINTER_REG 12
> >
> > +#define STR(x) #x
> > +#define XSTR(x) STR(x)
> > +
> > #ifdef __cplusplus
> > extern "C" {
> > #endif
> > +_Unwind_Ptr __attribute__((weak)) __gnu_Unwind_Find_got (_Unwind_Ptr);
> > +
> > +static inline _Unwind_Ptr gnu_Unwind_Find_got (_Unwind_Ptr ptr)
> > +{
> > + _Unwind_Ptr res;
> > +
> > + if (__gnu_Unwind_Find_got)
> > + res = __gnu_Unwind_Find_got (ptr);
> > + else
> > + {
> > + asm volatile ("mov %[result], r" XSTR(FDPIC_REGNUM)
> > + : [result]"=r" (res)
> > + :
> > + :);
> > + }
> > +
> > + return res;
> > +}
> > +
> > /* Decode an R_ARM_TARGET2 relocation. */
> > static inline _Unwind_Word
> > _Unwind_decode_typeinfo_ptr (_Unwind_Word base __attribute__
> ((unused)),
> > @@ -48,7 +70,12 @@ extern "C" {
> > if (!tmp)
> > return 0;
> >
> > -#if (defined(linux) && !defined(__uClinux__)) || defined(__NetBSD__) \
> > +#if __FDPIC__
> > + /* For FDPIC, we store the offset of the GOT entry. */
> > + /* So, first get GOT from dynamic linker and then use indirect
> access. */
> > + tmp += gnu_Unwind_Find_got (ptr);
> > + tmp = *(_Unwind_Word *) tmp;
> > +#elif (defined(linux) && !defined(__uClinux__)) || defined(__NetBSD__) \
> > || defined(__FreeBSD__) || defined(__fuchsia__)
> > /* Pc-relative indirect. */
> > #define _GLIBCXX_OVERRIDE_TTYPE_ENCODING (DW_EH_PE_pcrel |
> DW_EH_PE_indirect)
> > diff --git a/libgcc/unwind-arm-common.inc b/libgcc/unwind-arm-common.inc
> > index 76f8fc3..7187edd 100644
> > --- a/libgcc/unwind-arm-common.inc
> > +++ b/libgcc/unwind-arm-common.inc
> > @@ -62,6 +62,7 @@ __gnu_Unwind_Find_exidx (_Unwind_Ptr, int *);
> > #define UCB_PR_ADDR(ucbp) ((ucbp)->unwinder_cache.reserved2)
> > #define UCB_SAVED_CALLSITE_ADDR(ucbp) ((ucbp)->unwinder_cache.reserved3)
> > #define UCB_FORCED_STOP_ARG(ucbp) ((ucbp)->unwinder_cache.reserved4)
> > +#define UCB_PR_GOT(ucbp) ((ucbp)->unwinder_cache.reserved5)
> >
> > /* Unwind descriptors. */
> >
> > @@ -85,6 +86,15 @@ typedef struct __EIT_entry
> > _uw content;
> > } __EIT_entry;
> >
> > +#ifdef __FDPIC__
> > +
> > +/* Only used in FDPIC case. */
> > +struct funcdesc_t {
> > + unsigned int ptr;
> > + unsigned int got;
> > +};
>
> This isn't in GNU style.
>
> OK
> > +#endif
> > +
> > /* Assembly helper functions. */
> >
> > /* Restore core register state. Never returns. */
> > @@ -259,7 +269,21 @@ get_eit_entry (_Unwind_Control_Block *ucbp, _uw
> return_address)
> > {
> > /* One of the predefined standard routines. */
> > _uw idx = (*(_uw *) ucbp->pr_cache.ehtp >> 24) & 0xf;
> > +#if __FDPIC__
> > + {
> > + struct funcdesc_t *funcdesc
> > + = (struct funcdesc_t *) __gnu_unwind_get_pr_addr (idx);
> > + if (funcdesc)
> > + {
> > + UCB_PR_ADDR (ucbp) = funcdesc->ptr;
> > + UCB_PR_GOT (ucbp) = funcdesc->got;
> > + }
> > + else
> > + UCB_PR_ADDR (ucbp) = 0;
> > + }
> > +#else
> > UCB_PR_ADDR (ucbp) = __gnu_unwind_get_pr_addr (idx);
> > +#endif
> > if (UCB_PR_ADDR (ucbp) == 0)
> > {
> > /* Failed */
> > @@ -270,6 +294,10 @@ get_eit_entry (_Unwind_Control_Block *ucbp, _uw
> return_address)
> > {
> > /* Execute region offset to PR */
> > UCB_PR_ADDR (ucbp) = selfrel_offset31 (ucbp->pr_cache.ehtp);
> > +#if __FDPIC__
> > + UCB_PR_GOT (ucbp)
> > + = (unsigned int) gnu_Unwind_Find_got ((_Unwind_Ptr) UCB_PR_ADDR
> (ucbp));
> > +#endif
> > }
> > return _URC_OK;
> > }
> > @@ -291,14 +319,29 @@ unwind_phase2 (_Unwind_Control_Block * ucbp,
> phase2_vrs * vrs)
> > UCB_SAVED_CALLSITE_ADDR (ucbp) = VRS_PC(vrs);
> >
> > /* Call the pr to decide what to do. */
> > +#if __FDPIC__
> > + {
> > + volatile struct funcdesc_t funcdesc;
> > + funcdesc.ptr = UCB_PR_ADDR (ucbp);
> > + funcdesc.got = UCB_PR_GOT (ucbp);
> > + pr_result = ((personality_routine) &funcdesc)
> > + (_US_UNWIND_FRAME_STARTING, ucbp, (_Unwind_Context *) vrs);
> > + }
> > +#else
> > pr_result = ((personality_routine) UCB_PR_ADDR (ucbp))
> > (_US_UNWIND_FRAME_STARTING, ucbp, (_Unwind_Context *) vrs);
> > +#endif
> > }
> > while (pr_result == _URC_CONTINUE_UNWIND);
> >
> > if (pr_result != _URC_INSTALL_CONTEXT)
> > abort();
> >
> > +#if __FDPIC__
> > + /* r9 could have been lost due to PLT jump. Restore correct value.
> */
> > + vrs->core.r[FDPIC_REGNUM] = gnu_Unwind_Find_got (VRS_PC (vrs));
> > +#endif
> > +
> > uw_restore_core_regs (vrs, &vrs->core);
> > }
> >
> > @@ -346,8 +389,18 @@ unwind_phase2_forced (_Unwind_Control_Block *ucbp,
> phase2_vrs *entry_vrs,
> > next_vrs = saved_vrs;
> >
> > /* Call the pr to decide what to do. */
> > +#if __FDPIC__
> > + {
> > + volatile struct funcdesc_t funcdesc;
> > + funcdesc.ptr = UCB_PR_ADDR (ucbp);
> > + funcdesc.got = UCB_PR_GOT (ucbp);
> > + pr_result = ((personality_routine) &funcdesc)
> > + (action, ucbp, (void *) &next_vrs);
> > + }
> > +#else
> > pr_result = ((personality_routine) UCB_PR_ADDR (ucbp))
> > (action, ucbp, (void *) &next_vrs);
> > +#endif
> >
> > saved_vrs.prev_sp = VRS_SP (&next_vrs);
> > }
> > @@ -384,6 +437,11 @@ unwind_phase2_forced (_Unwind_Control_Block *ucbp,
> phase2_vrs *entry_vrs,
> > return _URC_FAILURE;
> > }
> >
> > +#if __FDPIC__
> > + /* r9 could have been lost due to PLT jump. Restore correct value.
> */
> > + saved_vrs.core.r[FDPIC_REGNUM] = gnu_Unwind_Find_got (VRS_PC
> (&saved_vrs));
> > +#endif
> > +
> > uw_restore_core_regs (&saved_vrs, &saved_vrs.core);
> > }
> >
> > @@ -429,8 +487,18 @@ __gnu_Unwind_RaiseException (_Unwind_Control_Block
> * ucbp,
> > return _URC_FAILURE;
> >
> > /* Call the pr to decide what to do. */
> > +#if __FDPIC__
> > + {
> > + volatile struct funcdesc_t funcdesc;
> > + funcdesc.ptr = UCB_PR_ADDR (ucbp);
> > + funcdesc.got = UCB_PR_GOT (ucbp);
> > + pr_result = ((personality_routine) &funcdesc)
> > + (_US_VIRTUAL_UNWIND_FRAME, ucbp, (void *) &saved_vrs);
> > + }
> > +#else
> > pr_result = ((personality_routine) UCB_PR_ADDR (ucbp))
> > (_US_VIRTUAL_UNWIND_FRAME, ucbp, (void *) &saved_vrs);
> > +#endif
> > }
> > while (pr_result == _URC_CONTINUE_UNWIND);
> >
> > @@ -488,13 +556,27 @@ __gnu_Unwind_Resume (_Unwind_Control_Block * ucbp,
> phase2_vrs * entry_vrs)
> > }
> >
> > /* Call the cached PR. */
> > +#if __FDPIC__
> > + {
> > + volatile struct funcdesc_t funcdesc;
> > + funcdesc.ptr = UCB_PR_ADDR (ucbp);
> > + funcdesc.got = UCB_PR_GOT (ucbp);
> > + pr_result = ((personality_routine) &funcdesc)
> > + (_US_UNWIND_FRAME_RESUME, ucbp, (_Unwind_Context *) entry_vrs);
> > + }
> > +#else
> > pr_result = ((personality_routine) UCB_PR_ADDR (ucbp))
> > (_US_UNWIND_FRAME_RESUME, ucbp, (_Unwind_Context *) entry_vrs);
> > +#endif
> >
> > switch (pr_result)
> > {
> > case _URC_INSTALL_CONTEXT:
> > /* Upload the registers to enter the landing pad. */
> > +#if __FDPIC__
> > + /* r9 could have been lost due to PLT jump. Restore correct
> value. */
> > + entry_vrs->core.r[FDPIC_REGNUM] = gnu_Unwind_Find_got (VRS_PC
> (entry_vrs));
> > +#endif
> > uw_restore_core_regs (entry_vrs, &entry_vrs->core);
> >
> > case _URC_CONTINUE_UNWIND:
> > @@ -586,9 +668,20 @@ __gnu_Unwind_Backtrace(_Unwind_Trace_Fn trace, void
> * trace_argument,
> > }
> >
> > /* Call the pr to decide what to do. */
> > +#if __FDPIC__
> > + {
> > + volatile struct funcdesc_t funcdesc;
> > + funcdesc.ptr = UCB_PR_ADDR (ucbp);
> > + funcdesc.got = UCB_PR_GOT (ucbp);
> > + code = ((personality_routine) &funcdesc)
> > + (_US_VIRTUAL_UNWIND_FRAME | _US_FORCE_UNWIND,
> > + ucbp, (void *) &saved_vrs);
> > + }
> > +#else
> > code = ((personality_routine) UCB_PR_ADDR (ucbp))
> > (_US_VIRTUAL_UNWIND_FRAME | _US_FORCE_UNWIND,
> > ucbp, (void *) &saved_vrs);
> > +#endif
> > }
> > while (code != _URC_END_OF_STACK
> > && code != _URC_FAILURE);
> > diff --git a/libgcc/unwind-pe.h b/libgcc/unwind-pe.h
> > index dd5ae95..9c5177f 100644
> > --- a/libgcc/unwind-pe.h
> > +++ b/libgcc/unwind-pe.h
> > @@ -259,10 +259,27 @@ read_encoded_value_with_base (unsigned char
> encoding, _Unwind_Ptr base,
> >
> > if (result != 0)
> > {
> > +#if __FDPIC__
> > + /* FDPIC relative addresses imply taking the GOT address
> > + into account. */
> > + if ((encoding & DW_EH_PE_pcrel) && (encoding &
> DW_EH_PE_indirect))
> > + {
> > + result += gnu_Unwind_Find_got ((_Unwind_Ptr) u);
> > + result = *(_Unwind_Internal_Ptr *) result;
> > + }
> > + else
> > + {
> > + result += ((encoding & 0x70) == DW_EH_PE_pcrel
> > + ? (_Unwind_Internal_Ptr) u : base);
> > + if (encoding & DW_EH_PE_indirect)
> > + result = *(_Unwind_Internal_Ptr *) result;
> > + }
> > +#else
> > result += ((encoding & 0x70) == DW_EH_PE_pcrel
> > ? (_Unwind_Internal_Ptr) u : base);
> > if (encoding & DW_EH_PE_indirect)
> > result = *(_Unwind_Internal_Ptr *) result;
> > +#endif
> > }
> > }
> >
> > diff --git a/libstdc++-v3/libsupc++/eh_personality.cc
> b/libstdc++-v3/libsupc++/eh_personality.cc
> > index 1b336c7..7b26b8d 100644
> > --- a/libstdc++-v3/libsupc++/eh_personality.cc
> > +++ b/libstdc++-v3/libsupc++/eh_personality.cc
> > @@ -93,7 +93,15 @@ get_ttype_entry (lsda_header_info *info, _uleb128_t i)
> > _Unwind_Ptr ptr;
> >
> > i *= size_of_encoded_value (info->ttype_encoding);
> > - read_encoded_value_with_base (info->ttype_encoding, info->ttype_base,
> > + read_encoded_value_with_base (
> > +#if __FDPIC__
> > + /* Force these flags to nake sure to
> > + take the GOT into account. */
> > + (DW_EH_PE_pcrel | DW_EH_PE_indirect),
> > +#else
> > + info->ttype_encoding,
> > +#endif
> > + info->ttype_base,
> > info->TType - i, &ptr);
> >
> > return reinterpret_cast<const std::type_info *>(ptr);
> >
>
> Nearly all of the above is outside of my comfort zone... I can't help
> feeling that there ought to be a less invasive way to do this, but
> without being particularly familiar with the code it's difficult to be
> sure.
>
> The last two files patched will need approval from a relevant component
> maintainer.
>
