On Thu, 4 Mar 2021, Richard Biener wrote: > On Wed, 3 Mar 2021, Richard Biener wrote: > > > On Wed, 3 Mar 2021, David Malcolm wrote: > > > > > On Wed, 2021-03-03 at 08:48 +0100, Richard Biener wrote: > > > > 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. > > > > > > This looks very much like the issue I ran into in > > > c46d057f55748520e819dcd8e04bca71be9902b2 (and, in retrospect, that > > > commit may have just been papering over the problem). > > > > > > The issue there was that pp_printf is not reentrant - if a handler for > > > a pp_printf format code ends up making a nested call to pp_printf, I > > > got behavior that looks like what you're seeing. > > > > > > That said, I've been poring over the output in PR middle-end/97855 and > > > comparing it to the various pretty-printer usage in the tree, and I'm > > > not seeing anywhere where a pp_printf seems to be used when generating: > > > MEM[(struct poly_int *)&save + 8B].D.6750.coeffs[0] > > > > I think it's the D.6750 which is printed via > > > > else if (TREE_CODE (node) == DEBUG_EXPR_DECL) > > { > > if (flags & TDF_NOUID) > > pp_string (pp, "D#xxxx"); > > else > > pp_printf (pp, "D#%i", DEBUG_TEMP_UID (node)); > > > > because this is a DECL_DEBUG_EXPR. One could experiment with > > avoiding pp_printf in dump_decl_name. > > > > > Is there a minimal reproducer (or a .i file?) > > > > No, you need to do a LTO bootstrap, repeat the link step of > > for example cc1 with -v -save-temps and pick an ltrans invocation > > that exhibits the issue ... > > > > I can poke at the above tomorrow again. I suppose we could > > also add some checking-assert into the pp_printf code at > > the problematic place (or is any recursion bogus?) to catch > > the case with an ICE. > > It ICEs _a_ _lot_. > > diff --git a/gcc/pretty-print.c b/gcc/pretty-print.c > index ade1933f86a..7755157a7d7 100644 > --- a/gcc/pretty-print.c > +++ b/gcc/pretty-print.c > @@ -1069,6 +1069,11 @@ static const char *get_end_url_string > (pretty_printer *); > void > pp_format (pretty_printer *pp, text_info *text) > { > + /* pp_format is not reentrant. */ > + static bool in_pp_format; > + gcc_checking_assert (!in_pp_format); > + in_pp_format = true; > + > output_buffer *buffer = pp_buffer (pp); > const char *p; > const char **args; > @@ -1500,6 +1505,8 @@ pp_format (pretty_printer *pp, text_info *text) > buffer->line_length = old_line_length; > pp_wrapping_mode (pp) = old_wrapping_mode; > pp_clear_state (pp); > + > + in_pp_format = false; > } > > /* Format of a message pointed to by TEXT. */ > > testresult summary attached (but it passes bootstrap).
OK, and after the patch and better recursion checking for the above (use pp->in_pp_format) the only ICE that remains is for FAIL: gcc.dg/noncompile/pr79758.c -O0 (test for warnings, line 5) FAIL: gcc.dg/noncompile/pr79758.c -O0 1 blank line(s) in output FAIL: gcc.dg/noncompile/pr79758.c -O0 (internal compiler error) FAIL: gcc.dg/noncompile/pr79758.c -O0 (test for excess errors) /home/rguenther/src/trunk/gcc/testsuite/gcc.dg/noncompile/pr79758.c:4:15: error: 'a' undeclared here (not in a function)^M /home/rguenther/src/trunk/gcc/testsuite/gcc.dg/noncompile/pr79758.c:5:6: error: redefinition of 'fn1'^M 'void(<type-error>^M ^M Internal compiler error: Error reporting routines re-entered.^M 0x807e00 pp_format(pretty_printer*, text_info*)^M /home/rguenther/src/trunk/gcc/pretty-print.c:1073^M 0x19a6556 diagnostic_report_diagnostic(diagnostic_context*, diagnostic_info*)^M /home/rguenther/src/trunk/gcc/diagnostic.c:1244^M 0x19a6ace diagnostic_impl^M /home/rguenther/src/trunk/gcc/diagnostic.c:1406^M 0x19a6e17 internal_error(char const*, ...)^M /home/rguenther/src/trunk/gcc/diagnostic.c:1808^M 0x807315 fancy_abort(char const*, int, char const*)^M /home/rguenther/src/trunk/gcc/diagnostic.c:1907^M 0x807e00 pp_format(pretty_printer*, text_info*)^M /home/rguenther/src/trunk/gcc/pretty-print.c:1073^M 0x19c4ae0 pp_format_verbatim(pretty_printer*, text_info*)^M /home/rguenther/src/trunk/gcc/pretty-print.c:1542^M 0x19c4ae0 pp_verbatim(pretty_printer*, char const*, ...)^M /home/rguenther/src/trunk/gcc/pretty-print.c:1798^M 0x8f5be1 pp_c_parameter_type_list(c_pretty_printer*, tree_node*)^M /home/rguenther/src/trunk/gcc/c-family/c-pretty-print.c:544^M 0x8f7df7 c_pretty_printer::direct_abstract_declarator(tree_node*)^M /home/rguenther/src/trunk/gcc/c-family/c-pretty-print.c:591^M 0x860c62 print_type^M /home/rguenther/src/trunk/gcc/c/c-objc-common.c:198^M 0x860fd5 c_tree_printer^M /home/rguenther/src/trunk/gcc/c/c-objc-common.c:310^M 0x860fd5 c_tree_printer^M /home/rguenther/src/trunk/gcc/c/c-objc-common.c:254^M 0x19c2a6c pp_format(pretty_printer*, text_info*)^M /home/rguenther/src/trunk/gcc/pretty-print.c:1479^M 0x19a6556 diagnostic_report_diagnostic(diagnostic_context*, diagnostic_info*)^M /home/rguenther/src/trunk/gcc/diagnostic.c:1244^M 0x19a9337 diagnostic_impl^M /home/rguenther/src/trunk/gcc/diagnostic.c:1406^M 0x19a9337 inform(unsigned int, char const*, ...)^M /home/rguenther/src/trunk/gcc/diagnostic.c:1485^M 0x816eee diagnose_mismatched_decls^M /home/rguenther/src/trunk/gcc/c/c-decl.c:2282^M 0x818624 duplicate_decls^M /home/rguenther/src/trunk/gcc/c/c-decl.c:2950^M 0x81afe9 pushdecl(tree_node*)^M /home/rguenther/src/trunk/gcc/c/c-decl.c:3143^M Please submit a full bug report,^M with preprocessed source if appropriate.^M Please include the complete backtrace with any bug report.^M See <https://gcc.gnu.org/bugs/> for instructions.^M which happens because #define pp_unsupported_tree(PP, T) \ pp_verbatim (PP, "%qs not supported by %s", \ get_tree_code_name (TREE_CODE (T)), __FUNCTION__) going to fix that. Richard