[PATCH] PR880088 Enable -Wtrampolines for no-exec-stack targets with -Wall.

[PATCH] PR880088 Enable -Wtrampolines for no-exec-stack targets with -Wall.

[Mark Wielaard]

With -Wtrampolines a warning is produced whenever gcc generates executable
code on the stack at runtime to support taking a nested function address
that is used to call the nested function indirectly when it needs to access
any variables in its lexical scope.

As a result the stack has to be marked as executable even for targets which
have a non-executable stack as default.

Define a new target macro TARGET_HAS_DEFAULT_NOEXEC_STACK for those targets
that have a non-executable default stack based on when they call
file_end_indicate_exec_stack.

Add -Wtrampolines to -Wall for those targets and update the documentation
to better explain when -Wtrampolines will warn.

gcc/ChangeLog

    PR c/880088
    * common.opt (Wtrampolines): Improve documentation.
    * doc/invoke.texi (-Wtrampolines): Likewise.
    * doc/tm.texi.in (TARGET_HAS_DEFAULT_NOEXEC_STACK): Document macro.
    * doc/tm.texi: Regenerate.
    * defaults.c (TARGET_HAS_DEFAULT_NOEXEC_STACK): Define to zero if
    not defined yet.
    * config/aarch64/aarch64-freebsd.h (TARGET_HAS_DEFAULT_NOEXEC_STACK):
    Define to 1.
    * config/aarch64/aarch64-linux.h (TARGET_HAS_DEFAULT_NOEXEC_STACK):
    Likewise.
    * config/alpha/linux-elf.h (TARGET_HAS_DEFAULT_NOEXEC_STACK):
    Likewise.
    * config/arc/elf.h (TARGET_HAS_DEFAULT_NOEXEC_STACK): Likewise.
    * config/arc/linux.h (TARGET_HAS_DEFAULT_NOEXEC_STACK): Likewise.
    * config/arm/arm.c (TARGET_HAS_DEFAULT_NOEXEC_STACK): Define to
    NEED_INDICATE_EXEC_STACK.
    * config/cris/cris.c (TARGET_HAS_DEFAULT_NOEXEC_STACK): Define to
    TARGET_LINUX.
    * config/i386/dragonfly.h (TARGET_HAS_DEFAULT_NOEXEC_STACK):
    Likewise.
    * config/i386/freebsd.h (TARGET_HAS_DEFAULT_NOEXEC_STACK): Likewise.
    * config/i386/gnu-user-common.h (TARGET_HAS_DEFAULT_NOEXEC_STACK):
    Likewise.
    * config/i386/linux-common.h (TARGET_HAS_DEFAULT_NOEXEC_STACK):
    Likewise.
    * config/m32r/linux.h (TARGET_HAS_DEFAULT_NOEXEC_STACK): Likewise.
    * config/m68k/linux.h (TARGET_HAS_DEFAULT_NOEXEC_STACK): Likewise.
    * config/microblaze/linux.h (TARGET_HAS_DEFAULT_NOEXEC_STACK):
    Likewise.
    * config/nds32/nds32.c (TARGET_HAS_DEFAULT_NOEXEC_STACK): Define to
    TARGET_LINUX_ABI.
    * config/nios2/nios2.c (TARGET_HAS_DEFAULT_NOEXEC_STACK): Likewise.
    * config/pa/pa.c (TARGET_HAS_DEFAULT_NOEXEC_STACK): Define to
    NEED_INDICATE_EXEC_STACK.
    * config/powerpcspe/sysv4.h (TARGET_HAS_DEFAULT_NOEXEC_STACK): Define
    to TARGET_32BIT || DEFAULT_ABI == ABI_ELFv2 for POWERPC_LINUX ||
    POWERPC_FREEBSD.
    * config/rs6000/sysv4.h (TARGET_HAS_DEFAULT_NOEXEC_STACK): Likewise.
    * config/riscv/linux.h (TARGET_HAS_DEFAULT_NOEXEC_STACK): Define to 1.
    * config/s390/linux.h (TARGET_HAS_DEFAULT_NOEXEC_STACK): Likewise.
    * config/s390/s390.c (TARGET_HAS_DEFAULT_NOEXEC_STACK): Likewise.
    * config/sh/linux.h (TARGET_HAS_DEFAULT_NOEXEC_STACK): Likewise.
    * config/sparc/sparc.c (TARGET_HAS_DEFAULT_NOEXEC_STACK): Define to
    NEED_INDICATE_EXEC_STACK.
    * config/tilegx/tilegx.c (TARGET_HAS_DEFAULT_NOEXEC_STACK): Likewise.
    * config/tilepro/tilepro.c (TARGET_HAS_DEFAULT_NOEXEC_STACK):
    Likewise.

gcc/c-family/ChangeLog

    PR c/880088
    * c.opt (Wtrampolines): Enable by -Wall when
    TARGET_HAS_DEFAULT_NOEXEC_STACK.
