* [PATCH] handle vector and aggregate stores in -Wstringop-overflow [PR 97027]
@ 2021-07-13 19:26 Martin Sebor
2021-07-14 7:01 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: Martin Sebor @ 2021-07-13 19:26 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 807 bytes --]
An existing, previously xfailed test that I recently removed
the xfail from made me realize that -Wstringop-overflow doesn't
properly detect buffer overflow resulting from vectorized stores.
Because of a difference in the IL the test passes on x86_64 but
fails on targets like aarch64. Other examples can be constructed
that -Wstringop-overflow fails to diagnose even on x86_64. For
INSTANCE, the overflow in the following function isn't diagnosed
when the loop is vectorized:
void* f (void)
{
char *p = __builtin_malloc (8);
for (int i = 0; i != 16; ++i)
p[i] = 1 << i;
return p;
}
The attached change enhances the warning to detect those as well.
It found a few bugs in vectorizer tests that the patch corrects.
Tested on x86_64-linux and with an aarch64 cross.
Martin
[-- Attachment #2: gcc-97027.diff --]
[-- Type: text/x-patch, Size: 32366 bytes --]
Detect buffer overflow by aggregate and vector stores [PR97027].
Resolves:
PR middle-end/97027 - missing warning on buffer overflow storing a larger scalar into a smaller array
gcc/ChangeLog:
PR middle-end/97027
* tree-ssa-strlen.c (handle_assign): New function.
(nonzero_bytes_for_type): New function.
(count_nonzero_bytes): Handle more tree types. Call
nonzero_bytes_for_tye.
(count_nonzero_bytes): Handle types.
(handle_store): Handle stores from function calls.
(strlen_check_and_optimize_call): Move code to handle_assign. Call
it for assignments from function calls.
gcc/testsuite/ChangeLog:
PR middle-end/97027
* gcc.dg/Wstringop-overflow-15.c: Remove an xfail.
* gcc.dg/Wstringop-overflow-47.c: Adjust xfails.
* gcc.dg/torture/pr69170.c: Avoid valid warnings.
* gcc.dg/torture/pr70025.c: Prune out a false positive.
* gcc.dg/vect/pr97769.c: Initialize a loop control variable.
* gcc.target/i386/pr92658-avx512bw-trunc.c: Increase buffer size
to avoid overflow.
* gcc.target/i386/pr92658-avx512f.c: Same.
* gcc.dg/Wstringop-overflow-68.c: New test.
* gcc.dg/Wstringop-overflow-69.c: New test.
* gcc.dg/Wstringop-overflow-70.c: New test.
* gcc.dg/Wstringop-overflow-71.c: New test.
* gcc.dg/strlenopt-95.c: New test.
diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-15.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-15.c
index 1907bac2722..87f8462d431 100644
--- a/gcc/testsuite/gcc.dg/Wstringop-overflow-15.c
+++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-15.c
@@ -30,7 +30,7 @@ void vla_bounded (int n)
a[0] = 0;
a[1] = 1;
a[n] = n; // { dg-warning "\\\[-Wstringop-overflow" "pr82608" { xfail *-*-* } }
- a[69] = n; // { dg-warning "\\\[-Wstringop-overflow" "pr82608" { xfail *-*-* } }
+ a[69] = n; // { dg-warning "\\\[-Wstringop-overflow" "pr82608" }
sink (&a);
}
diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-47.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-47.c
index 6412874e2f9..968f6ee4ad4 100644
--- a/gcc/testsuite/gcc.dg/Wstringop-overflow-47.c
+++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-47.c
@@ -31,15 +31,15 @@ void nowarn_c32 (char c)
void warn_c32 (char c)
{
- extern char warn_a32[32]; // { dg-message "at offset 32 into destination object 'warn_a32' of size 32" "pr97027" }
+ extern char warn_a32[32]; // { dg-message "at offset (32|1) into destination object 'warn_a32' of size 32" "pr97027" }
void *p = warn_a32 + 1;
- *(C32*)p = (C32){ c }; // { dg-warning "writing 1 byte into a region of size 0" "pr97027" }
+ *(C32*)p = (C32){ c }; // { dg-warning "writing (1 byte|32 bytes) into a region of size (0|31)" "pr97027" }
/* Verify a local variable too. */
char a32[32];
p = a32 + 1;
- *(C32*)p = (C32){ c }; // { dg-warning "writing 1 byte into a region of size 0" "pr97027" }
+ *(C32*)p = (C32){ c }; // { dg-warning "writing (1 byte|32 bytes) into a region of size (0|31)" "pr97027" }
sink (p);
}
@@ -60,15 +60,20 @@ void nowarn_i16_64 (int16_t i)
void warn_i16_64 (int16_t i)
{
- extern char warn_a64[64]; // { dg-message "at offset 128 to object 'warn_a64' with size 64" "pr97027" { xfail *-*-* } }
+/* The IL below that's visible to the warning changes from one target to
+ another. On some like aarch64 it's a single vector store, on others
+ like x86_64 it's a series of BIT_FIELD_REFs. The overflow by
+ the former is detected but the latter is not yet. */
+
+ extern char warn_a64[64]; // { dg-message "at offset (1|128) into destination object 'warn_a64' of size (63|64)" "pr97027 note" { xfail { ! aarch64-*-* } } }
void *p = warn_a64 + 1;
I16_64 *q = (I16_64*)p;
- *q = (I16_64){ i }; // { dg-warning "writing 1 byte into a region of size 0" "pr97027" { xfail *-*-* } }
+ *q = (I16_64){ i }; // { dg-warning "writing (1 byte|64 bytes) into a region of size (0|63)" "pr97027" { xfail { ! aarch64-*-* } } }
char a64[64];
p = a64 + 1;
q = (I16_64*)p;
- *q = (I16_64){ i }; // { dg-warning "writing 1 byte into a region of size 0" "pr97027" { xfail *-*-* } }
+ *q = (I16_64){ i }; // { dg-warning "writing (1 byte|64 bytes) into a region of size (0|63)" "pr97027" { xfail { ! aarch64-*-* } } }
sink (p);
}
diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-68.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-68.c
new file mode 100644
index 00000000000..d2d3ae5f853
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-68.c
@@ -0,0 +1,104 @@
+/* PR tree-optimization/97027 - missing warning on buffer overflow storing
+ a larger scalar into a smaller array
+ Verify overflow by aggregate stores.
+ { dg-do compile }
+ { dg-options "-O2" } */
+
+#define A(N) (A ## N)
+#define Ac1 (AC1){ 0 }
+#define Ac2 (AC2){ 0, 1 }
+#define Ac4 (AC4){ 0, 1, 2, 3 }
+#define Ac8 (AC8){ 0, 1, 2, 3, 4, 5, 6, 7 }
+#define Ac16 (AC16){ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15 }
+
+typedef struct AC1 { char a[1]; } AC1;
+typedef struct AC2 { char a[2]; } AC2;
+typedef struct AC3 { char a[3]; } AC3;
+typedef struct AC4 { char a[4]; } AC4;
+typedef struct AC5 { char a[5]; } AC5;
+typedef struct AC8 { char a[8]; } AC8;
+typedef struct AC16 { char a[16]; } AC16;
+
+extern char a1[1], a2[2], a3[3], a4[4], a5[5], a6[6], a7[7], a8[8], a15[15];
+
+extern AC1 ac1;
+extern AC2 ac2;
+extern AC4 ac4;
+extern AC8 ac8;
+extern AC16 ac16;
+
+extern AC1 fac1 (void);
+extern AC2 fac2 (void);
+extern AC4 fac4 (void);
+extern AC8 fac8 (void);
+extern AC16 fac16 (void);
+
+void nowarn (void)
+{
+ *(AC1*)a1 = Ac1;
+ *(AC2*)a2 = Ac2;
+ *(AC4*)a4 = Ac4;
+ *(AC4*)a5 = Ac4;
+ *(AC4*)a6 = Ac4;
+ *(AC4*)a7 = Ac4;
+ *(AC8*)a8 = Ac8;
+ *(AC8*)a15 = Ac8;
+}
+
+void warn_comp_lit_zero (void)
+{
+ *(AC2*)a1 = (AC2){ }; // { dg-warning "writing 2 bytes into a region of size 1" }
+ *(AC4*)a2 = (AC4){ }; // { dg-warning "writing 4 bytes into a region of size 2" }
+ *(AC4*)a3 = (AC4){ }; // { dg-warning "writing 4 bytes into a region of size 3" }
+ *(AC8*)a4 = (AC8){ }; // { dg-warning "writing 8 bytes into a region of size 4" }
+ *(AC8*)a7 = (AC8){ }; // { dg-warning "writing 8 bytes into a region of size 7" }
+ *(AC16*)a15 = (AC16){ };// { dg-warning "writing 16 bytes into a region of size 15" }
+}
+
+void warn_comp_lit (void)
+{
+ *(AC2*)a1 = Ac2; // { dg-warning "writing 2 bytes into a region of size 1" "pr??????" { xfail *-*-* } }
+ *(AC4*)a2 = Ac4; // { dg-warning "writing 4 bytes into a region of size 2" "pr??????" { xfail *-*-* } }
+ *(AC4*)a3 = Ac4; // { dg-warning "writing 4 bytes into a region of size 3" "pr??????" { xfail *-*-* } }
+ *(AC8*)a4 = Ac8; // { dg-warning "writing 8 bytes into a region of size 4" "pr??????" { xfail *-*-* } }
+ *(AC8*)a7 = Ac8; // { dg-warning "writing 8 bytes into a region of size 7" "pr??????" { xfail *-*-* } }
+ *(AC16*)a15 = Ac16; // { dg-warning "writing 16 bytes into a region of size 15" "pr??????" { xfail *-*-* } }
+}
+
+void warn_aggr_decl (void)
+{
+ *(AC2*)a1 = ac2; // { dg-warning "writing 2 bytes into a region of size 1" }
+ *(AC4*)a2 = ac4; // { dg-warning "writing 4 bytes into a region of size 2" }
+ *(AC4*)a3 = ac4; // { dg-warning "writing 4 bytes into a region of size 3" }
+ *(AC8*)a4 = ac8; // { dg-warning "writing 8 bytes into a region of size 4" }
+ *(AC8*)a7 = ac8; // { dg-warning "writing 8 bytes into a region of size 7" }
+ *(AC16*)a15 = ac16; // { dg-warning "writing 16 bytes into a region of size 15" }
+}
+
+void warn_aggr_parm (AC2 pc2, AC4 pc4, AC8 pc8, AC16 pc16)
+{
+ *(AC2*)a1 = pc2; // { dg-warning "writing 2 bytes into a region of size 1" }
+ *(AC4*)a2 = pc4; // { dg-warning "writing 4 bytes into a region of size 2" }
+ *(AC4*)a3 = pc4; // { dg-warning "writing 4 bytes into a region of size 3" }
+ *(AC8*)a4 = pc8; // { dg-warning "writing 8 bytes into a region of size 4" }
+ *(AC8*)a7 = pc8; // { dg-warning "writing 8 bytes into a region of size 7" }
+ *(AC16*)a15 = pc16; // { dg-warning "writing 16 bytes into a region of size 15" }
+}
+
+void warn_aggr_func (void)
+{
+ *(AC2*)a1 = fac2 (); // { dg-warning "writing 2 bytes into a region of size 1" }
+ *(AC4*)a2 = fac4 (); // { dg-warning "writing 4 bytes into a region of size 2" }
+ *(AC4*)a3 = fac4 (); // { dg-warning "writing 4 bytes into a region of size 3" }
+ *(AC8*)a4 = fac8 (); // { dg-warning "writing 8 bytes into a region of size 4" }
+ *(AC8*)a7 = fac8 (); // { dg-warning "writing 8 bytes into a region of size 7" }
+ *(AC16*)a15 = fac16 ();// { dg-warning "writing 16 bytes into a region of size 15" }
+
+ extern AC2 fac2_x ();
+
+ *(AC2*)a1 = fac2_x (); // { dg-warning "writing 2 bytes into a region of size 1" }
+
+ extern AC2 fac2_p (char*);
+
+ *(AC2*)a1 = fac2_p (0); // { dg-warning "writing 2 bytes into a region of size 1" }
+}
diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-69.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-69.c
new file mode 100644
index 00000000000..754b481d6cd
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-69.c
@@ -0,0 +1,84 @@
+/* PR tree-optimization/97027 - missing warning on buffer overflow storing
+ a larger scalar into a smaller array
+ Verify overflow by vector stores.
+ { dg-do compile }
+ { dg-options "-O2" } */
+
+#define V(N) __attribute__ ((vector_size (N)))
+#define C1 (VC1){ 0 }
+#define C2 (VC2){ 0, 1 }
+#define C4 (VC4){ 0, 1, 2, 3 }
+#define C8 (VC8){ 0, 1, 2, 3, 4, 5, 6, 7 }
+#define C16 (VC16){ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15 }
+
+typedef V (1) char VC1;
+typedef V (2) char VC2;
+typedef V (4) char VC4;
+typedef V (8) char VC8;
+typedef V (16) char VC16;
+
+extern char a1[1], a2[2], a3[3], a4[4], a5[5], a6[6], a7[7], a8[8], a15[15];
+
+extern VC1 c1;
+extern VC2 c2;
+extern VC4 c4;
+extern VC8 c8;
+extern VC16 c16;
+
+extern VC1 fc1 (void);
+extern VC2 fc2 (void);
+extern VC4 fc4 (void);
+extern VC8 fc8 (void);
+extern VC16 fc16 (void);
+
+void nowarn (void)
+{
+ *(VC1*)a1 = C1;
+ *(VC2*)a2 = C2;
+ *(VC4*)a4 = C4;
+ *(VC4*)a5 = C4;
+ *(VC4*)a6 = C4;
+ *(VC4*)a7 = C4;
+ *(VC8*)a8 = C8;
+ *(VC8*)a15 = C8;
+}
+
+void warn_vec_lit (void)
+{
+ *(VC2*)a1 = C2; // { dg-warning "writing 2 bytes into a region of size 1" }
+ *(VC4*)a2 = C4; // { dg-warning "writing 4 bytes into a region of size 2" }
+ *(VC4*)a3 = C4; // { dg-warning "writing 4 bytes into a region of size 3" }
+ *(VC8*)a4 = C8; // { dg-warning "writing 8 bytes into a region of size 4" }
+ *(VC8*)a7 = C8; // { dg-warning "writing 8 bytes into a region of size 7" }
+ *(VC16*)a15 = C16; // { dg-warning "writing 16 bytes into a region of size 15" }
+}
+
+void warn_vec_decl (void)
+{
+ *(VC2*)a1 = c2; // { dg-warning "writing 2 bytes into a region of size 1" }
+ *(VC4*)a2 = c4; // { dg-warning "writing 4 bytes into a region of size 2" }
+ *(VC4*)a3 = c4; // { dg-warning "writing 4 bytes into a region of size 3" }
+ *(VC8*)a4 = c8; // { dg-warning "writing 8 bytes into a region of size 4" }
+ *(VC8*)a7 = c8; // { dg-warning "writing 8 bytes into a region of size 7" }
+ *(VC16*)a15 = c16; // { dg-warning "writing 16 bytes into a region of size 15" }
+}
+
+void warn_vec_parm (VC2 pc2, VC4 pc4, VC8 pc8, VC16 pc16)
+{
+ *(VC2*)a1 = pc2; // { dg-warning "writing 2 bytes into a region of size 1" }
+ *(VC4*)a2 = pc4; // { dg-warning "writing 4 bytes into a region of size 2" }
+ *(VC4*)a3 = pc4; // { dg-warning "writing 4 bytes into a region of size 3" }
+ *(VC8*)a4 = pc8; // { dg-warning "writing 8 bytes into a region of size 4" }
+ *(VC8*)a7 = pc8; // { dg-warning "writing 8 bytes into a region of size 7" }
+ *(VC16*)a15 = pc16; // { dg-warning "writing 16 bytes into a region of size 15" }
+}
+
+void warn_vec_func (void)
+{
+ *(VC2*)a1 = fc2 (); // { dg-warning "writing 2 bytes into a region of size 1" }
+ *(VC4*)a2 = fc4 (); // { dg-warning "writing 4 bytes into a region of size 2" }
+ *(VC4*)a3 = fc4 (); // { dg-warning "writing 4 bytes into a region of size 3" }
+ *(VC8*)a4 = fc8 (); // { dg-warning "writing 8 bytes into a region of size 4" }
+ *(VC8*)a7 = fc8 (); // { dg-warning "writing 8 bytes into a region of size 7" }
+ *(VC16*)a15 = fc16 ();// { dg-warning "writing 16 bytes into a region of size 15" }
+}
diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-70.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-70.c
new file mode 100644
index 00000000000..5d8bfa9a704
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-70.c
@@ -0,0 +1,21 @@
+/* PR tree-optimization/97027 - missing warning on buffer overflow storing
+ a larger scalar into a smaller array
+ Verify overflow by vector stores.
+ { dg-do compile }
+ { dg-options "-O3" } */
+
+void* nowarn_loop (void)
+{
+ char *p = __builtin_malloc (16);
+ for (int i = 0; i != 16; ++i)
+ p[i] = i;
+ return p;
+}
+
+void* warn_loop (void)
+{
+ char *p = __builtin_malloc (15);
+ for (int i = 0; i != 16; ++i)
+ p[i] = i; // { dg-warning "writing 16 bytes into a region of size 15" }
+ return p;
+}
diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-71.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-71.c
new file mode 100644
index 00000000000..0522772959f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-71.c
@@ -0,0 +1,66 @@
+/* PR tree-optimization/97027 - missing warning on buffer overflow storing
+ a larger scalar into a smaller array
+ Verify warnings for overflow by stores of results of built-in functions.
+ { dg-do compile }
+ { dg-options "-O2" } */
+
+typedef __INT16_TYPE__ int16_t;
+typedef __SIZE_TYPE__ size_t;
+
+extern int abs (int);
+extern double nan (const char *);
+extern size_t strlen (const char *);
+extern char* strcpy (char *, const char *);
+
+
+extern unsigned char ax[], a1[1], a2[2], a8[8];
+
+
+void nowarn_abs (int i)
+{
+ *(int *)ax = abs (i);
+ *(char *)a1 = abs (i);
+}
+
+void warn_abs (int i)
+{
+ *(int *)a1 = abs (i); // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+void nowarn_nan (const char *s)
+{
+ *(double *)ax = nan (s);
+}
+
+void warn_nan (const char *s)
+{
+ *(double *)a1 = nan (s); // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+void nowarn_strlen (const char *s1, const char *s2, const char *s3)
+{
+ *(char *)ax = strlen (s1);
+ *(char *)a1 = strlen (s2);
+ *(size_t *)a8 = strlen (s3);
+}
+
+void warn_strlen (const char *s1, const char *s2)
+{
+ *(int16_t *)a1 = strlen (s1); // { dg-warning "\\\[-Wstringop-overflow" }
+ *(size_t *)a2 = strlen (s2); // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+void nowarn_strcpy (char *s1, char *s2, const char *s3)
+{
+ *(char **)ax = strcpy (s1, s2);
+ *(char **)a8 = strcpy (s2, s3);
+}
+
+void warn_strcpy (char *s1, char *s2, const char *s3)
+{
+ *(char **)a1 = strcpy (s1, s2); // { dg-warning "\\\[-Wstringop-overflow" }
+ *(char **)a2 = strcpy (s2, s3); // { dg-warning "\\\[-Wstringop-overflow" }
+}
diff --git a/gcc/testsuite/gcc.dg/strlenopt-95.c b/gcc/testsuite/gcc.dg/strlenopt-95.c
new file mode 100644
index 00000000000..505bc996a59
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/strlenopt-95.c
@@ -0,0 +1,65 @@
+/* Verify strlen results of vector assignments.
+ { dg-do compile }
+ { dg-options "-O2 -Wall" } */
+
+#include "strlenopt.h"
+
+#define V(N) __attribute__ ((vector_size (N)))
+
+typedef V (1) char VC1;
+typedef V (2) char VC2;
+typedef V (4) char VC4;
+typedef V (8) char VC8;
+typedef V (16) char VC16;
+
+extern char a[];
+
+#define A(expr) ((expr) ? (void)0 : abort ())
+
+void test_fold (int i)
+{
+ *(VC4*)a = (VC4){ };
+ A (strlen (a) == 0);
+ A (!a[1] && !a[2] && !a[3]);
+
+ *(VC4*)a = (VC4){ 0, 1 };
+ A (strlen (a) == 0);
+ A (a[1] == 1 && !a[2] && !a[3]);
+
+ *(VC4*)a = (VC4){ 1 };
+ A (strlen (a) == 1);
+ A (!a[1] && !a[2] && !a[3]);
+
+ *(VC4*)a = (VC4){ 1, 0, 3 };
+ A (strlen (a) == 1);
+ A (!a[1] && a[2] == 3 && !a[3]);
+
+ *(VC4*)a = (VC4){ 1, 2 };
+ A (strlen (a) == 2);
+ A (!a[2] && !a[3]);
+
+ *(VC4*)a = (VC4){ 1, 2, 0, 4 };
+ A (strlen (a) == 2);
+ A (!a[2] && a[3] == 4);
+
+ *(VC4*)a = (VC4){ 1, 2, 3 };
+ A (strlen (a) == 3);
+ A (!a[3]);
+
+ *(VC8*)a = (VC8){ 1, 2, 3, 0, 5 };
+ A (strlen (a) == 3);
+
+ *(VC8*)a = (VC8){ 1, 2, 3, 0, 5, 6 };
+ A (strlen (a) == 3);
+
+ *(VC8*)a = (VC8){ 1, 2, 3, 0, 5, 6, 7 };
+ A (strlen (a) == 3);
+ A (strlen (a + 1) == 2);
+ A (strlen (a + 2) == 1);
+ A (strlen (a + 3) == 0);
+
+ A (a[4] == 5 && a[5] == 6 && a[6] == 7 && a[7] == 8);
+}
+
+/* { dg-final { scan-tree-dump-not "abort \\(" "strlen1" } }
+ { dg-final { scan-tree-dump-not "strlen \\(" "strlen1" } } */
diff --git a/gcc/testsuite/gcc.dg/torture/pr69170.c b/gcc/testsuite/gcc.dg/torture/pr69170.c
index 2af0bde7dad..a39125a69cc 100644
--- a/gcc/testsuite/gcc.dg/torture/pr69170.c
+++ b/gcc/testsuite/gcc.dg/torture/pr69170.c
@@ -6,7 +6,7 @@ typedef struct {
char buf[];
} hash_state;
int a;
-hash_state b;
+extern hash_state b;
void fn1()
{
a = 0;
diff --git a/gcc/testsuite/gcc.dg/torture/pr70025.c b/gcc/testsuite/gcc.dg/torture/pr70025.c
index 6c43a0afbde..7cf28c452e3 100644
--- a/gcc/testsuite/gcc.dg/torture/pr70025.c
+++ b/gcc/testsuite/gcc.dg/torture/pr70025.c
@@ -80,3 +80,8 @@ main ()
__builtin_abort ();
return 0;
}
+
+/* At -O3 the loop in bar() is vectorized and results in a (possibly
+ unreachable) out-of-bounds store to p.d7[8]:
+ _22(D)->d7[8] = _122;
+ { dg-prune-output "-Wstringop-overflow" } */
diff --git a/gcc/testsuite/gcc.dg/vect/pr97769.c b/gcc/testsuite/gcc.dg/vect/pr97769.c
index 127f91aa8fd..59e0b464881 100644
--- a/gcc/testsuite/gcc.dg/vect/pr97769.c
+++ b/gcc/testsuite/gcc.dg/vect/pr97769.c
@@ -25,7 +25,7 @@ fn2(tmp *p1)
{
char *d = (char *)p1->d1;
int *b = p1->h1;
- for (int a; a; a++, d += 4)
+ for (int a = 0; a; a++, d += 4)
fn1(d, *b++);
}
diff --git a/gcc/testsuite/gcc.target/i386/pr92658-avx512bw-trunc.c b/gcc/testsuite/gcc.target/i386/pr92658-avx512bw-trunc.c
index c1982f91383..fa6d36d3492 100644
--- a/gcc/testsuite/gcc.target/i386/pr92658-avx512bw-trunc.c
+++ b/gcc/testsuite/gcc.target/i386/pr92658-avx512bw-trunc.c
@@ -13,7 +13,7 @@ typedef unsigned short v32hi __attribute__((vector_size (64)));
void
truncwb_512 (v32qi * dst, v32hi * __restrict src)
{
- unsigned char tem[8];
+ unsigned char tem[32];
tem[0] = (*src)[0];
tem[1] = (*src)[1];
tem[2] = (*src)[2];
@@ -52,7 +52,7 @@ truncwb_512 (v32qi * dst, v32hi * __restrict src)
void
truncwb_256 (v16qi * dst, v16hi * __restrict src)
{
- unsigned char tem[8];
+ unsigned char tem[16];
tem[0] = (*src)[0];
tem[1] = (*src)[1];
tem[2] = (*src)[2];
diff --git a/gcc/testsuite/gcc.target/i386/pr92658-avx512f.c b/gcc/testsuite/gcc.target/i386/pr92658-avx512f.c
index e9ee3d24232..e26b06ecd5a 100644
--- a/gcc/testsuite/gcc.target/i386/pr92658-avx512f.c
+++ b/gcc/testsuite/gcc.target/i386/pr92658-avx512f.c
@@ -54,7 +54,7 @@ truncqb (v8qi * dst, v8di * __restrict src)
void
truncdw (v16hi * dst, v16si * __restrict src)
{
- unsigned short tem[8];
+ unsigned short tem[16];
tem[0] = (*src)[0];
tem[1] = (*src)[1];
tem[2] = (*src)[2];
@@ -78,7 +78,7 @@ truncdw (v16hi * dst, v16si * __restrict src)
void
truncdb (v16qi * dst, v16si * __restrict src)
{
- unsigned char tem[8];
+ unsigned char tem[16];
tem[0] = (*src)[0];
tem[1] = (*src)[1];
tem[2] = (*src)[2];
diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
index 94257df1067..5d16e24a38d 100644
--- a/gcc/tree-ssa-strlen.c
+++ b/gcc/tree-ssa-strlen.c
@@ -192,6 +192,8 @@ struct laststmt_struct
static int get_stridx_plus_constant (strinfo *, unsigned HOST_WIDE_INT, tree);
static void handle_builtin_stxncpy_strncat (bool, gimple_stmt_iterator *);
+static bool handle_assign (gimple_stmt_iterator *, tree, bool *,
+ pointer_query &);
/* Sets MINMAX to either the constant value or the range VAL is in
and returns either the constant value or VAL on success or null
@@ -1944,14 +1946,21 @@ maybe_warn_overflow (gimple *stmt, tree len, pointer_query &ptr_qry,
/* The DECL of the function performing the write if it is done
by one. */
tree writefn = NULL_TREE;
- /* The destination expression involved in the store STMT. */
+ /* The destination expression involved in the store or call STMT. */
tree dest = NULL_TREE;
if (is_gimple_assign (stmt))
dest = gimple_assign_lhs (stmt);
else if (is_gimple_call (stmt))
{
- dest = gimple_call_arg (stmt, 0);
+ if (gimple_call_builtin_p (stmt, BUILT_IN_NORMAL)
+ && gimple_call_num_args (stmt))
+ dest = gimple_call_arg (stmt, 0);
+ else
+ dest = gimple_call_lhs (stmt);
+
+ if (!dest)
+ return;
writefn = gimple_call_fndecl (stmt);
}
else
@@ -4374,19 +4383,49 @@ handle_pointer_plus (gimple_stmt_iterator *gsi)
}
}
+/* Set LENRANGE to the number of nonzero bytes for a store of TYPE and
+ clear all flags. Return true on success and false on failure. */
+
+static bool
+nonzero_bytes_for_type (tree type, unsigned lenrange[3],
+ bool *nulterm, bool *allnul, bool *allnonnul)
+{
+ /* Use the size of the type of the expression as the size of the store,
+ and set the upper bound of the length range to that of the size.
+ Nothing is known about the contents so clear all flags. */
+ tree typesize = TYPE_SIZE_UNIT (type);
+ if (!type)
+ return false;
+
+ if (!tree_fits_uhwi_p (typesize))
+ return false;
+
+ unsigned HOST_WIDE_INT sz = tree_to_uhwi (typesize);
+ if (sz > UINT_MAX)
+ return false;
+
+ lenrange[2] = sz;
+ lenrange[1] = lenrange[2] ? lenrange[2] - 1 : 0;
+ lenrange[0] = 0;
+ *nulterm = false;
+ *allnul = false;
+ *allnonnul = false;
+ return true;
+}
+
static bool
count_nonzero_bytes_addr (tree, unsigned HOST_WIDE_INT, unsigned HOST_WIDE_INT,
unsigned [3], bool *, bool *, bool *,
range_query *, ssa_name_limit_t &);
-/* Determines the minimum and maximum number of leading non-zero bytes
- in the representation of EXP and set LENRANGE[0] and LENRANGE[1]
+/* Recursively determine the minimum and maximum number of leading nonzero
+ bytes in the representation of EXP and set LENRANGE[0] and LENRANGE[1]
to each.
Sets LENRANGE[2] to the total size of the access (which may be less
than LENRANGE[1] when what's being referenced by EXP is a pointer
rather than an array).
- Sets *NULTERM if the representation contains a zero byte, and sets
- *ALLNUL if all the bytes are zero.
+ Sets *NULTERM if the representation contains a zero byte, sets *ALLNUL
+ if all the bytes are zero, and *ALLNONNUL is all are nonzero.
OFFSET and NBYTES are the offset into the representation and
the size of the access to it determined from an ADDR_EXPR (i.e.,
a pointer) or MEM_REF or zero for other expressions.
@@ -4422,9 +4461,11 @@ count_nonzero_bytes (tree exp, unsigned HOST_WIDE_INT offset,
if (gimple_assign_single_p (stmt))
{
exp = gimple_assign_rhs1 (stmt);
- if (TREE_CODE (exp) != MEM_REF)
+ if (!DECL_P (exp)
+ && TREE_CODE (exp) != CONSTRUCTOR
+ && TREE_CODE (exp) != MEM_REF)
return false;
- /* Handle MEM_REF below. */
+ /* Handle DECLs, CONSTRUCTOR and MEM_REF below. */
}
else if (gimple_code (stmt) == GIMPLE_PHI)
{
@@ -4448,6 +4489,25 @@ count_nonzero_bytes (tree exp, unsigned HOST_WIDE_INT offset,
}
}
+ if (TREE_CODE (exp) == CONSTRUCTOR)
+ {
+ if (nbytes)
+ /* If NBYTES has already been determined by an outer MEM_REF
+ fail rather than overwriting it (this shouldn't happen). */
+ return false;
+
+ tree type = TREE_TYPE (exp);
+ tree size = TYPE_SIZE_UNIT (type);
+ if (!size || !tree_fits_uhwi_p (size))
+ return false;
+
+ unsigned HOST_WIDE_INT byte_size = tree_to_uhwi (size);
+ if (byte_size < offset)
+ return false;
+
+ nbytes = byte_size - offset;
+ }
+
if (TREE_CODE (exp) == MEM_REF)
{
if (nbytes)
@@ -4483,9 +4543,11 @@ count_nonzero_bytes (tree exp, unsigned HOST_WIDE_INT offset,
if (VAR_P (exp) || TREE_CODE (exp) == CONST_DECL)
{
- exp = ctor_for_folding (exp);
- if (!exp)
- return false;
+ /* If EXP can be folded into a constant use the result. Otherwise
+ proceed to use EXP to determine a range of the result. */
+ if (tree fold_exp = ctor_for_folding (exp))
+ if (fold_exp != error_mark_node)
+ exp = fold_exp;
}
const char *prep = NULL;
@@ -4533,7 +4595,8 @@ count_nonzero_bytes (tree exp, unsigned HOST_WIDE_INT offset,
}
if (!nbytes)
- return false;
+ return nonzero_bytes_for_type (TREE_TYPE (exp), lenrange,
+ nulterm, allnul, allnonnul);
/* Compute the number of leading nonzero bytes in the representation
and update the minimum and maximum. */
@@ -4696,14 +4759,19 @@ count_nonzero_bytes_addr (tree exp, unsigned HOST_WIDE_INT offset,
return true;
}
-/* Same as above except with an implicit SSA_NAME limit. RVALS is used
- to determine ranges of dynamically computed string lengths (the results
- of strlen). */
+/* Same as above except with an implicit SSA_NAME limit. When EXPR_OR_TYPE
+ is a type rather than an expression use its size to compute the range.
+ RVALS is used to determine ranges of dynamically computed string lengths
+ (the results of strlen). */
static bool
-count_nonzero_bytes (tree exp, unsigned lenrange[3], bool *nulterm,
+count_nonzero_bytes (tree expr_or_type, unsigned lenrange[3], bool *nulterm,
bool *allnul, bool *allnonnul, range_query *rvals)
{
+ if (TYPE_P (expr_or_type))
+ return nonzero_bytes_for_type (expr_or_type, lenrange,
+ nulterm, allnul, allnonnul);
+
/* Set to optimistic values so the caller doesn't have to worry about
initializing these and to what. On success, the function will clear
these if it determines their values are different but being recursive
@@ -4714,7 +4782,8 @@ count_nonzero_bytes (tree exp, unsigned lenrange[3], bool *nulterm,
*allnonnul = true;
ssa_name_limit_t snlim;
- return count_nonzero_bytes (exp, 0, 0, lenrange, nulterm, allnul, allnonnul,
+ tree expr = expr_or_type;
+ return count_nonzero_bytes (expr, 0, 0, lenrange, nulterm, allnul, allnonnul,
rvals, snlim);
}
@@ -4728,11 +4797,28 @@ static bool
handle_store (gimple_stmt_iterator *gsi, bool *zero_write,
pointer_query &ptr_qry)
{
- int idx = -1;
- strinfo *si = NULL;
gimple *stmt = gsi_stmt (*gsi);
- tree ssaname = NULL_TREE, lhs = gimple_assign_lhs (stmt);
- tree rhs = gimple_assign_rhs1 (stmt);
+ /* The LHS and RHS of the store. The RHS is null if STMT is a function
+ call. RHSTYPE is the type of the store. */
+ tree lhs, rhs, rhstype;
+ if (is_gimple_assign (stmt))
+ {
+ lhs = gimple_assign_lhs (stmt);
+ rhs = gimple_assign_rhs1 (stmt);
+ rhstype = TREE_TYPE (rhs);
+ }
+ else if (is_gimple_call (stmt))
+ {
+ lhs = gimple_call_lhs (stmt);
+ rhs = NULL_TREE;
+ rhstype = TREE_TYPE (gimple_call_fntype (stmt));
+ }
+ else
+ return true;
+
+ tree ssaname = NULL_TREE;
+ strinfo *si = NULL;
+ int idx = -1;
range_query *const rvals = ptr_qry.rvals;
@@ -4756,12 +4842,12 @@ handle_store (gimple_stmt_iterator *gsi, bool *zero_write,
ssaname = TREE_OPERAND (lhs, 0);
else if (si == NULL || compare_nonzero_chars (si, offset, rvals) < 0)
{
- *zero_write = initializer_zerop (rhs);
+ *zero_write = rhs ? initializer_zerop (rhs) : false;
bool dummy;
unsigned lenrange[] = { UINT_MAX, 0, 0 };
- if (count_nonzero_bytes (rhs, lenrange, &dummy, &dummy, &dummy,
- rvals))
+ if (count_nonzero_bytes (rhs ? rhs : rhstype, lenrange,
+ &dummy, &dummy, &dummy, rvals))
maybe_warn_overflow (stmt, lenrange[2], ptr_qry);
return true;
@@ -4793,9 +4879,10 @@ handle_store (gimple_stmt_iterator *gsi, bool *zero_write,
bool full_string_p;
const bool ranges_valid
- = count_nonzero_bytes (rhs, lenrange, &full_string_p,
+ = count_nonzero_bytes (rhs ? rhs : rhstype, lenrange, &full_string_p,
&storing_all_zeros_p, &storing_all_nonzero_p,
rvals);
+
if (ranges_valid)
{
rhs_minlen = lenrange[0];
@@ -4864,7 +4951,7 @@ handle_store (gimple_stmt_iterator *gsi, bool *zero_write,
&& storing_nonzero_p
&& lenrange[0] == lenrange[1]
&& lenrange[0] == lenrange[2]
- && TREE_CODE (TREE_TYPE (rhs)) == INTEGER_TYPE)
+ && TREE_CODE (rhstype) == INTEGER_TYPE)
{
/* Handle a store of one or more non-nul characters that ends
before the terminating nul of the destination and so does
@@ -5145,8 +5232,19 @@ strlen_check_and_optimize_call (gimple_stmt_iterator *gsi, bool *zero_write,
if (!gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))
{
tree fntype = gimple_call_fntype (stmt);
- if (fntype && lookup_attribute ("alloc_size", TYPE_ATTRIBUTES (fntype)))
- handle_alloc_call (BUILT_IN_NONE, gsi);
+ if (!fntype)
+ return true;
+
+ if (lookup_attribute ("alloc_size", TYPE_ATTRIBUTES (fntype)))
+ {
+ handle_alloc_call (BUILT_IN_NONE, gsi);
+ return true;
+ }
+
+ if (tree lhs = gimple_call_lhs (stmt))
+ handle_assign (gsi, lhs, zero_write, ptr_qry);
+
+ /* Proceed to handle user-defined formatting functions. */
}
/* When not optimizing we must be checking printf calls which
@@ -5362,6 +5460,48 @@ handle_integral_assign (gimple_stmt_iterator *gsi, bool *cleanup_eh,
}
}
+/* Handle assignment statement at *GSI to LHS. Set *ZERO_WRITE if
+ the assignent stores all zero bytes.. */
+
+static bool
+handle_assign (gimple_stmt_iterator *gsi, tree lhs, bool *zero_write,
+ pointer_query &ptr_qry)
+{
+ tree type = TREE_TYPE (lhs);
+ if (TREE_CODE (type) == ARRAY_TYPE)
+ type = TREE_TYPE (type);
+
+ bool is_char_store = is_char_type (type);
+ if (!is_char_store && TREE_CODE (lhs) == MEM_REF)
+ {
+ /* To consider stores into char objects via integer types other
+ than char but not those to non-character objects, determine
+ the type of the destination rather than just the type of
+ the access. */
+ for (int i = 0; i != 2; ++i)
+ {
+ tree ref = TREE_OPERAND (lhs, i);
+ type = TREE_TYPE (ref);
+ if (TREE_CODE (type) == POINTER_TYPE)
+ type = TREE_TYPE (type);
+ if (TREE_CODE (type) == ARRAY_TYPE)
+ type = TREE_TYPE (type);
+ if (is_char_type (type))
+ {
+ is_char_store = true;
+ break;
+ }
+ }
+ }
+
+ /* Handle a single or multibyte assignment. */
+ if (is_char_store && !handle_store (gsi, zero_write, ptr_qry))
+ return false;
+
+ return true;
+}
+
+
/* Attempt to check for validity of the performed access a single statement
at *GSI using string length knowledge, and to optimize it.
If the given basic block needs clean-up of EH, CLEANUP_EH is set to
@@ -5407,38 +5547,8 @@ check_and_optimize_stmt (gimple_stmt_iterator *gsi, bool *cleanup_eh,
/* Handle assignment to a character. */
handle_integral_assign (gsi, cleanup_eh, ptr_qry.rvals);
else if (TREE_CODE (lhs) != SSA_NAME && !TREE_SIDE_EFFECTS (lhs))
- {
- tree type = TREE_TYPE (lhs);
- if (TREE_CODE (type) == ARRAY_TYPE)
- type = TREE_TYPE (type);
-
- bool is_char_store = is_char_type (type);
- if (!is_char_store && TREE_CODE (lhs) == MEM_REF)
- {
- /* To consider stores into char objects via integer types
- other than char but not those to non-character objects,
- determine the type of the destination rather than just
- the type of the access. */
- for (int i = 0; i != 2; ++i)
- {
- tree ref = TREE_OPERAND (lhs, i);
- type = TREE_TYPE (ref);
- if (TREE_CODE (type) == POINTER_TYPE)
- type = TREE_TYPE (type);
- if (TREE_CODE (type) == ARRAY_TYPE)
- type = TREE_TYPE (type);
- if (is_char_type (type))
- {
- is_char_store = true;
- break;
- }
- }
- }
-
- /* Handle a single or multibyte assignment. */
- if (is_char_store && !handle_store (gsi, &zero_write, ptr_qry))
- return false;
- }
+ if (!handle_assign (gsi, lhs, &zero_write, ptr_qry))
+ return false;
}
else if (gcond *cond = dyn_cast<gcond *> (stmt))
{
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] handle vector and aggregate stores in -Wstringop-overflow [PR 97027]
2021-07-13 19:26 [PATCH] handle vector and aggregate stores in -Wstringop-overflow [PR 97027] Martin Sebor
@ 2021-07-14 7:01 ` Richard Biener
2021-07-14 18:46 ` Martin Sebor
0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2021-07-14 7:01 UTC (permalink / raw)
To: Martin Sebor; +Cc: gcc-patches
On Tue, Jul 13, 2021 at 9:27 PM Martin Sebor via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> An existing, previously xfailed test that I recently removed
> the xfail from made me realize that -Wstringop-overflow doesn't
> properly detect buffer overflow resulting from vectorized stores.
> Because of a difference in the IL the test passes on x86_64 but
> fails on targets like aarch64. Other examples can be constructed
> that -Wstringop-overflow fails to diagnose even on x86_64. For
> INSTANCE, the overflow in the following function isn't diagnosed
> when the loop is vectorized:
>
> void* f (void)
> {
> char *p = __builtin_malloc (8);
> for (int i = 0; i != 16; ++i)
> p[i] = 1 << i;
> return p;
> }
>
> The attached change enhances the warning to detect those as well.
> It found a few bugs in vectorizer tests that the patch corrects.
> Tested on x86_64-linux and with an aarch64 cross.
- dest = gimple_call_arg (stmt, 0);
+ if (gimple_call_builtin_p (stmt, BUILT_IN_NORMAL)
+ && gimple_call_num_args (stmt))
+ dest = gimple_call_arg (stmt, 0);
+ else
+ dest = gimple_call_lhs (stmt);
+
+ if (!dest)
+ return;
so this uses arg0 for memcpy (dst, src, 4) and also for bcopy (src, dst, 4)?
It looks quite fragile to me. I think you want to use the LHS only if it is
aggregate (and not a pointer or some random other value). Likewise
you should only use arg0 for a whitelist of builtins, not for any random one.
It's bad enough that compute_objsize decides for itself whether it is
passed a pointer or an object rather than the API being explicit about this.
if (VAR_P (exp) || TREE_CODE (exp) == CONST_DECL)
{
- exp = ctor_for_folding (exp);
- if (!exp)
- return false;
+ /* If EXP can be folded into a constant use the result. Otherwise
+ proceed to use EXP to determine a range of the result. */
+ if (tree fold_exp = ctor_for_folding (exp))
+ if (fold_exp != error_mark_node)
+ exp = fold_exp;
fold_exp can be NULL, meaning a zero-initializer but below you'll run into
const char *prep = NULL;
if (TREE_CODE (exp) == STRING_CST)
{
and crash. Either you handle a NULL fold_expr explicitely or conservatively
continue to return false.
+ /* The LHS and RHS of the store. The RHS is null if STMT is a function
+ call. RHSTYPE is the type of the store. */
+ tree lhs, rhs, rhstype;
+ if (is_gimple_assign (stmt))
+ {
+ lhs = gimple_assign_lhs (stmt);
+ rhs = gimple_assign_rhs1 (stmt);
+ rhstype = TREE_TYPE (rhs);
+ }
+ else if (is_gimple_call (stmt))
+ {
+ lhs = gimple_call_lhs (stmt);
+ rhs = NULL_TREE;
+ rhstype = TREE_TYPE (gimple_call_fntype (stmt));
+ }
The type of the store in a call is better determined from the LHS.
For internal function calls the above will crash.
Otherwise looks like reasonable changes.
Richard.
> Martin
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] handle vector and aggregate stores in -Wstringop-overflow [PR 97027]
2021-07-14 7:01 ` Richard Biener
@ 2021-07-14 18:46 ` Martin Sebor
2021-07-15 7:31 ` Richard Biener
0 siblings, 1 reply; 4+ messages in thread
From: Martin Sebor @ 2021-07-14 18:46 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 4254 bytes --]
On 7/14/21 1:01 AM, Richard Biener wrote:
> On Tue, Jul 13, 2021 at 9:27 PM Martin Sebor via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>>
>> An existing, previously xfailed test that I recently removed
>> the xfail from made me realize that -Wstringop-overflow doesn't
>> properly detect buffer overflow resulting from vectorized stores.
>> Because of a difference in the IL the test passes on x86_64 but
>> fails on targets like aarch64. Other examples can be constructed
>> that -Wstringop-overflow fails to diagnose even on x86_64. For
>> INSTANCE, the overflow in the following function isn't diagnosed
>> when the loop is vectorized:
>>
>> void* f (void)
>> {
>> char *p = __builtin_malloc (8);
>> for (int i = 0; i != 16; ++i)
>> p[i] = 1 << i;
>> return p;
>> }
>>
>> The attached change enhances the warning to detect those as well.
>> It found a few bugs in vectorizer tests that the patch corrects.
>> Tested on x86_64-linux and with an aarch64 cross.
>
> - dest = gimple_call_arg (stmt, 0);
> + if (gimple_call_builtin_p (stmt, BUILT_IN_NORMAL)
> + && gimple_call_num_args (stmt))
> + dest = gimple_call_arg (stmt, 0);
> + else
> + dest = gimple_call_lhs (stmt);
> +
> + if (!dest)
> + return;
>
> so this uses arg0 for memcpy (dst, src, 4) and also for bcopy (src, dst, 4)?
No. The code is only called for assignments like *p = f () and for
a handful of built-ins (memcpy, strcpy, and memset).
bcopy() returns void and so its result cannot be assigned. I believe
bcopy() and the other legacy bxxx() functions are also lowered into
memcpy/memmove etc. so we should see no calls to it in the middle end.
In any case, I have adjusted the function as described below to avoid
even this hypothetical issue.
> It looks quite fragile to me. I think you want to use the LHS only if it is
> aggregate (and not a pointer or some random other value). Likewise
> you should only use arg0 for a whitelist of builtins, not for any random one.
I've added an argument to the function to make the distinction
between a call result and argument explicit but I haven't been able
to create a test case to exercise it. For all the built-ins I've
tried in an assignment like:
extern char a[4];
*(double*)a = nan ("foo");
the call result ends up assigned to a temporary:
_1 = __builtin_nan (s_2(D));
MEM[(double *)&a] = _1;
I can only get a call and assignment in one for user-defined functions
that return an aggregate.
>
> It's bad enough that compute_objsize decides for itself whether it is
> passed a pointer or an object rather than the API being explicit about this.
>
> if (VAR_P (exp) || TREE_CODE (exp) == CONST_DECL)
> {
> - exp = ctor_for_folding (exp);
> - if (!exp)
> - return false;
> + /* If EXP can be folded into a constant use the result. Otherwise
> + proceed to use EXP to determine a range of the result. */
> + if (tree fold_exp = ctor_for_folding (exp))
^^^^^^^^^^^^^^^^^
> + if (fold_exp != error_mark_node)
> + exp = fold_exp;
>
> fold_exp can be NULL, meaning a zero-initializer but below you'll run into
fold_exp is assigned to exp if it's neither null (as I underlined
above) nor error_mark_node so I think it's correct as is.
>
> const char *prep = NULL;
> if (TREE_CODE (exp) == STRING_CST)
> {
>
> and crash. Either you handle a NULL fold_expr explicitely or conservatively
> continue to return false.
>
> + /* The LHS and RHS of the store. The RHS is null if STMT is a function
> + call. RHSTYPE is the type of the store. */
> + tree lhs, rhs, rhstype;
> + if (is_gimple_assign (stmt))
> + {
> + lhs = gimple_assign_lhs (stmt);
> + rhs = gimple_assign_rhs1 (stmt);
> + rhstype = TREE_TYPE (rhs);
> + }
> + else if (is_gimple_call (stmt))
> + {
> + lhs = gimple_call_lhs (stmt);
> + rhs = NULL_TREE;
> + rhstype = TREE_TYPE (gimple_call_fntype (stmt));
> + }
>
> The type of the store in a call is better determined from the LHS.
> For internal function calls the above will crash.
>
> Otherwise looks like reasonable changes.
Please see the attached revision.
Martin
[-- Attachment #2: gcc-97027.diff --]
[-- Type: text/x-patch, Size: 36373 bytes --]
Detect buffer overflow by aggregate and vector stores [PR97027].
Resolves:
PR middle-end/97027 - missing warning on buffer overflow storing a larger scalar into a smaller array
gcc/ChangeLog:
PR middle-end/97027
* tree-ssa-strlen.c (handle_assign): New function.
(maybe_warn_overflow): Add argument.
(nonzero_bytes_for_type): New function.
(count_nonzero_bytes): Handle more tree types. Call
nonzero_bytes_for_tye.
(count_nonzero_bytes): Handle types.
(handle_store): Handle stores from function calls.
(strlen_check_and_optimize_call): Move code to handle_assign. Call
it for assignments from function calls.
gcc/testsuite/ChangeLog:
PR middle-end/97027
* gcc.dg/Wstringop-overflow-15.c: Remove an xfail.
* gcc.dg/Wstringop-overflow-47.c: Adjust xfails.
* gcc.dg/torture/pr69170.c: Avoid valid warnings.
* gcc.dg/torture/pr70025.c: Prune out a false positive.
* gcc.dg/vect/pr97769.c: Initialize a loop control variable.
* gcc.target/i386/pr92658-avx512bw-trunc.c: Increase buffer size
to avoid overflow.
* gcc.target/i386/pr92658-avx512f.c: Same.
* gcc.dg/Wstringop-overflow-68.c: New test.
* gcc.dg/Wstringop-overflow-69.c: New test.
* gcc.dg/Wstringop-overflow-70.c: New test.
* gcc.dg/Wstringop-overflow-71.c: New test.
* gcc.dg/strlenopt-95.c: New test.
diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-15.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-15.c
index 1907bac2722..87f8462d431 100644
--- a/gcc/testsuite/gcc.dg/Wstringop-overflow-15.c
+++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-15.c
@@ -30,7 +30,7 @@ void vla_bounded (int n)
a[0] = 0;
a[1] = 1;
a[n] = n; // { dg-warning "\\\[-Wstringop-overflow" "pr82608" { xfail *-*-* } }
- a[69] = n; // { dg-warning "\\\[-Wstringop-overflow" "pr82608" { xfail *-*-* } }
+ a[69] = n; // { dg-warning "\\\[-Wstringop-overflow" "pr82608" }
sink (&a);
}
diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-47.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-47.c
index 6412874e2f9..968f6ee4ad4 100644
--- a/gcc/testsuite/gcc.dg/Wstringop-overflow-47.c
+++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-47.c
@@ -31,15 +31,15 @@ void nowarn_c32 (char c)
void warn_c32 (char c)
{
- extern char warn_a32[32]; // { dg-message "at offset 32 into destination object 'warn_a32' of size 32" "pr97027" }
+ extern char warn_a32[32]; // { dg-message "at offset (32|1) into destination object 'warn_a32' of size 32" "pr97027" }
void *p = warn_a32 + 1;
- *(C32*)p = (C32){ c }; // { dg-warning "writing 1 byte into a region of size 0" "pr97027" }
+ *(C32*)p = (C32){ c }; // { dg-warning "writing (1 byte|32 bytes) into a region of size (0|31)" "pr97027" }
/* Verify a local variable too. */
char a32[32];
p = a32 + 1;
- *(C32*)p = (C32){ c }; // { dg-warning "writing 1 byte into a region of size 0" "pr97027" }
+ *(C32*)p = (C32){ c }; // { dg-warning "writing (1 byte|32 bytes) into a region of size (0|31)" "pr97027" }
sink (p);
}
@@ -60,15 +60,20 @@ void nowarn_i16_64 (int16_t i)
void warn_i16_64 (int16_t i)
{
- extern char warn_a64[64]; // { dg-message "at offset 128 to object 'warn_a64' with size 64" "pr97027" { xfail *-*-* } }
+/* The IL below that's visible to the warning changes from one target to
+ another. On some like aarch64 it's a single vector store, on others
+ like x86_64 it's a series of BIT_FIELD_REFs. The overflow by
+ the former is detected but the latter is not yet. */
+
+ extern char warn_a64[64]; // { dg-message "at offset (1|128) into destination object 'warn_a64' of size (63|64)" "pr97027 note" { xfail { ! aarch64-*-* } } }
void *p = warn_a64 + 1;
I16_64 *q = (I16_64*)p;
- *q = (I16_64){ i }; // { dg-warning "writing 1 byte into a region of size 0" "pr97027" { xfail *-*-* } }
+ *q = (I16_64){ i }; // { dg-warning "writing (1 byte|64 bytes) into a region of size (0|63)" "pr97027" { xfail { ! aarch64-*-* } } }
char a64[64];
p = a64 + 1;
q = (I16_64*)p;
- *q = (I16_64){ i }; // { dg-warning "writing 1 byte into a region of size 0" "pr97027" { xfail *-*-* } }
+ *q = (I16_64){ i }; // { dg-warning "writing (1 byte|64 bytes) into a region of size (0|63)" "pr97027" { xfail { ! aarch64-*-* } } }
sink (p);
}
diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-68.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-68.c
new file mode 100644
index 00000000000..d2d3ae5f853
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-68.c
@@ -0,0 +1,104 @@
+/* PR tree-optimization/97027 - missing warning on buffer overflow storing
+ a larger scalar into a smaller array
+ Verify overflow by aggregate stores.
+ { dg-do compile }
+ { dg-options "-O2" } */
+
+#define A(N) (A ## N)
+#define Ac1 (AC1){ 0 }
+#define Ac2 (AC2){ 0, 1 }
+#define Ac4 (AC4){ 0, 1, 2, 3 }
+#define Ac8 (AC8){ 0, 1, 2, 3, 4, 5, 6, 7 }
+#define Ac16 (AC16){ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15 }
+
+typedef struct AC1 { char a[1]; } AC1;
+typedef struct AC2 { char a[2]; } AC2;
+typedef struct AC3 { char a[3]; } AC3;
+typedef struct AC4 { char a[4]; } AC4;
+typedef struct AC5 { char a[5]; } AC5;
+typedef struct AC8 { char a[8]; } AC8;
+typedef struct AC16 { char a[16]; } AC16;
+
+extern char a1[1], a2[2], a3[3], a4[4], a5[5], a6[6], a7[7], a8[8], a15[15];
+
+extern AC1 ac1;
+extern AC2 ac2;
+extern AC4 ac4;
+extern AC8 ac8;
+extern AC16 ac16;
+
+extern AC1 fac1 (void);
+extern AC2 fac2 (void);
+extern AC4 fac4 (void);
+extern AC8 fac8 (void);
+extern AC16 fac16 (void);
+
+void nowarn (void)
+{
+ *(AC1*)a1 = Ac1;
+ *(AC2*)a2 = Ac2;
+ *(AC4*)a4 = Ac4;
+ *(AC4*)a5 = Ac4;
+ *(AC4*)a6 = Ac4;
+ *(AC4*)a7 = Ac4;
+ *(AC8*)a8 = Ac8;
+ *(AC8*)a15 = Ac8;
+}
+
+void warn_comp_lit_zero (void)
+{
+ *(AC2*)a1 = (AC2){ }; // { dg-warning "writing 2 bytes into a region of size 1" }
+ *(AC4*)a2 = (AC4){ }; // { dg-warning "writing 4 bytes into a region of size 2" }
+ *(AC4*)a3 = (AC4){ }; // { dg-warning "writing 4 bytes into a region of size 3" }
+ *(AC8*)a4 = (AC8){ }; // { dg-warning "writing 8 bytes into a region of size 4" }
+ *(AC8*)a7 = (AC8){ }; // { dg-warning "writing 8 bytes into a region of size 7" }
+ *(AC16*)a15 = (AC16){ };// { dg-warning "writing 16 bytes into a region of size 15" }
+}
+
+void warn_comp_lit (void)
+{
+ *(AC2*)a1 = Ac2; // { dg-warning "writing 2 bytes into a region of size 1" "pr??????" { xfail *-*-* } }
+ *(AC4*)a2 = Ac4; // { dg-warning "writing 4 bytes into a region of size 2" "pr??????" { xfail *-*-* } }
+ *(AC4*)a3 = Ac4; // { dg-warning "writing 4 bytes into a region of size 3" "pr??????" { xfail *-*-* } }
+ *(AC8*)a4 = Ac8; // { dg-warning "writing 8 bytes into a region of size 4" "pr??????" { xfail *-*-* } }
+ *(AC8*)a7 = Ac8; // { dg-warning "writing 8 bytes into a region of size 7" "pr??????" { xfail *-*-* } }
+ *(AC16*)a15 = Ac16; // { dg-warning "writing 16 bytes into a region of size 15" "pr??????" { xfail *-*-* } }
+}
+
+void warn_aggr_decl (void)
+{
+ *(AC2*)a1 = ac2; // { dg-warning "writing 2 bytes into a region of size 1" }
+ *(AC4*)a2 = ac4; // { dg-warning "writing 4 bytes into a region of size 2" }
+ *(AC4*)a3 = ac4; // { dg-warning "writing 4 bytes into a region of size 3" }
+ *(AC8*)a4 = ac8; // { dg-warning "writing 8 bytes into a region of size 4" }
+ *(AC8*)a7 = ac8; // { dg-warning "writing 8 bytes into a region of size 7" }
+ *(AC16*)a15 = ac16; // { dg-warning "writing 16 bytes into a region of size 15" }
+}
+
+void warn_aggr_parm (AC2 pc2, AC4 pc4, AC8 pc8, AC16 pc16)
+{
+ *(AC2*)a1 = pc2; // { dg-warning "writing 2 bytes into a region of size 1" }
+ *(AC4*)a2 = pc4; // { dg-warning "writing 4 bytes into a region of size 2" }
+ *(AC4*)a3 = pc4; // { dg-warning "writing 4 bytes into a region of size 3" }
+ *(AC8*)a4 = pc8; // { dg-warning "writing 8 bytes into a region of size 4" }
+ *(AC8*)a7 = pc8; // { dg-warning "writing 8 bytes into a region of size 7" }
+ *(AC16*)a15 = pc16; // { dg-warning "writing 16 bytes into a region of size 15" }
+}
+
+void warn_aggr_func (void)
+{
+ *(AC2*)a1 = fac2 (); // { dg-warning "writing 2 bytes into a region of size 1" }
+ *(AC4*)a2 = fac4 (); // { dg-warning "writing 4 bytes into a region of size 2" }
+ *(AC4*)a3 = fac4 (); // { dg-warning "writing 4 bytes into a region of size 3" }
+ *(AC8*)a4 = fac8 (); // { dg-warning "writing 8 bytes into a region of size 4" }
+ *(AC8*)a7 = fac8 (); // { dg-warning "writing 8 bytes into a region of size 7" }
+ *(AC16*)a15 = fac16 ();// { dg-warning "writing 16 bytes into a region of size 15" }
+
+ extern AC2 fac2_x ();
+
+ *(AC2*)a1 = fac2_x (); // { dg-warning "writing 2 bytes into a region of size 1" }
+
+ extern AC2 fac2_p (char*);
+
+ *(AC2*)a1 = fac2_p (0); // { dg-warning "writing 2 bytes into a region of size 1" }
+}
diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-69.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-69.c
new file mode 100644
index 00000000000..754b481d6cd
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-69.c
@@ -0,0 +1,84 @@
+/* PR tree-optimization/97027 - missing warning on buffer overflow storing
+ a larger scalar into a smaller array
+ Verify overflow by vector stores.
+ { dg-do compile }
+ { dg-options "-O2" } */
+
+#define V(N) __attribute__ ((vector_size (N)))
+#define C1 (VC1){ 0 }
+#define C2 (VC2){ 0, 1 }
+#define C4 (VC4){ 0, 1, 2, 3 }
+#define C8 (VC8){ 0, 1, 2, 3, 4, 5, 6, 7 }
+#define C16 (VC16){ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15 }
+
+typedef V (1) char VC1;
+typedef V (2) char VC2;
+typedef V (4) char VC4;
+typedef V (8) char VC8;
+typedef V (16) char VC16;
+
+extern char a1[1], a2[2], a3[3], a4[4], a5[5], a6[6], a7[7], a8[8], a15[15];
+
+extern VC1 c1;
+extern VC2 c2;
+extern VC4 c4;
+extern VC8 c8;
+extern VC16 c16;
+
+extern VC1 fc1 (void);
+extern VC2 fc2 (void);
+extern VC4 fc4 (void);
+extern VC8 fc8 (void);
+extern VC16 fc16 (void);
+
+void nowarn (void)
+{
+ *(VC1*)a1 = C1;
+ *(VC2*)a2 = C2;
+ *(VC4*)a4 = C4;
+ *(VC4*)a5 = C4;
+ *(VC4*)a6 = C4;
+ *(VC4*)a7 = C4;
+ *(VC8*)a8 = C8;
+ *(VC8*)a15 = C8;
+}
+
+void warn_vec_lit (void)
+{
+ *(VC2*)a1 = C2; // { dg-warning "writing 2 bytes into a region of size 1" }
+ *(VC4*)a2 = C4; // { dg-warning "writing 4 bytes into a region of size 2" }
+ *(VC4*)a3 = C4; // { dg-warning "writing 4 bytes into a region of size 3" }
+ *(VC8*)a4 = C8; // { dg-warning "writing 8 bytes into a region of size 4" }
+ *(VC8*)a7 = C8; // { dg-warning "writing 8 bytes into a region of size 7" }
+ *(VC16*)a15 = C16; // { dg-warning "writing 16 bytes into a region of size 15" }
+}
+
+void warn_vec_decl (void)
+{
+ *(VC2*)a1 = c2; // { dg-warning "writing 2 bytes into a region of size 1" }
+ *(VC4*)a2 = c4; // { dg-warning "writing 4 bytes into a region of size 2" }
+ *(VC4*)a3 = c4; // { dg-warning "writing 4 bytes into a region of size 3" }
+ *(VC8*)a4 = c8; // { dg-warning "writing 8 bytes into a region of size 4" }
+ *(VC8*)a7 = c8; // { dg-warning "writing 8 bytes into a region of size 7" }
+ *(VC16*)a15 = c16; // { dg-warning "writing 16 bytes into a region of size 15" }
+}
+
+void warn_vec_parm (VC2 pc2, VC4 pc4, VC8 pc8, VC16 pc16)
+{
+ *(VC2*)a1 = pc2; // { dg-warning "writing 2 bytes into a region of size 1" }
+ *(VC4*)a2 = pc4; // { dg-warning "writing 4 bytes into a region of size 2" }
+ *(VC4*)a3 = pc4; // { dg-warning "writing 4 bytes into a region of size 3" }
+ *(VC8*)a4 = pc8; // { dg-warning "writing 8 bytes into a region of size 4" }
+ *(VC8*)a7 = pc8; // { dg-warning "writing 8 bytes into a region of size 7" }
+ *(VC16*)a15 = pc16; // { dg-warning "writing 16 bytes into a region of size 15" }
+}
+
+void warn_vec_func (void)
+{
+ *(VC2*)a1 = fc2 (); // { dg-warning "writing 2 bytes into a region of size 1" }
+ *(VC4*)a2 = fc4 (); // { dg-warning "writing 4 bytes into a region of size 2" }
+ *(VC4*)a3 = fc4 (); // { dg-warning "writing 4 bytes into a region of size 3" }
+ *(VC8*)a4 = fc8 (); // { dg-warning "writing 8 bytes into a region of size 4" }
+ *(VC8*)a7 = fc8 (); // { dg-warning "writing 8 bytes into a region of size 7" }
+ *(VC16*)a15 = fc16 ();// { dg-warning "writing 16 bytes into a region of size 15" }
+}
diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-70.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-70.c
new file mode 100644
index 00000000000..5d8bfa9a704
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-70.c
@@ -0,0 +1,21 @@
+/* PR tree-optimization/97027 - missing warning on buffer overflow storing
+ a larger scalar into a smaller array
+ Verify overflow by vector stores.
+ { dg-do compile }
+ { dg-options "-O3" } */
+
+void* nowarn_loop (void)
+{
+ char *p = __builtin_malloc (16);
+ for (int i = 0; i != 16; ++i)
+ p[i] = i;
+ return p;
+}
+
+void* warn_loop (void)
+{
+ char *p = __builtin_malloc (15);
+ for (int i = 0; i != 16; ++i)
+ p[i] = i; // { dg-warning "writing 16 bytes into a region of size 15" }
+ return p;
+}
diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-71.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-71.c
new file mode 100644
index 00000000000..dccee35cb3d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-71.c
@@ -0,0 +1,105 @@
+/* PR tree-optimization/97027 - missing warning on buffer overflow storing
+ a larger scalar into a smaller array
+ Verify warnings for overflow by stores of results of built-in functions.
+ { dg-do compile }
+ { dg-options "-O2" } */
+
+typedef __INT16_TYPE__ int16_t;
+typedef __SIZE_TYPE__ size_t;
+
+extern int abs (int);
+
+extern void* alloca (size_t);
+
+extern double nan (const char *);
+_Decimal32 nand32 (const char *);
+
+extern size_t strlen (const char *);
+extern char* strcpy (char *, const char *);
+
+
+extern unsigned char ax[], a1[1], a2[2], a8[8];
+
+
+void nowarn_abs (int i)
+{
+ *(int *)ax = abs (i);
+ *(char *)a1 = abs (i);
+}
+
+void warn_abs (int i)
+{
+ *(int *)a1 = abs (i); // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+void nowarn_alloca (size_t n)
+{
+ *(void **)ax = alloca (n);
+}
+
+void warn_alloca (size_t n)
+{
+ *(void **)a1 = alloca (n); // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+void nowarn_complex (double x, double i)
+{
+ *(_Complex double *)ax = __builtin_complex (x, i);
+}
+
+void warn_complex (double x, double i)
+{
+ _Complex double *p = (_Complex double *)a1;
+ *p = __builtin_complex (x, i); // { dg-warning "\\\[-Wstringop-overflow" "pr101455" { xfail *-*-* } }
+}
+
+
+void nowarn_nan (const char *s)
+{
+ *(double *)ax = nan (s);
+}
+
+void warn_nan (const char *s)
+{
+ *(double *)a1 = nan (s); // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+void nowarn_nand32 (const char *s)
+{
+ *(_Decimal32 *)ax = nand32 (s);
+}
+
+void warn_nand32 (const char *s)
+{
+ *(_Decimal32 *)a1 = nand32 (s); // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+void nowarn_strlen (const char *s1, const char *s2, const char *s3)
+{
+ *(char *)ax = strlen (s1);
+ *(char *)a1 = strlen (s2);
+ *(size_t *)a8 = strlen (s3);
+}
+
+void warn_strlen (const char *s1, const char *s2)
+{
+ *(int16_t *)a1 = strlen (s1); // { dg-warning "\\\[-Wstringop-overflow" }
+ *(size_t *)a2 = strlen (s2); // { dg-warning "\\\[-Wstringop-overflow" }
+}
+
+
+void nowarn_strcpy (char *s1, char *s2, const char *s3)
+{
+ *(char **)ax = strcpy (s1, s2);
+ *(char **)a8 = strcpy (s2, s3);
+}
+
+void warn_strcpy (char *s1, char *s2, const char *s3)
+{
+ *(char **)a1 = strcpy (s1, s2); // { dg-warning "\\\[-Wstringop-overflow" }
+ *(char **)a2 = strcpy (s2, s3); // { dg-warning "\\\[-Wstringop-overflow" }
+}
diff --git a/gcc/testsuite/gcc.dg/strlenopt-95.c b/gcc/testsuite/gcc.dg/strlenopt-95.c
new file mode 100644
index 00000000000..505bc996a59
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/strlenopt-95.c
@@ -0,0 +1,65 @@
+/* Verify strlen results of vector assignments.
+ { dg-do compile }
+ { dg-options "-O2 -Wall" } */
+
+#include "strlenopt.h"
+
+#define V(N) __attribute__ ((vector_size (N)))
+
+typedef V (1) char VC1;
+typedef V (2) char VC2;
+typedef V (4) char VC4;
+typedef V (8) char VC8;
+typedef V (16) char VC16;
+
+extern char a[];
+
+#define A(expr) ((expr) ? (void)0 : abort ())
+
+void test_fold (int i)
+{
+ *(VC4*)a = (VC4){ };
+ A (strlen (a) == 0);
+ A (!a[1] && !a[2] && !a[3]);
+
+ *(VC4*)a = (VC4){ 0, 1 };
+ A (strlen (a) == 0);
+ A (a[1] == 1 && !a[2] && !a[3]);
+
+ *(VC4*)a = (VC4){ 1 };
+ A (strlen (a) == 1);
+ A (!a[1] && !a[2] && !a[3]);
+
+ *(VC4*)a = (VC4){ 1, 0, 3 };
+ A (strlen (a) == 1);
+ A (!a[1] && a[2] == 3 && !a[3]);
+
+ *(VC4*)a = (VC4){ 1, 2 };
+ A (strlen (a) == 2);
+ A (!a[2] && !a[3]);
+
+ *(VC4*)a = (VC4){ 1, 2, 0, 4 };
+ A (strlen (a) == 2);
+ A (!a[2] && a[3] == 4);
+
+ *(VC4*)a = (VC4){ 1, 2, 3 };
+ A (strlen (a) == 3);
+ A (!a[3]);
+
+ *(VC8*)a = (VC8){ 1, 2, 3, 0, 5 };
+ A (strlen (a) == 3);
+
+ *(VC8*)a = (VC8){ 1, 2, 3, 0, 5, 6 };
+ A (strlen (a) == 3);
+
+ *(VC8*)a = (VC8){ 1, 2, 3, 0, 5, 6, 7 };
+ A (strlen (a) == 3);
+ A (strlen (a + 1) == 2);
+ A (strlen (a + 2) == 1);
+ A (strlen (a + 3) == 0);
+
+ A (a[4] == 5 && a[5] == 6 && a[6] == 7 && a[7] == 8);
+}
+
+/* { dg-final { scan-tree-dump-not "abort \\(" "strlen1" } }
+ { dg-final { scan-tree-dump-not "strlen \\(" "strlen1" } } */
diff --git a/gcc/testsuite/gcc.dg/torture/pr69170.c b/gcc/testsuite/gcc.dg/torture/pr69170.c
index 2af0bde7dad..a39125a69cc 100644
--- a/gcc/testsuite/gcc.dg/torture/pr69170.c
+++ b/gcc/testsuite/gcc.dg/torture/pr69170.c
@@ -6,7 +6,7 @@ typedef struct {
char buf[];
} hash_state;
int a;
-hash_state b;
+extern hash_state b;
void fn1()
{
a = 0;
diff --git a/gcc/testsuite/gcc.dg/torture/pr70025.c b/gcc/testsuite/gcc.dg/torture/pr70025.c
index 6c43a0afbde..7cf28c452e3 100644
--- a/gcc/testsuite/gcc.dg/torture/pr70025.c
+++ b/gcc/testsuite/gcc.dg/torture/pr70025.c
@@ -80,3 +80,8 @@ main ()
__builtin_abort ();
return 0;
}
+
+/* At -O3 the loop in bar() is vectorized and results in a (possibly
+ unreachable) out-of-bounds store to p.d7[8]:
+ _22(D)->d7[8] = _122;
+ { dg-prune-output "-Wstringop-overflow" } */
diff --git a/gcc/testsuite/gcc.dg/vect/pr97769.c b/gcc/testsuite/gcc.dg/vect/pr97769.c
index 127f91aa8fd..59e0b464881 100644
--- a/gcc/testsuite/gcc.dg/vect/pr97769.c
+++ b/gcc/testsuite/gcc.dg/vect/pr97769.c
@@ -25,7 +25,7 @@ fn2(tmp *p1)
{
char *d = (char *)p1->d1;
int *b = p1->h1;
- for (int a; a; a++, d += 4)
+ for (int a = 0; a; a++, d += 4)
fn1(d, *b++);
}
diff --git a/gcc/testsuite/gcc.target/i386/pr92658-avx512bw-trunc.c b/gcc/testsuite/gcc.target/i386/pr92658-avx512bw-trunc.c
index c1982f91383..fa6d36d3492 100644
--- a/gcc/testsuite/gcc.target/i386/pr92658-avx512bw-trunc.c
+++ b/gcc/testsuite/gcc.target/i386/pr92658-avx512bw-trunc.c
@@ -13,7 +13,7 @@ typedef unsigned short v32hi __attribute__((vector_size (64)));
void
truncwb_512 (v32qi * dst, v32hi * __restrict src)
{
- unsigned char tem[8];
+ unsigned char tem[32];
tem[0] = (*src)[0];
tem[1] = (*src)[1];
tem[2] = (*src)[2];
@@ -52,7 +52,7 @@ truncwb_512 (v32qi * dst, v32hi * __restrict src)
void
truncwb_256 (v16qi * dst, v16hi * __restrict src)
{
- unsigned char tem[8];
+ unsigned char tem[16];
tem[0] = (*src)[0];
tem[1] = (*src)[1];
tem[2] = (*src)[2];
diff --git a/gcc/testsuite/gcc.target/i386/pr92658-avx512f.c b/gcc/testsuite/gcc.target/i386/pr92658-avx512f.c
index e9ee3d24232..e26b06ecd5a 100644
--- a/gcc/testsuite/gcc.target/i386/pr92658-avx512f.c
+++ b/gcc/testsuite/gcc.target/i386/pr92658-avx512f.c
@@ -54,7 +54,7 @@ truncqb (v8qi * dst, v8di * __restrict src)
void
truncdw (v16hi * dst, v16si * __restrict src)
{
- unsigned short tem[8];
+ unsigned short tem[16];
tem[0] = (*src)[0];
tem[1] = (*src)[1];
tem[2] = (*src)[2];
@@ -78,7 +78,7 @@ truncdw (v16hi * dst, v16si * __restrict src)
void
truncdb (v16qi * dst, v16si * __restrict src)
{
- unsigned char tem[8];
+ unsigned char tem[16];
tem[0] = (*src)[0];
tem[1] = (*src)[1];
tem[2] = (*src)[2];
diff --git a/gcc/tree-ssa-strlen.c b/gcc/tree-ssa-strlen.c
index 94257df1067..799c21fddde 100644
--- a/gcc/tree-ssa-strlen.c
+++ b/gcc/tree-ssa-strlen.c
@@ -192,6 +192,8 @@ struct laststmt_struct
static int get_stridx_plus_constant (strinfo *, unsigned HOST_WIDE_INT, tree);
static void handle_builtin_stxncpy_strncat (bool, gimple_stmt_iterator *);
+static bool handle_assign (gimple_stmt_iterator *, tree, bool *,
+ pointer_query &);
/* Sets MINMAX to either the constant value or the range VAL is in
and returns either the constant value or VAL on success or null
@@ -1929,12 +1931,15 @@ maybe_set_strlen_range (tree lhs, tree src, tree bound)
/* Diagnose buffer overflow by a STMT writing LEN + PLUS_ONE bytes,
either into a region allocated for the object SI when non-null,
or into an object designated by the LHS of STMT otherwise.
+ For a call STMT, when CALL_LHS is set use its left hand side
+ as the destination, otherwise use argument zero.
When nonnull uses RVALS to determine range information.
RAWMEM may be set by memcpy and other raw memory functions
to allow accesses across subobject boundaries. */
static void
-maybe_warn_overflow (gimple *stmt, tree len, pointer_query &ptr_qry,
+maybe_warn_overflow (gimple *stmt, bool call_lhs, tree len,
+ pointer_query &ptr_qry,
strinfo *si = NULL, bool plus_one = false,
bool rawmem = false)
{
@@ -1944,14 +1949,23 @@ maybe_warn_overflow (gimple *stmt, tree len, pointer_query &ptr_qry,
/* The DECL of the function performing the write if it is done
by one. */
tree writefn = NULL_TREE;
- /* The destination expression involved in the store STMT. */
+ /* The destination expression involved in the store or call STMT. */
tree dest = NULL_TREE;
if (is_gimple_assign (stmt))
dest = gimple_assign_lhs (stmt);
else if (is_gimple_call (stmt))
{
- dest = gimple_call_arg (stmt, 0);
+ if (call_lhs)
+ dest = gimple_call_lhs (stmt);
+ else
+ {
+ gcc_assert (gimple_call_builtin_p (stmt, BUILT_IN_NORMAL));
+ dest = gimple_call_arg (stmt, 0);
+ }
+
+ if (!dest)
+ return;
writefn = gimple_call_fndecl (stmt);
}
else
@@ -2108,12 +2122,12 @@ maybe_warn_overflow (gimple *stmt, tree len, pointer_query &ptr_qry,
/* Convenience wrapper for the above. */
static inline void
-maybe_warn_overflow (gimple *stmt, unsigned HOST_WIDE_INT len,
+maybe_warn_overflow (gimple *stmt, bool call_lhs, unsigned HOST_WIDE_INT len,
pointer_query &ptr_qry, strinfo *si = NULL,
bool plus_one = false, bool rawmem = false)
{
- maybe_warn_overflow (stmt, build_int_cst (size_type_node, len), ptr_qry,
- si, plus_one, rawmem);
+ tree tlen = build_int_cst (size_type_node, len);
+ maybe_warn_overflow (stmt, call_lhs, tlen, ptr_qry, si, plus_one, rawmem);
}
/* Handle a strlen call. If strlen of the argument is known, replace
@@ -2443,7 +2457,7 @@ handle_builtin_strcpy (enum built_in_function bcode, gimple_stmt_iterator *gsi,
else if (idx < 0)
srclen = build_int_cst (size_type_node, ~idx);
- maybe_warn_overflow (stmt, srclen, ptr_qry, olddsi, true);
+ maybe_warn_overflow (stmt, false, srclen, ptr_qry, olddsi, true);
if (olddsi != NULL)
adjust_last_stmt (olddsi, stmt, false, ptr_qry);
@@ -3248,7 +3262,7 @@ handle_builtin_memcpy (enum built_in_function bcode, gimple_stmt_iterator *gsi,
if (olddsi != NULL
&& !integer_zerop (len))
{
- maybe_warn_overflow (stmt, len, ptr_qry, olddsi, false, true);
+ maybe_warn_overflow (stmt, false, len, ptr_qry, olddsi, false, true);
adjust_last_stmt (olddsi, stmt, false, ptr_qry);
}
@@ -3713,7 +3727,8 @@ handle_builtin_memset (gimple_stmt_iterator *gsi, bool *zero_write,
tree memset_size = gimple_call_arg (memset_stmt, 2);
/* Check for overflow. */
- maybe_warn_overflow (memset_stmt, memset_size, ptr_qry, NULL, false, true);
+ maybe_warn_overflow (memset_stmt, false, memset_size, ptr_qry, NULL,
+ false, true);
/* Bail when there is no statement associated with the destination
(the statement may be null even when SI1->ALLOC is not). */
@@ -4374,19 +4389,49 @@ handle_pointer_plus (gimple_stmt_iterator *gsi)
}
}
+/* Set LENRANGE to the number of nonzero bytes for a store of TYPE and
+ clear all flags. Return true on success and false on failure. */
+
+static bool
+nonzero_bytes_for_type (tree type, unsigned lenrange[3],
+ bool *nulterm, bool *allnul, bool *allnonnul)
+{
+ /* Use the size of the type of the expression as the size of the store,
+ and set the upper bound of the length range to that of the size.
+ Nothing is known about the contents so clear all flags. */
+ tree typesize = TYPE_SIZE_UNIT (type);
+ if (!type)
+ return false;
+
+ if (!tree_fits_uhwi_p (typesize))
+ return false;
+
+ unsigned HOST_WIDE_INT sz = tree_to_uhwi (typesize);
+ if (sz > UINT_MAX)
+ return false;
+
+ lenrange[2] = sz;
+ lenrange[1] = lenrange[2] ? lenrange[2] - 1 : 0;
+ lenrange[0] = 0;
+ *nulterm = false;
+ *allnul = false;
+ *allnonnul = false;
+ return true;
+}
+
static bool
count_nonzero_bytes_addr (tree, unsigned HOST_WIDE_INT, unsigned HOST_WIDE_INT,
unsigned [3], bool *, bool *, bool *,
range_query *, ssa_name_limit_t &);
-/* Determines the minimum and maximum number of leading non-zero bytes
- in the representation of EXP and set LENRANGE[0] and LENRANGE[1]
+/* Recursively determine the minimum and maximum number of leading nonzero
+ bytes in the representation of EXP and set LENRANGE[0] and LENRANGE[1]
to each.
Sets LENRANGE[2] to the total size of the access (which may be less
than LENRANGE[1] when what's being referenced by EXP is a pointer
rather than an array).
- Sets *NULTERM if the representation contains a zero byte, and sets
- *ALLNUL if all the bytes are zero.
+ Sets *NULTERM if the representation contains a zero byte, sets *ALLNUL
+ if all the bytes are zero, and *ALLNONNUL is all are nonzero.
OFFSET and NBYTES are the offset into the representation and
the size of the access to it determined from an ADDR_EXPR (i.e.,
a pointer) or MEM_REF or zero for other expressions.
@@ -4422,9 +4467,11 @@ count_nonzero_bytes (tree exp, unsigned HOST_WIDE_INT offset,
if (gimple_assign_single_p (stmt))
{
exp = gimple_assign_rhs1 (stmt);
- if (TREE_CODE (exp) != MEM_REF)
+ if (!DECL_P (exp)
+ && TREE_CODE (exp) != CONSTRUCTOR
+ && TREE_CODE (exp) != MEM_REF)
return false;
- /* Handle MEM_REF below. */
+ /* Handle DECLs, CONSTRUCTOR and MEM_REF below. */
}
else if (gimple_code (stmt) == GIMPLE_PHI)
{
@@ -4448,6 +4495,25 @@ count_nonzero_bytes (tree exp, unsigned HOST_WIDE_INT offset,
}
}
+ if (TREE_CODE (exp) == CONSTRUCTOR)
+ {
+ if (nbytes)
+ /* If NBYTES has already been determined by an outer MEM_REF
+ fail rather than overwriting it (this shouldn't happen). */
+ return false;
+
+ tree type = TREE_TYPE (exp);
+ tree size = TYPE_SIZE_UNIT (type);
+ if (!size || !tree_fits_uhwi_p (size))
+ return false;
+
+ unsigned HOST_WIDE_INT byte_size = tree_to_uhwi (size);
+ if (byte_size < offset)
+ return false;
+
+ nbytes = byte_size - offset;
+ }
+
if (TREE_CODE (exp) == MEM_REF)
{
if (nbytes)
@@ -4483,9 +4549,11 @@ count_nonzero_bytes (tree exp, unsigned HOST_WIDE_INT offset,
if (VAR_P (exp) || TREE_CODE (exp) == CONST_DECL)
{
- exp = ctor_for_folding (exp);
- if (!exp)
- return false;
+ /* If EXP can be folded into a constant use the result. Otherwise
+ proceed to use EXP to determine a range of the result. */
+ if (tree fold_exp = ctor_for_folding (exp))
+ if (fold_exp != error_mark_node)
+ exp = fold_exp;
}
const char *prep = NULL;
@@ -4533,7 +4601,8 @@ count_nonzero_bytes (tree exp, unsigned HOST_WIDE_INT offset,
}
if (!nbytes)
- return false;
+ return nonzero_bytes_for_type (TREE_TYPE (exp), lenrange,
+ nulterm, allnul, allnonnul);
/* Compute the number of leading nonzero bytes in the representation
and update the minimum and maximum. */
@@ -4696,14 +4765,19 @@ count_nonzero_bytes_addr (tree exp, unsigned HOST_WIDE_INT offset,
return true;
}
-/* Same as above except with an implicit SSA_NAME limit. RVALS is used
- to determine ranges of dynamically computed string lengths (the results
- of strlen). */
+/* Same as above except with an implicit SSA_NAME limit. When EXPR_OR_TYPE
+ is a type rather than an expression use its size to compute the range.
+ RVALS is used to determine ranges of dynamically computed string lengths
+ (the results of strlen). */
static bool
-count_nonzero_bytes (tree exp, unsigned lenrange[3], bool *nulterm,
+count_nonzero_bytes (tree expr_or_type, unsigned lenrange[3], bool *nulterm,
bool *allnul, bool *allnonnul, range_query *rvals)
{
+ if (TYPE_P (expr_or_type))
+ return nonzero_bytes_for_type (expr_or_type, lenrange,
+ nulterm, allnul, allnonnul);
+
/* Set to optimistic values so the caller doesn't have to worry about
initializing these and to what. On success, the function will clear
these if it determines their values are different but being recursive
@@ -4714,7 +4788,8 @@ count_nonzero_bytes (tree exp, unsigned lenrange[3], bool *nulterm,
*allnonnul = true;
ssa_name_limit_t snlim;
- return count_nonzero_bytes (exp, 0, 0, lenrange, nulterm, allnul, allnonnul,
+ tree expr = expr_or_type;
+ return count_nonzero_bytes (expr, 0, 0, lenrange, nulterm, allnul, allnonnul,
rvals, snlim);
}
@@ -4728,11 +4803,29 @@ static bool
handle_store (gimple_stmt_iterator *gsi, bool *zero_write,
pointer_query &ptr_qry)
{
- int idx = -1;
- strinfo *si = NULL;
gimple *stmt = gsi_stmt (*gsi);
- tree ssaname = NULL_TREE, lhs = gimple_assign_lhs (stmt);
- tree rhs = gimple_assign_rhs1 (stmt);
+ /* The LHS and RHS of the store. The RHS is null if STMT is a function
+ call. STORETYPE is the type of the store (determined from either
+ the RHS of the assignment statement or the LHS of a function call. */
+ tree lhs, rhs, storetype;
+ if (is_gimple_assign (stmt))
+ {
+ lhs = gimple_assign_lhs (stmt);
+ rhs = gimple_assign_rhs1 (stmt);
+ storetype = TREE_TYPE (rhs);
+ }
+ else if (is_gimple_call (stmt))
+ {
+ lhs = gimple_call_lhs (stmt);
+ rhs = NULL_TREE;
+ storetype = TREE_TYPE (lhs);
+ }
+ else
+ return true;
+
+ tree ssaname = NULL_TREE;
+ strinfo *si = NULL;
+ int idx = -1;
range_query *const rvals = ptr_qry.rvals;
@@ -4756,13 +4849,13 @@ handle_store (gimple_stmt_iterator *gsi, bool *zero_write,
ssaname = TREE_OPERAND (lhs, 0);
else if (si == NULL || compare_nonzero_chars (si, offset, rvals) < 0)
{
- *zero_write = initializer_zerop (rhs);
+ *zero_write = rhs ? initializer_zerop (rhs) : false;
bool dummy;
unsigned lenrange[] = { UINT_MAX, 0, 0 };
- if (count_nonzero_bytes (rhs, lenrange, &dummy, &dummy, &dummy,
- rvals))
- maybe_warn_overflow (stmt, lenrange[2], ptr_qry);
+ if (count_nonzero_bytes (rhs ? rhs : storetype, lenrange,
+ &dummy, &dummy, &dummy, rvals))
+ maybe_warn_overflow (stmt, true, lenrange[2], ptr_qry);
return true;
}
@@ -4793,16 +4886,17 @@ handle_store (gimple_stmt_iterator *gsi, bool *zero_write,
bool full_string_p;
const bool ranges_valid
- = count_nonzero_bytes (rhs, lenrange, &full_string_p,
+ = count_nonzero_bytes (rhs ? rhs : storetype, lenrange, &full_string_p,
&storing_all_zeros_p, &storing_all_nonzero_p,
rvals);
+
if (ranges_valid)
{
rhs_minlen = lenrange[0];
storing_nonzero_p = lenrange[1] > 0;
*zero_write = storing_all_zeros_p;
- maybe_warn_overflow (stmt, lenrange[2], ptr_qry);
+ maybe_warn_overflow (stmt, true, lenrange[2], ptr_qry);
}
else
{
@@ -4864,7 +4958,7 @@ handle_store (gimple_stmt_iterator *gsi, bool *zero_write,
&& storing_nonzero_p
&& lenrange[0] == lenrange[1]
&& lenrange[0] == lenrange[2]
- && TREE_CODE (TREE_TYPE (rhs)) == INTEGER_TYPE)
+ && TREE_CODE (storetype) == INTEGER_TYPE)
{
/* Handle a store of one or more non-nul characters that ends
before the terminating nul of the destination and so does
@@ -5145,8 +5239,19 @@ strlen_check_and_optimize_call (gimple_stmt_iterator *gsi, bool *zero_write,
if (!gimple_call_builtin_p (stmt, BUILT_IN_NORMAL))
{
tree fntype = gimple_call_fntype (stmt);
- if (fntype && lookup_attribute ("alloc_size", TYPE_ATTRIBUTES (fntype)))
- handle_alloc_call (BUILT_IN_NONE, gsi);
+ if (!fntype)
+ return true;
+
+ if (lookup_attribute ("alloc_size", TYPE_ATTRIBUTES (fntype)))
+ {
+ handle_alloc_call (BUILT_IN_NONE, gsi);
+ return true;
+ }
+
+ if (tree lhs = gimple_call_lhs (stmt))
+ handle_assign (gsi, lhs, zero_write, ptr_qry);
+
+ /* Proceed to handle user-defined formatting functions. */
}
/* When not optimizing we must be checking printf calls which
@@ -5362,6 +5467,48 @@ handle_integral_assign (gimple_stmt_iterator *gsi, bool *cleanup_eh,
}
}
+/* Handle assignment statement at *GSI to LHS. Set *ZERO_WRITE if
+ the assignent stores all zero bytes.. */
+
+static bool
+handle_assign (gimple_stmt_iterator *gsi, tree lhs, bool *zero_write,
+ pointer_query &ptr_qry)
+{
+ tree type = TREE_TYPE (lhs);
+ if (TREE_CODE (type) == ARRAY_TYPE)
+ type = TREE_TYPE (type);
+
+ bool is_char_store = is_char_type (type);
+ if (!is_char_store && TREE_CODE (lhs) == MEM_REF)
+ {
+ /* To consider stores into char objects via integer types other
+ than char but not those to non-character objects, determine
+ the type of the destination rather than just the type of
+ the access. */
+ for (int i = 0; i != 2; ++i)
+ {
+ tree ref = TREE_OPERAND (lhs, i);
+ type = TREE_TYPE (ref);
+ if (TREE_CODE (type) == POINTER_TYPE)
+ type = TREE_TYPE (type);
+ if (TREE_CODE (type) == ARRAY_TYPE)
+ type = TREE_TYPE (type);
+ if (is_char_type (type))
+ {
+ is_char_store = true;
+ break;
+ }
+ }
+ }
+
+ /* Handle a single or multibyte assignment. */
+ if (is_char_store && !handle_store (gsi, zero_write, ptr_qry))
+ return false;
+
+ return true;
+}
+
+
/* Attempt to check for validity of the performed access a single statement
at *GSI using string length knowledge, and to optimize it.
If the given basic block needs clean-up of EH, CLEANUP_EH is set to
@@ -5407,38 +5554,8 @@ check_and_optimize_stmt (gimple_stmt_iterator *gsi, bool *cleanup_eh,
/* Handle assignment to a character. */
handle_integral_assign (gsi, cleanup_eh, ptr_qry.rvals);
else if (TREE_CODE (lhs) != SSA_NAME && !TREE_SIDE_EFFECTS (lhs))
- {
- tree type = TREE_TYPE (lhs);
- if (TREE_CODE (type) == ARRAY_TYPE)
- type = TREE_TYPE (type);
-
- bool is_char_store = is_char_type (type);
- if (!is_char_store && TREE_CODE (lhs) == MEM_REF)
- {
- /* To consider stores into char objects via integer types
- other than char but not those to non-character objects,
- determine the type of the destination rather than just
- the type of the access. */
- for (int i = 0; i != 2; ++i)
- {
- tree ref = TREE_OPERAND (lhs, i);
- type = TREE_TYPE (ref);
- if (TREE_CODE (type) == POINTER_TYPE)
- type = TREE_TYPE (type);
- if (TREE_CODE (type) == ARRAY_TYPE)
- type = TREE_TYPE (type);
- if (is_char_type (type))
- {
- is_char_store = true;
- break;
- }
- }
- }
-
- /* Handle a single or multibyte assignment. */
- if (is_char_store && !handle_store (gsi, &zero_write, ptr_qry))
- return false;
- }
+ if (!handle_assign (gsi, lhs, &zero_write, ptr_qry))
+ return false;
}
else if (gcond *cond = dyn_cast<gcond *> (stmt))
{
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] handle vector and aggregate stores in -Wstringop-overflow [PR 97027]
2021-07-14 18:46 ` Martin Sebor
@ 2021-07-15 7:31 ` Richard Biener
0 siblings, 0 replies; 4+ messages in thread
From: Richard Biener @ 2021-07-15 7:31 UTC (permalink / raw)
To: Martin Sebor; +Cc: gcc-patches
On Wed, Jul 14, 2021 at 8:46 PM Martin Sebor <msebor@gmail.com> wrote:
>
> On 7/14/21 1:01 AM, Richard Biener wrote:
> > On Tue, Jul 13, 2021 at 9:27 PM Martin Sebor via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> >>
> >> An existing, previously xfailed test that I recently removed
> >> the xfail from made me realize that -Wstringop-overflow doesn't
> >> properly detect buffer overflow resulting from vectorized stores.
> >> Because of a difference in the IL the test passes on x86_64 but
> >> fails on targets like aarch64. Other examples can be constructed
> >> that -Wstringop-overflow fails to diagnose even on x86_64. For
> >> INSTANCE, the overflow in the following function isn't diagnosed
> >> when the loop is vectorized:
> >>
> >> void* f (void)
> >> {
> >> char *p = __builtin_malloc (8);
> >> for (int i = 0; i != 16; ++i)
> >> p[i] = 1 << i;
> >> return p;
> >> }
> >>
> >> The attached change enhances the warning to detect those as well.
> >> It found a few bugs in vectorizer tests that the patch corrects.
> >> Tested on x86_64-linux and with an aarch64 cross.
> >
> > - dest = gimple_call_arg (stmt, 0);
> > + if (gimple_call_builtin_p (stmt, BUILT_IN_NORMAL)
> > + && gimple_call_num_args (stmt))
> > + dest = gimple_call_arg (stmt, 0);
> > + else
> > + dest = gimple_call_lhs (stmt);
> > +
> > + if (!dest)
> > + return;
> >
> > so this uses arg0 for memcpy (dst, src, 4) and also for bcopy (src, dst, 4)?
>
> No. The code is only called for assignments like *p = f () and for
> a handful of built-ins (memcpy, strcpy, and memset).
I see - that wasn't obvious when looking at the patch.
> bcopy() returns void and so its result cannot be assigned. I believe
> bcopy() and the other legacy bxxx() functions are also lowered into
> memcpy/memmove etc. so we should see no calls to it in the middle end.
> In any case, I have adjusted the function as described below to avoid
> even this hypothetical issue.
>
> > It looks quite fragile to me. I think you want to use the LHS only if it is
> > aggregate (and not a pointer or some random other value). Likewise
> > you should only use arg0 for a whitelist of builtins, not for any random one.
>
> I've added an argument to the function to make the distinction
> between a call result and argument explicit but I haven't been able
> to create a test case to exercise it. For all the built-ins I've
> tried in an assignment like:
>
> extern char a[4];
> *(double*)a = nan ("foo");
>
> the call result ends up assigned to a temporary:
>
> _1 = __builtin_nan (s_2(D));
> MEM[(double *)&a] = _1;
>
> I can only get a call and assignment in one for user-defined functions
> that return an aggregate.
Yes, call LHS will be SSA names if the result is of register type.
> >
> > It's bad enough that compute_objsize decides for itself whether it is
> > passed a pointer or an object rather than the API being explicit about this.
> >
> > if (VAR_P (exp) || TREE_CODE (exp) == CONST_DECL)
> > {
> > - exp = ctor_for_folding (exp);
> > - if (!exp)
> > - return false;
> > + /* If EXP can be folded into a constant use the result. Otherwise
> > + proceed to use EXP to determine a range of the result. */
> > + if (tree fold_exp = ctor_for_folding (exp))
> ^^^^^^^^^^^^^^^^^
> > + if (fold_exp != error_mark_node)
> > + exp = fold_exp;
> >
> > fold_exp can be NULL, meaning a zero-initializer but below you'll run into
>
> fold_exp is assigned to exp if it's neither null (as I underlined
> above) nor error_mark_node so I think it's correct as is.
Oops. Somehow missed that - the
if (..)
if (..)
style for an && is a "bad" C++ requirement as "nice" as the
new if (tree x = ...) syntax is.
> >
> > const char *prep = NULL;
> > if (TREE_CODE (exp) == STRING_CST)
> > {
> >
> > and crash. Either you handle a NULL fold_expr explicitely or conservatively
> > continue to return false.
> >
> > + /* The LHS and RHS of the store. The RHS is null if STMT is a function
> > + call. RHSTYPE is the type of the store. */
> > + tree lhs, rhs, rhstype;
> > + if (is_gimple_assign (stmt))
> > + {
> > + lhs = gimple_assign_lhs (stmt);
> > + rhs = gimple_assign_rhs1 (stmt);
> > + rhstype = TREE_TYPE (rhs);
> > + }
> > + else if (is_gimple_call (stmt))
> > + {
> > + lhs = gimple_call_lhs (stmt);
> > + rhs = NULL_TREE;
> > + rhstype = TREE_TYPE (gimple_call_fntype (stmt));
> > + }
> >
> > The type of the store in a call is better determined from the LHS.
> > For internal function calls the above will crash.
> >
> > Otherwise looks like reasonable changes.
>
> Please see the attached revision.
LGTM.
Thanks,
Richard.
> Martin
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-07-15 7:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-13 19:26 [PATCH] handle vector and aggregate stores in -Wstringop-overflow [PR 97027] Martin Sebor
2021-07-14 7:01 ` Richard Biener
2021-07-14 18:46 ` Martin Sebor
2021-07-15 7:31 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).