https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120929

--- Comment #17 from Siddhesh Poyarekar <siddhesh 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?

That propagation is not incorrect, but it is incomplete, since (TIL) a MEM_REF
with zero offset could be the LHS of a valid gimple statement as well.  However
AFAICT there's no way to directly reach it like in case of an SSA, you'll need
to walk the function to find the statement with a matching LHS.

That is essentially option A.  I think that's the "correct" solution unless it
turns out to be infeasible (too slow, too kludgy, etc.), in which case option B
is a reasonable alternative I think.

Reply via email to