---
 gcc/ChangeLog                        | 51 ++++++++++++++++++++++++++++++++++++
 gcc/c-family/ChangeLog               |  6 +++++
 gcc/c-family/c.opt                   |  3 +++
 gcc/common.opt                       |  2 +-
 gcc/config/aarch64/aarch64-freebsd.h |  1 +
 gcc/config/aarch64/aarch64-linux.h   |  1 +
 gcc/config/alpha/linux-elf.h         |  1 +
 gcc/config/arc/elf.h                 |  3 +++
 gcc/config/arc/linux.h               |  3 +++
 gcc/config/arm/arm.c                 |  3 +++
 gcc/config/cris/cris.c               |  2 ++
 gcc/config/i386/dragonfly.h          |  1 +
 gcc/config/i386/freebsd.h            |  1 +
 gcc/config/i386/gnu-user-common.h    |  2 ++
 gcc/config/i386/linux-common.h       |  3 +++
 gcc/config/m32r/linux.h              |  1 +
 gcc/config/m68k/linux.h              |  1 +
 gcc/config/microblaze/linux.h        |  1 +
 gcc/config/nds32/nds32.c             |  3 +++
 gcc/config/nios2/nios2.c             |  3 +++
 gcc/config/pa/pa.c                   |  3 +++
 gcc/config/powerpcspe/sysv4.h        |  5 ++++
 gcc/config/riscv/linux.h             |  1 +
 gcc/config/rs6000/sysv4.h            |  5 ++++
 gcc/config/s390/linux.h              |  1 +
 gcc/config/s390/s390.c               |  3 +++
 gcc/config/sh/linux.h                |  1 +
 gcc/config/sparc/sparc.c             |  3 +++
 gcc/config/tilegx/tilegx.c           |  3 +++
 gcc/config/tilepro/tilepro.c         |  3 +++
 gcc/defaults.h                       |  6 +++++
 gcc/doc/invoke.texi                  | 20 +++++++++-----
 gcc/doc/tm.texi                      | 11 ++++++++
 gcc/doc/tm.texi.in                   | 11 ++++++++
 34 files changed, 160 insertions(+), 8 deletions(-)

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 6f88a10..30e13c6 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -806,6 +806,9 @@ Wtemplates
 C++ ObjC++ Var(warn_templates) Warning
 Warn on primary template declaration.
 
+Wtrampolines
+C Var(warn_trampolines) Warning LangEnabledBy(C,Wall,TARGET_HAS_DEFAULT_NOEXEC_STACK,0)
+
 Wmissing-attributes
 C ObjC C++ ObjC++ Var(warn_missing_attributes) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
 Warn about declarations of entities that may be missing attributes
diff --git a/gcc/common.opt b/gcc/common.opt
index 72a7135..0459b8d 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -765,7 +765,7 @@ Do not suppress warnings from system headers.
 
 Wtrampolines
 Common Var(warn_trampolines) Warning
-Warn whenever a trampoline is generated.
+Warn whenever an executable trampoline is generated on the stack.
 
 Wtype-limits
 Common Var(warn_type_limits) Warning EnabledBy(Wextra)
diff --git a/gcc/config/aarch64/aarch64-freebsd.h b/gcc/config/aarch64/aarch64-freebsd.h
index d0d8bc4..41f84f1 100644
--- a/gcc/config/aarch64/aarch64-freebsd.h
+++ b/gcc/config/aarch64/aarch64-freebsd.h
@@ -83,6 +83,7 @@
   }                                           \
   while (false)
 
+#define TARGET_HAS_DEFAULT_NOEXEC_STACK 1
 #define TARGET_ASM_FILE_END file_end_indicate_exec_stack
 
 /* Uninitialized common symbols in non-PIE executables, even with
diff --git a/gcc/config/aarch64/aarch64-linux.h b/gcc/config/aarch64/aarch64-linux.h
index 2bee7e5..3ee37f3 100644
--- a/gcc/config/aarch64/aarch64-linux.h
+++ b/gcc/config/aarch64/aarch64-linux.h
@@ -83,6 +83,7 @@
 
 #define GNU_USER_TARGET_D_CRITSEC_SIZE 48
 
+#define TARGET_HAS_DEFAULT_NOEXEC_STACK 1
 #define TARGET_ASM_FILE_END file_end_indicate_exec_stack
 
 /* Uninitialized common symbols in non-PIE executables, even with
diff --git a/gcc/config/alpha/linux-elf.h b/gcc/config/alpha/linux-elf.h
index 36b74dc..0a99466 100644
--- a/gcc/config/alpha/linux-elf.h
+++ b/gcc/config/alpha/linux-elf.h
@@ -50,4 +50,5 @@ along with GCC; see the file COPYING3.  If not see
 #define LIB_SPEC \
 "%{pthread:-lpthread} %{shared:-lc}%{!shared:%{profile:-lc_p}%{!profile:-lc}} "
 
+#define TARGET_HAS_DEFAULT_NOEXEC_STACK 1
 #define TARGET_ASM_FILE_END file_end_indicate_exec_stack
diff --git a/gcc/config/arc/elf.h b/gcc/config/arc/elf.h
index 3aabcf8..e8276a3 100644
--- a/gcc/config/arc/elf.h
+++ b/gcc/config/arc/elf.h
@@ -71,6 +71,9 @@ along with GCC; see the file COPYING3.  If not see
 #undef ATTRIBUTE_PCS
 #define ATTRIBUTE_PCS 2
 
+#undef TARGET_HAS_DEFAULT_NOEXEC_STACK
+#define TARGET_HAS_DEFAULT_NOEXEC_STACK 1
+
 #undef TARGET_ASM_FILE_END
 #define TARGET_ASM_FILE_END arc_file_end
 
diff --git a/gcc/config/arc/linux.h b/gcc/config/arc/linux.h
index 993f445..c27af3d 100644
--- a/gcc/config/arc/linux.h
+++ b/gcc/config/arc/linux.h
@@ -61,6 +61,9 @@ along with GCC; see the file COPYING3.  If not see
    %{shared:-lc} \
    %{!shared:%{profile:-lc_p}%{!profile:-lc}}"
 
+#undef TARGET_HAS_DEFAULT_NOEXEC_STACK
+#define TARGET_HAS_DEFAULT_NOEXEC_STACK 1
+
 #undef TARGET_ASM_FILE_END
 #define TARGET_ASM_FILE_END file_end_indicate_exec_stack
 
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 40f0574..3083a28 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -393,6 +393,9 @@ static const struct attribute_spec arm_attribute_table[] =
 #undef  TARGET_INSERT_ATTRIBUTES
 #define TARGET_INSERT_ATTRIBUTES arm_insert_attributes
 
+#undef TARGET_HAS_DEFAULT_NOEXEC_STACK
+#define TARGET_HAS_DEFAULT_NOEXEC_STACK NEED_INDICATE_EXEC_STACK
+
 #undef TARGET_ASM_FILE_START
 #define TARGET_ASM_FILE_START arm_file_start
 #undef TARGET_ASM_FILE_END
diff --git a/gcc/config/cris/cris.c b/gcc/config/cris/cris.c
index c652cb3..d77475b 100644
--- a/gcc/config/cris/cris.c
+++ b/gcc/config/cris/cris.c
@@ -213,6 +213,8 @@ int cris_cpu_version = CRIS_DEFAULT_CPU_VERSION;
 #undef TARGET_ASM_CAN_OUTPUT_MI_THUNK
 #define TARGET_ASM_CAN_OUTPUT_MI_THUNK default_can_output_mi_thunk_no_vcall
 
+#define TARGET_HAS_DEFAULT_NOEXEC_STACK TARGET_LINUX
+
 #undef TARGET_ASM_FILE_START
 #define TARGET_ASM_FILE_START cris_file_start
 #undef TARGET_ASM_FILE_END
diff --git a/gcc/config/i386/dragonfly.h b/gcc/config/i386/dragonfly.h
index 40774c0..9430745 100644
--- a/gcc/config/i386/dragonfly.h
+++ b/gcc/config/i386/dragonfly.h
@@ -97,4 +97,5 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 /* Support for i386 was removed from DragonFly in 2007  */
 #define SUBTARGET32_DEFAULT_CPU "i486"
 
