Message ID | 1534005653.22677.9.camel@med.uni-goettingen.de |
---|---|
State | New |
Headers | show |
Series | [RFC,C,ADA] use function descriptors instead of trampolines in C | expand |
When running the test suite with this patch applied and "-fno-trampolines", there are some errors. Most of it is expected (e.g. nested-6.c calls qsort which fails because it has not itself been compiled with -fno-trampolines). One test case for __builtin_call_with_static_chain in gcc.dg/cwsc1.cfails in an assertion in the new code at calls.c:288. This seems unrelated to my patch though. Eric, can you take a look? Maybe the assertion should be removed to allow overriding the static chain? Joseph, I mistyped your email address before. Could take a look whether the C changes make sense to you? Best, Martin $ gcc/xgcc -Bgcc -Wall -fno-trampolines -O3 cwsc1.c during RTL pass: expand cwsc1.c: In function ‘main’: cwsc1.c:39:46: internal compiler error: in prepare_call_address, at calls.c:288 void *x = __builtin_call_with_static_chain(ptr(), &c); ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~ 0x5bd37d prepare_call_address(tree_node*, rtx_def*, rtx_def*, rtx_def**, int, int) ../../gcc/gcc/calls.c:288 0x860947 expand_call(tree_node*, rtx_def*, int) ../../gcc/gcc/calls.c:4173 0x977358 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool) ../../gcc/gcc/expr.c:10914 0x98375d store_expr(tree_node*, rtx_def*, int, bool, bool) ../../gcc/gcc/expr.c:5614 0x984b1c expand_assignment(tree_node*, tree_node*, bool) ../../gcc/gcc/expr.c:5398 0x872ac2 expand_call_stmt ../../gcc/gcc/cfgexpand.c:2685 0x872ac2 expand_gimple_stmt_1 ../../gcc/gcc/cfgexpand.c:3575 0x872ac2 expand_gimple_stmt ../../gcc/gcc/cfgexpand.c:3734 0x873e3f expand_gimple_basic_block ../../gcc/gcc/cfgexpand.c:5769 0x878a17 execute ../../gcc/gcc/cfgexpand.c:6372 Please submit a full bug report, with preprocessed source if appropriate. Please include the complete backtrace with any bug report. See <https://gcc.gnu.org/bugs/> for instructions. Am Samstag, den 11.08.2018, 18:40 +0200 schrieb Martin Uecker: > > > A while ago Eric Botcazou added support for function descriptors > as a replacement for trampolines. This is used in Ada, but not > in C where it would also imply a change of ABI. Still, as nested > functions are generally not used across interface boundaries, > I thought it would be really useful to have the option to > replace trampolines with function descriptors in C too. > This then avoids the need for an executable stack. > The main downside is that most calls through function pointers then > need to test a bit in the pointer to identify descriptors. > > The following tiny patch (against recent git) is my initial attempt > to implement this idea. As most of the hard work was already done by > Eric for Ada, this turned out to be very simple. But as I am not > too familiar with the GCC code base, it might still be completely > wrong in some ways... In particular, I am not sure whether I mark > all the right tree nodes correctly in the C frontend. > > The patch essentially just adds the marking of the tree nodes > at two places in C FE. The rest of the PATCH is moving the check > for the command-line flag in the front ends to be able to have > different defaults for different languages. > > > I am not too deeply convinced about the security benefits of a > non-executable stack myself, but it some people like to > enforce this in general. So it might make sense to completely > transition to the use of function descriptors for nested functions. > A possible strategy for such a transition might be to first compile > all libraries with -fno-trampolines (but see downsides above). > Assuming these do not create trampolines themselves this should > cause no problems, but the libraries should then be compatible with > code compiled with trampolines and with code compiled with > descriptors. > > > The compiler passes the man or boy test by Donald Knuth with > '-ftrampolines' (default for C) and '-fno-trampolines' and also > my internal project which uses nested function still passes its > complete test suite. I also verified that an executable stack > is not used anymore. > > More testing is in progress. > > > Best, > Martin > > > diff --git a/gcc/ada/gcc-interface/trans.c b/gcc/ada/gcc-interface/trans.c > index 31e098a0c70..3eb2e71a2bd 100644 > --- a/gcc/ada/gcc-interface/trans.c > +++ b/gcc/ada/gcc-interface/trans.c > @@ -1755,7 +1755,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 > @@ -4327,7 +4328,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/c-objc-common.h b/gcc/c/c-objc-common.h > index 78e768c2366..ef039560eb9 100644 > --- a/gcc/c/c-objc-common.h > +++ b/gcc/c/c-objc-common.h > @@ -110,4 +110,7 @@ along with GCC; see the file COPYING3. If not see > > #undef LANG_HOOKS_TREE_INLINING_VAR_MOD_TYPE_P > #define LANG_HOOKS_TREE_INLINING_VAR_MOD_TYPE_P c_vla_unspec_p > + > +#undef LANG_HOOKS_CUSTOM_FUNCTION_DESCRIPTORS > +#define LANG_HOOKS_CUSTOM_FUNCTION_DESCRIPTORS true > #endif /* GCC_C_OBJC_COMMON */ > diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c > index 90ae306c99a..57f3eca320b 100644 > --- a/gcc/c/c-typeck.c > +++ b/gcc/c/c-typeck.c > @@ -1912,7 +1912,13 @@ 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) > + && (flag_trampolines == 0)) > + FUNC_ADDR_BY_DESCRIPTOR (r) = 1; > + > + return r; > } > > /* Mark EXP as read, not just set, for set but not used -Wunused > @@ -3130,6 +3136,11 @@ 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) > + && (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 384c0238748..1367e0305a3 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 5bb645291cf..9338edd7bb7 100644 > --- a/gcc/common.opt > +++ b/gcc/common.opt > @@ -2471,7 +2471,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/tree-nested.c b/gcc/tree-nested.c > index 4c8eda94f14..4b49024b917 100644 > --- a/gcc/tree-nested.c > +++ b/gcc/tree-nested.c > @@ -2497,7 +2497,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);
diff --git a/gcc/ada/gcc-interface/trans.c b/gcc/ada/gcc-interface/trans.c index 31e098a0c70..3eb2e71a2bd 100644 --- a/gcc/ada/gcc-interface/trans.c +++ b/gcc/ada/gcc-interface/trans.c @@ -1755,7 +1755,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 @@ -4327,7 +4328,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/c-objc-common.h b/gcc/c/c-objc-common.h index 78e768c2366..ef039560eb9 100644 --- a/gcc/c/c-objc-common.h +++ b/gcc/c/c-objc-common.h @@ -110,4 +110,7 @@ along with GCC; see the file COPYING3. If not see #undef LANG_HOOKS_TREE_INLINING_VAR_MOD_TYPE_P #define LANG_HOOKS_TREE_INLINING_VAR_MOD_TYPE_P c_vla_unspec_p + +#undef LANG_HOOKS_CUSTOM_FUNCTION_DESCRIPTORS +#define LANG_HOOKS_CUSTOM_FUNCTION_DESCRIPTORS true #endif /* GCC_C_OBJC_COMMON */ diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c index 90ae306c99a..57f3eca320b 100644 --- a/gcc/c/c-typeck.c +++ b/gcc/c/c-typeck.c @@ -1912,7 +1912,13 @@ 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) + && (flag_trampolines == 0)) + FUNC_ADDR_BY_DESCRIPTOR (r) = 1; + + return r; } /* Mark EXP as read, not just set, for set but not used -Wunused @@ -3130,6 +3136,11 @@ 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) + && (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 384c0238748..1367e0305a3 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 5bb645291cf..9338edd7bb7 100644 --- a/gcc/common.opt +++ b/gcc/common.opt @@ -2471,7 +2471,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/tree-nested.c b/gcc/tree-nested.c index 4c8eda94f14..4b49024b917 100644 --- a/gcc/tree-nested.c +++ b/gcc/tree-nested.c @@ -2497,7 +2497,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);