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]

RFC: need help with patch to check asm_fprintf format specifiers


Some day we'll have a nice way of defining custom format specifier
checks.  Until then, I think it would be useful to add hard-wired
checks for gcc's own custom formats so that we catch more bugs.

I added a new "format" called "asm_fprintf" to check that function's
specifiers.  It actually works and found a bunch of real bugs.
However there's a shortcoming I'd like some help thinking about how to
solve.

Essentially, we're hosed by the fact that the specifier validity may
change depending on the target.  One case is where the type of
HOST_WIDE_INT differs between the host and target.  If you build a
cross-compiler, then calls to asm_fprintf %wd are checked against the
host's notion of HOST_WIDE_INT, but you want it to check for HWI of
the target platform.

The word "host" is misleading here.  E.g. if I build a native gcc on
solaris2.7, HOST_WIDE_INT will be "long long".  When I take that
compiler and use it to build a cross to a 32-bit target, then HWI for
that target will be "long".  The mismatch happens because the first
compiler has hardwired %wd to match "long long" but when building the
cross-compiler it sees "long" but mistakenly warns about it.

The second discrepancy occurs when a platform defines
ASM_FPRINTF_EXTENSIONS.  Again the host may not agree with the target
on what if any extensions are valid.  I think so far only "arm"
defines it.  I worked around it by hard-wiring the extra specifiers
in.  IMHO, we should get rid of ASM_FPRINTF_EXTENSIONS anyway.  Just
make these few extentions always valid in asm_fprintf.

Anyway I'm still stuck on the %wd thing.  I think basically I need a
way to tell the host compiler at runtime what type %wd should check
for.  But then we get back into the extensible feature which is beyond
the scope of what I want to do.  Is there a middle ground between
complete extensibility and no extensibility where we could add some
sort of pragma or hook for just this minor issue?

		Thanks,
		--Kaveh

My in-progress raw patch is below for discussion purposes only:

diff -rup orig/egcc-CVS20030519/gcc/builtin-attrs.def egcc-CVS20030519/gcc/builtin-attrs.def
--- orig/egcc-CVS20030519/gcc/builtin-attrs.def	2003-05-04 18:15:21.000000000 -0400
+++ egcc-CVS20030519/gcc/builtin-attrs.def	2003-05-22 00:23:59.718941000 -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-CVS20030519/gcc/c-format.c egcc-CVS20030519/gcc/c-format.c
--- orig/egcc-CVS20030519/gcc/c-format.c	2003-05-17 18:18:38.000000000 -0400
+++ egcc-CVS20030519/gcc/c-format.c	2003-05-22 10:54:12.908464000 -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
 {
@@ -404,6 +404,7 @@ typedef struct
 /* Macros to fill out tables of these.  */
 #define BADLEN	{ 0, NULL, NULL }
 #define NOLENGTHS	{ BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }
+#define NOARGUMENTS	{ T89_V, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN, BADLEN }
 
 
 /* Structure describing a format conversion specifier (or a set of specifiers
@@ -574,6 +575,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_w, 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 +630,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 +795,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.  */
@@ -830,6 +880,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,
diff -rup orig/egcc-CVS20030519/gcc/hwint.h egcc-CVS20030519/gcc/hwint.h
--- orig/egcc-CVS20030519/gcc/hwint.h	2003-01-06 15:31:16.000000000 -0500
+++ egcc-CVS20030519/gcc/hwint.h	2003-05-22 00:41:19.536727000 -0400
@@ -47,14 +47,17 @@ extern char sizeof_long_long_must_be_8[s
 #if HOST_BITS_PER_LONG >= 64 || !defined NEED_64BIT_HOST_WIDE_INT
 #   define HOST_BITS_PER_WIDE_INT HOST_BITS_PER_LONG
 #   define HOST_WIDE_INT long
+#   define FMT_LEN_w FMT_LEN_l
 #else
 # if HOST_BITS_PER_LONGLONG >= 64
 #   define HOST_BITS_PER_WIDE_INT HOST_BITS_PER_LONGLONG
 #   define HOST_WIDE_INT long long
+#   define FMT_LEN_w FMT_LEN_ll
 # else
 #  if HOST_BITS_PER___INT64 >= 64
 #   define HOST_BITS_PER_WIDE_INT HOST_BITS_PER___INT64
 #   define HOST_WIDE_INT __int64
+#   define FMT_LEN_w FMT_LEN_ll /* asm_fprintf doesn't accept the 'j' length modifier.  */
 #  else
     #error "Unable to find a suitable type for HOST_WIDE_INT"
 #  endif
diff -rup orig/egcc-CVS20030519/gcc/output.h egcc-CVS20030519/gcc/output.h
--- orig/egcc-CVS20030519/gcc/output.h	2003-05-12 20:01:38.000000000 -0400
+++ egcc-CVS20030519/gcc/output.h	2003-05-22 00:21:34.100039000 -0400
@@ -105,7 +105,13 @@ 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)
+#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]