I have managed to reproduce the issue now, and the patch does appear to fix the ICE.
There is a second issue, which will need more investigation: When compiled with '-flto' the extra internal functions that VTV generates, and which are necessary for it to work correctly, do not get propagated into the final binary. You can see this compiling the bb_tests.cc test case in the libvtv testsuite, and grepping for 'GLOBAL' in the final output: $ /usr/local/google3/cmtice/gcc-fsf.root/usr/local/bin/g++ -o bb_tests bb_tests.cc -O1 -fvtable-verify=std // Compile without flto $ nm bb_tests | grep GLOBAL 0000000000601000 d _GLOBAL_OFFSET_TABLE_ 0000000000400930 t _GLOBAL__sub_I.00099__Z14get_cond_value $ /usr/local/google3/cmtice/gcc-fsf.root/usr/local/bin/g++ -o bb_tests_flto bb_tests.cc -O1 -flto -fvtable-verify=std // Compile with flto $ nm bb_tests_flto | grep GLOBAL 0000000000601000 d _GLOBAL_OFFSET_TABLE_ But as said, that's a separate issue, which I will need to investigate (if anyone has any suggestions as to the proper way to propagate the functions through -flto, I would love to hear them). I approve Richard's patch for fixing the ICE. -- Caroline Tice cmt...@google.com On Wed, Feb 20, 2019 at 6:30 AM Richard Biener <rguent...@suse.de> wrote: > > On Tue, 19 Feb 2019, Caroline Tice wrote: > > > On Tue, Feb 19, 2019 at 1:57 AM Richard Biener <rguent...@suse.de> wrote: > > > > > > > > Looks like vtv_generate_init_routine calls symtab->process_new_functions > > > () which has seriously bad effects on GCCs internal memory state. > > > > > > VTV has zero testsuite coverage it seems and besides its initial > > > commit didn't receive any love. > > > > > > > I am puzzled by these statements, as neither of them is true. VTV does > > have a testsuite in the libvtv directory. Naturally you can only run them > > when vtv is enabled and only from the libvtv build tree, as they will all > > fail if VTV is not enabled. I have fixed various bugs in VTV since the > > initial commit, and I have also approved several patches, especially for > > people migrating it to other architectures, suggesting that interest in it > > is not zero. > > > > > > > > > > > > Can we rip it out please? > > > > > > Is the patch OK? (Pointless) bootstrap and regtest running on > > > x86_64-unknown-linux-gnu. > > > > > > Has anybody "recently" tried to enable the feature and tested the > > > result? > > > > > > > I have not tried building it recently, but will do so. I will review your > > patch to see if it makes sense. I would prefer that VTV not be 'ripped > > out' of GCC, and will do whatever I can, within reason, to try to fix its > > issues. > > Meanwhile the patch passed bootstrap and testing with > --enable-vtable-verify. > > Richard. > > > > > > > > > Thanks, > > > Richard. > > > > > > 2019-02-19 Richard Biener <rguent...@suse.de> > > > > > > PR middle-end/89392 > > > cp/ > > > * vtable-class-hierarchy.c (vtv_generate_init_routine): Do not > > > make symtab process new functions here. > > > > > > Index: gcc/cp/vtable-class-hierarchy.c > > > =================================================================== > > > --- gcc/cp/vtable-class-hierarchy.c (revision 269009) > > > +++ gcc/cp/vtable-class-hierarchy.c (working copy) > > > @@ -1191,8 +1191,6 @@ vtv_generate_init_routine (void) > > > gimplify_function_tree (vtv_fndecl); > > > cgraph_node::add_new_function (vtv_fndecl, false); > > > > > > - symtab->process_new_functions (); > > > - > > > if (flag_vtable_verify == VTV_PREINIT_PRIORITY && !TARGET_PECOFF) > > > assemble_vtv_preinit_initializer (vtv_fndecl); > > > > > > > > > > -- > Richard Biener <rguent...@suse.de> > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB > 21284 (AG Nuernberg)
