This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Patch to check asm_fprintf format specifiers [take 2]


This is my second attempt to enable checking format specifiers for
asm_fprintf style calls.  The change from last time is the
introduction of a runtime hook for determining HOST_WIDE_INT in case
one is building a cross-compiler and the values for HWI between host
and target are different.

The hook introduces no new language constructs.  I simply declare a
magic typedef identifier and have GCC look it up when applying the
attribute.

I thought about wrapping this new stuff in a special macro like
ENABLE_CHECKING or a derivative as discussed last time.  However for
the warnings to appear this requires both the compiler being used and
the compiler being built to have ENABLE_CHECKING turned on.  We get
this guaranteed during a bootstrap in stage2 on mainline.  However it
occured to me that when building cross-compiler it's unlikely that
this will normally be the case because typically in the field one uses
an installed/released compiler as the host CC.  Thus we'll lose ever
checking a substantial number of cases.

Since the hook doesn't require a new language construct the situation
described in the previous thread, where a new GCC removes this feature
in favor of real extensible checking and then proceeds to try and
compile older GCC releases with this checking style and fails to parse
it, should not arise.  So I'm going to suggest that we leave this in
by default without a wrapping macro.  Recall it's only usable for the
specific syntax accepted by asm_fprintf, and requires the special
HOST_WIDE_INT identifier to be declared.  That should be discouraging
enough if anyone decides to use it outside of GCC, so I don't expect
anyone will be able to rely on it.  I hope this is acceptable.

I suppose what remains is documentation and a testsuite entry.
Regarding docs, since this is a GCC-only feature I included docs in
comments in the code.  I don't want to encourage anyone reading the
manual to want to use this feature.

Regarding a testcase, I'm currently in the process of
designing/creating one.  However I recently posted a patch for
updating the internals of asm_fprintf to fix broken and/or add missing
specifiers.  Until that settles down and/or is accepted/rejected, I'd
prefer to submit the testcase separately.

So...

