Does this patch have a change? This version seems risk-free and
is a clear improvement from simply doing nothing for
'-fno-trampolines'. Also it is useful in situations where
one cannot have an executable stack.
I am currently thinking about working
around this problem by calling nested functions with the
following macro (x86_64 only):
#define NESTED_CALL(p, args) ({ \
_auto_type __p = (p); \
struct { unsigned short mov1; void* addr; unsigned short mov2; void* chain;
unsigned int jmp; } \
__attribute__((packed))* __t = (void*)p; \
assert((0xbb49 == __t->mov1) && (0xba49 == __t->mov2) && (0x90e3ff49 ==
__t->jmp)); \
__builtin_call_with_static_chain(((__typeof__(__p))(__t->addr))args,
__t->chain); \
})
(the 'assert' could be replaced with an 'if' to call
regular functions when pointers to nested functions
and regular functions are mixed)
But I would rather have a proper solution...
Best,
Martin
Am Freitag, den 21.12.2018, 07:46 +0100 schrieb Martin Uecker:
> Am Montag, den 17.12.2018, 10:28 -0700 schrieb Jeff Law:
>
> > > But the alignment increase itself on 'i386' and 'aarch64'
> > > might be unacceptable. In this case, the only safe change
> > > is to make the higher alignment also depend on
> > > "-fno-trampolines". Would this be acceptable?
> >
> > Unclear at this point.
>
> Hi Jeff,
>
> in any case, here is another revision of this patch for
> consideration which implements just this.
>
> The lang hook is not activated for C anymore which means
> that this patch is a no-op when -fno-trampolines is not
> given. So the test about achieving the minimum alignment on
> i386 does not fail anymore.
>
> With -fno-trampolines the minimum alignment is increased
> as needed on platforms which support descriptors.
> For C and Ada (where it is the default) function
> descriptors are created instead of trampolines.
>
> If -fno-trampolines is given on platforms which do not
> support descriptors a warning is given (before the flag
> was silently ignored.)
>
> I also made the existing warnings about the ABI change
> more visible (similar to -freg-struct-return or similar
> flags).
>
> I consider the current behavior broken, where the flag
> is a silent no-op for most languages and on some targets
> while the documentation implies otherwise.
>
> Bootstrapped and regression tested on x86.
>
> I also tested -fno-trampolines on a project of mine which
> uses nested functions and has a test suite and there it
> works perfectly and removes the need for the executable stack.
>
> Finally, this patch is a tiny and self-contained change which
> could easily be reverted if there should be any unanticipated
> failures.
>
> Best,
> Martin
>
>
>
> diff --git a/gcc/ChangeLog b/gcc/ChangeLog
> index 4be16c15f86..8825d87b44f 100644
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,16 @@
> +2018-12-20 Martin Uecker <martin.uec...@med.uni-goettingen.de>
> +
> + * common.opt (flag_trampolines): Change default.
> + * calls.c (prepare_call_address): Remove check for
> + flag_trampolines. Decision is now made in FEs.
> + * defaults.h (FUNCTION_ALIGNMENT): Add test for flag_trampolines.
> + * tree-nested.c (convert_tramp_reference_op): Likewise.
> + * toplev.c (process_options): Add warning for -fno-trampolines on
> + unsupported targets.
> + * doc/invoke.texi (-fno-trampolines): Document support for C.
> + * doc/sourcebuild.texi (target attributes): Document new
> + "notrampolines" effective target keyword.
> +
> 2018-12-20 Vladimir Makarov <vmaka...@redhat.com>
>
> PR target/88457
> diff --git a/gcc/ada/ChangeLog b/gcc/ada/ChangeLog
> index ba974cdcb03..c2f3d0db281 100644
> --- a/gcc/ada/ChangeLog
> +++ b/gcc/ada/ChangeLog
> @@ -1,3 +1,8 @@
> +2018-12-20 Martin Uecker <martin.uec...@med.uni-goettingen.de>
> +
> + * gcc-interface/trans.c (Attribute_to_gnu): Add check for
> + flag_trampolines.
> +
> 2018-12-14 Eric Botcazou <ebotca...@adacore.com>
>
> * gcc-interface/decl.c (rm_size): Take into account the padding in
> diff --git a/gcc/ada/gcc-interface/trans.c b/gcc/ada/gcc-interface/trans.c
> index 620dbd3d36d..ae4139e9b84 100644
> --- a/gcc/ada/gcc-interface/trans.c
> +++ b/gcc/ada/gcc-interface/trans.c
> @@ -2239,7 +2239,8 @@ Attribute_to_gnu (Node_Id gnat_node, tree
> *gnu_result_type_p, int attribute)
> if ((attribute == Attr_Access
> || attribute == Attr_Unrestricted_Access)
> && targetm.calls.custom_function_descriptors > 0
> - && Can_Use_Internal_Rep (Etype (gnat_node)))
> + && Can_Use_Internal_Rep (Etype (gnat_node))
> + && flag_trampolines != 1)
> FUNC_ADDR_BY_DESCRIPTOR (gnu_expr) = 1;
>
> /* Otherwise, we need to check that we are not violating the
> @@ -5050,7 +5051,8 @@ Call_to_gnu (Node_Id gnat_node, tree
> *gnu_result_type_p, tree gnu_target,
> /* If the access type doesn't require foreign-compatible
> representation,
> be prepared for descriptors. */
> if (targetm.calls.custom_function_descriptors > 0
> - && Can_Use_Internal_Rep (Etype (Prefix (Name (gnat_node)))))
> + && Can_Use_Internal_Rep (Etype (Prefix (Name (gnat_node))))
> + && flag_trampolines != 1)
> by_descriptor = true;
> }
> else if (Nkind (Name (gnat_node)) == N_Attribute_Reference)
> diff --git a/gcc/c/ChangeLog b/gcc/c/ChangeLog
> index 6e12dda2331..71443846799 100644
> --- a/gcc/c/ChangeLog
> +++ b/gcc/c/ChangeLog
> @@ -1,3 +1,9 @@
> +2018-12-20 Martin Uecker <martin.uec...@med.uni-goettingen.de>
> +
> + * c-typeck.c (function_to_pointer_conversion): If using descriptors
> + instead of trampolines, amend function address with
> + FUNC_ADDR_BY_DESCRIPTOR and calls with ALL_EXPR_BY_DESCRIPTOR.
> +
> 2018-12-19 Segher Boessenkool <seg...@kernel.crashing.org>
>
> * c-parser.c (c_parser_asm_statement) <RID_CONST, RID_RESTRICT>: Give
> diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
> index 1ae5ede81e6..6363dfda3b8 100644
> --- a/gcc/c/c-typeck.c
> +++ b/gcc/c/c-typeck.c
> @@ -1913,7 +1913,14 @@ function_to_pointer_conversion (location_t loc, tree
> exp)
> if (TREE_NO_WARNING (orig_exp))
> TREE_NO_WARNING (exp) = 1;
>
> - return build_unary_op (loc, ADDR_EXPR, exp, false);
> + tree r = build_unary_op (loc, ADDR_EXPR, exp, false);
> +
> + if (TREE_CODE(r) == ADDR_EXPR
> + && targetm.calls.custom_function_descriptors > 0
> + && flag_trampolines == 0)
> + FUNC_ADDR_BY_DESCRIPTOR (r) = 1;
> +
> + return r;
> }
>
> /* Mark EXP as read, not just set, for set but not used -Wunused
> @@ -3135,6 +3142,12 @@ build_function_call_vec (location_t loc,
> vec<location_t> arg_loc,
> else
> result = build_call_array_loc (loc, TREE_TYPE (fntype),
> function, nargs, argarray);
> +
> + if (TREE_CODE (result) == CALL_EXPR
> + && targetm.calls.custom_function_descriptors > 0
> + && flag_trampolines == 0)
> + CALL_EXPR_BY_DESCRIPTOR (result) = 1;
> +
> /* If -Wnonnull warning has been diagnosed, avoid diagnosing it again
> later. */
> if (warned_p && TREE_CODE (result) == CALL_EXPR)
> diff --git a/gcc/calls.c b/gcc/calls.c
> index e3b4ef80e51..1b977c231ea 100644
> --- a/gcc/calls.c
> +++ b/gcc/calls.c
> @@ -230,7 +230,7 @@ prepare_call_address (tree fndecl_or_type, rtx funexp,
> rtx static_chain_value,
> {
> /* If it's an indirect call by descriptor, generate code to perform
> runtime identification of the pointer and load the descriptor. */
> - if ((flags & ECF_BY_DESCRIPTOR) && !flag_trampolines)
> + if (flags & ECF_BY_DESCRIPTOR)
> {
> const int bit_val = targetm.calls.custom_function_descriptors;
> rtx call_lab = gen_label_rtx ();
> diff --git a/gcc/common.opt b/gcc/common.opt
> index 45d7f6189e5..d85d77b5b91 100644
> --- a/gcc/common.opt
> +++ b/gcc/common.opt
> @@ -2546,7 +2546,7 @@ Common Report Var(flag_tracer) Optimization
> Perform superblock formation via tail duplication.
>
> ftrampolines
> -Common Report Var(flag_trampolines) Init(0)
> +Common Report Var(flag_trampolines) Init(-1)
> For targets that normally need trampolines for nested functions, always
> generate them instead of using descriptors.
>
> diff --git a/gcc/defaults.h b/gcc/defaults.h
> index 9035b333be8..66de347edef 100644
> --- a/gcc/defaults.h
> +++ b/gcc/defaults.h
> @@ -1050,7 +1050,8 @@ see the files COPYING3 and COPYING.RUNTIME
> respectively. If not, see
> /* Force minimum alignment to be able to use the least significant bits
> for distinguishing descriptor addresses from code addresses. */
> #define FUNCTION_ALIGNMENT(ALIGN) \
> - (lang_hooks.custom_function_descriptors \
> + (( lang_hooks.custom_function_descriptors
> \
> + || flag_trampolines == 0)
> \
> && targetm.calls.custom_function_descriptors > 0 \
> ? MAX ((ALIGN), \
> 2 * targetm.calls.custom_function_descriptors * BITS_PER_UNIT)\
> diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
> index ac2ee59d92c..5662abd1b97 100644
> --- a/gcc/doc/invoke.texi
> +++ b/gcc/doc/invoke.texi
> @@ -14030,9 +14030,10 @@ made executable in order for the program to work
> properly.
> basis to let the compiler avoid generating them, if it computes that this
> is safe, and replace them with descriptors. Descriptors are made up of data
> only, but the generated code must be prepared to deal with them. As of this
> -writing, @option{-fno-trampolines} is enabled by default only for Ada.
> +writing, @option{-fno-trampolines} is supported only for C and Ada and
> +enabled by default only for Ada.
>
> -Moreover, code compiled with @option{-ftrampolines} and code compiled with
> +@strong{Warning:} Code compiled with @option{-ftrampolines} and code
> compiled with
> @option{-fno-trampolines} are not binary compatible if nested functions are
> present. This option must therefore be used on a program-wide basis and be
> manipulated with extreme care.
> diff --git a/gcc/doc/sourcebuild.texi b/gcc/doc/sourcebuild.texi
> index 29c693b9644..d645d1def92 100644
> --- a/gcc/doc/sourcebuild.texi
> +++ b/gcc/doc/sourcebuild.texi
> @@ -2162,6 +2162,9 @@ Target supports Newlib.
> GCC was configured with @code{--enable-newlib-nano-formatted-io}, which
> reduces
> the code size of Newlib formatted I/O functions.
>
> +@item notrampolines
> +Target supports option @option{-fno-trampolines}.
> +
> @item pow10
> Target provides @code{pow10} function.
>
> diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
> index 33a4776b921..24f443bb26f 100644
> --- a/gcc/testsuite/ChangeLog
> +++ b/gcc/testsuite/ChangeLog
> @@ -1,3 +1,9 @@
> +2018-12-20 Martin Uecker <martin.uec...@med.uni-goettingen.de>
> +
> + * gcc.dg/trampoline-2.c: New test.
> + * lib/target-supports.exp
> + (check_effective_target_notrampolines): New.
> +
> 2018-12-20 Vladimir Makarov <vmaka...@redhat.com>
>
> PR target/88457
> diff --git a/gcc/testsuite/gcc.dg/trampoline-2.c
> b/gcc/testsuite/gcc.dg/trampoline-2.c
> new file mode 100644
> index 00000000000..06c1cf4f647
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/trampoline-2.c
> @@ -0,0 +1,23 @@
> +/* test that nested function work without trampolines for -fno-trampolines */
> +/* Origin: Martin Uecker <martin.uec...@med.uni-goettingen.de> */
> +/* { dg-require-effective-target notrampolines } */
> +/* { dg-options "-std=gnu11 -O2 -Wtrampolines -fno-trampolines" } */
> +
> +static int p(void) { return +1; }
> +static int m(void) { return -1; }
> +static int z(void) { return 0; }
> +
> +typedef int (*funptr_t)(void);
> +
> +static int A(int k, funptr_t a1, funptr_t a2, funptr_t a3, funptr_t a4,
> funptr_t a5)
> +{
> + int B(void) { return A(--k, B, a1, a2, a3, a4); }
> +
> + return (k <= 0) ? (a4() + a5()) : (B());
> +}
> +
> +int main(void)
> +{
> + return (0 == A(5, p, m, m, p, z)) ? 0 : 1;
> +}
> +
> diff --git a/gcc/testsuite/lib/target-supports.exp
> b/gcc/testsuite/lib/target-supports.exp
> index 7dec43233e0..43f9bb3891f 100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -924,6 +924,14 @@ proc check_effective_target_scheduling {} {
> } "-fschedule-insns"]
> }
>
> +# Return 1 if it is possible to use function descriptors instead of
> trampolines, 0 otherwise.
> +
> +proc check_effective_target_notrampolines {} {
> + return [check_no_compiler_messages notrampolines assembly {
> + void foo (void) { }
> + } "-fno-trampolines"]
> +}
> +
> # Return 1 if trapping arithmetic is available, 0 otherwise.
>
> proc check_effective_target_trapping {} {
> diff --git a/gcc/toplev.c b/gcc/toplev.c
> index ab20cd98969..765ce347172 100644
> --- a/gcc/toplev.c
> +++ b/gcc/toplev.c
> @@ -1698,6 +1698,12 @@ process_options (void)
> flag_prefetch_loop_arrays = 0;
> }
>
> + if (flag_trampolines == 0 && targetm.calls.custom_function_descriptors ==
> -1)
> + {
> + warning_at (UNKNOWN_LOCATION, 0,
> + "-fno-trampolines not supported for this target");
> + }
> +
> /* This combination of options isn't handled for i386 targets and doesn't
> make much sense anyway, so don't allow it. */
> if (flag_prefetch_loop_arrays > 0 && optimize_size)
> diff --git a/gcc/tree-nested.c b/gcc/tree-nested.c
> index 0ad469ada49..eb9bccb7a9d 100644
> --- a/gcc/tree-nested.c
> +++ b/gcc/tree-nested.c
> @@ -2544,7 +2544,7 @@ convert_tramp_reference_op (tree *tp, int
> *walk_subtrees, void *data)
> continue;
>
> /* Decide whether to generate a descriptor or a trampoline. */
> - descr = FUNC_ADDR_BY_DESCRIPTOR (t) && !flag_trampolines;
> + descr = FUNC_ADDR_BY_DESCRIPTOR (t);
>
> if (descr)
> x = lookup_descr_for_decl (i, decl, INSERT);
