http://gcc.gnu.org/ml/gcc-patches/2008-06/msg01641.html http://bugs.gentoo.org/248061 http://gcc.gnu.org/PR35965 From: Julian Brown To: gcc-patches at gcc dot gnu dot org Cc: paul at codesourcery dot com Date: Thu, 26 Jun 2008 11:08:39 +0100 Subject: [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 __________________________________________________________________