+#define TARGET_HAS_DEFAULT_NOEXEC_STACK 1
 #define TARGET_ASM_FILE_END file_end_indicate_exec_stack
diff --git a/gcc/config/i386/freebsd.h b/gcc/config/i386/freebsd.h
index caac6b3..7e58459 100644
--- a/gcc/config/i386/freebsd.h
+++ b/gcc/config/i386/freebsd.h
@@ -127,5 +127,6 @@ along with GCC; see the file COPYING3.  If not see
 
 #define SUBTARGET32_DEFAULT_CPU "i586"
 
+#define TARGET_HAS_DEFAULT_NOEXEC_STACK 1
 #define TARGET_ASM_FILE_END file_end_indicate_exec_stack
 
diff --git a/gcc/config/i386/gnu-user-common.h b/gcc/config/i386/gnu-user-common.h
index dc66daa..21d2789 100644
--- a/gcc/config/i386/gnu-user-common.h
+++ b/gcc/config/i386/gnu-user-common.h
@@ -57,6 +57,8 @@ along with GCC; see the file COPYING3.  If not see
   GNU_USER_TARGET_MATHFILE_SPEC " " \
   GNU_USER_TARGET_ENDFILE_SPEC
 
+#define TARGET_HAS_DEFAULT_NOEXEC_STACK 1
+
 #define TARGET_ASM_FILE_END file_end_indicate_exec_stack
 
 /* The stack pointer needs to be moved while checking the stack.  */
diff --git a/gcc/config/i386/linux-common.h b/gcc/config/i386/linux-common.h
index 1e7a055..b1748df 100644
--- a/gcc/config/i386/linux-common.h
+++ b/gcc/config/i386/linux-common.h
@@ -70,5 +70,8 @@ along with GCC; see the file COPYING3.  If not see
 
 extern void file_end_indicate_exec_stack_and_cet (void);
 
+#undef TARGET_HAS_DEFAULT_NOEXEC_STACK
+#define TARGET_HAS_DEFAULT_NOEXEC_STACK 1
+
 #undef TARGET_ASM_FILE_END
 #define TARGET_ASM_FILE_END file_end_indicate_exec_stack_and_cet
diff --git a/gcc/config/m32r/linux.h b/gcc/config/m32r/linux.h
index d65abe7..e1fe66d 100644
--- a/gcc/config/m32r/linux.h
+++ b/gcc/config/m32r/linux.h
@@ -88,4 +88,5 @@
                                                                                 
 #define TARGET_OS_CPP_BUILTINS() GNU_USER_TARGET_OS_CPP_BUILTINS()
 
+#define TARGET_HAS_DEFAULT_NOEXEC_STACK 1
 #define TARGET_ASM_FILE_END file_end_indicate_exec_stack
diff --git a/gcc/config/m68k/linux.h b/gcc/config/m68k/linux.h
index f584d19..2b599aa 100644
--- a/gcc/config/m68k/linux.h
+++ b/gcc/config/m68k/linux.h
@@ -223,6 +223,7 @@ along with GCC; see the file COPYING3.  If not see
      : "%d0", "%d2", "%d3");						\
 }
 
+#define TARGET_HAS_DEFAULT_NOEXEC_STACK 1
 #define TARGET_ASM_FILE_END file_end_indicate_exec_stack
 
 #undef DBX_REGISTER_NUMBER
diff --git a/gcc/config/microblaze/linux.h b/gcc/config/microblaze/linux.h
index d505424..8e7a45b 100644
--- a/gcc/config/microblaze/linux.h
+++ b/gcc/config/microblaze/linux.h
@@ -58,4 +58,5 @@
 #undef TARGET_OS_CPP_BUILTINS
 #define TARGET_OS_CPP_BUILTINS() GNU_USER_TARGET_OS_CPP_BUILTINS()
 
+#define TARGET_HAS_DEFAULT_NOEXEC_STACK 1
 #define TARGET_ASM_FILE_END file_end_indicate_exec_stack
