This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH, ARM] Fix PIC + -fstack-protector (PR35965)


Hi,

This patch fixes the test case in PR35965. The problem was that using
a -fstack-protector[-all] option causes arm.c:require_pic_register() to
be called from function.c:stack_protect_prologue() (via validize_mem).
The logic in require_pic_register is either expecting to be called "as
part of the rtx cost estimation process", or whilst emitting RTL for
a function.

During the stack_protect_prologue call, we have something like the
latter case: RTL is emitted (to load the _GLOBAL_OFFSET_TABLE_ pointer
into a pseudo-reg), although current_ir_type () still returns
IR_GIMPLE. This means that cfun->machine->pic_reg is initialised with
two different pseudo-regs representing the GOT pointer during the
compilation of a single function: an attempt is made to use the first to
access the stack-protector-related variable __stack_check_guard, but
only the second is actually loaded with the GOT pointer. The missing
GOT pointer is then optimised away leaving only the offset (relative to
address zero), and the resulting bad memory access causes a segfault at
runtime in the compiled code.

The attached patch just makes sure that the cfun->machine->pic_reg
variable is only set once per-function, which suffices to fix the
problem, but might not be the cleanest possible solution. E.g. the MIPS
backend checks the variable "currently_expanding_to_rtl" in a
(presumably) similar situation, but although that condition is true
during the stack_protect_prologue call, adding it to the condition in
arm.c:require_pic_register, e.g.:

  if (current_ir_type () != IR_GIMPLE || currently_expanding_to_rtl)

Causes a crash in the subsequent code:

    ...
    start_sequence ();

    arm_load_pic_register (0UL);

    seq = get_insns ();
    end_sequence ();
    emit_insn_after (seq, entry_of_function ()); // boom!
  }

because some BB-related fields are apparently not yet initialised
properly. Maybe the stack_protect_prologue() call needs to be lower
down cfgexpand.c:tree_expand_cfg for this to work.

The attached patch is tested with cross to arm-none-linux-gnueabi. OK
for mainline? (The bug is present on 4.3 too, so the patch might be
needed there also.)

Julian

ChangeLog

    gcc/
    * config/arm/arm.c (require_pic_register): Only set
    cfun->machine->pic_reg once per function.
Index: gcc/config/arm/arm.c
===================================================================
--- gcc/config/arm/arm.c	(revision 137103)
+++ gcc/config/arm/arm.c	(working copy)
@@ -3394,7 +3394,8 @@ require_pic_register (void)
       gcc_assert (can_create_pseudo_p ());
       if (arm_pic_register != INVALID_REGNUM)
 	{
-	  cfun->machine->pic_reg = gen_rtx_REG (Pmode, arm_pic_register);
+	  if (!cfun->machine->pic_reg)
+	    cfun->machine->pic_reg = gen_rtx_REG (Pmode, arm_pic_register);
 
 	  /* Play games to avoid marking the function as needing pic
 	     if we are being called as part of the cost-estimation
@@ -3406,7 +3407,8 @@ require_pic_register (void)
 	{
 	  rtx seq;
 
-	  cfun->machine->pic_reg = gen_reg_rtx (Pmode);
+	  if (!cfun->machine->pic_reg)
+	    cfun->machine->pic_reg = gen_reg_rtx (Pmode);
 
 	  /* Play games to avoid marking the function as needing pic
 	     if we are being called as part of the cost-estimation

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]