Re: PATCH: PR target/47766: [x32] -fstack-protector doesn't work

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Fri, Jul 29, 2011 at 3:14 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Thu, Jul 28, 2011 at 1:03 PM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> On Thu, Jul 28, 2011 at 9:03 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>>
>>>>> This patch adds x32 support to UNSPEC_SP_XXX patterns. ?OK for trunk?
>>>>
>>>> http://gcc.gnu.org/contribute.html#patches
>>>>
>>>
>>> Sorry. I should have mentioned testcase in:
>>>
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47766
>>>
>>> Actually, they are in gcc testsuite. ?I noticed them when
>>> I run gcc testsuite on x32.
>>
>> This looks like a middle-end problem to me.
>>
>> According to the documentation:
>>
>> --quote--
>> `stack_protect_set'
>> ? ? This pattern, if defined, moves a `Pmode' value from the memory in
>> ? ? operand 1 to the memory in operand 0 without leaving the value in
>> ? ? a register afterward. ?This is to avoid leaking the value some
>> ? ? place that an attacker might use to rewrite the stack guard slot
>> ? ? after having clobbered it.
>>
>> ? ? If this pattern is not defined, then a plain move pattern is
>> ? ? generated.
>>
>> `stack_protect_test'
>> ? ? This pattern, if defined, compares a `Pmode' value from the memory
>> ? ? in operand 1 with the memory in operand 0 without leaving the
>> ? ? value in a register afterward and branches to operand 2 if the
>> ? ? values weren't equal.
>>
>> ? ? If this pattern is not defined, then a plain compare pattern and
>> ? ? conditional branch pattern is used.
>> --quote--
>>
>> According to the documentation, x86 patterns are correct. However,
>> middle-end fails to extend ptr_mode value to Pmode, and in function.c,
>> stack_protect_prologue/stack_protect_epilogue, we already have
>> ptr_mode (SImode) operand:
>>
>> (mem/v/f/c/i:SI (plus:DI (reg/f:DI 54 virtual-stack-vars)
>> ? ? ? ?(const_int -4 [0xfffffffffffffffc])) [2 D.2704+0 S4 A32])
>>
>> (mem/v/f/c/i:SI (symbol_ref:DI ("__stack_chk_guard") [flags 0x40]
>> <var_decl 0x7ffc35aa0be0 __stack_chk_guard>) [2 __stack_chk_guard+0 S4
>> A32])
>>
>> An opinion of a RTL maintainer (CC'd) is needed here. Target
>> definition is OK in its current form.
>
> When -fstack-protector ?was added, there are
>
> tree
> default_stack_protect_guard (void)
> {
> ?tree t = stack_chk_guard_decl;
>
> ?if (t == NULL)
> ? ?{
> ? ? ?rtx x;
>
> ? ? ?t = build_decl (UNKNOWN_LOCATION,
> ? ? ? ? ? ? ? ? ? ? ?VAR_DECL, get_identifier ("__stack_chk_guard"),
> ? ? ? ? ? ? ? ? ? ? ?ptr_type_node);
> ? ? ? ? ? ? ? ? ? ? ?^^^^^^^^^^^^^^^^^
>
> I think ptr_mode is intended and Pmode is just a typo. ?Jakub, Richard,
> what are your thoughts on this?
>

Stack protector does use ptr_mode.  We have

/* i386 glibc provides __stack_chk_guard in %gs:0x14,
   x32 glibc provides it in %fs:0x18.
   x86_64 glibc provides it in %fs:0x28.  */
#define TARGET_THREAD_SSP_OFFSET \
  (TARGET_64BIT ? (TARGET_X32 ? 0x18 : 0x28) : 0x14)

in gnu-user64.h and

typedef struct
{
  void *tcb;            /* Pointer to the TCB.  Not necessarily the
                           thread descriptor used by libpthread.  */
  dtv_t *dtv;
  void *self;           /* Pointer to the thread descriptor.  */
  int multiple_threads;
  int gscope_flag;
  uintptr_t sysinfo;
  uintptr_t stack_guard;
  uintptr_t pointer_guard;

in sysdeps/x86_64/tls.h.  I believe it is a mistake to use Pmode in
the stack protector documentation.

-- 
H.J.