Re: [PATCH 1/4] enhance overflow and truncation detection in strncpy and strncat (PR 81117)

2017-08-14 Thread Martin Sebor

On 08/09/2017 10:44 PM, Jeff Law wrote:

On 08/06/2017 02:07 PM, Martin Sebor wrote:

The attached patch adds support for a new GCC format specifier,
G, that behaves like %K but accepts a gcall* argument.  This
makes it possible to provide inlining context for "artificial"
inline functions like strncpy (with _FORTIFY_SOURCE) in
diagnostics issued from the middle end.

Martin

gcc-81117-1.diff


PR c/81117 - Improve buffer overflow checking in strncpy

gcc/ChangeLog:

PR c/81117
* tree-diagnostic.c (default_tree_printer): Handle %G.
* tree-pretty-print.h (percent_G_format): Declare new function.
* tree-pretty-print.c (percent_K_format): Define a static overload.
(percent_G_format): New function.

gcc/c/ChangeLog:

PR c/81117
* c-objc-common.c (c_objc_common_init): Handle 'G'.

gcc/c-family/ChangeLog:

* c-format.h (T89_G): New macro.
* c-format.c (local_gcall_ptr_node): New variable.
(init_dynamic_diag_info): Initialize it.

gcc/cp/ChangeLog:

PR c/81117
* error.c (cp_printer): Handle 'G'.

gcc/testsuite/ChangeLog:

PR c/81117
* gcc.dg/format/gcc_diag-10.c: Exercise %G.

diff --git a/gcc/tree-diagnostic.c b/gcc/tree-diagnostic.c
index 52b7e7f..ad58b69 100644
--- a/gcc/tree-diagnostic.c
+++ b/gcc/tree-diagnostic.c
@@ -275,6 +275,10 @@ default_tree_printer (pretty_printer *pp, text_info *text, 
const char *spec,
   t = va_arg (*text->args_ptr, tree);
   break;

+case 'G':
+  percent_G_format (text);
+  return true;
+
 case 'K':
   percent_K_format (text);
   return true;
diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c
index 4d8177c..7c4c805 100644
--- a/gcc/tree-pretty-print.c
+++ b/gcc/tree-pretty-print.c
@@ -33,6 +33,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "dumpfile.h"
 #include "internal-fn.h"
 #include "gomp-constants.h"
+#include "gimple.h"

This is an indication you're probably putting the 'G' handling in the
wrong place.  Wouldn't gimple-pretty-print.c be more correct?

That's my only objection to this patch, so if it moves trivially, then
it's pre-approved.  If it's non-trivial, then we'll want another iteration.


I moved it and after retesting committed in r251098.  The diff
is attached for reference.

Btw., there are lots of test suite failures that make it tricky
to spot regressions, mostly in ASan tests but others too (e.g.,
g++.dg/debug/debug9.C, g++.dg/template/nontype10.C, and some
others).

Martin

Index: gcc/tree-pretty-print.c
===
--- gcc/tree-pretty-print.c	(revision 251097)
+++ gcc/tree-pretty-print.c	(revision 251098)
@@ -33,6 +33,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "dumpfile.h"
 #include "internal-fn.h"
 #include "gomp-constants.h"
+#include "gimple.h"
 
 /* Local functions, macros and variables.  */
 static const char *op_symbol (const_tree);
@@ -3970,18 +3971,17 @@ newline_and_indent (pretty_printer *pp, int spc)
   INDENT (spc);
 }
 
-/* Handle a %K format for TEXT.  Separate from default_tree_printer so
-   it can also be used in front ends.
-   %K: a statement, from which EXPR_LOCATION and TREE_BLOCK will be recorded.
-*/
+/* Handle the %K format for TEXT.  Separate from default_tree_printer
+   so it can also be used in front ends.
+   Argument is a statement from which EXPR_LOCATION and TREE_BLOCK will
+   be recorded.  */
 
 void
