From 10fcc1429cfad8b453712fe55319807769fa6370 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Thu, 16 Aug 2018 17:07:15 +0000 Subject: [PATCH] -Wmisleading-indentation: fix ICE in get_visual_column (PR c++/70693) PR c++/70693 reports a crash within -Wmisleading-indentation in get_visual_column, reading past the end of a source line. The issue occurs due to a stray carriage return aka '\r' aka ^M, occurring towards the end of line 35 of attachment 38289 - but not at the end itself. This carriage return confuses our line numbering: from that point in the file, the lexer (and thus location_t values) use line numbers that are one larger than those seen by input.c, "cat -n" and emacs. This discrepancy between the lexer's line numbering and input.c's line numbering leads to an out-of-range read in get_visual_column (trying to read column 8, to locate the first non-whitespace on the line containing "break;", but finding the next line, which is only 4 characters long). This patch fixes the ICE by adding a range check to get_visual_column before accessing the input.c line buffer. This is arguably papering over the root cause, but there are presumably other ways of triggering such an out-of-range read by writing to the source file after the lexer but before -Wmisleading-indentation, and we ought to be not ICE in the face of that. gcc/c-family/ChangeLog: PR c++/70693 * c-common.c (selftest::c_family_tests): Call selftest::c_indentation_c_tests. * c-common.h (selftest::c_indentation_c_tests): New decl. * c-indentation.c: Include "selftest.h". (next_tab_stop): Add "tab_width" param, rather than accessing cpp_opts. (get_visual_column): Likewise. Clarify comment. Bulletproof against reading past the end of the line. (get_first_nws_vis_column): Add "tab_width" param. (detect_intervening_unindent): Likewise. (should_warn_for_misleading_indentation): Read tab width from cpp_opts and pass around. (selftest::test_next_tab_stop): New test. (selftest::assert_get_visual_column_succeeds): New function. (ASSERT_GET_VISUAL_COLUMN_SUCCEEDS): New macro. (selftest::assert_get_visual_column_fails): New function. (ASSERT_GET_VISUAL_COLUMN_FAILS): New macro. (selftest::test_get_visual_column): New test. (selftest::c_indentation_c_tests): New function. gcc/testsuite/ChangeLog: PR c++/70693 * c-c++-common/Wmisleading-indentation-pr70693.c: New test. From-SVN: r263595 --- gcc/c-family/ChangeLog | 23 +++ gcc/c-family/c-common.c | 1 + gcc/c-family/c-common.h | 1 + gcc/c-family/c-indentation.c | 192 +++++++++++++++++++-- gcc/testsuite/ChangeLog | 5 + .../c-c++-common/Wmisleading-indentation-pr70693.c | 12 ++ 6 files changed, 220 insertions(+), 14 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/Wmisleading-indentation-pr70693.c diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog index d293d99..8cd6f4c 100644 --- a/gcc/c-family/ChangeLog +++ b/gcc/c-family/ChangeLog @@ -1,3 +1,26 @@ +2018-08-16 David Malcolm + + PR c++/70693 + * c-common.c (selftest::c_family_tests): Call + selftest::c_indentation_c_tests. + * c-common.h (selftest::c_indentation_c_tests): New decl. + * c-indentation.c: Include "selftest.h". + (next_tab_stop): Add "tab_width" param, rather than accessing + cpp_opts. + (get_visual_column): Likewise. Clarify comment. Bulletproof + against reading past the end of the line. + (get_first_nws_vis_column): Add "tab_width" param. + (detect_intervening_unindent): Likewise. + (should_warn_for_misleading_indentation): Read tab width from + cpp_opts and pass around. + (selftest::test_next_tab_stop): New test. + (selftest::assert_get_visual_column_succeeds): New function. + (ASSERT_GET_VISUAL_COLUMN_SUCCEEDS): New macro. + (selftest::assert_get_visual_column_fails): New function. + (ASSERT_GET_VISUAL_COLUMN_FAILS): New macro. + (selftest::test_get_visual_column): New test. + (selftest::c_indentation_c_tests): New function. + 2018-08-16 Nathan Sidwell * c-ada-spec.c (count_ada_macro): Use cpp_user_macro_p. diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index 55b2e50..95cff21 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -8370,6 +8370,7 @@ c_family_tests (void) { c_common_c_tests (); c_format_c_tests (); + c_indentation_c_tests (); c_pretty_print_c_tests (); c_spellcheck_cc_tests (); } diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h index 8a802bb..9b05e60 100644 --- a/gcc/c-family/c-common.h +++ b/gcc/c-family/c-common.h @@ -1338,6 +1338,7 @@ namespace selftest { /* Declarations for specific families of tests within c-family, by source file, in alphabetical order. */ extern void c_format_c_tests (void); + extern void c_indentation_c_tests (void); extern void c_pretty_print_c_tests (void); extern void c_spellcheck_cc_tests (void); diff --git a/gcc/c-family/c-indentation.c b/gcc/c-family/c-indentation.c index 44b1e1e..436d61b 100644 --- a/gcc/c-family/c-indentation.c +++ b/gcc/c-family/c-indentation.c @@ -23,15 +23,15 @@ along with GCC; see the file COPYING3. If not see #include "tm.h" #include "c-common.h" #include "c-indentation.h" +#include "selftest.h" extern cpp_options *cpp_opts; /* Round up VIS_COLUMN to nearest tab stop. */ static unsigned int -next_tab_stop (unsigned int vis_column) +next_tab_stop (unsigned int vis_column, unsigned int tab_width) { - const unsigned int tab_width = cpp_opts->tabstop; vis_column = ((vis_column + tab_width) / tab_width) * tab_width; return vis_column; } @@ -43,12 +43,13 @@ next_tab_stop (unsigned int vis_column) Returns true if a conversion was possible, writing the result to OUT, otherwise returns false. If FIRST_NWS is not NULL, then write to it the visual column corresponding to the first non-whitespace character - on the line. */ + on the line (up to or before EXPLOC). */ static bool get_visual_column (expanded_location exploc, location_t loc, unsigned int *out, - unsigned int *first_nws) + unsigned int *first_nws, + unsigned int tab_width) { /* PR c++/68819: if the column number is zero, we presumably had a location_t > LINE_MAP_MAX_LOCATION_WITH_COLS, and so @@ -73,6 +74,8 @@ get_visual_column (expanded_location exploc, location_t loc, char_span line = location_get_source_line (exploc.file, exploc.line); if (!line) return false; + if ((size_t)exploc.column > line.length ()) + return false; unsigned int vis_column = 0; for (int i = 1; i < exploc.column; i++) { @@ -85,7 +88,7 @@ get_visual_column (expanded_location exploc, location_t loc, } if (ch == '\t') - vis_column = next_tab_stop (vis_column); + vis_column = next_tab_stop (vis_column, tab_width); else vis_column++; } @@ -106,7 +109,8 @@ get_visual_column (expanded_location exploc, location_t loc, static bool get_first_nws_vis_column (const char *file, int line_num, - unsigned int *first_nws) + unsigned int *first_nws, + unsigned int tab_width) { gcc_assert (first_nws); @@ -125,7 +129,7 @@ get_first_nws_vis_column (const char *file, int line_num, } if (ch == '\t') - vis_column = next_tab_stop (vis_column); + vis_column = next_tab_stop (vis_column, tab_width); else vis_column++; } @@ -178,7 +182,8 @@ static bool detect_intervening_unindent (const char *file, int body_line, int next_stmt_line, - unsigned int vis_column) + unsigned int vis_column, + unsigned int tab_width) { gcc_assert (file); gcc_assert (next_stmt_line > body_line); @@ -186,7 +191,7 @@ detect_intervening_unindent (const char *file, for (int line = body_line + 1; line < next_stmt_line; line++) { unsigned int line_vis_column; - if (get_first_nws_vis_column (file, line, &line_vis_column)) + if (get_first_nws_vis_column (file, line, &line_vis_column, tab_width)) if (line_vis_column < vis_column) return true; } @@ -289,6 +294,8 @@ should_warn_for_misleading_indentation (const token_indent_info &guard_tinfo, expanded_location next_stmt_exploc = expand_location (next_stmt_loc); expanded_location guard_exploc = expand_location (guard_loc); + const unsigned int tab_width = cpp_opts->tabstop; + /* They must be in the same file. */ if (next_stmt_exploc.file != body_exploc.file) return false; @@ -334,7 +341,7 @@ should_warn_for_misleading_indentation (const token_indent_info &guard_tinfo, unsigned int guard_line_first_nws; if (!get_visual_column (guard_exploc, guard_loc, &guard_vis_column, - &guard_line_first_nws)) + &guard_line_first_nws, tab_width)) return false; /* Heuristic: only warn if the guard is the first thing on its line. */ @@ -394,15 +401,15 @@ should_warn_for_misleading_indentation (const token_indent_info &guard_tinfo, it's not clear that it's meaningful to look at indentation. */ if (!get_visual_column (next_stmt_exploc, next_stmt_loc, &next_stmt_vis_column, - &next_stmt_line_first_nws)) + &next_stmt_line_first_nws, tab_width)) return false; if (!get_visual_column (body_exploc, body_loc, &body_vis_column, - &body_line_first_nws)) + &body_line_first_nws, tab_width)) return false; if (!get_visual_column (guard_exploc, guard_loc, &guard_vis_column, - &guard_line_first_nws)) + &guard_line_first_nws, tab_width)) return false; /* If the line where the next stmt starts has non-whitespace @@ -486,7 +493,7 @@ should_warn_for_misleading_indentation (const token_indent_info &guard_tinfo, int vis_column = MIN (next_stmt_vis_column, body_vis_column); if (detect_intervening_unindent (body_exploc.file, body_exploc.line, next_stmt_exploc.line, - vis_column)) + vis_column, tab_width)) return false; /* Otherwise, they are visually aligned: issue a warning. */ @@ -611,3 +618,160 @@ warn_for_misleading_indentation (const token_indent_info &guard_tinfo, guard_tinfo_to_string (guard_tinfo.keyword)); } } + +#if CHECKING_P + +namespace selftest { + +/* Verify that next_tab_stop works as expected. */ + +static void +test_next_tab_stop () +{ + const unsigned int tab_width = 8; + + ASSERT_EQ (next_tab_stop (0, tab_width), 8); + ASSERT_EQ (next_tab_stop (1, tab_width), 8); + ASSERT_EQ (next_tab_stop (7, tab_width), 8); + + ASSERT_EQ (next_tab_stop (8, tab_width), 16); + ASSERT_EQ (next_tab_stop (9, tab_width), 16); + ASSERT_EQ (next_tab_stop (15, tab_width), 16); + + ASSERT_EQ (next_tab_stop (16, tab_width), 24); + ASSERT_EQ (next_tab_stop (17, tab_width), 24); + ASSERT_EQ (next_tab_stop (23, tab_width), 24); +} + +/* Verify that the given call to get_visual_column succeeds, with + the given results. */ + +static void +assert_get_visual_column_succeeds (const location &loc, + const char *file, int line, int column, + const unsigned int tab_width, + unsigned int expected_visual_column, + unsigned int expected_first_nws) +{ + expanded_location exploc; + exploc.file = file; + exploc.line = line; + exploc.column = column; + exploc.data = NULL; + exploc.sysp = false; + unsigned int actual_visual_column; + unsigned int actual_first_nws; + bool result = get_visual_column (exploc, UNKNOWN_LOCATION, + &actual_visual_column, + &actual_first_nws, tab_width); + ASSERT_TRUE_AT (loc, result); + ASSERT_EQ_AT (loc, actual_visual_column, expected_visual_column); + ASSERT_EQ_AT (loc, actual_first_nws, expected_first_nws); +} + +/* Verify that the given call to get_visual_column succeeds, with + the given results. */ + +#define ASSERT_GET_VISUAL_COLUMN_SUCCEEDS(FILENAME, LINE, COLUMN, \ + TAB_WIDTH, \ + EXPECTED_VISUAL_COLUMN, \ + EXPECTED_FIRST_NWS) \ + SELFTEST_BEGIN_STMT \ + assert_get_visual_column_succeeds (SELFTEST_LOCATION, \ + FILENAME, LINE, COLUMN, \ + TAB_WIDTH, \ + EXPECTED_VISUAL_COLUMN, \ + EXPECTED_FIRST_NWS); \ + SELFTEST_END_STMT + +/* Verify that the given call to get_visual_column fails gracefully. */ + +static void +assert_get_visual_column_fails (const location &loc, + const char *file, int line, int column, + const unsigned int tab_width) +{ + expanded_location exploc; + exploc.file = file; + exploc.line = line; + exploc.column = column; + exploc.data = NULL; + exploc.sysp = false; + unsigned int actual_visual_column; + unsigned int actual_first_nws; + bool result = get_visual_column (exploc, UNKNOWN_LOCATION, + &actual_visual_column, + &actual_first_nws, tab_width); + ASSERT_FALSE_AT (loc, result); +} + +/* Verify that the given call to get_visual_column fails gracefully. */ + +#define ASSERT_GET_VISUAL_COLUMN_FAILS(FILENAME, LINE, COLUMN, \ + TAB_WIDTH) \ + SELFTEST_BEGIN_STMT \ + assert_get_visual_column_fails (SELFTEST_LOCATION, \ + FILENAME, LINE, COLUMN, \ + TAB_WIDTH); \ + SELFTEST_END_STMT + +/* Verify that get_visual_column works as expected. */ + +static void +test_get_visual_column () +{ + /* Create a tempfile with a mixture of tabs and spaces. + + Both lines have either a space or a tab, then " line N", + for 8 characters in total. + + 1-based "columns" (w.r.t. to line 1): + .....................0000000001111. + .....................1234567890123. */ + const char *content = (" line 1\n" + "\t line 2\n"); + line_table_test ltt; + temp_source_file tmp (SELFTEST_LOCATION, ".txt", content); + + const unsigned int tab_width = 8; + const char *file = tmp.get_filename (); + + /* Line 1 (space-based indentation). */ + { + const int line = 1; + ASSERT_GET_VISUAL_COLUMN_SUCCEEDS (file, line, 1, tab_width, 0, 0); + ASSERT_GET_VISUAL_COLUMN_SUCCEEDS (file, line, 2, tab_width, 1, 1); + ASSERT_GET_VISUAL_COLUMN_SUCCEEDS (file, line, 3, tab_width, 2, 2); + /* first_nws should have stopped increasing. */ + ASSERT_GET_VISUAL_COLUMN_SUCCEEDS (file, line, 4, tab_width, 3, 2); + /* Verify the end-of-line boundary. */ + ASSERT_GET_VISUAL_COLUMN_SUCCEEDS (file, line, 8, tab_width, 7, 2); + ASSERT_GET_VISUAL_COLUMN_FAILS (file, line, 9, tab_width); + } + + /* Line 2 (tab-based indentation). */ + { + const int line = 2; + ASSERT_GET_VISUAL_COLUMN_SUCCEEDS (file, line, 1, tab_width, 0, 0); + ASSERT_GET_VISUAL_COLUMN_SUCCEEDS (file, line, 2, tab_width, 8, 8); + ASSERT_GET_VISUAL_COLUMN_SUCCEEDS (file, line, 3, tab_width, 9, 9); + /* first_nws should have stopped increasing. */ + ASSERT_GET_VISUAL_COLUMN_SUCCEEDS (file, line, 4, tab_width, 10, 9); + /* Verify the end-of-line boundary. */ + ASSERT_GET_VISUAL_COLUMN_SUCCEEDS (file, line, 8, tab_width, 14, 9); + ASSERT_GET_VISUAL_COLUMN_FAILS (file, line, 9, tab_width); + } +} + +/* Run all of the selftests within this file. */ + +void +c_indentation_c_tests () +{ + test_next_tab_stop (); + test_get_visual_column (); +} + +} // namespace selftest + +#endif /* CHECKING_P */ diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index e0f48ab..cb402d4 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2018-08-16 David Malcolm + + PR c++/70693 + * c-c++-common/Wmisleading-indentation-pr70693.c: New test. + 2018-08-16 Vlad Lazar * gcc.target/aarch64/imm_choice_comparison.c: New test. diff --git a/gcc/testsuite/c-c++-common/Wmisleading-indentation-pr70693.c b/gcc/testsuite/c-c++-common/Wmisleading-indentation-pr70693.c new file mode 100644 index 0000000..0869b11 --- /dev/null +++ b/gcc/testsuite/c-c++-common/Wmisleading-indentation-pr70693.c @@ -0,0 +1,12 @@ +/* { dg-options "-Wmisleading-indentation" } */ + +int in_what; /* */ + +void process_char(char c) { + switch( 0 ) { + case 0: + if( c == '>' ) in_what = 0; + break; + + } +} -- 2.7.4