[PATCH] Fixups for Martin's gimple-ssa-sprintf.c patch

David Malcolm dmalcolm@redhat.com
Thu Aug 25 15:54:00 GMT 2016


Martin: here are the fixups for your patch I needed to apply to make
it work with mine.  I couldn't actually get any of your existing test
cases to emit locations within the string literals, due to them all
being embedded in macro expansions (possibly relating to PR c/77328),
so I added a simple testcase using -fdiagnostics-show-caret, which
does successfully show a range within the string.

Posting in the hope that it's helpful; I haven't attempted a bootstrap
with it.

gcc/ChangeLog:
	* gimple-ssa-sprintf.c (class substring_loc): Delete.
	(g_string_concat_db): Delete.
	(substring_loc::get_location): Delete.
	(format_warning_va): Delete.
	(format_directive): Update use of substring_loc ctor to supply
	type of format string.
	(add_bytes): Likewise.

gcc/testsuite/ChangeLog:
	* gcc.dg/tree-ssa/builtin-sprintf-warn-4.c: New test case.
---
 gcc/gimple-ssa-sprintf.c                           | 129 +--------------------
 .../gcc.dg/tree-ssa/builtin-sprintf-warn-4.c       |  17 +++
 2 files changed, 21 insertions(+), 125 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-4.c

diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index ff8697c..fa63047 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -349,129 +349,6 @@ get_format_string (tree format, location_t *ploc)
   return fmtstr;
 }
 
