On Wed, Jan 28, 2026 at 3:24 PM Siddhesh Poyarekar <[email protected]> wrote: > > By the time the third instance of warn_access (waccess3) is executed, > its input pointer may have been replaced with a same-address equivalent > to optimize accesses. As a simple fix, avoid warning when it is evident > that the location being accessed may have an alias through its union > sibling. > > A future enhancement could be to actually analyze siblings in such cases > to see if there's a match, but it may be more effort than it's worth in > practice. > > gcc/ChangeLog: > > PR middle-end/123801 > * gimple-ssa-warn-access.cc (aliasing_union_addr_p): New > function. > (pass_waccess::maybe_check_access_sizes): Use it.
A few questions below. > > gcc/testsuite/ChangeLog: > > PR middle-end/123801 > * gcc.dg/Wstringop-overflow-pr123801.c: New test. > > Signed-off-by: Siddhesh Poyarekar <[email protected]> > --- > Testing: > > - x86_64 bootstrap in progress, no new regressions in non-bootstrap > testing. > - i686 build and test in progress > > gcc/gimple-ssa-warn-access.cc | 46 ++++++++++++++++++- > .../gcc.dg/Wstringop-overflow-pr123801.c | 34 ++++++++++++++ > 2 files changed, 78 insertions(+), 2 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/Wstringop-overflow-pr123801.c > > diff --git a/gcc/gimple-ssa-warn-access.cc b/gcc/gimple-ssa-warn-access.cc > index c8f10cae32f..f8533d48e11 100644 > --- a/gcc/gimple-ssa-warn-access.cc > +++ b/gcc/gimple-ssa-warn-access.cc > @@ -3378,6 +3378,45 @@ append_attrname (const std::pair<int, attr_access> > &access, > strcpy (attrstr, TREE_STRING_POINTER (str)); > } > > +/* In the late (waccess3) pass run, addresses to union members may have been > + replaced to optimize accesses. This may result in waccess seeing the > wrong > + union member and come to a wrong conclusion about its size. For now, just > + bail out when we see the possibility of such a situation. This could in > + future walk through the different types and see if there's a union member > at > + the same address that matches the size of the access. */ > +static inline bool > +aliasing_union_addr_p (tree ptr, bool early) > +{ > + if (early) > + return false; > + > + /* Thread through the chain of definitions to arrive at an address > + expression. */ Isn't at this point one look back enough to get the ADDR_EXPR? Or there cases where more than one is needed? Do you have a testcase for those included? > + while (ptr && TREE_CODE (ptr) == SSA_NAME) > + { > + gimple *stmt = SSA_NAME_DEF_STMT (ptr); > + if (gimple_code (stmt) == GIMPLE_ASSIGN) if (is_a <gassign *> (stmt)) > + ptr = gimple_assign_rhs1 (stmt); This brings up we looking past a in "a + b". I am not sure that is correct; yes we are looking for the ADDR_EXPR here though. Maybe gimple_assign_single_p is better than looking at all assigns. > + else > + ptr = NULL; Why not just return false here? > + } > + > + if (ptr && TREE_CODE (ptr) == ADDR_EXPR) And then this becomes: if (TREE_CODE (ptr) != ADDR_EXPR) return false; I think overall the idea here is solid, just the walk back I am questioning if needed. Thanks, Andrew > + { > + ptr = TREE_OPERAND (ptr, 0); > + > + while (handled_component_p (ptr)) > + { > + if (TREE_CODE (ptr) == COMPONENT_REF > + && (TREE_CODE (TREE_TYPE (ptr)) == UNION_TYPE > + || TREE_CODE (TREE_TYPE (ptr)) == QUAL_UNION_TYPE)) > + return true; > + ptr = TREE_OPERAND (ptr, 0); > + } > + } > + return false; > +} > + > /* Iterate over attribute access read-only, read-write, and write-only > arguments and diagnose past-the-end accesses and related problems > in the function call EXP. */ > @@ -3419,8 +3458,11 @@ pass_waccess::maybe_check_access_sizes (rdwr_map *rwm, > tree fndecl, tree fntype, > > /* The pointer is set to null for the entry corresponding to > the size argument. Skip it. It's handled when the entry > - corresponding to the pointer argument comes up. */ > - if (!access.second.ptr) > + corresponding to the pointer argument comes up. Also bail out in > + waccess3 when the input pointer could end up pointing to a different > + member of a union, potentially giving a misleading size. */ > + if (!access.second.ptr > + || aliasing_union_addr_p (access.second.ptr, m_early_checks_p)) > continue; > > tree ptrtype = fntype_argno_type (fntype, ptridx); > diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-pr123801.c > b/gcc/testsuite/gcc.dg/Wstringop-overflow-pr123801.c > new file mode 100644 > index 00000000000..2881d2b5ea7 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-pr123801.c > @@ -0,0 +1,34 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -Wstringop-overflow" } */ > +#define SIZE 8 > + > +struct probe_locals *pl; > + > +void strlcpy (char *, const char *, long) > + __attribute__ ((__access__ (__write_only__, 1, 3))); > + > +struct probe_locals > +{ > + union > + { > + struct > + { > + char __tmp7[SIZE]; > + int __tmp10; > + int __tmp11; > + }; > + struct > + { > + char __tmp15[SIZE]; > + char __tmp16[SIZE]; > + }; > + }; > +}; > + > +void > +probe_func(const char *in) > +{ > + pl->__tmp10 = pl->__tmp11 = 0; > + char *tmp = pl->__tmp16; > + strlcpy (tmp, in, SIZE); > +} > -- > 2.52.0 >
