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. Thanks, Richard