Tested via full bootstrap (minus java cause it's broken on solaris2)
and testsuite run on sparc-sun-solaris2.7, no regressions.  I also
built a cross-compiler targetted to arm-unknown-pe using a host
compiler with this patch and verified that we get warnings for the
various buggy asm_fprintf specifiers I found with V1 of this patch.

Ok for mainline?

		Thanks,
		--Kaveh


2003-05-30  Kaveh R. Ghazi  <ghazi@caip.rutgers.edu>

	* builtin-attrs.def (ATTR_ASM_FPRINTF): New.
	* c-format.c (enum format_type): Add asm_fprintf_format_type.
	(NOARGUMENTS, asm_fprintf_length_specs, asm_fprintf_flag_specs,
	asm_fprintf_flag_pairs, asm_fprintf_char_table): New.
	(format_types_orig): Renamed from format_types.  Add new data.
	(format_types): Declare as pointer.
	(handle_format_attribute): Move later in file so we have all
	necessary declarations.  Add section to capture HOST_WIDE_INT.
	* output.h (ATTRIBUTE_ASM_FPRINTF, __gcc_host_wide_int__): New.
	(asm_fprintf): Mark with ATTRIBUTE_ASM_FPRINTF.

diff -rup orig/egcc-CVS20030529/gcc/builtin-attrs.def egcc-CVS20030529/gcc/builtin-attrs.def
--- orig/egcc-CVS20030529/gcc/builtin-attrs.def	2003-05-04 18:15:21.000000000 -0400
+++ egcc-CVS20030529/gcc/builtin-attrs.def	2003-05-30 20:26:27.189053000 -0400
@@ -86,6 +86,7 @@ DEF_ATTR_IDENT (ATTR_NONNULL, "nonnull")
 DEF_ATTR_IDENT (ATTR_NORETURN, "noreturn")
 DEF_ATTR_IDENT (ATTR_NOTHROW, "nothrow")
 DEF_ATTR_IDENT (ATTR_PRINTF, "printf")
+DEF_ATTR_IDENT (ATTR_ASM_FPRINTF, "asm_fprintf")
 DEF_ATTR_IDENT (ATTR_PURE, "pure")
 DEF_ATTR_IDENT (ATTR_SCANF, "scanf")
 DEF_ATTR_IDENT (ATTR_STRFMON, "strfmon")
diff -rup orig/egcc-CVS20030529/gcc/c-format.c egcc-CVS20030529/gcc/c-format.c
--- orig/egcc-CVS20030529/gcc/c-format.c	2003-05-30 19:38:35.107708000 -0400
+++ egcc-CVS20030529/gcc/c-format.c	2003-05-30 20:28:57.216076000 -0400
@@ -56,9 +56,9 @@ set_Wformat (setting)
 
 /* This must be in the same order as format_types, with format_type_error
    last.  */
-enum format_type { printf_format_type, scanf_format_type,
-		   strftime_format_type, strfmon_format_type,
-		   format_type_error };
+enum format_type { printf_format_type, asm_fprintf_format_type,
+		   scanf_format_type, strftime_format_type,
+		   strfmon_format_type, format_type_error };
 
 typedef struct function_format_info
 {
@@ -71,74 +71,6 @@ static bool decode_format_attr		PARAMS (
 						 function_format_info *, int));
 static enum format_type decode_format_type	PARAMS ((const char *));
 
-/* Handle a "format" attribute; arguments as in
-   struct attribute_spec.handler.  */
-tree
-handle_format_attribute (node, name, args, flags, no_add_attrs)
-     tree *node;
-     tree name ATTRIBUTE_UNUSED;
-     tree args;
-     int flags;
-     bool *no_add_attrs;
-{
-  tree type = *node;
-  function_format_info info;
-  tree argument;
-  unsigned HOST_WIDE_INT arg_num;
-
-  if (!decode_format_attr (args, &info, 0))
-    {
-      *no_add_attrs = true;
-      return NULL_TREE;
-    }
-
-  /* If a parameter list is specified, verify that the format_num
-     argument is actually a string, in case the format attribute
-     is in error.  */
-  argument = TYPE_ARG_TYPES (type);
-  if (argument)
-    {
-      for (arg_num = 1; argument != 0 && arg_num != info.format_num;
-	   ++arg_num, argument = TREE_CHAIN (argument))
-	;
-
-      if (! argument
-	  || TREE_CODE (TREE_VALUE (argument)) != POINTER_TYPE
-	  || (TYPE_MAIN_VARIANT (TREE_TYPE (TREE_VALUE (argument)))
-	      != char_type_node))
-	{
-	  if (!(flags & (int) ATTR_FLAG_BUILT_IN))
-	    error ("format string arg not a string type");
-	  *no_add_attrs = true;
-	  return NULL_TREE;
-	}
-
-      else if (info.first_arg_num != 0)
-	{
-	  /* Verify that first_arg_num points to the last arg,
-	     the ...  */
-	  while (argument)
-	    arg_num++, argument = TREE_CHAIN (argument);
-
-	  if (arg_num != info.first_arg_num)
-	    {
-	      if (!(flags & (int) ATTR_FLAG_BUILT_IN))
-		error ("args to be formatted is not '...'");
-	      *no_add_attrs = true;
-	      return NULL_TREE;
-	    }
-	}
-    }
-
-  if (info.format_type == strftime_format_type && info.first_arg_num != 0)
-    {
-      error ("strftime formats cannot format arguments");
-      *no_add_attrs = true;
-      return NULL_TREE;
-    }
-
-  return NULL_TREE;
-}
 
 
 /* Handle a "format_arg" attribute; arguments as in
@@ -402,6 +334,7 @@ typedef struct
 
 
 /* Macros to fill out tables of these.  */
+#define NOARGUMENTS	{ T89_V, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }
 #define BADLEN	{ 0, NULL, NULL }
 #define NOLENGTHS	{ BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }
 
@@ -574,6 +507,13 @@ static const format_length_info printf_l
   { NULL, 0, 0, NULL, 0, 0 }
 };
 
