On Tue, 25 Nov 2025, Jakub Jelinek wrote:
> Hi!
>
> The following testcase is miscompiled by modref thinking one store is not
> needed:
> ipa-modref: call stmt IteratorImp::operator++ (&clast.d_imp);
> ipa-modref: call to void IteratorImp::operator++()/2 does not use ref: MEM
> <struct PMF *> [(struct iterator *)&clast + 8B] alias sets: 5->5
> ipa-modref: call stmt IteratorImp::operator++ (&clast.d_imp);
> ipa-modref: call to void IteratorImp::operator++()/2 does not use ref: MEM
> <struct PMF *> [(struct iterator *)&clast + 8B] alias sets: 5->5
> ipa-modref: call stmt IteratorImp::operator++ (&clast.d_imp);
> ipa-modref: call to void IteratorImp::operator++()/2 kills clast
> Deleted dead store: MEM <struct PMF *> [(struct iterator *)&clast + 8B] =
> &MEM <struct PMF[16]> [(void *)&mX + 120B];
> This only happens when PMF is pointer to member function, not when PMF
> is say user structure with void * and unsigned long members (as an
> example of something with the same size as PMF) or when PMF is
> char (as an example of something with 0 alias set, which is what
> PMF for some reason has).
> Apparently it is related to the r0-82711 change (PR22369 and PR22451)
> which made also PMF * types to have alias set 0. While probably
> modref should be analyzed what's going on, I find it really weird and
> IMHO unnecessary to use alias set 0 for any pointers, we don't use
> alias set 0 for e.g. void * pointers. Commenting out the
> INDIRECT_TYPE_P (t) && TYPE_PTRMEMFUNC_P (TREE_TYPE (t))
> special case fixes the testcase, and so does the following patch
> which treats PMF * for aliasing the same as void *.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, tried also
> the testcases from those 2 PRs (which weren't checked in or maybe came
> from the testsuite) and they still work fine.
>
> 2025-11-25 Jakub Jelinek <[email protected]>
>
> PR c++/119969
> * cp-objcp-common.cc (cxx_get_alias_set): Return
> get_alias_set (ptr_type_node) for pointer/reference to
> TYPE_PTRMEMFUNC_P instead of 0.
>
> * g++.dg/torture/pr119969.C: New test.
>
> --- gcc/cp/cp-objcp-common.cc.jj 2025-10-09 22:41:19.871273246 +0200
> +++ gcc/cp/cp-objcp-common.cc 2025-11-25 12:37:38.251339146 +0100
> @@ -181,10 +181,10 @@ cxx_get_alias_set (tree t)
> return get_alias_set (TYPE_CONTEXT (t));
>
> /* Punt on PMFs until we canonicalize functions properly. */
> - if (TYPE_PTRMEMFUNC_P (t)
> - || (INDIRECT_TYPE_P (t)
> - && TYPE_PTRMEMFUNC_P (TREE_TYPE (t))))
> + if (TYPE_PTRMEMFUNC_P (t))
> return 0;
> + if (INDIRECT_TYPE_P (t) && TYPE_PTRMEMFUNC_P (TREE_TYPE (t)))
> + return get_alias_set (ptr_type_node);
I think we'd get the same effect from alias.cc itself iff
TYPE_PTRMEMFUNC_P were TYPE_STRUCTURAL_EQUALITY_P. In particular
I'll not that a PMF ** would not end up invoking the langhook for
PMF *. While the pointer code in get_alias_set eventually
looks at the "base", it does not re-query the frontend about it,
but only special-case
if (TREE_CODE (p) == VOID_TYPE || TYPE_STRUCTURAL_EQUALITY_P (p))
set = get_alias_set (ptr_type_node);
so given it seems we don't exactly know what goes wrong I fear while
the fix makes sense, it's not complete? Unless TYPE_PTRMEMFUNC_P
already is TYPE_STRUCTURAL_EQUALITY_P, in which case all the
special-casing could go?
Richard.
>
> return c_common_get_alias_set (t);
> }
> --- gcc/testsuite/g++.dg/torture/pr119969.C.jj 2025-11-25
> 12:38:20.120143562 +0100
> +++ gcc/testsuite/g++.dg/torture/pr119969.C 2025-11-25 12:38:42.407758775
> +0100
> @@ -0,0 +1,46 @@
> +// PR c++/119969
> +// { dg-do run }
> +
> +struct S {};
> +using PMF = void (S::*)();
> +using Block = PMF[16];
> +using BlockPtr = Block*;
> +
> +struct IteratorImp {
> + Block** d_blockPtr_p;
> + PMF* d_value_p;
> +
> + void operator++();
> + PMF& operator*() const { return *d_value_p; }
> +};
> +
> +void IteratorImp::operator++() {
> + int offset = 1 + (d_value_p - **d_blockPtr_p);
> + d_blockPtr_p += offset / 16;
> + d_value_p = **d_blockPtr_p + (offset % 16);
> +}
> +
> +struct iterator {
> + IteratorImp d_imp;
> +};
> +
> +struct D {
> + Block* d_blockPtrs[1];
> + Block d_block;
> + PMF* d_start_p;
> +};
> +
> +D mX;
> +
> +void privateInit(int numElements) {
> + mX.d_blockPtrs[0] = &mX.d_block;
> + mX.d_start_p = mX.d_block + (numElements + 7);
> +}
> +
> +int main() {
> + privateInit(0);
> + iterator cbgn = {{mX.d_blockPtrs, mX.d_block + 7}};
> + auto clast = cbgn;
> + ++clast.d_imp;
> + if (&*cbgn.d_imp == &*clast.d_imp) return 1;
> +}
>
> Jakub
>
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