diff --git a/gcc/config/nds32/nds32.c b/gcc/config/nds32/nds32.c
index 1ae34fc..b87d143 100644
--- a/gcc/config/nds32/nds32.c
+++ b/gcc/config/nds32/nds32.c
@@ -5756,6 +5756,9 @@ nds32_use_blocks_for_constant_p (machine_mode mode,
 
 /* -- The Overall Framework of an Assembler File.  */
 
+#undef TARGET_HAS_DEFAULT_NOEXEC_STACK
+#define TARGET_HAS_DEFAULT_NOEXEC_STACK TARGET_LINUX_ABI
+
 #undef TARGET_ASM_FILE_START
 #define TARGET_ASM_FILE_START nds32_asm_file_start
 #undef TARGET_ASM_FILE_END
diff --git a/gcc/config/nios2/nios2.c b/gcc/config/nios2/nios2.c
index e266924..7870979 100644
--- a/gcc/config/nios2/nios2.c
+++ b/gcc/config/nios2/nios2.c
@@ -5548,6 +5548,9 @@ nios2_adjust_reg_alloc_order (void)
 #undef TARGET_ASM_OUTPUT_ADDR_CONST_EXTRA
 #define TARGET_ASM_OUTPUT_ADDR_CONST_EXTRA nios2_output_addr_const_extra
 
+#undef TARGET_HAS_DEFAULT_NOEXEC_STACK
+#define TARGET_HAS_DEFAULT_NOEXEC_STACK TARGET_LINUX_ABI
+
 #undef TARGET_ASM_FILE_END
 #define TARGET_ASM_FILE_END nios2_asm_file_end
 
diff --git a/gcc/config/pa/pa.c b/gcc/config/pa/pa.c
index 2bf48e4..22f67aa 100644
--- a/gcc/config/pa/pa.c
+++ b/gcc/config/pa/pa.c
@@ -298,6 +298,9 @@ static size_t n_deferred_plabels = 0;
 #undef TARGET_ASM_CAN_OUTPUT_MI_THUNK
 #define TARGET_ASM_CAN_OUTPUT_MI_THUNK default_can_output_mi_thunk_no_vcall
 
+#undef TARGET_HAS_DEFAULT_NOEXEC_STACK
+#define TARGET_HAS_DEFAULT_NOEXEC_STACK NEED_INDICATE_EXEC_STACK
+
 #undef TARGET_ASM_FILE_END
 #define TARGET_ASM_FILE_END pa_file_end
 
diff --git a/gcc/config/powerpcspe/sysv4.h b/gcc/config/powerpcspe/sysv4.h
index 756e9e5..0eb7b48 100644
--- a/gcc/config/powerpcspe/sysv4.h
+++ b/gcc/config/powerpcspe/sysv4.h
@@ -964,6 +964,11 @@ ncrtn.o%s"
 /* Generate entries in .fixup for relocatable addresses.  */
 #define RELOCATABLE_NEEDS_FIXUP 1
 
+#if defined (POWERPC_LINUX) || defined (POWERPC_FREEBSD)
+  #define TARGET_HAS_DEFAULT_NOEXEC_STACK (TARGET_32BIT \
+					   || DEFAULT_ABI == ABI_ELFv2)
+#endif
+
 #define TARGET_ASM_FILE_END rs6000_elf_file_end
 
 #undef TARGET_ASAN_SHADOW_OFFSET
diff --git a/gcc/config/riscv/linux.h b/gcc/config/riscv/linux.h
index e208c95..e679cf8 100644
--- a/gcc/config/riscv/linux.h
+++ b/gcc/config/riscv/linux.h
@@ -67,4 +67,5 @@ along with GCC; see the file COPYING3.  If not see
       -dynamic-linker " GNU_USER_DYNAMIC_LINKER "} \
     %{static:-static}}"
 
+#define TARGET_HAS_DEFAULT_NOEXEC_STACK 1
 #define TARGET_ASM_FILE_END file_end_indicate_exec_stack
diff --git a/gcc/config/rs6000/sysv4.h b/gcc/config/rs6000/sysv4.h
index 0c67634..9330acf 100644
--- a/gcc/config/rs6000/sysv4.h
+++ b/gcc/config/rs6000/sysv4.h
@@ -972,6 +972,11 @@ ncrtn.o%s"
 /* Generate entries in .fixup for relocatable addresses.  */
 #define RELOCATABLE_NEEDS_FIXUP 1
 
+#if defined (POWERPC_LINUX) || defined (POWERPC_FREEBSD)
+  #define TARGET_HAS_DEFAULT_NOEXEC_STACK (TARGET_32BIT \
+					   || DEFAULT_ABI == ABI_ELFv2)
+#endif
+
 #define TARGET_ASM_FILE_END rs6000_elf_file_end
 
 #undef TARGET_ASAN_SHADOW_OFFSET
diff --git a/gcc/config/s390/linux.h b/gcc/config/s390/linux.h
index 480030a..0125488 100644
--- a/gcc/config/s390/linux.h
+++ b/gcc/config/s390/linux.h
@@ -89,6 +89,7 @@ along with GCC; see the file COPYING3.  If not see
 
 #define CPP_SPEC "%{posix:-D_POSIX_SOURCE} %{pthread:-D_REENTRANT}"
 
+#define TARGET_HAS_DEFAULT_NOEXEC_STACK 1
 #define TARGET_ASM_FILE_END file_end_indicate_exec_stack
 
 #ifdef TARGET_LIBC_PROVIDES_SSP
diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 277d555..048a546 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -16309,6 +16309,9 @@ s390_case_values_threshold (void)
 #define TARGET_ASM_FILE_START s390_asm_file_start
 #endif
 
+#undef TARGET_HAS_DEFAULT_NOEXEC_STACK
+#define TARGET_HAS_DEFAULT_NOEXEC_STACK 1
+
 #undef TARGET_ASM_FILE_END
 #define TARGET_ASM_FILE_END s390_asm_file_end
 
diff --git a/gcc/config/sh/linux.h b/gcc/config/sh/linux.h
index 6d2ccd0..f813ae9 100644
--- a/gcc/config/sh/linux.h
+++ b/gcc/config/sh/linux.h
@@ -41,6 +41,7 @@ along with GCC; see the file COPYING3.  If not see
 #define TARGET_DEFAULT \
   (TARGET_CPU_DEFAULT | TARGET_ENDIAN_DEFAULT | TARGET_OPT_DEFAULT)
 
+#define TARGET_HAS_DEFAULT_NOEXEC_STACK 1
 #define TARGET_ASM_FILE_END file_end_indicate_exec_stack
 
 #if TARGET_ENDIAN_DEFAULT == MASK_LITTLE_ENDIAN
