[v2 of PATCH 13/14] c-format.c: handle location wrappers

[David Malcolm]

On Mon, 2017-12-11 at 18:45 -0500, Jason Merrill wrote:
> On 11/10/2017 04:45 PM, David Malcolm wrote:
> > gcc/c-family/ChangeLog:
> >        * c-format.c (check_format_arg): Strip any location wrapper
> > around
> >        format_tree.
> > ---
> >    gcc/c-family/c-format.c | 9 ++++++++-
> >    1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c
> > index 164d035..6b436ec 100644
> > --- a/gcc/c-family/c-format.c
> > +++ b/gcc/c-family/c-format.c
> > @@ -1536,6 +1536,8 @@ check_format_arg (void *ctx, tree
> > format_tree,
> >
> >      location_t fmt_param_loc = EXPR_LOC_OR_LOC (format_tree,
> > input_location);
> >
> > +  STRIP_ANY_LOCATION_WRAPPER (format_tree);
> > +
> >      if (VAR_P (format_tree))
> >        {
> >          /* Pull out a constant value if the front end didn't.  */
>
> It seems like we want fold_for_warn here instead of the special
> variable
> handling.  That probably makes sense for the other places you change
> in
> this patch, too.
>
> Jason

Here's an updated version of the patch which uses fold_for_warn,
rather than STRIP_ANY_LOCATION_WRAPPER.

In one place it was necessary to add a STRIP_NOPS, since the
fold_for_warn can add a cast around a:
  ADDR_EXPR <POINTER_TYPE(ARRAY_TYPE(char))> (
    STRING_CST)
turning it into a:
  NOP_EXPR<POINTER_TYPE(char))> (
    ADDR_EXPR <POINTER_TYPE(ARRAY_TYPE(char))> (
      STRING_CST))
which without a STRIP_NOPS leads to a bail-out here:
  1596    if (TREE_CODE (format_tree) != ADDR_EXPR)
  1597      {
  1598        res->number_non_literal++;
  1599        return;
  1600      }
and thus -Wformat-security not recognizing string literals.

Successfully bootstrapped&regrtested on x86_64-pc-linux-gnu, as
part of the kit.

Is this OK for trunk, assuming the rest of the kit is approved?

gcc/c-family/ChangeLog:
        * c-format.c (check_format_arg): Strip any location wrapper around
        format_tree.
---
 gcc/c-family/c-format.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/gcc/c-family/c-format.c b/gcc/c-family/c-format.c
index 164d035..47aca55 100644
--- a/gcc/c-family/c-format.c
+++ b/gcc/c-family/c-format.c
@@ -1536,6 +1536,8 @@ check_format_arg (void *ctx, tree format_tree,
 
   location_t fmt_param_loc = EXPR_LOC_OR_LOC (format_tree, input_location);
 
+  format_tree = fold_for_warn (format_tree);
+
   if (VAR_P (format_tree))
     {
       /* Pull out a constant value if the front end didn't.  */
@@ -1591,13 +1593,14 @@ check_format_arg (void *ctx, tree format_tree,
        }
       offset = int_cst_value (arg1);
     }
+  STRIP_NOPS (format_tree);
   if (TREE_CODE (format_tree) != ADDR_EXPR)
     {
       res->number_non_literal++;
       return;
     }
   res->format_string_loc = EXPR_LOC_OR_LOC (format_tree, input_location);
-  format_tree = TREE_OPERAND (format_tree, 0);
+  format_tree = fold_for_warn (TREE_OPERAND (format_tree, 0));
   if (format_types[info->format_type].flags 
       & (int) FMT_FLAG_PARSE_ARG_CONVERT_EXTERNAL)
     {
@@ -1634,7 +1637,9 @@ check_format_arg (void *ctx, tree format_tree,
   if (TREE_CODE (format_tree) == ARRAY_REF
       && tree_fits_shwi_p (TREE_OPERAND (format_tree, 1))
       && (offset += tree_to_shwi (TREE_OPERAND (format_tree, 1))) >= 0)
-    format_tree = TREE_OPERAND (format_tree, 0);
+    {
+      format_tree = fold_for_warn (TREE_OPERAND (format_tree, 0));
+    }
   if (offset < 0)
     {
       res->number_non_literal++;
-- 
1.8.5.3