+/* Length specifiers valid for asm_fprintf.  */
+static const format_length_info asm_fprintf_length_specs[] =
+{
+  { "l", FMT_LEN_l, STD_C89, "ll", FMT_LEN_ll, STD_C89 },
+  { "w", FMT_LEN_none, STD_C89, NULL, 0, 0 },
+  { NULL, 0, 0, NULL, 0, 0 }
+};
 
 /* This differs from printf_length_specs only in that "Z" is not accepted.  */
 static const format_length_info scanf_length_specs[] =
@@ -622,6 +562,26 @@ static const format_flag_pair printf_fla
   { 0, 0, 0, 0 }
 };
 
+static const format_flag_spec asm_fprintf_flag_specs[] =
+{
+  { ' ',  0, 0, N_("` ' flag"),        N_("the ` ' printf flag"),              STD_C89 },
+  { '+',  0, 0, N_("`+' flag"),        N_("the `+' printf flag"),              STD_C89 },
+  { '#',  0, 0, N_("`#' flag"),        N_("the `#' printf flag"),              STD_C89 },
+  { '0',  0, 0, N_("`0' flag"),        N_("the `0' printf flag"),              STD_C89 },
+  { '-',  0, 0, N_("`-' flag"),        N_("the `-' printf flag"),              STD_C89 },
+  { 'w',  0, 0, N_("field width"),     N_("field width in printf format"),     STD_C89 },
+  { 'p',  0, 0, N_("precision"),       N_("precision in printf format"),       STD_C89 },
+  { 'L',  0, 0, N_("length modifier"), N_("length modifier in printf format"), STD_C89 },
+  { 0, 0, 0, NULL, NULL, 0 }
+};
+
+static const format_flag_pair asm_fprintf_flag_pairs[] =
+{
+  { ' ', '+', 1, 0   },
+  { '0', '-', 1, 0   },
+  { '0', 'p', 1, 'i' },
+  { 0, 0, 0, 0 }
+};
 
 static const format_flag_spec scanf_flag_specs[] =
 {
@@ -767,6 +727,28 @@ static const format_char_info print_char
   { NULL,  0, 0, NOLENGTHS, NULL, NULL }
 };
 
