On Tue, 2 Mar 2021, Martin Sebor wrote: > On 3/2/21 9:52 AM, Jeff Law via Gcc-patches wrote: > > > > > > On 3/1/21 1:39 AM, Richard Biener wrote: > >> The default diagnostic tree printer relies on dump_generic_node which > >> for some reason manages to clobber the diagnostic pretty-printer state > >> so we see garbled diagnostics like > >> > >> /home/rguenther/src/trunk/gcc/calls.c: In function 'expand_call': > >> D.6750.coeffs[0]'/home/rguenther/src/trunk/gcc/dojump.c:118:28: warning: > >> may be used uninitialized in this function [-Wmaybe-uninitialized] > >> > >> when the diagnostic is emitted by the LTO fronted. The following > >> approach using a temporary pretty-printer for the dump_generic_node > >> call fixes this for some unknown reason and we issue > >> > >> /home/rguenther/src/trunk/gcc/calls.c: In function 'expand_call': > >> /home/rguenther/src/trunk/gcc/dojump.c:118:28: warning: 'MEM[(struct > >> poly_int *)&save].D.6750.coeffs[0]' may be used uninitialized in this > >> function [-Wmaybe-uninitialized] > >> > >> [LTO] Bootstrapped and tested on x86_64-unknown-linux-gnu, OK for trunk? > >> > >> Thanks, > >> Richard. > >> > >> 2021-02-26 Richard Biener <rguent...@suse.de> > >> > >> PR middle-end/97855 > >> * tree-diagnostic.c (default_tree_printer): Use a temporary > >> pretty-printer when formatting a tree via dump_generic_node. > > It'd be good to know why this helps, but I trust your judgment that this > > is an improvement. > > I don't know if it's related but pr98492 tracks a problem in the C++ > front end caused by reinitializing the pretty printer in a number of > functions in cp/error.c. When one of these functions is called while > the pretty printer is formatting something, the effect of > the reinitialization is to drop the already formatted contents > of the printer's buffer. > > IIRC, I tripped over this when working on the MEM_REF formatting > improvement for -Wuninitialized.
I've poked quite a bit with breakpoints on suspicious pretty-printer functions and watch points on the pp state but found nothing in the case I was looking at (curiously also -Wuninitialized). But I also wasn't able to understand why the caller should work at all. And yes, the C/C++ tree printers also simply format to the passed pretty-printer... Hoping that David could shed some light on how this should play together. Most specifically pp_format (context->printer, &diagnostic->message); ^^^ this is the path affected by the patch (*diagnostic_starter (context)) (context, diagnostic); ^^^ this somehow messes things up, it does pp_set_prefix on context->printer but also does some formatting pp_output_formatted_text (context->printer); and now we expect this to magically output the composed pieces. Note swapping the first two lines didn't have any effect (I don't remember if it changed anything so details might have changed but it was definitely still broken). That said, the only hint I have is that the diagnostic plus prefix is quite long, but any problem in the generic code should eventually affect non-LTO as well but the PR is reported for LTO only (bogus diagnostics shown during LTO bootstrap). The patch fixes all bogus diagnostics during LTO bootstrap. I originally thought there's maybe a pp_flush too much but maybe there's a pp_flush missing ... I'll wait for Davids feedback but will eventually install the patch to close the bug. Richard.
