| Message ID | cce982a1fa24eab3a90881ff442115910e2f0c09.1477933704.git.segher@kernel.crashing.org |
|---|---|
| State | New |
| Headers | show |
On 31/10/16 17:15, Segher Boessenkool wrote: > If maybe_record_trace_start fails because the CFI is inconsistent on two > paths into a block it currently just ICEs. This changes it to also dump > the CFI on those two paths in the dump file; debugging it without that > information is hopeless. > > Tested on powerpc64-linux {-m32,-m64}. Is this okay for trunk? +1 from me as a variant of this patch has helped me debug issues with the CFI. Kyrill > > Segher > > > 2016-10-31 Segher Boessenkool <segher@kernel.crashing.org> > > * dwarf2cfi.c (dump_cfi_row): Add forward declaration. > (maybe_record_trace_start): If the CFI is different on the new and > old paths, print out both to the dump file before ICEing. > > --- > gcc/dwarf2cfi.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/gcc/dwarf2cfi.c b/gcc/dwarf2cfi.c > index da9da52..19edc28 100644 > --- a/gcc/dwarf2cfi.c > +++ b/gcc/dwarf2cfi.c > @@ -2239,6 +2239,8 @@ add_cfis_to_fde (void) > } > } > > +static void dump_cfi_row (FILE *f, dw_cfi_row *row); > + > /* If LABEL is the start of a trace, then initialize the state of that > trace from CUR_TRACE and CUR_ROW. */ > > @@ -2282,7 +2284,21 @@ maybe_record_trace_start (rtx_insn *start, rtx_insn *origin) > /* We ought to have the same state incoming to a given trace no > matter how we arrive at the trace. Anything else means we've > got some kind of optimization error. */ > - gcc_checking_assert (cfi_row_equal_p (cur_row, ti->beg_row)); > +#if CHECKING_P > + if (!cfi_row_equal_p (cur_row, ti->beg_row)) > + { > + if (dump_file) > + { > + fprintf (dump_file, "Inconsistent CFI state!\n"); > + fprintf (dump_file, "SHOULD have:\n"); > + dump_cfi_row (dump_file, ti->beg_row); > + fprintf (dump_file, "DO have:\n"); > + dump_cfi_row (dump_file, cur_row); > + } > + > + gcc_unreachable (); > + } > +#endif > > /* The args_size is allowed to conflict if it isn't actually used. */ > if (ti->beg_true_args_size != args_size)
Ping. On Mon, Oct 31, 2016 at 05:15:53PM +0000, Segher Boessenkool wrote: > If maybe_record_trace_start fails because the CFI is inconsistent on two > paths into a block it currently just ICEs. This changes it to also dump > the CFI on those two paths in the dump file; debugging it without that > information is hopeless. > > Tested on powerpc64-linux {-m32,-m64}. Is this okay for trunk? > > > Segher > > > 2016-10-31 Segher Boessenkool <segher@kernel.crashing.org> > > * dwarf2cfi.c (dump_cfi_row): Add forward declaration. > (maybe_record_trace_start): If the CFI is different on the new and > old paths, print out both to the dump file before ICEing. > > --- > gcc/dwarf2cfi.c | 18 +++++++++++++++++- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/gcc/dwarf2cfi.c b/gcc/dwarf2cfi.c > index da9da52..19edc28 100644 > --- a/gcc/dwarf2cfi.c > +++ b/gcc/dwarf2cfi.c > @@ -2239,6 +2239,8 @@ add_cfis_to_fde (void) > } > } > > +static void dump_cfi_row (FILE *f, dw_cfi_row *row); > + > /* If LABEL is the start of a trace, then initialize the state of that > trace from CUR_TRACE and CUR_ROW. */ > > @@ -2282,7 +2284,21 @@ maybe_record_trace_start (rtx_insn *start, rtx_insn *origin) > /* We ought to have the same state incoming to a given trace no > matter how we arrive at the trace. Anything else means we've > got some kind of optimization error. */ > - gcc_checking_assert (cfi_row_equal_p (cur_row, ti->beg_row)); > +#if CHECKING_P > + if (!cfi_row_equal_p (cur_row, ti->beg_row)) > + { > + if (dump_file) > + { > + fprintf (dump_file, "Inconsistent CFI state!\n"); > + fprintf (dump_file, "SHOULD have:\n"); > + dump_cfi_row (dump_file, ti->beg_row); > + fprintf (dump_file, "DO have:\n"); > + dump_cfi_row (dump_file, cur_row); > + } > + > + gcc_unreachable (); > + } > +#endif > > /* The args_size is allowed to conflict if it isn't actually used. */ > if (ti->beg_true_args_size != args_size) > -- > 1.9.3
On Thu, Nov 10, 2016 at 11:29:12AM -0600, Segher Boessenkool wrote: > Ping. > > On Mon, Oct 31, 2016 at 05:15:53PM +0000, Segher Boessenkool wrote: > > If maybe_record_trace_start fails because the CFI is inconsistent on two > > paths into a block it currently just ICEs. This changes it to also dump > > the CFI on those two paths in the dump file; debugging it without that > > information is hopeless. > > > > Tested on powerpc64-linux {-m32,-m64}. Is this okay for trunk? > > > > > > Segher > > > > > > 2016-10-31 Segher Boessenkool <segher@kernel.crashing.org> > > > > * dwarf2cfi.c (dump_cfi_row): Add forward declaration. > > (maybe_record_trace_start): If the CFI is different on the new and > > old paths, print out both to the dump file before ICEing. Ok, thanks. Jakub
diff --git a/gcc/dwarf2cfi.c b/gcc/dwarf2cfi.c index da9da52..19edc28 100644 --- a/gcc/dwarf2cfi.c +++ b/gcc/dwarf2cfi.c @@ -2239,6 +2239,8 @@ add_cfis_to_fde (void) } } +static void dump_cfi_row (FILE *f, dw_cfi_row *row); + /* If LABEL is the start of a trace, then initialize the state of that trace from CUR_TRACE and CUR_ROW. */ @@ -2282,7 +2284,21 @@ maybe_record_trace_start (rtx_insn *start, rtx_insn *origin) /* We ought to have the same state incoming to a given trace no matter how we arrive at the trace. Anything else means we've got some kind of optimization error. */ - gcc_checking_assert (cfi_row_equal_p (cur_row, ti->beg_row)); +#if CHECKING_P + if (!cfi_row_equal_p (cur_row, ti->beg_row)) + { + if (dump_file) + { + fprintf (dump_file, "Inconsistent CFI state!\n"); + fprintf (dump_file, "SHOULD have:\n"); + dump_cfi_row (dump_file, ti->beg_row); + fprintf (dump_file, "DO have:\n"); + dump_cfi_row (dump_file, cur_row); + } + + gcc_unreachable (); + } +#endif /* The args_size is allowed to conflict if it isn't actually used. */ if (ti->beg_true_args_size != args_size)
If maybe_record_trace_start fails because the CFI is inconsistent on two paths into a block it currently just ICEs. This changes it to also dump the CFI on those two paths in the dump file; debugging it without that information is hopeless. Tested on powerpc64-linux {-m32,-m64}. Is this okay for trunk? Segher 2016-10-31 Segher Boessenkool <segher@kernel.crashing.org> * dwarf2cfi.c (dump_cfi_row): Add forward declaration. (maybe_record_trace_start): If the CFI is different on the new and old paths, print out both to the dump file before ICEing. --- gcc/dwarf2cfi.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-)