Jan Hubicka <hubi...@ucw.cz> writes:
>> Jan Hubicka <hubi...@ucw.cz> writes:
>> >> The testcase has failed since r9-5035, because obj_type_ref_class
>> >> tries to look up an ODR type when no ODR type information is
>> >> available. (The information was available earlier in the
>> >> compilation, but was freed during pass_ipa_free_lang_data.)
>> >> We then crash dereferencing the null get_odr_type result.
>> >>
>> >> The test passes with -O2. However, it fails again if -fdump-tree-all
>> >> is used, since obj_type_ref_class is called indirectly from the
>> >> dump routines.
>> >>
>> >> Other code seems to create ODR type entries on the fly by passing
>> >> "true" as the insert parameter. But since obj_type_ref_class is
>> >> used by dump routines, I guess it should have no side-effects and
>> >> should simply punt in this case.
>> > Thanks for looking into this!
>> >
>> > The ODR type hash is indeed supposed to be populated lazilly except for
>> > LTO when we populate it at stream in time. I think just making function
>> > return NULL in case it should not is fragile, because we may hit missed
>> > optimizations and other surprised elsewhere.
>> >
>> > What about adding new "for_dump" parameter set implicitly to false that
>> > will make the function return ret in case get_odr_type is NULL?
>> > For dumping it probably does not matter what ODR variant you see, since
>> > we end up with a type name in the dump anyway.
>>
>> I think there are two problematic cases (at least as far as this
>> testcase goes). One is the direct call to obj_type_ref_class in
>> tree-pretty-print.c. The other is an indirect call via
>> virtual_method_call_p, which is also called by tree-pretty-print.c.
>> So I guess both of them would need a "for_dump" parameter.
>>
>> TBH doing it that way feels fragile too. Even if we get it right now,
>> it's going to be hard to ensure that all indirect calls to obj_type_ref_class
>> in future make the correct choice between lazy initialisation and remaining
>> side-effect free.
>
> Yep, it is not very pretty :(. In general the output of optimizations
> does not depend much on what is inserted into the type inheritance graph
> (because we must assume that types are derived in other units anyway)
> except for anonymous types. However speculative devirt is an exception
> here, so I guess we want to go woth for_dump markers (it is useful to
> have obj type ref pretty printed in a meaningful way).
OK, this patch (belatedly) does that.
Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK for trunk and
GCC 9+?
Thanks,
Richard
gcc/
PR middle-end/95114
* tree.h (virtual_method_call_p): Add a default-false parameter
that indicates whether the function is being called from dump
routines.
(obj_type_ref_class): Likewise.
* tree.c (virtual_method_call_p): Likewise.
* ipa-devirt.c (obj_type_ref_class): Likewise. Lazily add ODR
type information for the type when the parameter is false.
* tree-pretty-print.c (dump_generic_node): Update calls to
virtual_method_call_p and obj_type_ref_class accordingly.
gcc/testsuite/
PR middle-end/95114
* g++.target/aarch64/pr95114.C: New test.
---
gcc/ipa-devirt.c | 9 ++++++---
gcc/testsuite/g++.target/aarch64/pr95114.C | 3 +++
gcc/tree-pretty-print.c | 5 +++--
gcc/tree.c | 7 ++++---
gcc/tree.h | 4 ++--
5 files changed, 18 insertions(+), 10 deletions(-)
create mode 100644 gcc/testsuite/g++.target/aarch64/pr95114.C
diff --git a/gcc/tree.h b/gcc/tree.h
index cf546ed9491..866d9ba8fbc 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -5241,8 +5241,8 @@ extern location_t *block_nonartificial_location (tree);
extern location_t tree_nonartificial_location (tree);
extern tree block_ultimate_origin (const_tree);
extern tree get_binfo_at_offset (tree, poly_int64, tree);
-extern bool virtual_method_call_p (const_tree);
-extern tree obj_type_ref_class (const_tree ref);
+extern bool virtual_method_call_p (const_tree, bool = false);
+extern tree obj_type_ref_class (const_tree ref, bool = false);
extern bool types_same_for_odr (const_tree type1, const_tree type2);
extern bool contains_bitfld_component_ref_p (const_tree);
extern bool block_may_fallthru (const_tree);
diff --git a/gcc/tree.c b/gcc/tree.c
index 342da55bba7..3d9968fd7a0 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -12810,10 +12810,11 @@ lhd_gcc_personality (void)
OBJ_TYPE_REF representing an virtual call of C++ method.
(As opposed to OBJ_TYPE_REF representing objc calls
through a cast where middle-end devirtualization machinery
- can't apply.) */
+ can't apply.) FOR_DUMP_P is true when being called from
+ the dump routines. */
bool
-virtual_method_call_p (const_tree target)
+virtual_method_call_p (const_tree target, bool for_dump_p)
{
if (TREE_CODE (target) != OBJ_TYPE_REF)
return false;
@@ -12826,7 +12827,7 @@ virtual_method_call_p (const_tree target)
/* If we do not have BINFO associated, it means that type was built
without devirtualization enabled. Do not consider this a virtual
call. */
- if (!TYPE_BINFO (obj_type_ref_class (target)))
+ if (!TYPE_BINFO (obj_type_ref_class (target, for_dump_p)))
return false;
return true;
}
diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
index 0340decba9b..3ab7049734f 100644
--- a/gcc/ipa-devirt.c
+++ b/gcc/ipa-devirt.c
@@ -1883,10 +1883,11 @@ add_type_duplicate (odr_type val, tree type)
return build_bases;
}
-/* REF is OBJ_TYPE_REF, return the class the ref corresponds to. */
+/* REF is OBJ_TYPE_REF, return the class the ref corresponds to.
+ FOR_DUMP_P is true when being called from the dump routines. */
tree
-obj_type_ref_class (const_tree ref)
+obj_type_ref_class (const_tree ref, bool for_dump_p)
{
gcc_checking_assert (TREE_CODE (ref) == OBJ_TYPE_REF);
ref = TREE_TYPE (ref);
@@ -1902,8 +1903,10 @@ obj_type_ref_class (const_tree ref)
tree ret = TREE_TYPE (ref);
if (!in_lto_p && !TYPE_STRUCTURAL_EQUALITY_P (ret))
ret = TYPE_CANONICAL (ret);
+ else if (odr_type ot = get_odr_type (ret, !for_dump_p))
+ ret = ot->type;
else
- ret = get_odr_type (ret)->type;
+ gcc_assert (for_dump_p);
return ret;
}
diff --git a/gcc/tree-pretty-print.c b/gcc/tree-pretty-print.c
index 4f50f37b0da..be1ed906c1d 100644
--- a/gcc/tree-pretty-print.c
+++ b/gcc/tree-pretty-print.c
@@ -3167,10 +3167,11 @@ dump_generic_node (pretty_printer *pp, tree node, int
spc, dump_flags_t flags,
libstdc++-prettyprinters/shared_ptr.cc with and without -g,
for example, at occurrences of OBJ_TYPE_REF. */
if (!(flags & (TDF_SLIM | TDF_COMPARE_DEBUG))
- && virtual_method_call_p (node))
+ && virtual_method_call_p (node, true))
{
pp_string (pp, "(");
- dump_generic_node (pp, obj_type_ref_class (node), spc, flags, false);
+ dump_generic_node (pp, obj_type_ref_class (node, true),
+ spc, flags, false);
pp_string (pp, ")");
}
dump_generic_node (pp, OBJ_TYPE_REF_OBJECT (node), spc, flags, false);
diff --git a/gcc/testsuite/g++.target/aarch64/pr95114.C
b/gcc/testsuite/g++.target/aarch64/pr95114.C
new file mode 100644
index 00000000000..1689159e47c
--- /dev/null
+++ b/gcc/testsuite/g++.target/aarch64/pr95114.C
@@ -0,0 +1,3 @@
+template<typename T> struct foo { virtual void f() = 0; };
+extern foo<__Int8x8_t> &x;
+void f() { x.f(); }