diff --git a/gcc/config/sparc/sparc.c b/gcc/config/sparc/sparc.c
index dcdaef2..8a721ce 100644
--- a/gcc/config/sparc/sparc.c
+++ b/gcc/config/sparc/sparc.c
@@ -877,6 +877,9 @@ char sparc_hard_reg_printed[8];
 #define TARGET_ASM_OUTPUT_DWARF_DTPREL sparc_output_dwarf_dtprel
 #endif
 
+#undef TARGET_HAS_DEFAULT_NOEXEC_STACK
+#define TARGET_HAS_DEFAULT_NOEXEC_STACK NEED_INDICATE_EXEC_STACK
+
 #undef TARGET_ASM_FILE_END
 #define TARGET_ASM_FILE_END sparc_file_end
 
diff --git a/gcc/config/tilegx/tilegx.c b/gcc/config/tilegx/tilegx.c
index 14d8171..b258ca6 100644
--- a/gcc/config/tilegx/tilegx.c
+++ b/gcc/config/tilegx/tilegx.c
@@ -5724,6 +5724,9 @@ tilegx_truly_noop_truncation (poly_uint64 outprec, poly_uint64 inprec)
 #undef  TARGET_PRINT_OPERAND_ADDRESS
 #define TARGET_PRINT_OPERAND_ADDRESS tilegx_print_operand_address
 
+#undef TARGET_HAS_DEFAULT_NOEXEC_STACK
+#define TARGET_HAS_DEFAULT_NOEXEC_STACK NEED_INDICATE_EXEC_STACK
+
 #undef  TARGET_ASM_FILE_END
 #define TARGET_ASM_FILE_END tilegx_file_end
 
diff --git a/gcc/config/tilepro/tilepro.c b/gcc/config/tilepro/tilepro.c
index 65380b5..22c6a63 100644
--- a/gcc/config/tilepro/tilepro.c
+++ b/gcc/config/tilepro/tilepro.c
@@ -5087,6 +5087,9 @@ tilepro_file_end (void)
 #undef  TARGET_PRINT_OPERAND_ADDRESS
 #define TARGET_PRINT_OPERAND_ADDRESS tilepro_print_operand_address
 
+#undef TARGET_HAS_DEFAULT_NOEXEC_STACK
+#define TARGET_HAS_DEFAULT_NOEXEC_STACK NEED_INDICATE_EXEC_STACK
+
 #undef  TARGET_ASM_FILE_END
 #define TARGET_ASM_FILE_END tilepro_file_end
 
diff --git a/gcc/defaults.h b/gcc/defaults.h
index 9035b33..116fda7 100644
--- a/gcc/defaults.h
+++ b/gcc/defaults.h
@@ -1455,4 +1455,10 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 #define DWARF_GNAT_ENCODINGS_DEFAULT DWARF_GNAT_ENCODINGS_GDB
 #endif
 
+/* Whether the target has a non executable stack by default.
+   Used to enable warnings with -Wall that might change the default.  */
+#ifndef TARGET_HAS_DEFAULT_NOEXEC_STACK
+#define TARGET_HAS_DEFAULT_NOEXEC_STACK 0
+#endif
+
 #endif  /* ! GCC_DEFAULTS_H */
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 38e27a5..591a8ec 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -6221,13 +6221,19 @@ This warning is enabled by @option{-Wall}.
 @item -Wtrampolines
 @opindex Wtrampolines
 @opindex Wno-trampolines
-Warn about trampolines generated for pointers to nested functions.
-A trampoline is a small piece of data or code that is created at run
-time on the stack when the address of a nested function is taken, and is
-used to call the nested function indirectly.  For some targets, it is
-made up of data only and thus requires no special treatment.  But, for
-most targets, it is made up of code and thus requires the stack to be
-made executable in order for the program to work properly.
+Warn about executable trampolines generated for pointers to nested
+functions placed on the stack.  A trampoline is a small piece of data
+or code that is created at run time on the stack when the address of a
+nested function is taken, and is used to call the nested function
+indirectly so it can access any variables in its lexical scope.  For
+some targets, it is made up of data only and thus requires no special
+treatment.  But, for most targets, it is made up of code and thus
+requires the stack to be made executable in order for the program to
+work properly.
+
+This warning is enabled by @option{-Wall} for the C language if the
+target defaults to a non-executable stack and this trampoline would
+require the stack to be made executable.
 
 @item -Wfloat-equal
 @opindex Wfloat-equal
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index e348f0a..1c2ccbd 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -12065,3 +12065,14 @@ This target hook can be used to generate a target-specific code
 @deftypefn {Target Hook} void TARGET_RUN_TARGET_SELFTESTS (void)
 If selftests are enabled, run any selftests for this target.
 @end deftypefn
+
+@defmac TARGET_HAS_DEFAULT_NOEXEC_STACK
+Macro used to determine whether to warn when generating executable
+code on the stack for trampolines.  Define this macro to 1 if your
+target defaults to a non-executable stack.  The default is 0.
+
+If you define this macro to 1 then you also need a way to indicate
+that the stack needs to be made executable in the case an executable
+trampoline is generated on the stack.  See @code{TARGET_ASM_FILE_END}
+for an example of how to do that.
+@end defmac
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index f1ad80d..4340ced 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -8169,3 +8169,14 @@ maintainer is familiar with.
 @hook TARGET_SPECULATION_SAFE_VALUE
 
 @hook TARGET_RUN_TARGET_SELFTESTS
+
+@defmac TARGET_HAS_DEFAULT_NOEXEC_STACK
+Macro used to determine whether to warn when generating executable
+code on the stack for trampolines.  Define this macro to 1 if your
+target defaults to a non-executable stack.  The default is 0.
+
+If you define this macro to 1 then you also need a way to indicate
+that the stack needs to be made executable in the case an executable
+trampoline is generated on the stack.  See @code{TARGET_ASM_FILE_END}
+for an example of how to do that.
+@end defmac
-- 
1.8.3.1

