From 69708e0afbf3a3757b9f689bb54acff1d7e8d9ec Mon Sep 17 00:00:00 2001 From: Mihail Ionescu Date: Wed, 8 May 2019 14:36:15 +0000 Subject: [PATCH] [arm][PR88167] Fix __builtin_return_address returns invalid address This patch fixes a problem with the thumb1 prologue code where the link register could be unconditionally used as a scratch register even if the return value was still live at the end of the prologue. Additionally, the patch improves the code generated when we are not using many low call-saved registers to make use of any unused call clobbered registers to help with the saving of high registers that cannot be pushed directly (quite rare in normal code as the register allocator correctly prefers low registers). 2019-05-08 Mihail Ionescu Richard Earnshaw gcc: PR target/88167 * config/arm/arm.c (thumb1_prologue_unused_call_clobbered_lo_regs): New function. (thumb1_epilogue_unused_call_clobbered_lo_regs): New function. (thumb1_compute_save_core_reg_mask): Don't force a spare work register if both the epilogue and prologue can use call-clobbered regs. (thumb1_unexpanded_epilogue): Use thumb1_epilogue_unused_call_clobbered_lo_regs. Reverse the logic for picking temporaries for restoring high regs to match that of the prologue where possible. (thumb1_expand_prologue): Add any usable call-clobbered low registers to the list of work registers. Detect if the return address is still live at the end of the prologue and avoid using it for a work register if so. If the return address is not live, add LR to the list of pushable regs after the first pass. gcc/testsuite: PR target/88167 * gcc.target/arm/pr88167-1.c: New test. * gcc.target/arm/pr88167-2.c: New test. Co-Authored-By: Richard Earnshaw From-SVN: r271012 --- gcc/ChangeLog | 20 ++++++ gcc/config/arm/arm.c | 84 +++++++++++++++++++----- gcc/testsuite/ChangeLog | 7 ++ gcc/testsuite/gcc.target/arm/pr88167-1.c | 15 +++++ gcc/testsuite/gcc.target/arm/pr88167-2.c | 18 +++++ 5 files changed, 127 insertions(+), 17 deletions(-) create mode 100644 gcc/testsuite/gcc.target/arm/pr88167-1.c create mode 100644 gcc/testsuite/gcc.target/arm/pr88167-2.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 08b37f27d45..7723381047d 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,23 @@ +2019-05-08 Mihail Ionescu + Richard Earnshaw + + PR target/88167 + * config/arm/arm.c (thumb1_prologue_unused_call_clobbered_lo_regs): New + function. + (thumb1_epilogue_unused_call_clobbered_lo_regs): New function. + (thumb1_compute_save_core_reg_mask): Don't force a spare work + register if both the epilogue and prologue can use call-clobbered + regs. + (thumb1_unexpanded_epilogue): Use + thumb1_epilogue_unused_call_clobbered_lo_regs. Reverse the logic for + picking temporaries for restoring high regs to match that of the + prologue where possible. + (thumb1_expand_prologue): Add any usable call-clobbered low registers to + the list of work registers. Detect if the return address is still live + at the end of the prologue and avoid using it for a work register if so. + If the return address is not live, add LR to the list of pushable regs + after the first pass. + 2019-05-08 Bin Cheng PR tree-optimization/90078 diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index 45abcd89963..91bb65130b8 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -19670,6 +19670,35 @@ arm_compute_save_core_reg_mask (void) return save_reg_mask; } +/* Return a mask for the call-clobbered low registers that are unused + at the end of the prologue. */ +static unsigned long +thumb1_prologue_unused_call_clobbered_lo_regs (void) +{ + unsigned long mask = 0; + + for (int reg = 0; reg <= LAST_LO_REGNUM; reg++) + if (!callee_saved_reg_p (reg) + && !REGNO_REG_SET_P (df_get_live_out (ENTRY_BLOCK_PTR_FOR_FN (cfun)), + reg)) + mask |= 1 << reg; + return mask; +} + +/* Similarly for the start of the epilogue. */ +static unsigned long +thumb1_epilogue_unused_call_clobbered_lo_regs (void) +{ + unsigned long mask = 0; + + for (int reg = 0; reg <= LAST_LO_REGNUM; reg++) + if (!callee_saved_reg_p (reg) + && !REGNO_REG_SET_P (df_get_live_in (EXIT_BLOCK_PTR_FOR_FN (cfun)), + reg)) + mask |= 1 << reg; + return mask; +} + /* Compute a bit mask of which core registers need to be saved on the stack for the current function. */ static unsigned long @@ -19701,10 +19730,19 @@ thumb1_compute_save_core_reg_mask (void) if (mask & 0xff || thumb_force_lr_save ()) mask |= (1 << LR_REGNUM); - /* Make sure we have a low work register if we need one. - We will need one if we are going to push a high register, - but we are not currently intending to push a low register. */ + bool call_clobbered_scratch + = (thumb1_prologue_unused_call_clobbered_lo_regs () + && thumb1_epilogue_unused_call_clobbered_lo_regs ()); + + /* Make sure we have a low work register if we need one. We will + need one if we are going to push a high register, but we are not + currently intending to push a low register. However if both the + prologue and epilogue have a spare call-clobbered low register, + then we won't need to find an additional work register. It does + not need to be the same register in the prologue and + epilogue. */ if ((mask & 0xff) == 0 + && !call_clobbered_scratch && ((mask & 0x0f00) || TARGET_BACKTRACE)) { /* Use thumb_find_work_register to choose which register @@ -24930,12 +24968,7 @@ thumb1_unexpanded_epilogue (void) unsigned long mask = live_regs_mask & 0xff; int next_hi_reg; - /* The available low registers depend on the size of the value we are - returning. */ - if (size <= 12) - mask |= 1 << 3; - if (size <= 8) - mask |= 1 << 2; + mask |= thumb1_epilogue_unused_call_clobbered_lo_regs (); if (mask == 0) /* Oh dear! We have no low registers into which we can pop @@ -24943,7 +24976,7 @@ thumb1_unexpanded_epilogue (void) internal_error ("no low registers available for popping high registers"); - for (next_hi_reg = 8; next_hi_reg < 13; next_hi_reg++) + for (next_hi_reg = 12; next_hi_reg > LAST_LO_REGNUM; next_hi_reg--) if (live_regs_mask & (1 << next_hi_reg)) break; @@ -24951,7 +24984,7 @@ thumb1_unexpanded_epilogue (void) { /* Find lo register(s) into which the high register(s) can be popped. */ - for (regno = 0; regno <= LAST_LO_REGNUM; regno++) + for (regno = LAST_LO_REGNUM; regno >= 0; regno--) { if (mask & (1 << regno)) high_regs_pushed--; @@ -24959,20 +24992,22 @@ thumb1_unexpanded_epilogue (void) break; } - mask &= (2 << regno) - 1; /* A noop if regno == 8 */ + if (high_regs_pushed == 0 && regno >= 0) + mask &= ~((1 << regno) - 1); /* Pop the values into the low register(s). */ thumb_pop (asm_out_file, mask); /* Move the value(s) into the high registers. */ - for (regno = 0; regno <= LAST_LO_REGNUM; regno++) + for (regno = LAST_LO_REGNUM; regno >= 0; regno--) { if (mask & (1 << regno)) { asm_fprintf (asm_out_file, "\tmov\t%r, %r\n", next_hi_reg, regno); - for (next_hi_reg++; next_hi_reg < 13; next_hi_reg++) + for (next_hi_reg--; next_hi_reg > LAST_LO_REGNUM; + next_hi_reg--) if (live_regs_mask & (1 << next_hi_reg)) break; } @@ -25354,10 +25389,20 @@ thumb1_expand_prologue (void) break; /* Here we need to mask out registers used for passing arguments - even if they can be pushed. This is to avoid using them to stash the high - registers. Such kind of stash may clobber the use of arguments. */ + even if they can be pushed. This is to avoid using them to + stash the high registers. Such kind of stash may clobber the + use of arguments. */ pushable_regs = l_mask & (~arg_regs_mask); - if (lr_needs_saving) + pushable_regs |= thumb1_prologue_unused_call_clobbered_lo_regs (); + + /* Normally, LR can be used as a scratch register once it has been + saved; but if the function examines its own return address then + the value is still live and we need to avoid using it. */ + bool return_addr_live + = REGNO_REG_SET_P (df_get_live_out (ENTRY_BLOCK_PTR_FOR_FN (cfun)), + LR_REGNUM); + + if (lr_needs_saving || return_addr_live) pushable_regs &= ~(1 << LR_REGNUM); if (pushable_regs == 0) @@ -25398,6 +25443,11 @@ thumb1_expand_prologue (void) push_mask |= 1 << LR_REGNUM; real_regs_mask |= 1 << LR_REGNUM; lr_needs_saving = false; + /* If the return address is not live at this point, we + can add LR to the list of registers that we can use + for pushes. */ + if (!return_addr_live) + pushable_regs |= 1 << LR_REGNUM; } insn = thumb1_emit_multi_reg_push (push_mask, real_regs_mask); diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 3042f825ff8..ca9d2055a06 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,10 @@ +2019-05-08 Mihail Ionescu + Richard Earnshaw + + PR target/88167 + * gcc.target/arm/pr88167-1.c: New test. + * gcc.target/arm/pr88167-2.c: New test. + 2018-05-08 Bin Cheng PR tree-optimization/90078 diff --git a/gcc/testsuite/gcc.target/arm/pr88167-1.c b/gcc/testsuite/gcc.target/arm/pr88167-1.c new file mode 100644 index 00000000000..517a86d6e4b --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pr88167-1.c @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-require-effective-target arm_thumb1_ok } */ +/* { dg-options "-O2 -mthumb" } */ + +void *retaddr; + +void foo (void) { + retaddr = __builtin_return_address (0); + + /* Used for enforcing registers stacking. */ + asm volatile ("" : : : "r0", "r1", "r2", "r3", "r4", "r5", "r6", "r7", + "r8", "r9", "r10", "r11", "r12"); +} + +/* { dg-final { scan-assembler-not "mov\tlr," } } */ diff --git a/gcc/testsuite/gcc.target/arm/pr88167-2.c b/gcc/testsuite/gcc.target/arm/pr88167-2.c new file mode 100644 index 00000000000..6a303345eb9 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/pr88167-2.c @@ -0,0 +1,18 @@ +/* { dg-do run } */ +/* { dg-options "-O2" } */ +/* { dg-skip-if "" { ! { arm_thumb1 } } } */ + +int __attribute__((noclone, noinline)) +foo (int a, long long b) { + /* Used for enforcing registers stacking. */ + asm volatile ("" : : : "r0", "r1", "r2", "r3", + "r8", "r9", "r10", "r11", "r12"); + return (int) b; +} + +int main () +{ + if (foo (1, 0x1000000000000003LL) != 3) + __builtin_abort (); + __builtin_exit (0); +} -- 2.30.2