Hi: The fixits-pr84852-1.c fails on mips64el target.
FAIL: gcc.dg/fixits-pr84852-1.c (test for excess errors) FAIL: gcc.dg/fixits-pr84852-1.c dg-regexp 25 not found: ".*fixits-pr84852.c:-812156810:25:" see this patch: diff --git a/gcc/testsuite/gcc.dg/fixits-pr84852-1.c b/gcc/testsuite/gcc.dg/fixits-pr84852-1.c index ed88434..98087ab 100644 --- a/gcc/testsuite/gcc.dg/fixits-pr84852-1.c +++ b/gcc/testsuite/gcc.dg/fixits-pr84852-1.c @@ -22,4 +22,4 @@ int foo (void) { return strlen(""); } #endif /* We need this, to consume a stray line marker for the bogus line. */ -/* { dg-regexp ".*fixits-pr84852.c:-812156810:25:" } */ +/* { dg-regexp ".*fixits-pr84852-1.c:-812156810:25:" } */ Thanks. On Wed, Mar 14, 2018 at 10:10 PM, David Malcolm <dmalc...@redhat.com> wrote: > PR c/84852 reports an ICE inside diagnostic_show_locus when printing > a diagnostic for a source file with a #line >= 2^31: > > #line 7777777777 > int foo (void) { return strlen(""); } > > where we're attempting to print a fix-it hint at the top of the file > and underline the "strlen" (two "line spans"). > > The > #line 7777777777 > won't fix within the 32-bit linenum_type, and is truncated from > 0x1cf977871 > to > 0xcf977871 > i.e. 3482810481 in decimal. > > Such a #line is reported by -pedantic and -pedantic-errors, but we > shouldn't ICE. > > The ICE is an assertion failure within layout::calculate_line_spans, > where the line spans have not been properly sorted. > > The layout_ranges are stored as int, rather than linenum_type, > giving line -812156815 for the error, and line 1 for the fix-it hint. > > However, line_span uses linenum_type rather than int. > > line_span::comparator compares these values as int, and hence > decides that (linenum_type)3482810481 aka (int)-812156815 is less > than line 1. > > This leads to this assertion failing in layout::calculate_line_spans: > > 1105 gcc_assert (next->m_first_line >= current->m_first_line); > > since it isn't the case that 1 >= 3482810481. > > The underlying problem is the mix of types for storing line numbers: > in parts of libcpp and diagnostic-show-locus.c we use linenum_type; > in other places (including libcpp's expanded_location) we use int. > > I looked at using linenum_type throughout, but doing so turned into > a large patch, so this patch fixes the ICE in a less invasive way > by merely using linenum_type more consistently just within > diagnostic-show-locus.c, and fixing line_span::comparator to properly > handle line numbers (and line number differences) >= 2^31, by using > a new helper function for linenum_type differences, computing the > difference using long long, and using the sign of the difference > (as the difference might not fit in the "int" return type imposed > by qsort). > > (The new testcases assume the host's "unsigned int" is 32 bits; is > there anything we support where that isn't the case?) > > I can self-approve the libcpp, diagnostic-show-locus.c and input.c > changes. > > As part of the selftests, I needed to add ASSERT_GT and ASSERT_LT > to selftest.h; I'm treating those parts of the patch as "obvious". > > Successfully bootstrapped and regression-tested on x86_64-pc-linux-gnu; > adds 14 PASS results to gcc.sum. > > Committed to trunk as r258526. > > gcc/ChangeLog: > PR c/84852 > * diagnostic-show-locus.c (class layout_point): Convert m_line > from int to linenum_type. > (line_span::comparator): Use linenum "compare" function when > comparing line numbers. > (test_line_span): New function. > (layout_range::contains_point): Convert param "row" from int to > linenum_type. > (layout_range::intersects_line_p): Likewise. > (layout::will_show_line_p): Likewise. > (layout::print_source_line): Likewise. > (layout::should_print_annotation_line_p): Likewise. > (layout::print_annotation_line): Likewise. > (layout::print_leading_fixits): Likewise. > (layout::annotation_line_showed_range_p): Likewise. > (struct line_corrections): Likewise for field m_row. > (line_corrections::line_corrections): Likewise for param "row". > (layout::print_trailing_fixits): Likewise. > (layout::get_state_at_point): Likewise. > (layout::get_x_bound_for_row): Likewise. > (layout::print_line): Likewise. > (diagnostic_show_locus): Likewise for locals "last_line" and > "row". > (selftest::diagnostic_show_locus_c_tests): Call test_line_span. > * input.c (selftest::test_linenum_comparisons): New function. > (selftest::input_c_tests): Call it. > * selftest.c (selftest::test_assertions): Test ASSERT_GT, > ASSERT_GT_AT, ASSERT_LT, and ASSERT_LT_AT. > * selftest.h (ASSERT_GT): New macro. > (ASSERT_GT_AT): New macro. > (ASSERT_LT): New macro. > (ASSERT_LT_AT): New macro. > > gcc/testsuite/ChangeLog: > PR c/84852 > * gcc.dg/fixits-pr84852-1.c: New test. > * gcc.dg/fixits-pr84852-2.c: New test. > > libcpp/ChangeLog: > * include/line-map.h (compare): New function on linenum_type. > --- > gcc/diagnostic-show-locus.c | 105 > ++++++++++++++++++++++---------- > gcc/input.c | 16 +++++ > gcc/selftest.c | 4 ++ > gcc/selftest.h | 38 ++++++++++++ > gcc/testsuite/gcc.dg/fixits-pr84852-1.c | 25 ++++++++ > gcc/testsuite/gcc.dg/fixits-pr84852-2.c | 25 ++++++++ > libcpp/include/line-map.h | 12 ++++ > 7 files changed, 192 insertions(+), 33 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/fixits-pr84852-1.c > create mode 100644 gcc/testsuite/gcc.dg/fixits-pr84852-2.c > > diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c > index 5eee3cc..bdf608a 100644 > --- a/gcc/diagnostic-show-locus.c > +++ b/gcc/diagnostic-show-locus.c > @@ -115,7 +115,7 @@ class layout_point > : m_line (exploc.line), > m_column (exploc.column) {} > > - int m_line; > + linenum_type m_line; > int m_column; > }; > > @@ -129,8 +129,8 @@ class layout_range > bool show_caret_p, > const expanded_location *caret_exploc); > > - bool contains_point (int row, int column) const; > - bool intersects_line_p (int row) const; > + bool contains_point (linenum_type row, int column) const; > + bool intersects_line_p (linenum_type row) const; > > layout_point m_start; > layout_point m_finish; > @@ -172,16 +172,52 @@ struct line_span > { > const line_span *ls1 = (const line_span *)p1; > const line_span *ls2 = (const line_span *)p2; > - int first_line_diff = (int)ls1->m_first_line - (int)ls2->m_first_line; > - if (first_line_diff) > - return first_line_diff; > - return (int)ls1->m_last_line - (int)ls2->m_last_line; > + int first_line_cmp = compare (ls1->m_first_line, ls2->m_first_line); > + if (first_line_cmp) > + return first_line_cmp; > + return compare (ls1->m_last_line, ls2->m_last_line); > } > > linenum_type m_first_line; > linenum_type m_last_line; > }; > > +#if CHECKING_P > + > +/* Selftests for line_span. */ > + > +static void > +test_line_span () > +{ > + line_span line_one (1, 1); > + ASSERT_EQ (1, line_one.get_first_line ()); > + ASSERT_EQ (1, line_one.get_last_line ()); > + ASSERT_FALSE (line_one.contains_line_p (0)); > + ASSERT_TRUE (line_one.contains_line_p (1)); > + ASSERT_FALSE (line_one.contains_line_p (2)); > + > + line_span lines_1_to_3 (1, 3); > + ASSERT_EQ (1, lines_1_to_3.get_first_line ()); > + ASSERT_EQ (3, lines_1_to_3.get_last_line ()); > + ASSERT_TRUE (lines_1_to_3.contains_line_p (1)); > + ASSERT_TRUE (lines_1_to_3.contains_line_p (3)); > + > + ASSERT_EQ (0, line_span::comparator (&line_one, &line_one)); > + ASSERT_GT (line_span::comparator (&lines_1_to_3, &line_one), 0); > + ASSERT_LT (line_span::comparator (&line_one, &lines_1_to_3), 0); > + > + /* A linenum > 2^31. */ > + const linenum_type LARGEST_LINE = 0xffffffff; > + line_span largest_line (LARGEST_LINE, LARGEST_LINE); > + ASSERT_EQ (LARGEST_LINE, largest_line.get_first_line ()); > + ASSERT_EQ (LARGEST_LINE, largest_line.get_last_line ()); > + > + ASSERT_GT (line_span::comparator (&largest_line, &line_one), 0); > + ASSERT_LT (line_span::comparator (&line_one, &largest_line), 0); > +} > + > +#endif /* #if CHECKING_P */ > + > /* A class to control the overall layout when printing a diagnostic. > > The layout is determined within the constructor. > @@ -207,18 +243,18 @@ class layout > > expanded_location get_expanded_location (const line_span *) const; > > - void print_line (int row); > + void print_line (linenum_type row); > > private: > - bool will_show_line_p (int row) const; > - void print_leading_fixits (int row); > - void print_source_line (int row, const char *line, int line_width, > + bool will_show_line_p (linenum_type row) const; > + void print_leading_fixits (linenum_type row); > + void print_source_line (linenum_type row, const char *line, int line_width, > line_bounds *lbounds_out); > - bool should_print_annotation_line_p (int row) const; > - void print_annotation_line (int row, const line_bounds lbounds); > - void print_trailing_fixits (int row); > + bool should_print_annotation_line_p (linenum_type row) const; > + void print_annotation_line (linenum_type row, const line_bounds lbounds); > + void print_trailing_fixits (linenum_type row); > > - bool annotation_line_showed_range_p (int line, int start_column, > + bool annotation_line_showed_range_p (linenum_type line, int start_column, > int finish_column) const; > void show_ruler (int max_column) const; > > @@ -230,13 +266,13 @@ class layout > > bool > get_state_at_point (/* Inputs. */ > - int row, int column, > + linenum_type row, int column, > int first_non_ws, int last_non_ws, > /* Outputs. */ > point_state *out_state); > > int > - get_x_bound_for_row (int row, int caret_column, > + get_x_bound_for_row (linenum_type row, int caret_column, > int last_non_ws); > > void > @@ -417,7 +453,7 @@ layout_range::layout_range (const expanded_location > *start_exploc, > - 'a' indicates a subsequent point *after* the range. */ > > bool > -layout_range::contains_point (int row, int column) const > +layout_range::contains_point (linenum_type row, int column) const > { > gcc_assert (m_start.m_line <= m_finish.m_line); > /* ...but the equivalent isn't true for the columns; > @@ -478,7 +514,7 @@ layout_range::contains_point (int row, int column) const > /* Does this layout_range contain any part of line ROW? */ > > bool > -layout_range::intersects_line_p (int row) const > +layout_range::intersects_line_p (linenum_type row) const > { > gcc_assert (m_start.m_line <= m_finish.m_line); > if (row < m_start.m_line) > @@ -940,7 +976,7 @@ layout::maybe_add_location_range (const location_range > *loc_range, > /* Return true iff ROW is within one of the line spans for this layout. */ > > bool > -layout::will_show_line_p (int row) const > +layout::will_show_line_p (linenum_type row) const > { > for (int line_span_idx = 0; line_span_idx < get_num_line_spans (); > line_span_idx++) > @@ -1138,7 +1174,7 @@ layout::calculate_line_spans () > is its width. */ > > void > -layout::print_source_line (int row, const char *line, int line_width, > +layout::print_source_line (linenum_type row, const char *line, int > line_width, > line_bounds *lbounds_out) > { > m_colorizer.set_normal_text (); > @@ -1201,7 +1237,7 @@ layout::print_source_line (int row, const char *line, > int line_width, > i.e. if any of m_layout_ranges contains ROW. */ > > bool > -layout::should_print_annotation_line_p (int row) const > +layout::should_print_annotation_line_p (linenum_type row) const > { > layout_range *range; > int i; > @@ -1215,7 +1251,7 @@ layout::should_print_annotation_line_p (int row) const > source line. */ > > void > -layout::print_annotation_line (int row, const line_bounds lbounds) > +layout::print_annotation_line (linenum_type row, const line_bounds lbounds) > { > int x_bound = get_x_bound_for_row (row, m_exploc.column, > lbounds.m_last_non_ws); > @@ -1263,7 +1299,7 @@ layout::print_annotation_line (int row, const > line_bounds lbounds) > itself, with a leading '+'. */ > > void > -layout::print_leading_fixits (int row) > +layout::print_leading_fixits (linenum_type row) > { > for (unsigned int i = 0; i < m_fixit_hints.length (); i++) > { > @@ -1301,7 +1337,7 @@ layout::print_leading_fixits (int row) > the exact range from START_COLUMN to FINISH_COLUMN. */ > > bool > -layout::annotation_line_showed_range_p (int line, int start_column, > +layout::annotation_line_showed_range_p (linenum_type line, int start_column, > int finish_column) const > { > layout_range *range; > @@ -1552,7 +1588,7 @@ correction::ensure_terminated () > > struct line_corrections > { > - line_corrections (const char *filename, int row) > + line_corrections (const char *filename, linenum_type row) > : m_filename (filename), m_row (row) > {} > ~line_corrections (); > @@ -1560,7 +1596,7 @@ struct line_corrections > void add_hint (const fixit_hint *hint); > > const char *m_filename; > - int m_row; > + linenum_type m_row; > auto_vec <correction *> m_corrections; > }; > > @@ -1674,7 +1710,7 @@ line_corrections::add_hint (const fixit_hint *hint) > in layout::print_leading_fixits. */ > > void > -layout::print_trailing_fixits (int row) > +layout::print_trailing_fixits (linenum_type row) > { > /* Build a list of correction instances for the line, > potentially consolidating hints (for the sake of readability). */ > @@ -1761,7 +1797,7 @@ layout::print_newline () > > bool > layout::get_state_at_point (/* Inputs. */ > - int row, int column, > + linenum_type row, int column, > int first_non_ws, int last_non_ws, > /* Outputs. */ > point_state *out_state) > @@ -1806,7 +1842,7 @@ layout::get_state_at_point (/* Inputs. */ > character of source (as determined when printing the source line). */ > > int > -layout::get_x_bound_for_row (int row, int caret_column, > +layout::get_x_bound_for_row (linenum_type row, int caret_column, > int last_non_ws_column) > { > int result = caret_column + 1; > @@ -1897,7 +1933,7 @@ layout::show_ruler (int max_column) const > consisting of any caret/underlines, then any fixits. > If the source line can't be read, print nothing. */ > void > -layout::print_line (int row) > +layout::print_line (linenum_type row) > { > int line_width; > const char *line = location_get_source_line (m_exploc.file, row, > @@ -1977,8 +2013,9 @@ diagnostic_show_locus (diagnostic_context * context, > expanded_location exploc = layout.get_expanded_location (line_span); > context->start_span (context, exploc); > } > - int last_line = line_span->get_last_line (); > - for (int row = line_span->get_first_line (); row <= last_line; row++) > + linenum_type last_line = line_span->get_last_line (); > + for (linenum_type row = line_span->get_first_line (); > + row <= last_line; row++) > layout.print_line (row); > } > > @@ -3129,6 +3166,8 @@ test_fixit_deletion_affecting_newline (const > line_table_case &case_) > void > diagnostic_show_locus_c_tests () > { > + test_line_span (); > + > test_layout_range_for_single_point (); > test_layout_range_for_single_line (); > test_layout_range_for_multiple_lines (); > diff --git a/gcc/input.c b/gcc/input.c > index 081e785..b667576 100644 > --- a/gcc/input.c > +++ b/gcc/input.c > @@ -1595,6 +1595,21 @@ get_num_source_ranges_for_substring (cpp_reader *pfile, > > /* Selftests of location handling. */ > > +/* Verify that compare() on linenum_type handles comparisons over the full > + range of the type. */ > + > +static void > +test_linenum_comparisons () > +{ > + linenum_type min_line (0); > + linenum_type max_line (0xffffffff); > + ASSERT_EQ (0, compare (min_line, min_line)); > + ASSERT_EQ (0, compare (max_line, max_line)); > + > + ASSERT_GT (compare (max_line, min_line), 0); > + ASSERT_LT (compare (min_line, max_line), 0); > +} > + > /* Helper function for verifying location data: when location_t > values are > LINE_MAP_MAX_LOCATION_WITH_COLS, they are treated > as having column 0. */ > @@ -3528,6 +3543,7 @@ for_each_line_table_case (void (*testcase) (const > line_table_case &)) > void > input_c_tests () > { > + test_linenum_comparisons (); > test_should_have_column_data_p (); > test_unknown_location (); > test_builtins (); > diff --git a/gcc/selftest.c b/gcc/selftest.c > index 0f6c5e4..5709110 100644 > --- a/gcc/selftest.c > +++ b/gcc/selftest.c > @@ -288,6 +288,10 @@ test_assertions () > ASSERT_EQ (1, 1); > ASSERT_EQ_AT (SELFTEST_LOCATION, 1, 1); > ASSERT_NE (1, 2); > + ASSERT_GT (2, 1); > + ASSERT_GT_AT (SELFTEST_LOCATION, 2, 1); > + ASSERT_LT (1, 2); > + ASSERT_LT_AT (SELFTEST_LOCATION, 1, 2); > ASSERT_STREQ ("test", "test"); > ASSERT_STREQ_AT (SELFTEST_LOCATION, "test", "test"); > ASSERT_STR_CONTAINS ("foo bar baz", "bar"); > diff --git a/gcc/selftest.h b/gcc/selftest.h > index 67d55eb..e3117c6 100644 > --- a/gcc/selftest.h > +++ b/gcc/selftest.h > @@ -333,6 +333,44 @@ extern int num_passes; > ::selftest::fail ((LOC), desc); \ > SELFTEST_END_STMT > > +/* Evaluate LHS and RHS and compare them with >, calling > + ::selftest::pass if LHS > RHS, > + ::selftest::fail otherwise. */ > + > +#define ASSERT_GT(LHS, RHS) \ > + ASSERT_GT_AT ((SELFTEST_LOCATION), (LHS), (RHS)) > + > +/* Like ASSERT_GT, but treat LOC as the effective location of the > + selftest. */ > + > +#define ASSERT_GT_AT(LOC, LHS, RHS) \ > + SELFTEST_BEGIN_STMT \ > + const char *desc_ = "ASSERT_GT (" #LHS ", " #RHS ")"; \ > + if ((LHS) > (RHS)) \ > + ::selftest::pass ((LOC), desc_); \ > + else \ > + ::selftest::fail ((LOC), desc_); \ > + SELFTEST_END_STMT > + > +/* Evaluate LHS and RHS and compare them with <, calling > + ::selftest::pass if LHS < RHS, > + ::selftest::fail otherwise. */ > + > +#define ASSERT_LT(LHS, RHS) \ > + ASSERT_LT_AT ((SELFTEST_LOCATION), (LHS), (RHS)) > + > +/* Like ASSERT_LT, but treat LOC as the effective location of the > + selftest. */ > + > +#define ASSERT_LT_AT(LOC, LHS, RHS) \ > + SELFTEST_BEGIN_STMT \ > + const char *desc_ = "ASSERT_LT (" #LHS ", " #RHS ")"; \ > + if ((LHS) < (RHS)) \ > + ::selftest::pass ((LOC), desc_); \ > + else \ > + ::selftest::fail ((LOC), desc_); \ > + SELFTEST_END_STMT > + > /* Evaluate EXPECTED and ACTUAL and compare them with strcmp, calling > ::selftest::pass if they are equal, > ::selftest::fail if they are non-equal. */ > diff --git a/gcc/testsuite/gcc.dg/fixits-pr84852-1.c > b/gcc/testsuite/gcc.dg/fixits-pr84852-1.c > new file mode 100644 > index 0000000..ed88434 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/fixits-pr84852-1.c > @@ -0,0 +1,25 @@ > +/* This is padding (to avoid the output containing DejaGnu directives). */ > + > +/* We need -fdiagnostics-show-caret to trigger the ICE. */ > + > +/* { dg-options "-fdiagnostics-show-caret -pedantic-errors > -Wno-implicit-function-declaration" } */ > + > +#line 3482810481 /* { dg-error "line number out of range" } */ > +/* { dg-begin-multiline-output "" } > + #line 3482810481 > + ^~~~~~~~~~ > + { dg-end-multiline-output "" } */ > + > +int foo (void) { return strlen(""); } > + > +/* { dg-warning "incompatible implicit declaration of built-in function > 'strlen'" "" { target *-*-* } -812156810 } */ > +/* { dg-message "include '<string.h>' or provide a declaration of 'strlen'" > "" { target *-*-* } -812156810 } */ > +#if 0 > +{ dg-begin-multiline-output "" } > ++#include <string.h> > + /* This is padding (to avoid the output containing DejaGnu directives). */ > +{ dg-end-multiline-output "" } > +#endif > + > +/* We need this, to consume a stray line marker for the bogus line. */ > +/* { dg-regexp ".*fixits-pr84852.c:-812156810:25:" } */ > diff --git a/gcc/testsuite/gcc.dg/fixits-pr84852-2.c > b/gcc/testsuite/gcc.dg/fixits-pr84852-2.c > new file mode 100644 > index 0000000..0674ef5 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/fixits-pr84852-2.c > @@ -0,0 +1,25 @@ > +/* This is padding (to avoid the output containing DejaGnu directives). */ > + > +/* We need -fdiagnostics-show-caret to trigger the ICE. */ > + > +/* { dg-options "-fdiagnostics-show-caret -pedantic-errors > -Wno-implicit-function-declaration" } */ > + > +#line 7777777777 /* { dg-error "line number out of range" } */ > +/* { dg-begin-multiline-output "" } > + #line 7777777777 > + ^~~~~~~~~~ > + { dg-end-multiline-output "" } */ > + > +int foo (void) { return strlen(""); } > + > +/* { dg-warning "incompatible implicit declaration of built-in function > 'strlen'" "" { target *-*-* } -812156810 } */ > +/* { dg-message "include '<string.h>' or provide a declaration of 'strlen'" > "" { target *-*-* } -812156810 } */ > +#if 0 > +{ dg-begin-multiline-output "" } > ++#include <string.h> > + /* This is padding (to avoid the output containing DejaGnu directives). */ > +{ dg-end-multiline-output "" } > +#endif > + > +/* We need this, to consume a stray line marker for the bogus line. */ > +/* { dg-regexp ".*fixits-pr84852-2.c:-812156810:25:" } */ > diff --git a/libcpp/include/line-map.h b/libcpp/include/line-map.h > index f6242fc..d6cf816 100644 > --- a/libcpp/include/line-map.h > +++ b/libcpp/include/line-map.h > @@ -49,6 +49,18 @@ along with this program; see the file COPYING3. If not see > /* The type of line numbers. */ > typedef unsigned int linenum_type; > > +/* A function for for use by qsort for comparing line numbers. */ > + > +inline int compare (linenum_type lhs, linenum_type rhs) > +{ > + /* Avoid truncation issues by using long long for the comparison, > + and only consider the sign of the result. */ > + long long diff = (long long)lhs - (long long)rhs; > + if (diff) > + return diff > 0 ? 1 : -1; > + return 0; > +} > + > /* Reason for creating a new line map with linemap_add. LC_ENTER is > when including a new file, e.g. a #include directive in C. > LC_LEAVE is when reaching a file's end. LC_RENAME is when a file > -- > 1.8.5.3 >