Martin, I'd like your thoughts here.

As noted in the BZ our #pragmas aren't working to suppress a warning.

I did some debugging and ultimately found that the location passed down to the
diagnostic code is indeed outside the scope of the pragmas.

Further digging uncovered that we had a reasonable location passed to
maybe_diag_access_bounds, but rather than using it, we extracted a location
attached to the builtin_memref structure.

So if we look at the test:


char one[50];
char two[50];

void
test_strncat (void)
{
  (void) __builtin_strcpy (one, "gh");
  (void) __builtin_strcpy (two, "ef");
 
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wstringop-overflow="
#pragma GCC diagnostic ignored "-Warray-bounds"
  (void) __builtin_strncat (one, two, 99); 
#pragma GCC diagnostic pop
}

The location we end up passing to the diagnostic code comes from the destination
of the call (via DSTREF which is passed to maybe_diag_access_bounds and becomes
REF in that context).

> (gdb) p debug_tree (ref.ptr)
>  <addr_expr 0x7fffea93d0a0
>     type <pointer_type 0x7fffea937540
>         type <array_type 0x7fffea9372a0 type <integer_type 0x7fffea8173f0 
> char>
>             BLK
>             size <integer_cst 0x7fffea81c3a8 constant 400>
>             unit-size <integer_cst 0x7fffea933f90 constant 50>
>             align:8 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
> 0x7fffea9372a0 domain <integer_type 0x7fffea9371f8>
>             pointer_to_this <pointer_type 0x7fffea937540>>
>         unsigned DI
>         size <integer_cst 0x7fffea7fecd8 constant 64>
>         unit-size <integer_cst 0x7fffea7fecf0 constant 8>
>         align:64 warn_if_not_align:0 symtab:0 alias-set -1 canonical-type
> 0x7fffea937540>
>     constant
>     arg:0 <var_decl 0x7ffff7ffbb40 one type <array_type 0x7fffea9372a0>
>         addressable used public static read BLK j.c:2:6 size <integer_cst
> 0x7fffea81c3a8 400> unit-size <integer_cst 0x7fffea933f90 50>
>         align:256 warn_if_not_align:0 context <translation_unit_decl
> 0x7fffea80bac8 j.c>
>         chain <var_decl 0x7ffff7ffbbd0 two type <array_type 0x7fffea9372a0>
>             addressable used public static read BLK j.c:3:6 size <integer_cst
> 0x7fffea81c3a8 400> unit-size <integer_cst 0x7fffea933f90 50>
>             align:256 warn_if_not_align:0 context <translation_unit_decl
> 0x7fffea80bac8 j.c> chain <function_decl 0x7fffea92f500 test_strncat>>>
>     j.c:8:28 start: j.c:8:28 finish: j.c:8:30>
> 

Note the location.  j.c line 8 column 28-30.  THat location makes absolutely no
sense (it's the first call to strcpy) given we're trying to diagnose the 
strncat.

I have no idea why we're using the location from ref.ptr rather than the 
location
of the statement we're analyzing (which is passed in as LOC).  It's no surprise
the pragma didn't work.

Anyway, this patch seems to fix the problem and doesn't regress anything.  But
I'm hesitant to go forward with it given I don't know the motivation behind
extracting the location from ref.ptr.

Another thought would be to just remove the code that pulls the location from
ref.ptr.  I haven't tested that, but certainly could easily.


Martin, thoughts?

        PR tree-optimization/91890
        * gimple-ssa-warn-restrict.c (maybe_diag_access_bounds): Only
        extract a location from the reference if there is currently
        no valid location information.

        * gcc.dg/pragma-diag-8.c: New test.

diff --git a/gcc/gimple-ssa-warn-restrict.c b/gcc/gimple-ssa-warn-restrict.c
index 2c582a670eb..9421d3d6899 100644
--- a/gcc/gimple-ssa-warn-restrict.c
+++ b/gcc/gimple-ssa-warn-restrict.c
@@ -1711,7 +1711,7 @@ maybe_diag_access_bounds (location_t loc, gimple *call, 
tree func, int strict,
 
       if (warn_stringop_overflow)
        {
-         if (EXPR_HAS_LOCATION (ref.ptr))
+         if (loc == UNKNOWN_LOCATION && EXPR_HAS_LOCATION (ref.ptr))
            loc = EXPR_LOCATION (ref.ptr);
 
          loc = expansion_point_location_if_in_system_header (loc);
@@ -1754,7 +1754,7 @@ maybe_diag_access_bounds (location_t loc, gimple *call, 
tree func, int strict,
       || (ref.ref && TREE_NO_WARNING (ref.ref)))
     return false;
 
-  if (EXPR_HAS_LOCATION (ref.ptr))
+  if (loc == UNKNOWN_LOCATION && EXPR_HAS_LOCATION (ref.ptr))
     loc = EXPR_LOCATION (ref.ptr);
 
   loc = expansion_point_location_if_in_system_header (loc);
diff --git a/gcc/testsuite/gcc.dg/pragma-diag-8.c 
b/gcc/testsuite/gcc.dg/pragma-diag-8.c
new file mode 100644
index 00000000000..00780606e9b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pragma-diag-8.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -Wall" } */
+
+
+char one[50];
+char two[50];
+
+void
+test_strncat (void)
+{
+  (void) __builtin_strcpy (one, "gh");
+  (void) __builtin_strcpy (two, "ef");
+ 
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wstringop-overflow="
+#pragma GCC diagnostic ignored "-Warray-bounds"
+  (void) __builtin_strncat (one, two, 99); 
+#pragma GCC diagnostic pop
+}
+

Reply via email to