On Wed, Mar 4, 2026 at 1:41 AM Andrew Pinski
<[email protected]> wrote:
>
> When merging the modref summary after inlining, we merge all of the flags
> of the outer functions that was inlined into. But things go wrong as
> now the flags includes both ECF_NORETURN and ECF_NOTHROW. This happens
> because the function which was being inlined had ECF_NOTHROW while caller
> had ECF_NORETURN. When both of these are set, ignore_stores_p and
> ignore_nondeterminism_p
> return true. But in this case the inner function is still just nothrow
> and not noreturn.
> Basically we only want to combine in the CONST/PURE/NOVOPS related flags
> instead.
>
> Bootstrapped and tested on x86_64-linux-gnu.
>
> PR tree-optimization/120987
>
> gcc/ChangeLog:
>
> * ipa-modref.cc (ipa_merge_modref_summary_after_inlining): Mask
> off non const/pure/novops related flags when combining of outer
> functions.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/torture/pr120987-1.C: New test.
>
> Signed-off-by: Andrew Pinski <[email protected]>
> ---
> gcc/ipa-modref.cc | 11 ++++-
> gcc/testsuite/g++.dg/torture/pr120987-1.C | 57 +++++++++++++++++++++++
> 2 files changed, 66 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/torture/pr120987-1.C
>
> diff --git a/gcc/ipa-modref.cc b/gcc/ipa-modref.cc
> index fc00acecfce..3158e9757fc 100644
> --- a/gcc/ipa-modref.cc
> +++ b/gcc/ipa-modref.cc
> @@ -5345,9 +5345,16 @@ ipa_merge_modref_summary_after_inlining (cgraph_edge
> *edge)
> int flags = flags_from_decl_or_type (edge->callee->decl);
> /* Combine in outer flags. */
> cgraph_node *n;
> + /* Only combine const/pure/novops related flags.
> + NoReturn and NoThrow cannot be combined.
> + An outer noreturn function might throw but an inner might be marked as
> nothrow. */
Also in an inline chain A -> B -> C, B might be marked as noreturn but
that does not mean
A is noreturn. So when working on the last bug in this function I was
very confused (but that
bug turned out to be unrelated, so I didn't dare touch this code).
> + int flags_mask;
> + flags_mask = ECF_CONST | ECF_PURE | ECF_NOVOPS | ECF_LOOPING_CONST_OR_PURE;
> for (n = edge->caller; n->inlined_to; n = n->callers->caller)
> - flags |= flags_from_decl_or_type (n->decl);
> - flags |= flags_from_decl_or_type (n->decl);
> + flags |= flags_from_decl_or_type (n->decl) & flags_mask;
> +
> + /* Combine in the outer most. */
> + flags |= flags_from_decl_or_type (n->decl) & flags_mask;
> bool ignore_stores = ignore_stores_p (edge->caller->decl, flags);
I also noticed we pass edge->caller->decl here (B), not A that B is
inlined into. Though
as we only check flag_exceptions on that decl and inlining should not
happen on a
flag_exceptions change boundary this is probably harmless.
Note we're using 'flags' for many checks in the following code. I
think this is all mostly
"optimistic" merging, aka if a caller was modref analyzed to some ECF_* then the
actual instance of a callee (thus when inlined) has to adhere to that,
otherwise the
modref analysis was wrong. But I do not see how this should allow for
the reverse
merging of callee flags to caller flags. For the ignore_stores_p it
does effectively
merge caller flags to the callee of the inlined edge. In this context
I do not understand:
> + An outer noreturn function might throw but an inner might be marked as
> nothrow. */
If A is noreturn, but not nothrow, B is nothrow, then for the purpose
of inlining B -> C we
are currently treating the caller as noreturn + nothrow which should
be OK for the stores
recorded on 'C'? Not. But then the whole scheme falls apart again.
As said, I'm confused by this code. In my previous attempt I just wiped it all.
Meaning, I'll defer to Honza.
This code needs a big fat comment explaining what it does.
Thanks for trying.
Richard.
>
> if (!callee_info && to_info)
> diff --git a/gcc/testsuite/g++.dg/torture/pr120987-1.C
> b/gcc/testsuite/g++.dg/torture/pr120987-1.C
> new file mode 100644
> index 00000000000..4209679bc03
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/torture/pr120987-1.C
> @@ -0,0 +1,57 @@
> +// { dg-do run { target c++11 } }
> +// { dg-skip-if "requires hosted libstdc++ for string/memory" { ! hostedlib
> } }
> +// PR tree-optimization/120987
> +
> +#include <memory>
> +#include <string>
> +#include <cstdlib>
> +
> +#define ERROR_STRING "012345678901234567"
> +
> +struct gdb_exception
> +{
> + gdb_exception (const char *s)
> + : message (std::make_shared<std::string> (s))
> + {}
> +
> + explicit gdb_exception (gdb_exception &&other) noexcept
> + : message (std::move (other.message))
> + {
> + volatile int a = 1;
> + if (a != 1)
> + abort ();
> + }
> +
> +
> + std::shared_ptr<std::string> message;
> +};
> +
> +void __attribute__((noinline, noclone))
> +throw_exception (gdb_exception &&exception)
> +{
> + throw gdb_exception (std::move (exception));
> +}
> +
> +static void __attribute__((noinline, noclone))
> +parse_linespec (void)
> +{
> + struct gdb_exception file_exception (ERROR_STRING);
> + throw_exception (std::move (file_exception));
> +}
> +
> +int
> +main (void)
> +{
> + try
> + {
> + parse_linespec ();
> + }
> + catch (const gdb_exception &e)
> + {
> + if (*e.message != ERROR_STRING)
> + __builtin_abort();
> + return 0;
> + }
> +
> + __builtin_abort();
> +}
> --
> 2.43.0
>
