[PATCH] Add __builtin_va_arg_pack_len () builtin for {open{,at}{,64},mq_open} fortification

[Jakub Jelinek <jakub at redhat dot com>]

Hi!

ATM glibc with -D_FORTIFY_SOURCE{,=2} implements checking for open(2)
etc. arguments, because it is a common source of security problems
when people use O_CREAT in two argument open invocation.
But it is only implementable as a function-like preprocessor macro,
which has several disadvantages:
1) it can't be used in C++, where say
#include <fcntl.h>
namespace myN
{
  char *open (char *, char *, int, int);
}
   is valid
2) although POSIX allows us to implement open in C as function-like macro,
   it breaks code where people have some open function pointer and call
   that rather than open(2):
      my_struct->open (&x, 5, 2, 1);
   rather than
      (my_struct->open) (&x, 5, 2, 1);
   or
      (*my_struct->open) (&x, 5, 2, 1);
   People could certainly fix all their code and in Fedora many people
   have done that already, but it is a common problem in many sources

For C++ we really need to implement this checking open as
__extern_always_inline or some other way without using function-like
macros and for C it is certainly more polite to the users.

We have discussed several possibilities, including:
1) hardwiring this kind of checking into gcc with
   __attribute__((__open_check__ (O_CREAT, 2, __open_2)))
   with the meaning of arguments: O_CREAT bit value, index of the oflag
   argument (... is expected to follow that argument) and alternate
   function to call instead if O_CREAT is or might be set in oflag
   and only two arguments are passed (while this would allow
   to describe all of {open{,at}{,64},mq_open} checking, it is really
   not very narrow hack)
2) adding __builtin_va_arg_inline builtin which would have 2 arguments
   - constant index of the anonymous argument and type similar to va_arg
   This would issue diagnostics if it is used with non-matching type
   or that argument is missing completely (but allowing DCE to remove
   it first)
3) allowing __builtin_va_arg_pack () as initializer to [] arrays
   - but it would be a nightmare to handle a type which would be
   VLA before inlining but fixed length after inlining

The following approach is IMHO the least invasive for GCC and is quite
natural extension to __builtin_va_arg_pack ().  Although with that
we won't be able to verify the third argument is mode_t/int/unsigned int
or whatever other va_arg compatible type to mode_t, I don't think
that is a big deal - I doubt people do mistakes with that.

The patch introduces __builtin_va_arg_pack_len () which returns int
count of the anonymous arguments.  Otherwise it works similarly
to __builtin_va_arg_pack () - can be only used in function bodies
that are actually inlined, otherwise when it is seen during
expand it issues error.

The testcases model basically how open checking will be implemented
in glibc using this extension, just replace myopen with open,
myopenva with __open_alias with __asm ("open") and myopen2
with __open_2 and 0x40 with O_CREAT.

The link time warnings resp. errors aren't perfect, linker will
in these cases show location inside of the inline fn with -g,
which won't help much finding the bug, without -g it will show
section plus offset, which is possible to look up.  We have
the same issue with e.g. memset swapped argument checking in C++,
I will try to address it later with a separate patch.  But even if
that is not resolved, this is useful on its own and is generic enough
that it could be used for many other uses as well.

Ok for trunk?

2007-09-08  Jakub Jelinek  <jakub@redhat.com>

	* builtins.def (BUILT_IN_VA_ARG_PACK_LEN): New builtin.
	* builtins.c (expand_builtin) <case BUILT_IN_VA_ARG_PACK_LEN>: Issue
	error if __builtin_va_arg_pack_len () wasn't optimized out during
	inlining.
	* tree-inline.c (copy_bb): Replace __builtin_va_arg_pack_len ()
	with the number of inline's anonymous arguments.
	* doc/extend.texi: Document __builtin_va_arg_pack_len ().

	* gcc.dg/va-arg-pack-len-1.c: New test.
	* g++.dg/va-arg-pack-len-1.C: New test.

