Re: [PATCH] tree-optimization/105726 - adjust array bound heuristic

2022-05-25 Thread Martin Sebor via Gcc-patches

On 5/25/22 11:19, Martin Sebor wrote:

On 5/25/22 04:49, Richard Biener wrote:

...

[*] For example, no warning is issued for the following overread:


Scratch that, there is no overread with strncpy.  When there is like
with memcpy, it is diagnosed as it should be.



struct A a;

void g (char *d)
{
   struct B *q = &a.b;
   __builtin_strncpy (d, q->a, 123);   // { dg-warning 
"-Wstringop-overread" "pr???" { xfail *-*-* } }

}




Re: [PATCH] tree-optimization/105726 - adjust array bound heuristic

2022-05-25 Thread Martin Sebor via Gcc-patches

On 5/25/22 04:49, Richard Biener wrote:

There's heuristic to detect ptr[1].a[...] out of bound accesses
reasoning that if ptr points to an array of aggregates a trailing
incomplete array has to have size zero.  The following more
thoroughly constrains the cases this applies to avoid false
positive diagnostics.


The code in the area below does look like it's missing some logic
(like what you're adding).  The whole -Wrestrict pass is messy and
could use some cleanup (sorry).  The -Warray-bounds logic should
probably be removed (-Wstringop-overflow should now catch most of
the valid instances it issues, and if it doesn't(*) it should be
enhanced).  The pass could even be moved into the access warning
pass and probably also simplified quite a bit.

For what it's worth, a simple C test case for the same bug is:

struct A
{
  int i;
  struct B { char a[1]; } b;
};

void f (char *d, struct A *p)
{
  struct B *q = &p->b;
  __builtin_strncpy (d, q->a, 1);   // { dg-bogus "-Warray-bounds" }
}

Martin

[*] For example, no warning is issued for the following overread:

struct A a;

void g (char *d)
{
  struct B *q = &a.b;
  __builtin_strncpy (d, q->a, 123);   // { dg-warning 
"-Wstringop-overread" "pr???" { xfail *-*-* } }

}



Bootstrapped and tested on x86_64-unknown-linux-gnu.

OK?

Thanks,
Richard.

2022-05-25  Richard Biener  

PR tree-optimization/105726
* gimple-ssa-warn-restrict.cc (builtin_memref::set_base_and_offset):
Constrain array-of-flexarray case more.

* g++.dg/warn/Warray-bounds-27.C: New testcase.
---
  gcc/gimple-ssa-warn-restrict.cc  | 22 
  gcc/testsuite/g++.dg/warn/Warray-bounds-27.C | 16 ++
  2 files changed, 29 insertions(+), 9 deletions(-)
  create mode 100644 gcc/testsuite/g++.dg/warn/Warray-bounds-27.C

diff --git a/gcc/gimple-ssa-warn-restrict.cc b/gcc/gimple-ssa-warn-restrict.cc
index b678e806da3..734cdd7f5b4 100644
--- a/gcc/gimple-ssa-warn-restrict.cc
+++ b/gcc/gimple-ssa-warn-restrict.cc
@@ -525,7 +525,6 @@ builtin_memref::set_base_and_offset (tree expr)
  {
tree memrefoff = fold_convert (ptrdiff_type_node, TREE_OPERAND (base, 
1));
extend_offset_range (memrefoff);
-  base = TREE_OPERAND (base, 0);
  
if (refoff != HOST_WIDE_INT_MIN

  && TREE_CODE (expr) == COMPONENT_REF)
@@ -538,14 +537,19 @@ builtin_memref::set_base_and_offset (tree expr)
 REFOFF is set to s[1].b - (char*)s.  */
  offset_int off = tree_to_shwi (memrefoff);
  refoff += off;
-   }
-
-  if (!integer_zerop (memrefoff))
-   /* A non-zero offset into an array of struct with flexible array
-  members implies that the array is empty because there is no
-  way to initialize such a member when it belongs to an array.
-  This must be some sort of a bug.  */
-   refsize = 0;
+
+ if (!integer_zerop (memrefoff)
+ && !COMPLETE_TYPE_P (TREE_TYPE (expr))
+ && multiple_of_p (sizetype, memrefoff,
+   TYPE_SIZE_UNIT (TREE_TYPE (base)), true))
+   /* A non-zero offset into an array of struct with flexible array
+  members implies that the array is empty because there is no
+  way to initialize such a member when it belongs to an array.
+  This must be some sort of a bug.  */
+   refsize = 0;
+   }
+
+  base = TREE_OPERAND (base, 0);
  }
  