-/* Describes a location range outlining a substring within a string
-   literal.  */
-
-class substring_loc
-{
- public:
-  substring_loc (location_t fmt_string_loc,
-		 int caret_idx, int start_idx, int end_idx)
-    : m_fmt_string_loc (fmt_string_loc),
-    m_caret_idx (caret_idx), m_start_idx (start_idx), m_end_idx (end_idx)
-  { }
-
-  const char *get_location (location_t *out_loc) const;
-
-  location_t get_fmt_string_loc () const { return m_fmt_string_loc; }
-
- private:
-  location_t m_fmt_string_loc;
-  int m_caret_idx;
-  int m_start_idx;
-  int m_end_idx;
-};
-
-/* The global record of string concatentations, for use in extracting
-   locations within string literals.  */
-
-GTY(()) string_concat_db *g_string_concat_db;
-
-/* Attempt to determine the source location of the substring.
-   If successful, return NULL and write the source location to *OUT_LOC.
-   Otherwise return an error message.  Error messages are intended
-   for GCC developers (to help debugging) rather than for end-users.  */
-
-const char *
-substring_loc::get_location (location_t *out_loc) const
-{
-  gcc_assert (out_loc);
-
-  if (!g_string_concat_db)
-    g_string_concat_db
-      = new (ggc_alloc <string_concat_db> ()) string_concat_db ();
-
-  static struct cpp_reader* parse_in;
-  if (!parse_in)
-    {
-      /* Create and initialize a preprocessing reader.  */
-      parse_in = cpp_create_reader (CLK_GNUC99, ident_hash, line_table);
-      cpp_init_iconv (parse_in);
-    }
-
-  return get_source_location_for_substring (parse_in, g_string_concat_db,
-					    m_fmt_string_loc, CPP_STRING,
-					    m_caret_idx, m_start_idx, m_end_idx,
-					    out_loc);
-}
-
-static ATTRIBUTE_GCC_DIAG (5,0) bool
-format_warning_va (const substring_loc &fmt_loc,
-		   const source_range *param_range,
-		   const char *corrected_substring,
-		   int opt, const char *gmsgid, va_list *ap)
-{
-  bool substring_within_range = false;
-  location_t primary_loc;
-  location_t fmt_substring_loc = UNKNOWN_LOCATION;
-  source_range fmt_loc_range
-    = get_range_from_loc (line_table, fmt_loc.get_fmt_string_loc ());
-  const char *err = fmt_loc.get_location (&fmt_substring_loc);
-  source_range fmt_substring_range
-    = get_range_from_loc (line_table, fmt_substring_loc);
-  if (err)
-    /* Case 3: unable to get substring location.  */
-    primary_loc = fmt_loc.get_fmt_string_loc ();
-  else
-    {
-      if (fmt_substring_range.m_start >= fmt_loc_range.m_start
-	  && fmt_substring_range.m_finish <= fmt_loc_range.m_finish)
-	/* Case 1.  */
-	{
-	  substring_within_range = true;
-	  primary_loc = fmt_substring_loc;
-	}
-      else
-	/* Case 2.  */
-	{
-	  substring_within_range = false;
-	  primary_loc = fmt_loc.get_fmt_string_loc ();
-	}
-    }
-
-  rich_location richloc (line_table, primary_loc);
-
-  if (param_range)
-    {
-      location_t param_loc = make_location (param_range->m_start,
-					    param_range->m_start,
-					    param_range->m_finish);
-      richloc.add_range (param_loc, false);
-    }
-
-  if (!err && corrected_substring && substring_within_range)
-    richloc.add_fixit_replace (fmt_substring_range, corrected_substring);
-
-  diagnostic_info diagnostic;
-  diagnostic_set_info (&diagnostic, gmsgid, ap, &richloc, DK_WARNING);
-  diagnostic.option_index = opt;
-  bool warned = report_diagnostic (&diagnostic);
-
-  if (!err && fmt_substring_loc && !substring_within_range)
-    /* Case 2.  */
-    if (warned)
-      {
-	rich_location substring_richloc (line_table, fmt_substring_loc);
-	if (corrected_substring)
-	  substring_richloc.add_fixit_replace (fmt_substring_range,
-					       corrected_substring);
-	inform_at_rich_loc (&substring_richloc,
-			    "format string is defined here");
-      }
-
-  return warned;
-}
-
 static ATTRIBUTE_GCC_DIAG (5,0) bool
 fmtwarn (const substring_loc &fmt_loc,
 	 const source_range *param_range,
@@ -1695,7 +1572,8 @@ format_directive (const pass_sprintf_length::call_info &info,
 
   /* Create a location for the whole directive from the % to the format
      specifier.  */
-  substring_loc dirloc (info.fmtloc, offset, offset, offset + cvtlen - 1);
+  substring_loc dirloc (info.fmtloc, TREE_TYPE (info.format), offset, offset,
+			offset + cvtlen - 1);
 
   /* Also create a location range for the argument if possible.
      This doesn't work for integer literals or function calls.  */
@@ -1920,7 +1798,8 @@ add_bytes (const pass_sprintf_length::call_info &info,
       size_t len = strlen (info.fmtstr + off);
 
       substring_loc loc
-	(info.fmtloc, off - !len, len ? off : 0, off + len - !!len);
+	(info.fmtloc, TREE_TYPE (info.format), off - !len, len ? off : 0,
+	 off + len - !!len);
 
       /* Is the output of the last directive the result of the argument
 	 being within a range whose lower bound would fit in the buffer
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-4.c b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-4.c
new file mode 100644
index 0000000..b10a9cd
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-4.c
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+/* { dg-options "-std=c99 -O2 -Wformat -Wformat-length=1 -fdiagnostics-show-caret" } */
+
+void test (void)
+{
+  char a[4];
+  __builtin_sprintf (a, "abc%sghi", "def"); /* { dg-warning "29: .%s. directive writing 3 bytes into a region of size 1" } */
+  /* { dg-begin-multiline-output "" }
+   __builtin_sprintf (a, "abc%sghi", "def");
+                             ^~      ~~~~~
+     { dg-end-multiline-output "" } */
+
+  /* { dg-begin-multiline-output "" }
+   __builtin_sprintf (a, "abc%sghi", "def");
+   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+     { dg-end-multiline-output "" } */
+}
-- 
1.8.5.3



More information about the Gcc-patches mailing list