Re: [RFC] [PATCH][C][ADA] use function descriptors instead of trampolines in C

[Uecker, Martin]

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(), );
 ~^~
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  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 @@ 

[RFC] [PATCH][C][ADA] use function descriptors instead of trampolines in C

[Uecker, Martin]

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 
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