* [Bug c++/110914] Optimization eliminating necessary assignment before 0-byte memcpy
2023-08-05 15:46 [Bug c++/110914] New: Optimization eliminating necessary assignment before 0-byte memcpy erosenberger at kinetica dot com
@ 2023-08-05 15:47 ` erosenberger at kinetica dot com
2023-08-05 16:04 ` [Bug tree-optimization/110914] [11/12/13/14 Regression] " pinskia at gcc dot gnu.org
` (8 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: erosenberger at kinetica dot com @ 2023-08-05 15:47 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110914
--- Comment #1 from Eric Rosenberger <erosenberger at kinetica dot com> ---
Created attachment 55693
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55693&action=edit
Test program
Note: preprocessed .ii is too big to attach
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug tree-optimization/110914] [11/12/13/14 Regression] Optimization eliminating necessary assignment before 0-byte memcpy
2023-08-05 15:46 [Bug c++/110914] New: Optimization eliminating necessary assignment before 0-byte memcpy erosenberger at kinetica dot com
2023-08-05 15:47 ` [Bug c++/110914] " erosenberger at kinetica dot com
@ 2023-08-05 16:04 ` pinskia at gcc dot gnu.org
2023-08-05 16:14 ` pinskia at gcc dot gnu.org
` (7 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-08-05 16:04 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110914
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|Optimization eliminating |[11/12/13/14 Regression]
|necessary assignment before |Optimization eliminating
|0-byte memcpy |necessary assignment before
| |0-byte memcpy
Known to work| |9.5.0
Component|c++ |tree-optimization
Target Milestone|--- |11.5
Known to fail| |10.5.0, 14.0
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug tree-optimization/110914] [11/12/13/14 Regression] Optimization eliminating necessary assignment before 0-byte memcpy
2023-08-05 15:46 [Bug c++/110914] New: Optimization eliminating necessary assignment before 0-byte memcpy erosenberger at kinetica dot com
2023-08-05 15:47 ` [Bug c++/110914] " erosenberger at kinetica dot com
2023-08-05 16:04 ` [Bug tree-optimization/110914] [11/12/13/14 Regression] " pinskia at gcc dot gnu.org
@ 2023-08-05 16:14 ` pinskia at gcc dot gnu.org
2023-08-29 15:50 ` [Bug tree-optimization/110914] [11/12/13/14 Regression] Optimization eliminating necessary assignment before 0-byte memcpy since r10-5451 jakub at gcc dot gnu.org
` (6 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: pinskia at gcc dot gnu.org @ 2023-08-05 16:14 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110914
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Last reconfirmed| |2023-08-05
Ever confirmed|0 |1
Status|UNCONFIRMED |NEW
--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed, The strlen pass is removing it ...
Self contained testcase:
```
struct Int8_buf
{
Int8_buf(const char* s, __SIZE_TYPE__ l)
{
val_int = 0;
__builtin_memcpy(&val_int, s, l!=0);
}
unsigned char val_int;
};
int __attribute__ ((noinline)) foo(const char* s, __SIZE_TYPE__ l)
{
Int8_buf b(s, l);
return b.val_int;
}
int main()
{
const char *cs="123456";
int a = foo(cs, __builtin_strlen(cs) - 5);
int b = foo(cs, __builtin_strlen(cs) - 6);
if (a != '1')
__builtin_abort();
if (b != 0)
__builtin_abort();
}
```
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug tree-optimization/110914] [11/12/13/14 Regression] Optimization eliminating necessary assignment before 0-byte memcpy since r10-5451
2023-08-05 15:46 [Bug c++/110914] New: Optimization eliminating necessary assignment before 0-byte memcpy erosenberger at kinetica dot com
` (2 preceding siblings ...)
2023-08-05 16:14 ` pinskia at gcc dot gnu.org
@ 2023-08-29 15:50 ` jakub at gcc dot gnu.org
2023-08-29 16:55 ` jakub at gcc dot gnu.org
` (5 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-08-29 15:50 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110914
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|[11/12/13/14 Regression] |[11/12/13/14 Regression]
|Optimization eliminating |Optimization eliminating
|necessary assignment before |necessary assignment before
|0-byte memcpy |0-byte memcpy since
| |r10-5451
CC| |jakub at gcc dot gnu.org
--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Started with r10-5451-gef29b12cfbb4979a89b3c
C testcase for -O2:
__attribute__ ((noipa)) int
foo (const char *s, unsigned long l)
{
unsigned char r = 0;
__builtin_memcpy (&r, s, l != 0);
return r;
}
int
main ()
{
const char *p = "123456";
int a = foo (p, __builtin_strlen (p) - 5);
int b = foo (p, __builtin_strlen (p) - 6);
if (a != '1')
__builtin_abort ();
if (b != 0)
__builtin_abort ();
}
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug tree-optimization/110914] [11/12/13/14 Regression] Optimization eliminating necessary assignment before 0-byte memcpy since r10-5451
2023-08-05 15:46 [Bug c++/110914] New: Optimization eliminating necessary assignment before 0-byte memcpy erosenberger at kinetica dot com
` (3 preceding siblings ...)
2023-08-29 15:50 ` [Bug tree-optimization/110914] [11/12/13/14 Regression] Optimization eliminating necessary assignment before 0-byte memcpy since r10-5451 jakub at gcc dot gnu.org
@ 2023-08-29 16:55 ` jakub at gcc dot gnu.org
2023-08-30 9:22 ` cvs-commit at gcc dot gnu.org
` (4 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-08-29 16:55 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110914
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED
Assignee|unassigned at gcc dot gnu.org |jakub at gcc dot gnu.org
--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 55813
--> https://gcc.gnu.org/bugzilla/attachment.cgi?id=55813&action=edit
gcc14-pr110914.patch
Untested fix.
The bogus change in the above mentioned commit was the:
if (olddsi != NULL
- && tree_fits_uhwi_p (len)
&& !integer_zerop (len))
- adjust_last_stmt (olddsi, stmt, false);
+ {
+ maybe_warn_overflow (stmt, len, rvals, olddsi, false, true);
+ adjust_last_stmt (olddsi, stmt, false);
+ }
part. I haven't analyzed what exactly maybe_warn_overflow does, it is some
warning stuff and perhaps can be called when len is not constant, but the
previous guarding of adjust_last_stmt was completely intentional.
As adjust_last_stmt function comment says:
If the last .MEM setter statement before STMT is
memcpy (x, y, strlen (y) + 1), the only .MEM use of it is STMT
and STMT is known to overwrite x[strlen (x)], adjust the last memcpy to
just memcpy (x, y, strlen (y)). SI must be the zero length
strinfo.
so obviously the fact that memcpy (the second one) doesn't have last argument
constant 0 doesn't mean that it is non-zero length memcpy, we only know it
either if it is constant non-zero length, or variable where say value-range
could prove it is not zero. We have an adjust_last_stmt call later in the
function which handles length of strlen (x) + 1 though. So, this patch just
reverts the guard of that function back to what it was before.
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug tree-optimization/110914] [11/12/13/14 Regression] Optimization eliminating necessary assignment before 0-byte memcpy since r10-5451
2023-08-05 15:46 [Bug c++/110914] New: Optimization eliminating necessary assignment before 0-byte memcpy erosenberger at kinetica dot com
` (4 preceding siblings ...)
2023-08-29 16:55 ` jakub at gcc dot gnu.org
@ 2023-08-30 9:22 ` cvs-commit at gcc dot gnu.org
2023-08-30 9:33 ` cvs-commit at gcc dot gnu.org
` (3 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-08-30 9:22 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110914
--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:398842e7038ea0f34054f0f694014d0ecd656846
commit r14-3564-g398842e7038ea0f34054f0f694014d0ecd656846
Author: Jakub Jelinek <jakub@redhat.com>
Date: Wed Aug 30 11:21:45 2023 +0200
tree-ssa-strlen: Fix up handling of conditionally zero memcpy [PR110914]
The following testcase is miscompiled since r279392 aka
r10-5451-gef29b12cfbb4979
The strlen pass has adjust_last_stmt function, which performs mainly strcat
or strcat-like optimizations (say strcpy (x, "abcd"); strcat (x, p);
or equivalent memcpy (x, "abcd", strlen ("abcd") + 1); char *q = strchr (x,
0);
memcpy (x, p, strlen (p)); etc. where the first stmt stores '\0' character
at the end but next immediately overwrites it and so the first memcpy can
be
adjusted to store 1 fewer bytes. handle_builtin_memcpy called this
function
in two spots, the first one guarded like:
if (olddsi != NULL
&& tree_fits_uhwi_p (len)
&& !integer_zerop (len))
adjust_last_stmt (olddsi, stmt, false);
i.e. only for constant non-zero length. The other spot can call it even
for non-constant length but in that case we punt before that if that length
isn't length of some string + 1, so again non-zero.
The r279392 change I assume wanted to add some warning stuff and changed it
like
if (olddsi != NULL
- && tree_fits_uhwi_p (len)
&& !integer_zerop (len))
- adjust_last_stmt (olddsi, stmt, false);
+ {
+ maybe_warn_overflow (stmt, len, rvals, olddsi, false, true);
+ adjust_last_stmt (olddsi, stmt, false);
+ }
While maybe_warn_overflow possibly handles non-constant length fine,
adjust_last_stmt really relies on length to be non-zero, which
!integer_zerop (len) alone doesn't guarantee. While we could for
len being SSA_NAME ask the ranger or tree_expr_nonzero_p, I think
adjust_last_stmt will not benefit from it much, so the following patch
just restores the above condition/previous behavior for the
adjust_last_stmt
call only.
2023-08-30 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/110914
* tree-ssa-strlen.cc (strlen_pass::handle_builtin_memcpy): Don't
call
adjust_last_stmt unless len is known constant.
* gcc.c-torture/execute/pr110914.c: New test.
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug tree-optimization/110914] [11/12/13/14 Regression] Optimization eliminating necessary assignment before 0-byte memcpy since r10-5451
2023-08-05 15:46 [Bug c++/110914] New: Optimization eliminating necessary assignment before 0-byte memcpy erosenberger at kinetica dot com
` (5 preceding siblings ...)
2023-08-30 9:22 ` cvs-commit at gcc dot gnu.org
@ 2023-08-30 9:33 ` cvs-commit at gcc dot gnu.org
2023-08-30 9:49 ` cvs-commit at gcc dot gnu.org
` (2 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-08-30 9:33 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110914
--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:8b3d6cfad21a61cdc65bb0bb445cba907c190c21
commit r13-7768-g8b3d6cfad21a61cdc65bb0bb445cba907c190c21
Author: Jakub Jelinek <jakub@redhat.com>
Date: Wed Aug 30 11:21:45 2023 +0200
tree-ssa-strlen: Fix up handling of conditionally zero memcpy [PR110914]
The following testcase is miscompiled since r279392 aka
r10-5451-gef29b12cfbb4979
The strlen pass has adjust_last_stmt function, which performs mainly strcat
or strcat-like optimizations (say strcpy (x, "abcd"); strcat (x, p);
or equivalent memcpy (x, "abcd", strlen ("abcd") + 1); char *q = strchr (x,
0);
memcpy (x, p, strlen (p)); etc. where the first stmt stores '\0' character
at the end but next immediately overwrites it and so the first memcpy can
be
adjusted to store 1 fewer bytes. handle_builtin_memcpy called this
function
in two spots, the first one guarded like:
if (olddsi != NULL
&& tree_fits_uhwi_p (len)
&& !integer_zerop (len))
adjust_last_stmt (olddsi, stmt, false);
i.e. only for constant non-zero length. The other spot can call it even
for non-constant length but in that case we punt before that if that length
isn't length of some string + 1, so again non-zero.
The r279392 change I assume wanted to add some warning stuff and changed it
like
if (olddsi != NULL
- && tree_fits_uhwi_p (len)
&& !integer_zerop (len))
- adjust_last_stmt (olddsi, stmt, false);
+ {
+ maybe_warn_overflow (stmt, len, rvals, olddsi, false, true);
+ adjust_last_stmt (olddsi, stmt, false);
+ }
While maybe_warn_overflow possibly handles non-constant length fine,
adjust_last_stmt really relies on length to be non-zero, which
!integer_zerop (len) alone doesn't guarantee. While we could for
len being SSA_NAME ask the ranger or tree_expr_nonzero_p, I think
adjust_last_stmt will not benefit from it much, so the following patch
just restores the above condition/previous behavior for the
adjust_last_stmt
call only.
2023-08-30 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/110914
* tree-ssa-strlen.cc (strlen_pass::handle_builtin_memcpy): Don't
call
adjust_last_stmt unless len is known constant.
* gcc.c-torture/execute/pr110914.c: New test.
(cherry picked from commit 398842e7038ea0f34054f0f694014d0ecd656846)
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug tree-optimization/110914] [11/12/13/14 Regression] Optimization eliminating necessary assignment before 0-byte memcpy since r10-5451
2023-08-05 15:46 [Bug c++/110914] New: Optimization eliminating necessary assignment before 0-byte memcpy erosenberger at kinetica dot com
` (6 preceding siblings ...)
2023-08-30 9:33 ` cvs-commit at gcc dot gnu.org
@ 2023-08-30 9:49 ` cvs-commit at gcc dot gnu.org
2023-08-30 9:57 ` cvs-commit at gcc dot gnu.org
2023-08-30 10:00 ` jakub at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-08-30 9:49 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110914
--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-12 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:487390818c6b10daa29dd1db205fa8566296c084
commit r12-9837-g487390818c6b10daa29dd1db205fa8566296c084
Author: Jakub Jelinek <jakub@redhat.com>
Date: Wed Aug 30 11:21:45 2023 +0200
tree-ssa-strlen: Fix up handling of conditionally zero memcpy [PR110914]
The following testcase is miscompiled since r279392 aka
r10-5451-gef29b12cfbb4979
The strlen pass has adjust_last_stmt function, which performs mainly strcat
or strcat-like optimizations (say strcpy (x, "abcd"); strcat (x, p);
or equivalent memcpy (x, "abcd", strlen ("abcd") + 1); char *q = strchr (x,
0);
memcpy (x, p, strlen (p)); etc. where the first stmt stores '\0' character
at the end but next immediately overwrites it and so the first memcpy can
be
adjusted to store 1 fewer bytes. handle_builtin_memcpy called this
function
in two spots, the first one guarded like:
if (olddsi != NULL
&& tree_fits_uhwi_p (len)
&& !integer_zerop (len))
adjust_last_stmt (olddsi, stmt, false);
i.e. only for constant non-zero length. The other spot can call it even
for non-constant length but in that case we punt before that if that length
isn't length of some string + 1, so again non-zero.
The r279392 change I assume wanted to add some warning stuff and changed it
like
if (olddsi != NULL
- && tree_fits_uhwi_p (len)
&& !integer_zerop (len))
- adjust_last_stmt (olddsi, stmt, false);
+ {
+ maybe_warn_overflow (stmt, len, rvals, olddsi, false, true);
+ adjust_last_stmt (olddsi, stmt, false);
+ }
While maybe_warn_overflow possibly handles non-constant length fine,
adjust_last_stmt really relies on length to be non-zero, which
!integer_zerop (len) alone doesn't guarantee. While we could for
len being SSA_NAME ask the ranger or tree_expr_nonzero_p, I think
adjust_last_stmt will not benefit from it much, so the following patch
just restores the above condition/previous behavior for the
adjust_last_stmt
call only.
2023-08-30 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/110914
* tree-ssa-strlen.cc (strlen_pass::handle_builtin_memcpy): Don't
call
adjust_last_stmt unless len is known constant.
* gcc.c-torture/execute/pr110914.c: New test.
(cherry picked from commit 398842e7038ea0f34054f0f694014d0ecd656846)
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug tree-optimization/110914] [11/12/13/14 Regression] Optimization eliminating necessary assignment before 0-byte memcpy since r10-5451
2023-08-05 15:46 [Bug c++/110914] New: Optimization eliminating necessary assignment before 0-byte memcpy erosenberger at kinetica dot com
` (7 preceding siblings ...)
2023-08-30 9:49 ` cvs-commit at gcc dot gnu.org
@ 2023-08-30 9:57 ` cvs-commit at gcc dot gnu.org
2023-08-30 10:00 ` jakub at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2023-08-30 9:57 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110914
--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:
https://gcc.gnu.org/g:cf3aa538317d6c525739f339b79010ae82dc20f5
commit r11-10969-gcf3aa538317d6c525739f339b79010ae82dc20f5
Author: Jakub Jelinek <jakub@redhat.com>
Date: Wed Aug 30 11:21:45 2023 +0200
tree-ssa-strlen: Fix up handling of conditionally zero memcpy [PR110914]
The following testcase is miscompiled since r279392 aka
r10-5451-gef29b12cfbb4979
The strlen pass has adjust_last_stmt function, which performs mainly strcat
or strcat-like optimizations (say strcpy (x, "abcd"); strcat (x, p);
or equivalent memcpy (x, "abcd", strlen ("abcd") + 1); char *q = strchr (x,
0);
memcpy (x, p, strlen (p)); etc. where the first stmt stores '\0' character
at the end but next immediately overwrites it and so the first memcpy can
be
adjusted to store 1 fewer bytes. handle_builtin_memcpy called this
function
in two spots, the first one guarded like:
if (olddsi != NULL
&& tree_fits_uhwi_p (len)
&& !integer_zerop (len))
adjust_last_stmt (olddsi, stmt, false);
i.e. only for constant non-zero length. The other spot can call it even
for non-constant length but in that case we punt before that if that length
isn't length of some string + 1, so again non-zero.
The r279392 change I assume wanted to add some warning stuff and changed it
like
if (olddsi != NULL
- && tree_fits_uhwi_p (len)
&& !integer_zerop (len))
- adjust_last_stmt (olddsi, stmt, false);
+ {
+ maybe_warn_overflow (stmt, len, rvals, olddsi, false, true);
+ adjust_last_stmt (olddsi, stmt, false);
+ }
While maybe_warn_overflow possibly handles non-constant length fine,
adjust_last_stmt really relies on length to be non-zero, which
!integer_zerop (len) alone doesn't guarantee. While we could for
len being SSA_NAME ask the ranger or tree_expr_nonzero_p, I think
adjust_last_stmt will not benefit from it much, so the following patch
just restores the above condition/previous behavior for the
adjust_last_stmt
call only.
2023-08-30 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/110914
* tree-ssa-strlen.c (strlen_pass::handle_builtin_memcpy): Don't
call
adjust_last_stmt unless len is known constant.
* gcc.c-torture/execute/pr110914.c: New test.
(cherry picked from commit 398842e7038ea0f34054f0f694014d0ecd656846)
^ permalink raw reply [flat|nested] 11+ messages in thread* [Bug tree-optimization/110914] [11/12/13/14 Regression] Optimization eliminating necessary assignment before 0-byte memcpy since r10-5451
2023-08-05 15:46 [Bug c++/110914] New: Optimization eliminating necessary assignment before 0-byte memcpy erosenberger at kinetica dot com
` (8 preceding siblings ...)
2023-08-30 9:57 ` cvs-commit at gcc dot gnu.org
@ 2023-08-30 10:00 ` jakub at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: jakub at gcc dot gnu.org @ 2023-08-30 10:00 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110914
Jakub Jelinek <jakub at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Resolution|--- |FIXED
--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.
^ permalink raw reply [flat|nested] 11+ messages in thread