public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug tree-optimization/113630] New: -fno-strict-aliasing introduces out-of-bounds memory access
@ 2024-01-27 17:34 kristerw at gcc dot gnu.org
2024-01-27 17:42 ` [Bug tree-optimization/113630] " pinskia at gcc dot gnu.org
` (9 more replies)
0 siblings, 10 replies; 11+ messages in thread
From: kristerw at gcc dot gnu.org @ 2024-01-27 17:34 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113630
Bug ID: 113630
Summary: -fno-strict-aliasing introduces out-of-bounds memory
access
Product: gcc
Version: 14.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: tree-optimization
Assignee: unassigned at gcc dot gnu.org
Reporter: kristerw at gcc dot gnu.org
Target Milestone: ---
The test gcc.dg/torture/pr110799.c crashes because of an out of bounds memory
access when compiled with "-O2 -fno-strict-aliasing".
What is happening is that the pre pass has changed
struct S {
int a;
};
struct M {
int a, b;
};
__attribute__((noipa, noinline, noclone, no_icf))
int f (struct S * p, int c, int d)
{
int r;
<bb 2>:
if (c_2(D) != 0)
goto <bb 3>;
else
goto <bb 6>;
<bb 3>:
if (d_6(D) != 0)
goto <bb 4>;
else
goto <bb 5>;
<bb 4>
r_8 = p_4(D)->a;
goto <bb 7>;
<bb 5>
r_7 = MEM[(struct M *)p_4(D)].a;
goto <bb 7>;
<bb 6>
r_5 = MEM[(struct M *)p_4(D)].b;
<bb 7>
# r_1 = PHI <r_7(5), r_5(6), r_8(4)>
return r_1;
}
by combining bb 4 and bb 5 and doing all accesses as struct M:
__attribute__((noipa, noinline, noclone, no_icf))
int f (struct S * p, int c, int d)
{
int r;
int pretmp_9;
<bb 2>:
if (c_2(D) != 0)
goto <bb 3>; [50.00%]
else
goto <bb 4>; [50.00%]
<bb 3>:
pretmp_9 = MEM[(struct M *)p_4(D)].a;
goto <bb 5>;
<bb 4>:
r_5 = MEM[(struct M *)p_4(D)].b;
<bb 5>:
# r_1 = PHI <pretmp_9(3), r_5(4)>
return r_1;
}
This in turn allows later passes to hoist the two loads
__attribute__((noipa, noinline, noclone, no_icf))
int f (struct S * p, int c, int d)
{
int r;
int pretmp_9;
<bb 2>:
pretmp_9 = MEM[(struct M *)p_4(D)].a;
r_5 = MEM[(struct M *)p_4(D)].b;
if (c_2(D) != 0)
goto <bb 3>;
else
goto <bb 4>;
<bb 3>:
<bb 4>:
# r_1 = PHI <pretmp_9(3), r_5(2)>
return r_1;
}
which now reads out of bounds when we pass a struct S as f(&s, 1, 1).
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug tree-optimization/113630] -fno-strict-aliasing introduces out-of-bounds memory access
2024-01-27 17:34 [Bug tree-optimization/113630] New: -fno-strict-aliasing introduces out-of-bounds memory access kristerw at gcc dot gnu.org
@ 2024-01-27 17:42 ` pinskia at gcc dot gnu.org
2024-01-27 23:34 ` [Bug tree-optimization/113630] [11/12/13/14 Regression] " pinskia at gcc dot gnu.org
` (8 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: pinskia at gcc dot gnu.org @ 2024-01-27 17:42 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113630
--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I suspect pre us doing the right thing. It is phi-opt code that hoists is doing
the wrong thing for non strict aliasing.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug tree-optimization/113630] [11/12/13/14 Regression] -fno-strict-aliasing introduces out-of-bounds memory access
2024-01-27 17:34 [Bug tree-optimization/113630] New: -fno-strict-aliasing introduces out-of-bounds memory access kristerw at gcc dot gnu.org
2024-01-27 17:42 ` [Bug tree-optimization/113630] " pinskia at gcc dot gnu.org
@ 2024-01-27 23:34 ` pinskia at gcc dot gnu.org
2024-01-27 23:42 ` pinskia at gcc dot gnu.org
` (7 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: pinskia at gcc dot gnu.org @ 2024-01-27 23:34 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113630
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Target Milestone|--- |11.5
See Also| |https://gcc.gnu.org/bugzill
| |a/show_bug.cgi?id=110799
Known to fail| |10.1.0, 14.0, 7.1.0
Known to work| |5.1.0, 6.1.0, 6.4.0
Keywords| |needs-bisection, wrong-code
Summary|-fno-strict-aliasing |[11/12/13/14 Regression]
|introduces out-of-bounds |-fno-strict-aliasing
|memory access |introduces out-of-bounds
| |memory access
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug tree-optimization/113630] [11/12/13/14 Regression] -fno-strict-aliasing introduces out-of-bounds memory access
2024-01-27 17:34 [Bug tree-optimization/113630] New: -fno-strict-aliasing introduces out-of-bounds memory access kristerw at gcc dot gnu.org
2024-01-27 17:42 ` [Bug tree-optimization/113630] " pinskia at gcc dot gnu.org
2024-01-27 23:34 ` [Bug tree-optimization/113630] [11/12/13/14 Regression] " pinskia at gcc dot gnu.org
@ 2024-01-27 23:42 ` pinskia at gcc dot gnu.org
2024-01-28 0:11 ` pinskia at gcc dot gnu.org
` (6 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: pinskia at gcc dot gnu.org @ 2024-01-27 23:42 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113630
Andrew Pinski <pinskia at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|UNCONFIRMED |NEW
Last reconfirmed| |2024-01-27
Ever confirmed|0 |1
Known to work|5.1.0, 6.1.0, 6.4.0 |
--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Confirmed.
I really think what PRE does is correct here since we have an aliasing set of 0
for both. Now what is incorrect is hoist_adjacent_loads which cannot do either
of any of the aliasing sets are 0 ...
I think even the function below is valid for non-strict aliasing:
```
int __attribute__((noipa,noinline))
f(struct S *p, int c, int d)
{
int r;
if (c)
{
r = ((struct M*)p)->a;
}
else
r = ((struct M*)p)->b;
return r;
}
```
That is hoist_adjacent_loads is broken for non-strict-aliasing in general and
has been since 4.8.0 when it was added (r0-117275-g372a6eb8d991eb).
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug tree-optimization/113630] [11/12/13/14 Regression] -fno-strict-aliasing introduces out-of-bounds memory access
2024-01-27 17:34 [Bug tree-optimization/113630] New: -fno-strict-aliasing introduces out-of-bounds memory access kristerw at gcc dot gnu.org
` (2 preceding siblings ...)
2024-01-27 23:42 ` pinskia at gcc dot gnu.org
@ 2024-01-28 0:11 ` pinskia at gcc dot gnu.org
2024-01-29 8:04 ` rguenth at gcc dot gnu.org
` (5 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: pinskia at gcc dot gnu.org @ 2024-01-28 0:11 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113630
--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Note LLVM produces decent code here by only using one load:
```
xor eax, eax
test esi, esi
sete al
mov eax, dword ptr [rdi + 4*rax]
```
Maybe GCC could do the same ...
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug tree-optimization/113630] [11/12/13/14 Regression] -fno-strict-aliasing introduces out-of-bounds memory access
2024-01-27 17:34 [Bug tree-optimization/113630] New: -fno-strict-aliasing introduces out-of-bounds memory access kristerw at gcc dot gnu.org
` (3 preceding siblings ...)
2024-01-28 0:11 ` pinskia at gcc dot gnu.org
@ 2024-01-29 8:04 ` rguenth at gcc dot gnu.org
2024-01-29 8:11 ` rguenth at gcc dot gnu.org
` (4 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: rguenth at gcc dot gnu.org @ 2024-01-29 8:04 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113630
--- Comment #4 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #3)
> Note LLVM produces decent code here by only using one load:
> ```
> xor eax, eax
> test esi, esi
> sete al
> mov eax, dword ptr [rdi + 4*rax]
> ```
>
> Maybe GCC could do the same ...
IIRC there's duplicate bugs about this - phiprop does kind-of the reverse.
The sink pass can now sink two exactly same stores but doesn't try sinking
a "compatible" store by introducing a PHI for the address.
/* ??? We could handle differing SSA uses in the LHS by inserting
PHIs for them. */
else if (! operand_equal_p (gimple_assign_lhs (first_store),
gimple_assign_lhs (def), 0)
|| (gimple_clobber_p (first_store)
!= gimple_clobber_p (def)))
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug tree-optimization/113630] [11/12/13/14 Regression] -fno-strict-aliasing introduces out-of-bounds memory access
2024-01-27 17:34 [Bug tree-optimization/113630] New: -fno-strict-aliasing introduces out-of-bounds memory access kristerw at gcc dot gnu.org
` (4 preceding siblings ...)
2024-01-29 8:04 ` rguenth at gcc dot gnu.org
@ 2024-01-29 8:11 ` rguenth at gcc dot gnu.org
2024-01-31 11:35 ` cvs-commit at gcc dot gnu.org
` (3 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: rguenth at gcc dot gnu.org @ 2024-01-29 8:11 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113630
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |ASSIGNED
Assignee|unassigned at gcc dot gnu.org |rguenth at gcc dot gnu.org
--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #2)
> Confirmed.
>
> I really think what PRE does is correct here since we have an aliasing set
> of 0 for both. Now what is incorrect is hoist_adjacent_loads which cannot do
> either of any of the aliasing sets are 0 ...
>
>
>
> I think even the function below is valid for non-strict aliasing:
> ```
> int __attribute__((noipa,noinline))
> f(struct S *p, int c, int d)
> {
> int r;
> if (c)
> {
> r = ((struct M*)p)->a;
> }
> else
> r = ((struct M*)p)->b;
> return r;
> }
> ```
>
> That is hoist_adjacent_loads is broken for non-strict-aliasing in general
> and has been since 4.8.0 when it was added (r0-117275-g372a6eb8d991eb).
It looks it relies on
/* The zeroth operand of the two component references must be
identical. It is not sufficient to compare get_base_address of
the two references, because this could allow for different
elements of the same array in the two trees. It is not safe to
assume that the existence of one array element implies the
existence of a different one. */
if (!operand_equal_p (TREE_OPERAND (ref1, 0), TREE_OPERAND (ref2, 0), 0))
continue;
for the correctness test. Note the MEM accesses are of size sizeof (struct M).
With -fno-strict-aliasing we're not wiping that detail so I think it _is_
a bug in PRE that it merges the two accesses.
I'll have a more detailed look.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug tree-optimization/113630] [11/12/13/14 Regression] -fno-strict-aliasing introduces out-of-bounds memory access
2024-01-27 17:34 [Bug tree-optimization/113630] New: -fno-strict-aliasing introduces out-of-bounds memory access kristerw at gcc dot gnu.org
` (5 preceding siblings ...)
2024-01-29 8:11 ` rguenth at gcc dot gnu.org
@ 2024-01-31 11:35 ` cvs-commit at gcc dot gnu.org
2024-01-31 11:35 ` [Bug tree-optimization/113630] [11/12/13 " rguenth at gcc dot gnu.org
` (2 subsequent siblings)
9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2024-01-31 11:35 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113630
--- Comment #6 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Richard Biener <rguenth@gcc.gnu.org>:
https://gcc.gnu.org/g:724b64304ff5c8ac08a913509afd6fde38d7b767
commit r14-8653-g724b64304ff5c8ac08a913509afd6fde38d7b767
Author: Richard Biener <rguenther@suse.de>
Date: Wed Jan 31 11:28:50 2024 +0100
tree-optimization/113630 - invalid code hoisting
The following avoids code hoisting (but also PRE insertion) of
expressions that got value-numbered to another one that are not
a valid replacement (but still compute the same value). This time
because the access path ends in a structure with different size,
meaning we consider a related access as not trapping because of the
size of the base of the access.
PR tree-optimization/113630
* tree-ssa-pre.cc (compute_avail): Avoid registering a
reference with a representation with not matching base
access size.
* gcc.dg/torture/pr113630.c: New testcase.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug tree-optimization/113630] [11/12/13 Regression] -fno-strict-aliasing introduces out-of-bounds memory access
2024-01-27 17:34 [Bug tree-optimization/113630] New: -fno-strict-aliasing introduces out-of-bounds memory access kristerw at gcc dot gnu.org
` (6 preceding siblings ...)
2024-01-31 11:35 ` cvs-commit at gcc dot gnu.org
@ 2024-01-31 11:35 ` rguenth at gcc dot gnu.org
2024-05-06 13:15 ` cvs-commit at gcc dot gnu.org
2024-05-06 13:18 ` [Bug tree-optimization/113630] [11/12 " rguenth at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: rguenth at gcc dot gnu.org @ 2024-01-31 11:35 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113630
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Priority|P3 |P2
Summary|[11/12/13/14 Regression] |[11/12/13 Regression]
|-fno-strict-aliasing |-fno-strict-aliasing
|introduces out-of-bounds |introduces out-of-bounds
|memory access |memory access
Known to work| |14.0
--- Comment #7 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed on trunk sofar.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug tree-optimization/113630] [11/12/13 Regression] -fno-strict-aliasing introduces out-of-bounds memory access
2024-01-27 17:34 [Bug tree-optimization/113630] New: -fno-strict-aliasing introduces out-of-bounds memory access kristerw at gcc dot gnu.org
` (7 preceding siblings ...)
2024-01-31 11:35 ` [Bug tree-optimization/113630] [11/12/13 " rguenth at gcc dot gnu.org
@ 2024-05-06 13:15 ` cvs-commit at gcc dot gnu.org
2024-05-06 13:18 ` [Bug tree-optimization/113630] [11/12 " rguenth at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2024-05-06 13:15 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113630
--- Comment #8 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-13 branch has been updated by Richard Biener
<rguenth@gcc.gnu.org>:
https://gcc.gnu.org/g:47cd06042237bf2d4f05b8355362bc038f6fa445
commit r13-8693-g47cd06042237bf2d4f05b8355362bc038f6fa445
Author: Richard Biener <rguenther@suse.de>
Date: Wed Jan 31 11:28:50 2024 +0100
tree-optimization/113630 - invalid code hoisting
The following avoids code hoisting (but also PRE insertion) of
expressions that got value-numbered to another one that are not
a valid replacement (but still compute the same value). This time
because the access path ends in a structure with different size,
meaning we consider a related access as not trapping because of the
size of the base of the access.
PR tree-optimization/113630
* tree-ssa-pre.cc (compute_avail): Avoid registering a
reference with a representation with not matching base
access size.
* gcc.dg/torture/pr113630.c: New testcase.
(cherry picked from commit 724b64304ff5c8ac08a913509afd6fde38d7b767)
^ permalink raw reply [flat|nested] 11+ messages in thread
* [Bug tree-optimization/113630] [11/12 Regression] -fno-strict-aliasing introduces out-of-bounds memory access
2024-01-27 17:34 [Bug tree-optimization/113630] New: -fno-strict-aliasing introduces out-of-bounds memory access kristerw at gcc dot gnu.org
` (8 preceding siblings ...)
2024-05-06 13:15 ` cvs-commit at gcc dot gnu.org
@ 2024-05-06 13:18 ` rguenth at gcc dot gnu.org
9 siblings, 0 replies; 11+ messages in thread
From: rguenth at gcc dot gnu.org @ 2024-05-06 13:18 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113630
Richard Biener <rguenth at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |rguenth at gcc dot gnu.org
Known to fail| |13.2.0
Status|ASSIGNED |NEW
Known to work| |13.2.1
Summary|[11/12/13 Regression] |[11/12 Regression]
|-fno-strict-aliasing |-fno-strict-aliasing
|introduces out-of-bounds |introduces out-of-bounds
|memory access |memory access
Assignee|rguenth at gcc dot gnu.org |unassigned at gcc dot gnu.org
--- Comment #9 from Richard Biener <rguenth at gcc dot gnu.org> ---
Backported as far as I want.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2024-05-06 13:18 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-27 17:34 [Bug tree-optimization/113630] New: -fno-strict-aliasing introduces out-of-bounds memory access kristerw at gcc dot gnu.org
2024-01-27 17:42 ` [Bug tree-optimization/113630] " pinskia at gcc dot gnu.org
2024-01-27 23:34 ` [Bug tree-optimization/113630] [11/12/13/14 Regression] " pinskia at gcc dot gnu.org
2024-01-27 23:42 ` pinskia at gcc dot gnu.org
2024-01-28 0:11 ` pinskia at gcc dot gnu.org
2024-01-29 8:04 ` rguenth at gcc dot gnu.org
2024-01-29 8:11 ` rguenth at gcc dot gnu.org
2024-01-31 11:35 ` cvs-commit at gcc dot gnu.org
2024-01-31 11:35 ` [Bug tree-optimization/113630] [11/12/13 " rguenth at gcc dot gnu.org
2024-05-06 13:15 ` cvs-commit at gcc dot gnu.org
2024-05-06 13:18 ` [Bug tree-optimization/113630] [11/12 " rguenth at gcc dot gnu.org
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).