[Bug tree-optimization/113630] [11/12/13/14 Regression] -fno-strict-aliasing introduces out-of-bounds memory access
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113630 --- Comment #6 from GCC Commits --- The master branch has been updated by Richard Biener : https://gcc.gnu.org/g:724b64304ff5c8ac08a913509afd6fde38d7b767 commit r14-8653-g724b64304ff5c8ac08a913509afd6fde38d7b767 Author: Richard Biener 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.
[Bug tree-optimization/113630] [11/12/13/14 Regression] -fno-strict-aliasing introduces out-of-bounds memory access
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113630
Richard Biener 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 ---
(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.
[Bug tree-optimization/113630] [11/12/13/14 Regression] -fno-strict-aliasing introduces out-of-bounds memory access
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113630 --- Comment #4 from Richard Biener --- (In reply to Andrew Pinski from comment #3) > Note LLVM produces decent code here by only using one load: > ``` > xor eax, eax > testesi, esi > seteal > 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)))
[Bug tree-optimization/113630] [11/12/13/14 Regression] -fno-strict-aliasing introduces out-of-bounds memory access
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113630 --- Comment #3 from Andrew Pinski --- Note LLVM produces decent code here by only using one load: ``` xor eax, eax testesi, esi seteal mov eax, dword ptr [rdi + 4*rax] ``` Maybe GCC could do the same ...
[Bug tree-optimization/113630] [11/12/13/14 Regression] -fno-strict-aliasing introduces out-of-bounds memory access
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113630
Andrew Pinski 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 ---
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).
[Bug tree-optimization/113630] [11/12/13/14 Regression] -fno-strict-aliasing introduces out-of-bounds memory access
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=113630 Andrew Pinski 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