Re: [PATCH] PR880088 Enable -Wtrampolines for no-exec-stack targets with -Wall.

[Paul Koning]

> On Nov 26, 2018, at 4:13 AM, Mark Wielaard <mark@klomp.org> wrote:
> 
> With -Wtrampolines a warning is produced whenever gcc generates executable
> code on the stack at runtime to support taking a nested function address
> that is used to call the nested function indirectly when it needs to access
> any variables in its lexical scope.
> 
> As a result the stack has to be marked as executable even for targets which
> have a non-executable stack as default.
> 
> Define a new target macro TARGET_HAS_DEFAULT_NOEXEC_STACK for those targets
> that have a non-executable default stack based on when they call
> file_end_indicate_exec_stack.

The word "default" suggests this is a constant attribute of the platform.  Can it be variable?  Whether the stack is executable or not may depend on -m<something> switches.

	paul

Re: [PATCH] PR880088 Enable -Wtrampolines for no-exec-stack targets with -Wall.

[Mark Wielaard]

On Mon, 2018-11-26 at 08:19 -0500, Paul Koning wrote:
> > On Nov 26, 2018, at 4:13 AM, Mark Wielaard <mark@klomp.org> wrote:
> > 
> > With -Wtrampolines a warning is produced whenever gcc generates
> > executable
> > code on the stack at runtime to support taking a nested function
> > address
> > that is used to call the nested function indirectly when it needs
> > to access
> > any variables in its lexical scope.
> > 
> > As a result the stack has to be marked as executable even for
> > targets which
> > have a non-executable stack as default.
> > 
> > Define a new target macro TARGET_HAS_DEFAULT_NOEXEC_STACK for those
> > targets
> > that have a non-executable default stack based on when they call
> > file_end_indicate_exec_stack.
> 
> The word "default" suggests this is a constant attribute of the
> platform.  Can it be variable?

No. The new target macro is just for those targets that have no
executable stack by default. It basically tracks those targets that
have a static rule for calling file_end_indicate_exec_stack () from
their TARGET_ASM_FILE_END hook.

See also the new documentation for the target macro and the existing
documentation for the TARGET_ASM_FILE_END hook.

I would also be fine with having -Wall enable -Wtrampolines by default.
But in the bug report it was argued that it should only for targets
with a default non-executable stack where gcc would otherwise silently
make the whole stack executable.

Cheers,

Mark

Re: [PATCH] PR880088 Enable -Wtrampolines for no-exec-stack targets with -Wall.

[Joseph Myers]

On Mon, 26 Nov 2018, Mark Wielaard wrote:

> Define a new target macro TARGET_HAS_DEFAULT_NOEXEC_STACK for those targets
> that have a non-executable default stack based on when they call
> file_end_indicate_exec_stack.

Some targets (e.g. ia64) may default to no-exec stacks without needing 
that hook (e.g. if they use function descriptors, so trampolines have no 
need for an executable stack).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH] PR880088 Enable -Wtrampolines for no-exec-stack targets with -Wall.

[Mark Wielaard]

On Mon, 2018-11-26 at 18:29 +0000, Joseph Myers wrote:
> On Mon, 26 Nov 2018, Mark Wielaard wrote:
> 
> > Define a new target macro TARGET_HAS_DEFAULT_NOEXEC_STACK for those
> > targets
> > that have a non-executable default stack based on when they call
> > file_end_indicate_exec_stack.
> 
> Some targets (e.g. ia64) may default to no-exec stacks without
> needing 
> that hook (e.g. if they use function descriptors, so trampolines have
> no 
> need for an executable stack).

Yes, but for such targets it doesn't really matter whether or not they
define this new target macro. The -Wtrampolines warning only warns when
creating an executable runtime trampoline on the stack. It won't for
non-executable trampolines. So yes, then this target macro doesn't have
to be defined even if the stack is non-executable by default. Defining
the TARGET_HAS_DEFAULT_NOEXEC_STACK macro for such a target will then
make it so that -Wall enables -Wtrampolines by default, but
-Wtrampolines still won't warn because the trampolines aren't
executable code on the stack. But I believe that is the correct
behavior. The warning should only happen if gcc would otherwise
silently make the stack executable for a target that has a default non-
executable stack (and it won't for target that uses function
descriptors).

I thought the documentation updates for the warning made that more
clear, but maybe they can be improved even more?

A case could be made that taking a nested function pointer that escapes
the lexical scope from which it takes some context variable always is a
dangerous thing to do and that it should even warn if supporting that
doesn't involve placing an executable runtime trampoline on the stack.
Which is actually the case I made in the original bug. But I don't
believe I got consensus for that.

Thanks,

Mark

Re: [PATCH] PR880088 Enable -Wtrampolines for no-exec-stack targets with -Wall.

[Segher Boessenkool]

Hi Mark,

On Mon, Nov 26, 2018 at 10:13:29AM +0100, Mark Wielaard wrote:
> With -Wtrampolines a warning is produced whenever gcc generates executable
> code on the stack at runtime to support taking a nested function address
> that is used to call the nested function indirectly when it needs to access
> any variables in its lexical scope.
> 
> As a result the stack has to be marked as executable even for targets which
> have a non-executable stack as default.
> 
> Define a new target macro TARGET_HAS_DEFAULT_NOEXEC_STACK for those targets
> that have a non-executable default stack based on when they call
> file_end_indicate_exec_stack.

As Paul says, that name isn't so good.

TARGET_NEEDS_MAKING_THE_STACK_EXECUTABLE_FOR_TRAMPOLINES, or similar?

> diff --git a/gcc/config/rs6000/sysv4.h b/gcc/config/rs6000/sysv4.h
> index 0c67634..9330acf 100644
> --- a/gcc/config/rs6000/sysv4.h
> +++ b/gcc/config/rs6000/sysv4.h
> @@ -972,6 +972,11 @@ ncrtn.o%s"
>  /* Generate entries in .fixup for relocatable addresses.  */
>  #define RELOCATABLE_NEEDS_FIXUP 1
>  
> +#if defined (POWERPC_LINUX) || defined (POWERPC_FREEBSD)
> +  #define TARGET_HAS_DEFAULT_NOEXEC_STACK (TARGET_32BIT \
> +					   || DEFAULT_ABI == ABI_ELFv2)
> +#endif