-percent_K_format (text_info *text)
+percent_K_format (text_info *text, tree t)
 {
-  tree t = va_arg (*text->args_ptr, tree), block;
   text->set_location (0, EXPR_LOCATION (t), true);
   gcc_assert (pp_ti_abstract_origin (text) != NULL);
-  block = TREE_BLOCK (t);
+  tree block = TREE_BLOCK (t);
   *pp_ti_abstract_origin (text) = NULL;
 
   if (in_lto_p)
Index: gcc/c-family/ChangeLog
===
--- gcc/c-family/ChangeLog	(revision 251097)
+++ gcc/c-family/ChangeLog	(revision 251098)
@@ -1,3 +1,10 @@
+2017-08-14  Martin Sebor  
+
+	PR c/81117
+	* c-format.h (T89_G): New macro.
+	* c-format.c (local_gcall_ptr_node): New variable.
+	(init_dynamic_diag_info): Initialize it.
+
 2017-08-11  Martin Liska  
 
 	* c-opts.c (c_common_post_options): Replace ASM_OUTPUT_DEF with
Index: gcc/c-family/c-format.c
===
--- gcc/c-family/c-format.c	(revision 251097)
+++ gcc/c-family/c-format.c	(revision 251098)
@@ -56,6 +56,7 @@ struct function_format_info
 
 /* Initialized in init_dynamic_diag_info.  */
 static GTY(()) tree local_tree_type_node;
+static GTY(()) tree local_gcall_ptr_node;
 static GTY(()) tree locus;
 
 static bool decode_format_attr (tree, function_format_info *, int);
@@ -689,7 +690,9 @@ static const format_char_info gcc_diag_char_table[
 
   /* Custom conversion specifiers.  */
 
-  /* These will require a "tree" at runtime.  */
+  /* G requires a "gcall*" arg

Re: [PATCH 1/4] enhance overflow and truncation detection in strncpy and strncat (PR 81117)

2017-08-09 Thread Jeff Law
On 08/06/2017 02:07 PM, Martin Sebor wrote:
> The attached patch adds support for a new GCC format specifier,
> G, that behaves like %K but accepts a gcall* argument.  This
> makes it possible to provide inlining context for "artificial"
> inline functions like strncpy (with _FORTIFY_SOURCE) in
> diagnostics issued from the middle end.
> 
> Martin
> 
> gcc-81117-1.diff
> 
> 
> PR c/81117 - Improve buffer overflow checking in strncpy
> 
> gcc/ChangeLog:
> 
>   PR c/81117
>   * tree-diagnostic.c (default_tree_printer): Handle %G.
>   * tree-pretty-print.h (percent_G_format): Declare new function.
>   * tree-pretty-print.c (percent_K_format): Define a static overload.
>   (percent_G_format): New function.
> 
> gcc/c/ChangeLog:
> 
>   PR c/81117
>   * c-objc-common.c (c_objc_common_init): Handle 'G'.
> 
> gcc/c-family/ChangeLog:
> 
>   * c-format.h (T89_G): New macro.
>   * c-format.c (local_gcall_ptr_node): New variable.
>   (init_dynamic_diag_info): Initialize it.
> 
> gcc/cp/ChangeLog:
> 
>   PR c/81117
>   * error.c (cp_printer): Handle 'G'.
> 
> gcc/testsuite/ChangeLog:
> 
>   PR c/81117
>   * gcc.dg/format/gcc_diag-10.c: Exercise %G.
> 
> diff --git a/gcc/tree-diagnostic.c b/gcc/tree-diagnostic.c
> index 52b7e7f..ad58b69 100644
> --- a/gcc/tree-diagnostic.c
> +++ b/gcc/tree-diagnostic.c
> @@ -275,6 +275,10 @@ default_tree_printer (pretty_printer *pp, text_info 
> *text, const char *spec,
>t = va_arg (*text->args_ptr, tree);
>break;
>  
> +case 'G':
> +  percent_G_format (text);
> +  return true;
> +
>  case 'K':
>percent_K_format (text);
>return true;
> diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c
> index 4d8177c..7c4c805 100644
> --- a/gcc/tree-pretty-print.c
> +++ b/gcc/tree-pretty-print.c
> @@ -33,6 +33,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "dumpfile.h"
>  #include "internal-fn.h"
>  #include "gomp-constants.h"
> +#include "gimple.h"
This is an indication you're probably putting the 'G' handling in the
wrong place.  Wouldn't gimple-pretty-print.c be more correct?

That's my only objection to this patch, so if it moves trivially, then
it's pre-approved.  If it's non-trivial, then we'll want another iteration.

jeff