+static const format_char_info asm_fprintf_char_table[] =
+{
+  /* C89 conversion specifiers.  */
+  { "di",  0, STD_C89, { T89_I,   BADLEN,  BADLEN,  T89_L,   T9L_LL,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "-wp0 +",  "i" },
+  { "oxX", 0, STD_C89, { T89_UI,  BADLEN,  BADLEN,  T89_UL,  T9L_ULL, BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "-wp0#",   "i" },
+  { "u",   0, STD_C89, { T89_UI,  BADLEN,  BADLEN,  T89_UL,  T9L_ULL, BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "-wp0",    "i" },
+  { "fge", 0, STD_C89, { T89_D,   BADLEN,  BADLEN,  T89_D,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "-wp0 +#",  "" },
+  { "c",   0, STD_C89, { T89_I,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "-w",       "" },
+  { "s",   1, STD_C89, { T89_C,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "-wp",    "cR" },
+  { "p",   1, STD_C89, { T89_V,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "-w",      "c" },
+
+  /* asm_fprintf conversion specifiers.  */
+  { "O",   0, STD_C89, NOARGUMENTS, "",      ""   },
+  { "R",   0, STD_C89, NOARGUMENTS, "",      ""   },
+  { "I",   0, STD_C89, NOARGUMENTS, "",      ""   },
+  { "L",   0, STD_C89, NOARGUMENTS, "",      ""   },
+  { "U",   0, STD_C89, NOARGUMENTS, "",      ""   },
+  { "r",   0, STD_C89, { T89_I,   BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN,  BADLEN  }, "",  "" },
+  { "@",   0, STD_C89, NOARGUMENTS, "",      ""   },
+  { NULL,  0, 0, NOLENGTHS, NULL, NULL }
+};
+
 static const format_char_info scan_char_table[] =
 {
   /* C89 conversion specifiers.  */
@@ -822,7 +804,7 @@ static const format_char_info monetary_c
 
 
 /* This must be in the same order as enum format_type.  */
-static const format_kind_info format_types[] =
+static const format_kind_info format_types_orig[] =
 {
   { "printf",   printf_length_specs,  print_char_table, " +#0-'I", NULL, 
     printf_flag_specs, printf_flag_pairs,
@@ -830,6 +812,12 @@ static const format_kind_info format_typ
     'w', 0, 'p', 0, 'L',
     &integer_type_node, &integer_type_node
   },
+  { "asm_fprintf",   asm_fprintf_length_specs,  asm_fprintf_char_table, " +#0-", NULL, 
+    asm_fprintf_flag_specs, asm_fprintf_flag_pairs,
+    FMT_FLAG_ARG_CONVERT|FMT_FLAG_EMPTY_PREC_OK,
+    'w', 0, 'p', 0, 'L',
+    &integer_type_node, &integer_type_node
+  },
   { "scanf",    scanf_length_specs,   scan_char_table,  "*'I", NULL, 
     scanf_flag_specs, scanf_flag_pairs,
     FMT_FLAG_ARG_CONVERT|FMT_FLAG_SCANF_A_KLUDGE|FMT_FLAG_USE_DOLLAR|FMT_FLAG_ZERO_WIDTH_BAD|FMT_FLAG_DOLLAR_GAP_POINTER_OK,
@@ -848,6 +836,10 @@ static const format_kind_info format_typ
   }
 };
 
+/* This layer of indirection allows GCC to reassign format_types with
+   new data if necessary, while still allowing the original data to be
+   const.  */
+static const format_kind_info *format_types = format_types_orig;
 
 /* Structure detailing the results of checking a format function call
    where the format expression may be a conditional expression with
@@ -2359,3 +2351,116 @@ check_format_types (status, types)
       }
     }
 }
+
+/* Handle a "format" attribute; arguments as in
+   struct attribute_spec.handler.  */
+tree
+handle_format_attribute (node, name, args, flags, no_add_attrs)
+     tree *node;
+     tree name ATTRIBUTE_UNUSED;
+     tree args;
+     int flags;
+     bool *no_add_attrs;
+{
+  tree type = *node;
+  function_format_info info;
+  tree argument;
+  unsigned HOST_WIDE_INT arg_num;
+
+  if (!decode_format_attr (args, &info, 0))
+    {
+      *no_add_attrs = true;
+      return NULL_TREE;
+    }
+
+  /* If a parameter list is specified, verify that the format_num
+     argument is actually a string, in case the format attribute
+     is in error.  */
+  argument = TYPE_ARG_TYPES (type);
+  if (argument)
+    {
+      for (arg_num = 1; argument != 0 && arg_num != info.format_num;
+	   ++arg_num, argument = TREE_CHAIN (argument))
+	;
+
+      if (! argument
+	  || TREE_CODE (TREE_VALUE (argument)) != POINTER_TYPE
+	  || (TYPE_MAIN_VARIANT (TREE_TYPE (TREE_VALUE (argument)))
+	      != char_type_node))
+	{
+	  if (!(flags & (int) ATTR_FLAG_BUILT_IN))
+	    error ("format string arg not a string type");
+	  *no_add_attrs = true;
+	  return NULL_TREE;
+	}
+
+      else if (info.first_arg_num != 0)
+	{
+	  /* Verify that first_arg_num points to the last arg,
+	     the ...  */
+	  while (argument)
+	    arg_num++, argument = TREE_CHAIN (argument);
+
+	  if (arg_num != info.first_arg_num)
+	    {
+	      if (!(flags & (int) ATTR_FLAG_BUILT_IN))
+		error ("args to be formatted is not '...'");
+	      *no_add_attrs = true;
+	      return NULL_TREE;
+	    }
+	}
+    }
+
+  if (info.format_type == strftime_format_type && info.first_arg_num != 0)
+    {
+      error ("strftime formats cannot format arguments");
+      *no_add_attrs = true;
+      return NULL_TREE;
+    }
+
+  /* If this is format type __asm_fprintf__, we have to initialize
+     GCC's notion of HOST_WIDE_INT for checking %wd.  */
+  if (info.format_type == asm_fprintf_format_type)
+    {
+      static tree hwi;
+      tree orig;
+      
+      /* For this custom check to work, one must have issued:
+	 "typedef HOST_WIDE_INT __gcc_host_wide_int__;"
+	 in your source code prior to using this attribute.  */
+      if (!hwi)
+        {
+	  format_kind_info *new_format_types;
+	  format_length_info *new_asm_fprintf_length_specs;
+	  
+	  if (!(hwi = maybe_get_identifier ("__gcc_host_wide_int__")))
+	    abort();
+
+	  /* Create a new (writable) copy of asm_fprintf_length_specs.  */
+	  new_asm_fprintf_length_specs =
+	    xmalloc (sizeof (asm_fprintf_length_specs));
+	  memcpy (new_asm_fprintf_length_specs, asm_fprintf_length_specs,
+		  sizeof (asm_fprintf_length_specs));
+
+	  /* Create a new (writable) copy of format_types.  */
+	  new_format_types = xmalloc (sizeof (format_types_orig));
+	  memcpy (new_format_types, format_types_orig, sizeof (format_types_orig));
+	  
+	  /* Find the underlying type for HOST_WIDE_INT.  */
+	  orig = DECL_ORIGINAL_TYPE (identifier_global_value (hwi));
+	  if (orig == long_integer_type_node)
+	    new_asm_fprintf_length_specs[1].index = FMT_LEN_l;
+	  else if (orig == long_long_integer_type_node)
+	    new_asm_fprintf_length_specs[1].index = FMT_LEN_ll;
+	  else
+	    abort();
+
+	  /* Assign the new data for use.  */
+	  new_format_types[asm_fprintf_format_type].length_char_specs =
+	    new_asm_fprintf_length_specs;
+	  format_types = new_format_types;
+	}
+    }
+
+  return NULL_TREE;
+}
diff -rup orig/egcc-CVS20030529/gcc/output.h egcc-CVS20030529/gcc/output.h
--- orig/egcc-CVS20030529/gcc/output.h	2003-05-12 20:01:38.000000000 -0400
+++ egcc-CVS20030529/gcc/output.h	2003-05-30 20:26:39.817930000 -0400
@@ -105,7 +105,17 @@ extern void output_addr_const PARAMS ((F
 
 /* Output a string of assembler code, substituting numbers, strings
    and fixed syntactic prefixes.  */
-extern void asm_fprintf		PARAMS ((FILE *file, const char *p, ...));
+#if GCC_VERSION >= 3004
+#define ATTRIBUTE_ASM_FPRINTF(m, n) __attribute__ ((__format__ (__asm_fprintf__, m, n))) ATTRIBUTE_NONNULL(m)
+/* This is a magic identifier which allows GCC to figure out the type
+   of HOST_WIDE_INT for %wd specifier checks.  You must issue this
+   typedef before using the __asm_fprintf__ format attribute.  */
+typedef HOST_WIDE_INT __gcc_host_wide_int__;
+#else
+#define ATTRIBUTE_ASM_FPRINTF(m, n) ATTRIBUTE_NONNULL(m)
+#endif
+
+extern void asm_fprintf		PARAMS ((FILE *file, const char *p, ...)) ATTRIBUTE_ASM_FPRINTF(2, 3);
 
 /* Split up a CONST_DOUBLE or integer constant rtx into two rtx's for single
    words.  */


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]