I don't think this belongs in sysv4.h .


Segher

Re: [PATCH] PR880088 Enable -Wtrampolines for no-exec-stack targets with -Wall.

[Mark Wielaard]

Hi,

On Tue, 2018-11-27 at 12:37 -0600, Segher Boessenkool wrote:
> > Define a new target macro TARGET_HAS_DEFAULT_NOEXEC_STACK for those
> > targets
> > that have a non-executable default stack based on when they call
> > file_end_indicate_exec_stack.
> 
> As Paul says, that name isn't so good.
> 
> TARGET_NEEDS_MAKING_THE_STACK_EXECUTABLE_FOR_TRAMPOLINES, or similar?

Would the slightly shorter
TARGET_NEEDS_EXEC_STACK_MARKER_FOR_TRAMPOLINES be descriptive enough?

> > diff --git a/gcc/config/rs6000/sysv4.h b/gcc/config/rs6000/sysv4.h
> > index 0c67634..9330acf 100644
> > --- a/gcc/config/rs6000/sysv4.h
> > +++ b/gcc/config/rs6000/sysv4.h
> > @@ -972,6 +972,11 @@ ncrtn.o%s"
> >  /* Generate entries in .fixup for relocatable addresses.  */
> >  #define RELOCATABLE_NEEDS_FIXUP 1
> >  
> > +#if defined (POWERPC_LINUX) || defined (POWERPC_FREEBSD)
> > +  #define TARGET_HAS_DEFAULT_NOEXEC_STACK (TARGET_32BIT \
> > +					   || DEFAULT_ABI == ABI_ELFv2)
> > +#endif
> 
> I don't think this belongs in sysv4.h .

I might have gotten lost in the tree of defines/macros.

There are two sysv4.h files gcc/config/rs6000/sysv4.h and
gcc/config/powerpcspe/sysv4.h

Both define the TARGET_ASM_FILE_END hook to rs6000_elf_file_end.
Which are defined in config/rs6000/rs6000.c and
config/powerpcspe/powerpcspe.c. Both have:

#if defined (POWERPC_LINUX) || defined (POWERPC_FREEBSD)
  if (TARGET_32BIT || DEFAULT_ABI == ABI_ELFv2)
    file_end_indicate_exec_stack ();
#endif

And file_end_indicate_exec_stack () is what you call to flip the stack
to be executable (if an executable stack marker is needed for
generating the trampoline).

That is why both sysv4.h files have the new target macro defined the
same way. But maybe only one of them really needs it?

Thanks,

Mark

Re: [PATCH] PR880088 Enable -Wtrampolines for no-exec-stack targets with -Wall.

[Segher Boessenkool]

On Tue, Nov 27, 2018 at 08:54:07PM +0100, Mark Wielaard wrote:
> On Tue, 2018-11-27 at 12:37 -0600, Segher Boessenkool wrote:
> > > Define a new target macro TARGET_HAS_DEFAULT_NOEXEC_STACK for those
> > > targets
> > > that have a non-executable default stack based on when they call
> > > file_end_indicate_exec_stack.
> > 
> > As Paul says, that name isn't so good.
> > 
> > TARGET_NEEDS_MAKING_THE_STACK_EXECUTABLE_FOR_TRAMPOLINES, or similar?
> 
> Would the slightly shorter
> TARGET_NEEDS_EXEC_STACK_MARKER_FOR_TRAMPOLINES be descriptive enough?

"MARKER", is that some official name for it?  If no, just "FLAG"?
Fine with me, sure.

> > > diff --git a/gcc/config/rs6000/sysv4.h b/gcc/config/rs6000/sysv4.h
> > > index 0c67634..9330acf 100644
> > > --- a/gcc/config/rs6000/sysv4.h
> > > +++ b/gcc/config/rs6000/sysv4.h
> > > @@ -972,6 +972,11 @@ ncrtn.o%s"
> > >  /* Generate entries in .fixup for relocatable addresses.  */
> > >  #define RELOCATABLE_NEEDS_FIXUP 1
> > >  
> > > +#if defined (POWERPC_LINUX) || defined (POWERPC_FREEBSD)
> > > +  #define TARGET_HAS_DEFAULT_NOEXEC_STACK (TARGET_32BIT \
> > > +					   || DEFAULT_ABI == ABI_ELFv2)
> > > +#endif
> > 
> > I don't think this belongs in sysv4.h .
> 
> I might have gotten lost in the tree of defines/macros.
> 
> There are two sysv4.h files gcc/config/rs6000/sysv4.h and
> gcc/config/powerpcspe/sysv4.h

Forget about powerpcspe please, I am talking about rs6000 only.

You want linux.h and freebsd.h, maybe the "64" versions of those separately.
Or put this in rs6000.h.  sysv4.h is a random header for this, it doesn't
belong there.


Segher

Re: [PATCH] PR880088 Enable -Wtrampolines for no-exec-stack targets with -Wall.

[Mark Wielaard]

Hi Segher,

On Wed, 2018-11-28 at 15:00 -0600, Segher Boessenkool wrote:
> On Tue, Nov 27, 2018 at 08:54:07PM +0100, Mark Wielaard wrote:
> > On Tue, 2018-11-27 at 12:37 -0600, Segher Boessenkool wrote:
> > > > Define a new target macro TARGET_HAS_DEFAULT_NOEXEC_STACK for
> > > > those
> > > > targets
> > > > that have a non-executable default stack based on when they
> > > > call
> > > > file_end_indicate_exec_stack.
> > > 
> > > As Paul says, that name isn't so good.
> > > 
> > > TARGET_NEEDS_MAKING_THE_STACK_EXECUTABLE_FOR_TRAMPOLINES, or
> > > similar?
> > 
> > Would the slightly shorter
> > TARGET_NEEDS_EXEC_STACK_MARKER_FOR_TRAMPOLINES be descriptive
> > enough?
> 
> "MARKER", is that some official name for it?  If no, just "FLAG"?
> Fine with me, sure.

