From 03f82a6a634ddfa5828843b647c896968aee3702 Mon Sep 17 00:00:00 2001 From: Iain Sandoe Date: Sun, 27 Nov 2016 14:50:58 +0000 Subject: [PATCH] [Darwin] Fix PR57438 by avoiding empty function bodies and trailing labels. A. Empty function bodies causes two problems for Darwin's linker (i) zero-length FDEs and (ii) coincident label addresses that might point to items of differing weakness. B. Trailing local labels can be problematic when they end a function because similarly they might apparently point to a following weak function, leading to the linker concluding that there's a pointer-diff to a weak symbol (which is not allowed). Both conditions arise from __builtin_unreachable() lowering to a barrier. The solution for both is to emit some finite amount of code; in the case of A a trap is emitted, in the case of B a nop. gcc/ 2016-11-27 Iain Sandoe PR target/57438 * config/i386/i386.c (ix86_code_end): Note that we emitted code where the function might otherwise appear empty for picbase thunks. (ix86_output_function_epilogue): If we find a zero-sized function assume that reaching it is UB and trap. If we find a trailing label append a nop. * config/rs6000/rs6000.c (rs6000_output_function_epilogue): If we find a zero-sized function assume that reaching it is UB and trap. If we find a trailing label, append a nop. gcc/testsuite/ 2016-11-27 Iain Sandoe PR target/57438 * gcc.dg/pr57438-1.c: New Test. * gcc.dg/pr57438-2.c: New Test. From-SVN: r242897 --- gcc/ChangeLog | 12 +++++ gcc/config/i386/i386.c | 91 ++++++++++++++++++++++---------- gcc/config/rs6000/rs6000.c | 45 +++++++++++++--- gcc/testsuite/ChangeLog | 6 +++ gcc/testsuite/gcc.dg/pr57438-1.c | 16 ++++++ gcc/testsuite/gcc.dg/pr57438-2.c | 23 ++++++++ 6 files changed, 159 insertions(+), 34 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/pr57438-1.c create mode 100644 gcc/testsuite/gcc.dg/pr57438-2.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index ba2526c99c3..a57eaab8b20 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,15 @@ +2016-11-27 Iain Sandoe + + PR target/57438 + * config/i386/i386.c (ix86_code_end): Note that we emitted code + where the function might otherwise appear empty for picbase thunks. + (ix86_output_function_epilogue): If we find a zero-sized function + assume that reaching it is UB and trap. If we find a trailing label + append a nop. + * config/rs6000/rs6000.c (rs6000_output_function_epilogue): If we + find a zero-sized function assume that reaching it is UB and trap. + If we find a trailing label, append a nop. + 2016-11-27 Iain Sandoe PR target/71767 diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 1dd5669bfea..5018ccb633a 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -11920,6 +11920,9 @@ ix86_code_end (void) current_function_decl = decl; allocate_struct_function (decl, false); init_function_start (decl); + /* We're about to hide the function body from callees of final_* by + emitting it directly; tell them we're a thunk, if they care. */ + cfun->is_thunk = true; first_function_block_is_cold = false; /* Make sure unwind info is emitted for the thunk if needed. */ final_start_function (emit_barrier (), asm_out_file, 1); @@ -14625,36 +14628,68 @@ ix86_output_function_epilogue (FILE *file ATTRIBUTE_UNUSED, HOST_WIDE_INT) if (pic_offset_table_rtx && !ix86_use_pseudo_pic_reg ()) SET_REGNO (pic_offset_table_rtx, REAL_PIC_OFFSET_TABLE_REGNUM); -#if TARGET_MACHO - /* Mach-O doesn't support labels at the end of objects, so if - it looks like we might want one, insert a NOP. */ - { - rtx_insn *insn = get_last_insn (); - rtx_insn *deleted_debug_label = NULL; - while (insn - && NOTE_P (insn) - && NOTE_KIND (insn) != NOTE_INSN_DELETED_LABEL) - { - /* Don't insert a nop for NOTE_INSN_DELETED_DEBUG_LABEL - notes only, instead set their CODE_LABEL_NUMBER to -1, - otherwise there would be code generation differences - in between -g and -g0. */ - if (NOTE_P (insn) && NOTE_KIND (insn) == NOTE_INSN_DELETED_DEBUG_LABEL) - deleted_debug_label = insn; + + if (TARGET_MACHO) + { + rtx_insn *insn = get_last_insn (); + rtx_insn *deleted_debug_label = NULL; + + /* Mach-O doesn't support labels at the end of objects, so if + it looks like we might want one, take special action. + First, collect any sequence of deleted debug labels. */ + while (insn + && NOTE_P (insn) + && NOTE_KIND (insn) != NOTE_INSN_DELETED_LABEL) + { + /* Don't insert a nop for NOTE_INSN_DELETED_DEBUG_LABEL + notes only, instead set their CODE_LABEL_NUMBER to -1, + otherwise there would be code generation differences + in between -g and -g0. */ + if (NOTE_P (insn) && NOTE_KIND (insn) + == NOTE_INSN_DELETED_DEBUG_LABEL) + deleted_debug_label = insn; + insn = PREV_INSN (insn); + } + + /* If we have: + label: + barrier + then this needs to be detected, so skip past the barrier. */ + + if (insn && BARRIER_P (insn)) insn = PREV_INSN (insn); - } - if (insn - && (LABEL_P (insn) - || (NOTE_P (insn) - && NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL))) - fputs ("\tnop\n", file); - else if (deleted_debug_label) - for (insn = deleted_debug_label; insn; insn = NEXT_INSN (insn)) - if (NOTE_KIND (insn) == NOTE_INSN_DELETED_DEBUG_LABEL) - CODE_LABEL_NUMBER (insn) = -1; - } -#endif + /* Up to now we've only seen notes or barriers. */ + if (insn) + { + if (LABEL_P (insn) + || (NOTE_P (insn) + && NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL)) + /* Trailing label. */ + fputs ("\tnop\n", file); + else if (cfun && ! cfun->is_thunk) + { + /* See if we have a completely empty function body, skipping + the special case of the picbase thunk emitted as asm. */ + while (insn && ! INSN_P (insn)) + insn = PREV_INSN (insn); + /* If we don't find any insns, we've got an empty function body; + I.e. completely empty - without a return or branch. This is + taken as the case where a function body has been removed + because it contains an inline __builtin_unreachable(). GCC + declares that reaching __builtin_unreachable() means UB so + we're not obliged to do anything special; however, we want + non-zero-sized function bodies. To meet this, and help the + user out, let's trap the case. */ + if (insn == NULL) + fputs ("\tud2\n", file); + } + } + else if (deleted_debug_label) + for (insn = deleted_debug_label; insn; insn = NEXT_INSN (insn)) + if (NOTE_KIND (insn) == NOTE_INSN_DELETED_DEBUG_LABEL) + CODE_LABEL_NUMBER (insn) = -1; + } } /* Return a scratch register to use in the split stack prologue. The diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c index fce4e39128a..6c28e6aaf65 100644 --- a/gcc/config/rs6000/rs6000.c +++ b/gcc/config/rs6000/rs6000.c @@ -30234,11 +30234,15 @@ rs6000_output_function_epilogue (FILE *file, { #if TARGET_MACHO macho_branch_islands (); - /* Mach-O doesn't support labels at the end of objects, so if - it looks like we might want one, insert a NOP. */ + { rtx_insn *insn = get_last_insn (); rtx_insn *deleted_debug_label = NULL; + + /* Mach-O doesn't support labels at the end of objects, so if + it looks like we might want one, take special action. + + First, collect any sequence of deleted debug labels. */ while (insn && NOTE_P (insn) && NOTE_KIND (insn) != NOTE_INSN_DELETED_LABEL) @@ -30251,11 +30255,40 @@ rs6000_output_function_epilogue (FILE *file, deleted_debug_label = insn; insn = PREV_INSN (insn); } - if (insn - && (LABEL_P (insn) + + /* Second, if we have: + label: + barrier + then this needs to be detected, so skip past the barrier. */ + + if (insn && BARRIER_P (insn)) + insn = PREV_INSN (insn); + + /* Up to now we've only seen notes or barriers. */ + if (insn) + { + if (LABEL_P (insn) || (NOTE_P (insn) - && NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL))) - fputs ("\tnop\n", file); + && NOTE_KIND (insn) == NOTE_INSN_DELETED_LABEL)) + /* Trailing label: . */ + fputs ("\tnop\n", file); + else + { + /* Lastly, see if we have a completely empty function body. */ + while (insn && ! INSN_P (insn)) + insn = PREV_INSN (insn); + /* If we don't find any insns, we've got an empty function body; + I.e. completely empty - without a return or branch. This is + taken as the case where a function body has been removed + because it contains an inline __builtin_unreachable(). GCC + states that reaching __builtin_unreachable() means UB so we're + not obliged to do anything special; however, we want + non-zero-sized function bodies. To meet this, and help the + user out, let's trap the case. */ + if (insn == NULL) + fputs ("\ttrap\n", file); + } + } else if (deleted_debug_label) for (insn = deleted_debug_label; insn; insn = NEXT_INSN (insn)) if (NOTE_KIND (insn) == NOTE_INSN_DELETED_DEBUG_LABEL) diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index bd2d5405ba7..d7eacc3543e 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2016-11-27 Iain Sandoe + + PR target/57438 + * gcc.dg/pr57438-1.c: New Test. + * gcc.dg/pr57438-2.c: New Test. + 2016-11-27 Dominique d'Humieres Iain Sandoe diff --git a/gcc/testsuite/gcc.dg/pr57438-1.c b/gcc/testsuite/gcc.dg/pr57438-1.c new file mode 100644 index 00000000000..9bfd8b9d911 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr57438-1.c @@ -0,0 +1,16 @@ +/* { dg-do compile { target *-*-darwin* } } */ +/* { dg-options "-O1" } */ +/* { dg-additional-options "-mdynamic-no-pic" { target powerpc*-*-darwin* } } + +/* This is testing that a completely empty function body results in the + insertion of a ud2/trap instruction to prevent a zero-sized FDE, and/or + the function label apparently pointing to following code. */ + +__attribute__((noinline)) +void foo (void) +{ + __builtin_unreachable(); +} + +/* { dg-final { scan-assembler "ud2" { target { i?86-*-darwin* x86_64-*-darwin* } } } } */ +/* { dg-final { scan-assembler "trap" { target { powerpc*-*-darwin* } } } } */ diff --git a/gcc/testsuite/gcc.dg/pr57438-2.c b/gcc/testsuite/gcc.dg/pr57438-2.c new file mode 100644 index 00000000000..f3ff1dc1ed2 --- /dev/null +++ b/gcc/testsuite/gcc.dg/pr57438-2.c @@ -0,0 +1,23 @@ +/* { dg-do compile { target *-*-darwin* } } */ +/* { dg-options "--param case-values-threshold=3 -O2" } */ +/* { dg-additional-options "-funwind-tables" { target powerpc*-*-darwin* } } + +/* This is testing that a trailing local label is followed by a + nop where required. */ + +int foo (int x) +{ + switch (x) + { + case 0: + return 10; + case 3: + return -1; + case 5: + return 29; + default: + __builtin_unreachable(); + } +} + +/* { dg-final { scan-assembler "nop\\nLFE.*" { target { *-*-darwin* } } } } */ -- 2.30.2