if (TREE_CODE (ref) == COMPONENT_REF)

diff --git a/gcc/testsuite/g++.dg/warn/Warray-bounds-27.C 
b/gcc/testsuite/g++.dg/warn/Warray-bounds-27.C
new file mode 100644
index 000..06ce089c4b0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Warray-bounds-27.C
@@ -0,0 +1,16 @@
+// PR105726
+// { dg-do compile }
+// { dg-require-effective-target c++11 }
+// { dg-options "-O2 -Warray-bounds" }
+
+#include 
+#include 
+
+struct X {
+char pad[4];
+std::array mField;
+};
+
+void encode(char* aBuffer, const X& aMessage) {
+strncpy(aBuffer, aMessage.mField.data(), 1); // { dg-bogus "bounds" }
+}




[PATCH] tree-optimization/105726 - adjust array bound heuristic

2022-05-25 Thread Richard Biener via Gcc-patches
There's heuristic to detect ptr[1].a[...] out of bound accesses
reasoning that if ptr points to an array of aggregates a trailing
incomplete array has to have size zero.  The following more
thoroughly constrains the cases this applies to avoid false
positive diagnostics.

Bootstrapped and tested on x86_64-unknown-linux-gnu.

OK?

Thanks,
Richard.

2022-05-25  Richard Biener  

PR tree-optimization/105726
* gimple-ssa-warn-restrict.cc (builtin_memref::set_base_and_offset):
Constrain array-of-flexarray case more.

* g++.dg/warn/Warray-bounds-27.C: New testcase.
---
 gcc/gimple-ssa-warn-restrict.cc  | 22 
 gcc/testsuite/g++.dg/warn/Warray-bounds-27.C | 16 ++
 2 files changed, 29 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/Warray-bounds-27.C

diff --git a/gcc/gimple-ssa-warn-restrict.cc b/gcc/gimple-ssa-warn-restrict.cc
index b678e806da3..734cdd7f5b4 100644
--- a/gcc/gimple-ssa-warn-restrict.cc
+++ b/gcc/gimple-ssa-warn-restrict.cc
@@ -525,7 +525,6 @@ builtin_memref::set_base_and_offset (tree expr)
 {
   tree memrefoff = fold_convert (ptrdiff_type_node, TREE_OPERAND (base, 
1));
   extend_offset_range (memrefoff);
-  base = TREE_OPERAND (base, 0);
 
   if (refoff != HOST_WIDE_INT_MIN
  && TREE_CODE (expr) == COMPONENT_REF)
@@ -538,14 +537,19 @@ builtin_memref::set_base_and_offset (tree expr)
 REFOFF is set to s[1].b - (char*)s.  */
  offset_int off = tree_to_shwi (memrefoff);
  refoff += off;
-   }
-
-  if (!integer_zerop (memrefoff))
-   /* A non-zero offset into an array of struct with flexible array
-  members implies that the array is empty because there is no
-  way to initialize such a member when it belongs to an array.
-  This must be some sort of a bug.  */
-   refsize = 0;
+
+ if (!integer_zerop (memrefoff)
+ && !COMPLETE_TYPE_P (TREE_TYPE (expr))
+ && multiple_of_p (sizetype, memrefoff,
+   TYPE_SIZE_UNIT (TREE_TYPE (base)), true))
+   /* A non-zero offset into an array of struct with flexible array
+  members implies that the array is empty because there is no
+  way to initialize such a member when it belongs to an array.
+  This must be some sort of a bug.  */
+   refsize = 0;
+   }
+
+  base = TREE_OPERAND (base, 0);
 }
 
   if (TREE_CODE (ref) == COMPONENT_REF)
diff --git a/gcc/testsuite/g++.dg/warn/Warray-bounds-27.C 
b/gcc/testsuite/g++.dg/warn/Warray-bounds-27.C
new file mode 100644
index 000..06ce089c4b0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Warray-bounds-27.C
@@ -0,0 +1,16 @@
+// PR105726
+// { dg-do compile }
+// { dg-require-effective-target c++11 }
+// { dg-options "-O2 -Warray-bounds" }
+
+#include 
+#include 
+
+struct X {
+char pad[4];
+std::array mField;
+};
+
+void encode(char* aBuffer, const X& aMessage) {
+strncpy(aBuffer, aMessage.mField.data(), 1); // { dg-bogus "bounds" }
+}
-- 
2.35.3