[RFC PATCH] __builtin_va_arg_pack ()

[Richard Guenther]

On 8/30/07, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> As I already wrote in
> http://gcc.gnu.org/ml/gcc-patches/2007-07/msg01033.html
> Alex's __attribute__((__gnu_inline__)) support for C++ allows glibc
> to implement most of the -D_FORTIFY_SOURCE fortification even for C++,
> with minimal changes, but there is a notable exception that *printf
> family of functions can't be protected that way.
> The problem is that these functions have variable number of arguments
> and for fortification we need to write trivial wrappers for them
> which can be inlined.
> For C we currently define these as macros, which has issues on its own,
> although it is permitted by POSIX, if using a function pointer
> with the same name as one of the functions one has to prevent
> expanding them as function-like macros, so say
> #include <stdio.h>
> ...
>  foo->printf (a, b, c);
> won't compile while
>  (foo->printf) (a, b, c);
> will.  For C++ we can't use function-like macros here, because
> one can use the same identifier in his own namespace for different
> purpose.
>
> The following patch allows using inline functions for those cases,
> by introducing a __builtin_va_arg_pack () builtin which during
> inlining expands to all anonymous arguments of the inline function.
> It is similar to how __VA_ARGS__ can be used in the preprocessor,
> with a few limitations.
> Using it in an out-of-line function is an error, it doesn't make sense
> in that case, so it should be only used in always_inline inlines
> or gnu_inline extern inline functions (the latter, if it can't be inlined,
> will just result in an external call and that external implementation
> obviously can't use __builtin_va_arg_pack (), but the inline function
> can).  Also, it can be only used in the context of anonymous arguments
> of another variable number of arguments call.
>
> What do you think about this?

I think the idea is sound.  Though for instrumenting the standard functions
like printf couldn't you dispatch to the vprintf variants instead?  I
see we'd need
to force the compiler to not inline that call then which works by re-declaring
it like

extern inline __attribute__((__gnu_inline__)) void foo(void) {}

extern inline __attribute__((__gnu_inline__)) void bar(void)
{
  extern __attribute__((noinline)) void foo(void);

  foo();
}

but unfortunately you get an extra warning for from that and further calls
to foo() outside of bar() are not inlined anymore.  Leaving the only
other possibility to add a #pragma to mark a specific call as not-to-inline.

> +                 new_call = build_call_array (TREE_TYPE (call),
> +                                              CALL_EXPR_FN (call),
> +                                              nargs + call_expr_nargs (call),
> +                                              argarray);
> +                 CALL_EXPR_STATIC_CHAIN (new_call)
> +                   = CALL_EXPR_STATIC_CHAIN (call);
> +                 CALL_EXPR_TAILCALL (new_call) = CALL_EXPR_TAILCALL (call);
> +                 CALL_EXPR_RETURN_SLOT_OPT (new_call)
> +                   = CALL_EXPR_RETURN_SLOT_OPT (call);

in tree.h I also find CALL_FROM_THUNK_P, CALL_CANNOT_INLINE_P and
TREE_NOTHROW.


> @@ -10413,14 +10419,26 @@ fold_call_expr (tree exp, bool ignore)
>    tree fndecl = get_callee_fndecl (exp);
>    if (fndecl
>        && TREE_CODE (fndecl) == FUNCTION_DECL
> -      && DECL_BUILT_IN (fndecl))
> +      && DECL_BUILT_IN (fndecl)
> +      && !CALL_EXPR_VA_ARG_PACK (exp))
>      {
> +      int nargs = call_expr_nargs (exp);
> +
> +      if (nargs && TREE_CODE (CALL_EXPR_ARG (exp, nargs - 1)) == CALL_EXPR)
> +       {
> +         tree fndecl2 = get_callee_fndecl (CALL_EXPR_ARG (exp, nargs - 1));
> +         if (fndecl2
> +             && TREE_CODE (fndecl2) == FUNCTION_DECL
> +             && DECL_BUILT_IN (fndecl2)
> +             && DECL_FUNCTION_CODE (fndecl2) == BUILT_IN_VA_ARG_PACK)
> +           return NULL_TREE;
> +       }
> +

I think this needs a comment explaining what is happening.  Also for all places
you check DECL_BUILT_IN and DECL_FUNCTION_CODE for
BUILT_IN_VA_ARG_PACK you need to check the DECL_BUILT_IN_CLASS
for BUILT_IN_NORMAL.


> @@ -10506,6 +10524,15 @@ fold_builtin_call_array (tree type,
>      if (TREE_CODE (fndecl) == FUNCTION_DECL
>          && DECL_BUILT_IN (fndecl))
>        {
> +       if (n && TREE_CODE (argarray[n - 1]) == CALL_EXPR)
> +         {
> +           tree fndecl2 = get_callee_fndecl (argarray[n - 1]);
> +           if (fndecl2
> +               && TREE_CODE (fndecl2) == FUNCTION_DECL
> +               && DECL_BUILT_IN (fndecl2)
> +               && DECL_FUNCTION_CODE (fndecl2) == BUILT_IN_VA_ARG_PACK)
> +             return build_call_array (type, fn, n, argarray);
> +         }

Likewise.

> +@smallexample
> +extern inline __attribute__ ((__always_inline__)) int

I think this example should use __gnu_inline__ attribute.

Richard.