--- gcc/builtins.def.jj	2007-09-06 01:22:16.000000000 +0200
+++ gcc/builtins.def	2007-09-08 09:57:42.000000000 +0200
@@ -702,6 +702,7 @@ DEF_GCC_BUILTIN        (BUILT_IN_VA_COPY
 DEF_GCC_BUILTIN        (BUILT_IN_VA_END, "va_end", BT_FN_VOID_VALIST_REF, ATTR_NULL)
 DEF_GCC_BUILTIN        (BUILT_IN_VA_START, "va_start", BT_FN_VOID_VALIST_REF_VAR, ATTR_NULL)
 DEF_GCC_BUILTIN        (BUILT_IN_VA_ARG_PACK, "va_arg_pack", BT_FN_INT, ATTR_PURE_NOTHROW_LIST)
+DEF_GCC_BUILTIN        (BUILT_IN_VA_ARG_PACK_LEN, "va_arg_pack_len", BT_FN_INT, ATTR_PURE_NOTHROW_LIST)
 DEF_EXT_LIB_BUILTIN    (BUILT_IN__EXIT, "_exit", BT_FN_VOID_INT, ATTR_NORETURN_NOTHROW_LIST)
 DEF_C99_BUILTIN        (BUILT_IN__EXIT2, "_Exit", BT_FN_VOID_INT, ATTR_NORETURN_NOTHROW_LIST)
 
--- gcc/builtins.c.jj	2007-09-07 11:15:06.000000000 +0200
+++ gcc/builtins.c	2007-09-08 09:58:45.000000000 +0200
@@ -6276,6 +6276,12 @@ expand_builtin (tree exp, rtx target, rt
       error ("invalid use of %<__builtin_va_arg_pack ()%>");
       return const0_rtx;
 
+    case BUILT_IN_VA_ARG_PACK_LEN:
+      /* All valid uses of __builtin_va_arg_pack_len () are removed during
+	 inlining.  */
+      error ("invalid use of %<__builtin_va_arg_pack_len ()%>");
+      return const0_rtx;
+
       /* Return the address of the first anonymous stack arg.  */
     case BUILT_IN_NEXT_ARG:
       if (fold_builtin_next_arg (exp, false))
--- gcc/tree-inline.c.jj	2007-09-05 21:41:11.000000000 +0200
+++ gcc/tree-inline.c	2007-09-08 10:39:27.000000000 +0200
@@ -867,6 +867,33 @@ copy_bb (copy_body_data *id, basic_block
 		  stmt = *stmtp;
 		  update_stmt (stmt);
 		}
+	      else if (call
+		       && id->call_expr
+		       && (decl = get_callee_fndecl (call))
+		       && DECL_BUILT_IN_CLASS (decl) == BUILT_IN_NORMAL
+		       && DECL_FUNCTION_CODE (decl)
+			  == BUILT_IN_VA_ARG_PACK_LEN)
+		{
+		  /* __builtin_va_arg_pack_len () should be replaced by
+		     the number of anonymous arguments.  */
+		  int nargs = call_expr_nargs (id->call_expr);
+		  tree count, *call_ptr, p;
+
+		  for (p = DECL_ARGUMENTS (id->src_fn); p; p = TREE_CHAIN (p))
+		    nargs--;
+
+		  count = build_int_cst (integer_type_node, nargs);
+		  call_ptr = stmtp;
+		  if (TREE_CODE (*call_ptr) == GIMPLE_MODIFY_STMT)
+		    call_ptr = &GIMPLE_STMT_OPERAND (*call_ptr, 1);
+		  if (TREE_CODE (*call_ptr) == WITH_SIZE_EXPR)
+		    call_ptr = &TREE_OPERAND (*call_ptr, 0);
+		  gcc_assert (*call_ptr == call && call_ptr != stmtp);
+		  *call_ptr = count;
+		  stmt = *stmtp;
+		  update_stmt (stmt);
+		  call = NULL_TREE;
+		}
 
 	      /* Statements produced by inlining can be unfolded, especially
 		 when we constant propagated some operands.  We can't fold
--- gcc/doc/extend.texi.jj	2007-09-07 10:29:32.000000000 +0200
+++ gcc/doc/extend.texi	2007-09-08 13:20:20.000000000 +0200
@@ -583,6 +583,41 @@ myprintf (FILE *f, const char *format, .
 @end smallexample
 @end deftypefn
 
+@deftypefn {Built-in Function} __builtin_va_arg_pack_len ()
+This built-in function returns the number of anonymous arguments of
+an inline function.  It can be used only in inline functions which
+will be always inlined, never compiled as a separate function, such
+as those using @code{__attribute__ ((__always_inline__))} or
+@code{__attribute__ ((__gnu_inline__))} extern inline functions.
+For example following will do link or runtime checking of open
+arguments for optimized code:
+@smallexample
+#ifdef __OPTIMIZE__
+extern inline __attribute__((__gnu_inline__)) int
+myopen (const char *path, int oflag, ...)
+@{
+  if (__builtin_va_arg_pack_len () > 1)
+    warn_open_too_many_arguments ();
+
+  if (__builtin_constant_p (oflag))
+    @{
+      if ((oflag & O_CREAT) != 0 && __builtin_va_arg_pack_len () < 1)
+        @{
+          warn_open_missing_mode ();
+          return __open_2 (path, oflag);
+        @}
+      return open (path, oflag, __builtin_va_arg_pack ());
+    @}
+    
+  if (__builtin_va_arg_pack_len () < 1)
+    return __open_2 (path, oflag);
+
+  return open (path, oflag, __builtin_va_arg_pack ());
+@}
+#endif
+@end smallexample
+@end deftypefn
+
 @node Typeof
 @section Referring to a Type with @code{typeof}
 @findex typeof
--- gcc/testsuite/gcc.dg/va-arg-pack-len-1.c.jj	2007-09-08 13:07:50.000000000 +0200
+++ gcc/testsuite/gcc.dg/va-arg-pack-len-1.c	2007-09-08 13:10:38.000000000 +0200
@@ -0,0 +1,120 @@
+/* { dg-do run } */
+/* { dg-options "-O2" } */
+
+#include <stdarg.h>
+
+extern int warn_open_missing_mode (void);
+extern int warn_open_too_many_arguments (void);
+extern void abort (void);
+
+char expected_char;
+
+__attribute__((noinline)) int
+myopen2 (const char *path, int oflag)
+{
+  if (expected_char++ != path[0] || path[1] != '\0')
+    abort ();
+  switch (path[0])
+    {
+    case 'f':
+      if (oflag != 0x2)
+	abort ();
+      break;
+    case 'g':
+      if (oflag != 0x43)
+	abort ();
+      /* In real __open_2 this would terminate the program:
+	 open with O_CREAT without third argument.  */
+      return -6;
+    default:
+      abort ();
+    }
+  return 0;
+}
+
+__attribute__((noinline)) int
+myopenva (const char *path, int oflag, ...)
+{
+  int mode = 0;
+  va_list ap;
+  if ((oflag & 0x40) != 0)
+    {
+      va_start (ap, oflag);
+      mode = va_arg (ap, int);
+      va_end (ap);
+    }
+  if (expected_char++ != path[0] || path[1] != '\0')
+    abort ();
+  switch (path[0])
+    {
+    case 'a':
+      if (oflag != 0x43 || mode != 0644)
+	abort ();
+      break;
+    case 'b':
+      if (oflag != 0x3)
+	abort ();
+      break;
+    case 'c':
+      if (oflag != 0x2)
+	abort ();
+      break;
+    case 'd':
+      if (oflag != 0x43 || mode != 0600)
+	abort ();
+      break;
+    case 'e':
+      if (oflag != 0x3)
+	abort ();
+      break;
+    default:
+      abort ();
+    }
+  return 0;
+}
+
+extern inline __attribute__((always_inline, gnu_inline)) int
+myopen (const char *path, int oflag, ...)
+{
+  if (__builtin_va_arg_pack_len () > 1)
+    warn_open_too_many_arguments ();
+
+  if (__builtin_constant_p (oflag))
+    {
+      if ((oflag & 0x40) != 0 && __builtin_va_arg_pack_len () < 1)
+	{
+	  warn_open_missing_mode ();
+	  return myopen2 (path, oflag);
+	}
+      return myopenva (path, oflag, __builtin_va_arg_pack ());
+    }
+
+  if (__builtin_va_arg_pack_len () < 1)
+    return myopen2 (path, oflag);
+
+  return myopenva (path, oflag, __builtin_va_arg_pack ());
+}
+
+volatile int l0;
+
+int
+main (void)
+{
+  expected_char = 'a';
+  if (myopen ("a", 0x43, 0644))
+    abort ();
+  if (myopen ("b", 0x3, 0755))
+    abort ();
+  if (myopen ("c", 0x2))
+    abort ();
+  if (myopen ("d", l0 + 0x43, 0600))
+    abort ();
+  if (myopen ("e", l0 + 0x3, 0700))
+    abort ();
+  if (myopen ("f", l0 + 0x2))
+    abort ();
+  /* Invalid use of myopen, but only detectable at runtime.  */
+  if (myopen ("g", l0 + 0x43) != -6)
+    abort ();
+  return 0;
+}
--- gcc/testsuite/g++.dg/ext/va-arg-pack-len-1.C.jj	2007-09-08 13:10:00.000000000 +0200
+++ gcc/testsuite/g++.dg/ext/va-arg-pack-len-1.C	2007-09-08 13:10:16.000000000 +0200
@@ -0,0 +1,120 @@
+// { dg-do run }
+// { dg-options "-O2" }
+
+#include <stdarg.h>
+
+extern "C" int warn_open_missing_mode (void);
+extern "C" int warn_open_too_many_arguments (void);
+extern "C" void abort (void);
+
+char expected_char;
+
+__attribute__((noinline)) int
+myopen2 (const char *path, int oflag)
+{
+  if (expected_char++ != path[0] || path[1] != '\0')
+    abort ();
+  switch (path[0])
+    {
+    case 'f':
+      if (oflag != 0x2)
+	abort ();
+      break;
+    case 'g':
+      if (oflag != 0x43)
+	abort ();
+      // In real __open_2 this would terminate the program:
+      // open with O_CREAT without third argument.
+      return -6;
+    default:
+      abort ();
+    }
+  return 0;
+}
+
+__attribute__((noinline)) int
+myopenva (const char *path, int oflag, ...)
+{
+  int mode = 0;
+  va_list ap;
+  if ((oflag & 0x40) != 0)
+    {
+      va_start (ap, oflag);
+      mode = va_arg (ap, int);
+      va_end (ap);
+    }
+  if (expected_char++ != path[0] || path[1] != '\0')
+    abort ();
+  switch (path[0])
+    {
+    case 'a':
+      if (oflag != 0x43 || mode != 0644)
+	abort ();
+      break;
+    case 'b':
+      if (oflag != 0x3)
+	abort ();
+      break;
+    case 'c':
+      if (oflag != 0x2)
+	abort ();
+      break;
+    case 'd':
+      if (oflag != 0x43 || mode != 0600)
+	abort ();
+      break;
+    case 'e':
+      if (oflag != 0x3)
+	abort ();
+      break;
+    default:
+      abort ();
+    }
+  return 0;
+}
+
+extern inline __attribute__((always_inline, gnu_inline)) int
+myopen (const char *path, int oflag, ...)
+{
+  if (__builtin_va_arg_pack_len () > 1)
+    warn_open_too_many_arguments ();
+
+  if (__builtin_constant_p (oflag))
+    {
+      if ((oflag & 0x40) != 0 && __builtin_va_arg_pack_len () < 1)
+	{
+	  warn_open_missing_mode ();
+	  return myopen2 (path, oflag);
+	}
+      return myopenva (path, oflag, __builtin_va_arg_pack ());
+    }
+
+  if (__builtin_va_arg_pack_len () < 1)
+    return myopen2 (path, oflag);
+
+  return myopenva (path, oflag, __builtin_va_arg_pack ());
+}
+
+volatile int l0;
+
+int
+main (void)
+{
+  expected_char = 'a';
+  if (myopen ("a", 0x43, 0644))
+    abort ();
+  if (myopen ("b", 0x3, 0755))
+    abort ();
+  if (myopen ("c", 0x2))
+    abort ();
+  if (myopen ("d", l0 + 0x43, 0600))
+    abort ();
+  if (myopen ("e", l0 + 0x3, 0700))
+    abort ();
+  if (myopen ("f", l0 + 0x2))
+    abort ();
+  // Invalid use of myopen, but only detectable at runtime.
+  if (myopen ("g", l0 + 0x43) != -6)
+    abort ();
+  return 0;
+}

	Jakub