https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120929
--- Comment #20 from Richard Biener <rguenth at gcc dot gnu.org> ---
(In reply to qinzhao from comment #16)
> (In reply to Siddhesh Poyarekar from comment #12)
> > This is interesting here's the IR dump right after objsz:
> >
> > The key bit is:
> >
> > map2_4 = __builtin_malloc (8);
> > pin_pointer (&buf);
> > _1 = &map2_4->magic;
> > _9 = __builtin_malloc (9);
> > *_1 = _9;
> > goto <bb 4>; [100.00%]
> >
> > <bb 3> [local count: 1073741824]:
> > b = "";
> > ptr_10 = *_1;
> > _11 = 8;
> > __builtin___memcpy_chk (ptr_10, &b, 9, _11);
> >
> > where *_1 gets updated to _9, but when one follows the *_1 through ptr_10,
> > it doesn't end up with _9, the def statement is:
> >
> > _1 = &map2_4->magic;
> >
> > which leads to the incorrect value for the object size. This is because the
> > pass doesn't know that a MEM_REF could be the LHS (for the zero byte offset
> > case like it is here) for an assignment, i.e. this:
> >
> > *_1 = _9;
>
> There are two possible solutions to the above issue:
>
> A. Add handling for *_1 = _9 to enable the object size propagate through the
> correct data flow path.
> Or
> B. in my latest change that triggered this issue:
>
> /* Handle the following stmt #2 to propagate the size from the
> stmt #1 to #3:
> 1 _1 = .ACCESS_WITH_SIZE (_3, _4, 1, 0, -1, 0B);
> 2 _5 = *_1;
> 3 _6 = __builtin_dynamic_object_size (_5, 1);
> */
> else if (TREE_CODE (rhs) == MEM_REF
> && POINTER_TYPE_P (TREE_TYPE (rhs))
> && TREE_CODE (TREE_OPERAND (rhs, 0)) == SSA_NAME
> && integer_zerop (TREE_OPERAND (rhs, 1)))
> reexamine = merge_object_sizes (osi, var, TREE_OPERAND (rhs,
> 0));
>
>
> I feel that propagating the size through _5 = *_1 might not be correct in
> general, we should only limit it to the case when the RHS is a pointer
> defined by .ACCESS_WITH_SIZE?
>
> what do you think?
I'm confused as to what this does for the case in question. The IL is
_1 = &map2_4->magic;
_9 = __builtin_malloc (9);
*_1 = _9;
ptr_10 = *_1;
_11 = __builtin_dynamic_object_size (ptr_10, 0);
ptr_10 is equal to _9, but the objsize dump suggests we are instead
looking at the size of what _1 points to, instead of the size of what
*_1 points to?! That would be completely incorrect of course!
Also if that's not the case but we still look at *_1 but with _1
from the earlier def that cannot be done either because you skipped
an intermediate definition of *_1.
I think the code is obviously bogus. Quoting more in full:
else if (gimple_assign_single_p (stmt)
|| gimple_assign_unary_nop_p (stmt))
{
if (TREE_CODE (rhs) == SSA_NAME
&& POINTER_TYPE_P (TREE_TYPE (rhs)))
reexamine = merge_object_sizes (osi, var, rhs);
/* Handle the following stmt #2 to propagate the size from the
stmt #1 to #3:
1 _1 = .ACCESS_WITH_SIZE (_3, _4, 1, 0, -1, 0B);
2 _5 = *_1;
3 _6 = __builtin_dynamic_object_size (_5, 1);
*/
else if (TREE_CODE (rhs) == MEM_REF
&& POINTER_TYPE_P (TREE_TYPE (rhs))
&& TREE_CODE (TREE_OPERAND (rhs, 0)) == SSA_NAME
&& integer_zerop (TREE_OPERAND (rhs, 1)))
reexamine = merge_object_sizes (osi, var, TREE_OPERAND (rhs, 0));
so for
_1 = _2;
we merge from _2. For
_1 = *_2;
we _also_ merge from _2. But those are semantically not the same!
IMO this change was bogus and should be reverted.