FLAG it is then! It is even a little shorter :)

> > > > diff --git a/gcc/config/rs6000/sysv4.h
> > > > b/gcc/config/rs6000/sysv4.h
> > > > index 0c67634..9330acf 100644
> > > > --- a/gcc/config/rs6000/sysv4.h
> > > > +++ b/gcc/config/rs6000/sysv4.h
> > > > @@ -972,6 +972,11 @@ ncrtn.o%s"
> > > >  /* Generate entries in .fixup for relocatable addresses.  */
> > > >  #define RELOCATABLE_NEEDS_FIXUP 1
> > > >  
> > > > +#if defined (POWERPC_LINUX) || defined (POWERPC_FREEBSD)
> > > > +  #define TARGET_HAS_DEFAULT_NOEXEC_STACK (TARGET_32BIT \
> > > > +					   || DEFAULT_ABI ==
> > > > ABI_ELFv2)
> > > > +#endif
> > > 
> > > I don't think this belongs in sysv4.h .
> > 
> > I might have gotten lost in the tree of defines/macros.
> > 
> > There are two sysv4.h files gcc/config/rs6000/sysv4.h and
> > gcc/config/powerpcspe/sysv4.h
> 
> Forget about powerpcspe please, I am talking about rs6000 only.

I don't know the differences between the config backends, but it looks
like at least some of the configs were just copy/pasted which might
explain why they both define things the same (in sysv4.h). And they
both use the same TARGET_ASM_FILE_END hook (set to rs6000_elf_file_end
although that function also seems copy/pasted between powerpcspe.c and
rs6000.c.

Could you explain why I should forget about powerpcspe and/or why where
and how to setup the new target marco would be different between the
two config backends?

> You want linux.h and freebsd.h, maybe the "64" versions of those separately.
> Or put this in rs6000.h.  sysv4.h is a random header for this, it doesn't
> belong there.

The reason I added it to sysv4.h is because it matches where the
TARGET_ASM_FILE_END hook is setup. It might make sense to have
specialized TARGET_ASM_FILE_END hooks too for [linux,freebsd)(64)].h.
But that is probably a different discussion.

So if I understand correctly you would like to have:

rs6000/linux.h and rs6000/freebsd.h:
#define TARGET_NEEDS_EXEC_STACK_FLAG_FOR_TRAMPOLINES 1

rs6000/linux64.h and rs6000/freebsd64.h:
#define TARGET_NEEDS_EXEC_STACK_FLAG_FOR_TRAMPOLINES (DEFAULT_ABI == ABI_ELFv2)

If so, shouldn't the same be done for powerpcspe?
Sorry, you told me to forget about it, but I just cannot :)

Thanks,

Mark

Re: [PATCH] PR880088 Enable -Wtrampolines for no-exec-stack targets with -Wall.

[Segher Boessenkool]

Hi Mark,

On Thu, Nov 29, 2018 at 12:57:03PM +0100, Mark Wielaard wrote:
> On Wed, 2018-11-28 at 15:00 -0600, Segher Boessenkool wrote:
> > On Tue, Nov 27, 2018 at 08:54:07PM +0100, Mark Wielaard wrote:
> > > On Tue, 2018-11-27 at 12:37 -0600, Segher Boessenkool wrote:
> > > > > diff --git a/gcc/config/rs6000/sysv4.h
> > > > > b/gcc/config/rs6000/sysv4.h
> > > > > index 0c67634..9330acf 100644
> > > > > --- a/gcc/config/rs6000/sysv4.h
> > > > > +++ b/gcc/config/rs6000/sysv4.h
> > > > > @@ -972,6 +972,11 @@ ncrtn.o%s"
> > > > >  /* Generate entries in .fixup for relocatable addresses.  */
> > > > >  #define RELOCATABLE_NEEDS_FIXUP 1
> > > > >  
> > > > > +#if defined (POWERPC_LINUX) || defined (POWERPC_FREEBSD)
> > > > > +  #define TARGET_HAS_DEFAULT_NOEXEC_STACK (TARGET_32BIT \
> > > > > +					   || DEFAULT_ABI ==
> > > > > ABI_ELFv2)
> > > > > +#endif
> > > > 
> > > > I don't think this belongs in sysv4.h .
> > > 
> > > I might have gotten lost in the tree of defines/macros.
> > > 
> > > There are two sysv4.h files gcc/config/rs6000/sysv4.h and
> > > gcc/config/powerpcspe/sysv4.h
> > 
> > Forget about powerpcspe please, I am talking about rs6000 only.
> 
> I don't know the differences between the config backends, but it looks
> like at least some of the configs were just copy/pasted which might
> explain why they both define things the same (in sysv4.h). And they

Yes, things are copied.  My point is, my review is for rs6000/ only :-)

> > You want linux.h and freebsd.h, maybe the "64" versions of those separately.
> > Or put this in rs6000.h.  sysv4.h is a random header for this, it doesn't
> > belong there.
> 
> The reason I added it to sysv4.h is because it matches where the
> TARGET_ASM_FILE_END hook is setup. It might make sense to have

Yeah, but it does not make sense to put it there, since you are handling
other ABIs as well.

> So if I understand correctly you would like to have:
> 
> rs6000/linux.h and rs6000/freebsd.h:
> #define TARGET_NEEDS_EXEC_STACK_FLAG_FOR_TRAMPOLINES 1
> 
> rs6000/linux64.h and rs6000/freebsd64.h:
> #define TARGET_NEEDS_EXEC_STACK_FLAG_FOR_TRAMPOLINES (DEFAULT_ABI == ABI_ELFv2)

Something like that yes.  Or in rs6000.h, maybe.  Some location that makes
sense :-)


Segher