From 3d0a5393466ef5d2755dd0dc8e8c10750e9bb4b9 Mon Sep 17 00:00:00 2001 From: David Malcolm Date: Mon, 29 Oct 2018 23:58:34 +0000 Subject: [PATCH] Fix ICE in get_substring_ranges_for_loc on __FILE__ (PR c++/87721) PR c++/87721 reports a crash in get_substring_ranges_for_loc introduced by r265271, my fix for PR 87562. The new issue occurs when attempting to get a location with a string literal inside a macro in which the first token is __FILE__ (formed via concatenation). Attempting to get the spelling location of __FILE__ fails, leading to NULL for start_ord_map and final_ord_map, and thus a NULL pointer dereference. Given that our "on-demand" substring locations approach reparses the string literals, there isn't a good way to access the locations inside such string literals: attempting to reparse __FILE__ fails with a "missing open quote". This patch applies the easy fix by gracefully rejecting the case where the spelling locations for the start or finish give us NULL maps. gcc/ChangeLog: PR c++/87721 * input.c (get_substring_ranges_for_loc): Detect if linemap_resolve_location gives us a NULL map, and reject this case. gcc/testsuite/ChangeLog: PR c++/87721 * c-c++-common/substring-location-PR-87721.c: New test. * gcc.dg/plugin/diagnostic-test-string-literals-1.c: Add test for PR 87721. * gcc.dg/plugin/diagnostic_plugin_test_string_literals.c (test_string_literals): Fold the index arguments before checking for INTEGER_CST. From-SVN: r265611 --- gcc/ChangeLog | 7 +++++ gcc/input.c | 2 ++ gcc/testsuite/ChangeLog | 10 +++++++ .../substring-location-PR-87721.c | 11 +++++++ .../diagnostic-test-string-literals-1.c | 30 +++++++++++++++++++ .../diagnostic_plugin_test_string_literals.c | 6 ++-- 6 files changed, 63 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/substring-location-PR-87721.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 216b59318465..1ebf3dbe91ca 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,10 @@ +2018-10-29 David Malcolm + + PR c++/87721 + * input.c (get_substring_ranges_for_loc): Detect if + linemap_resolve_location gives us a NULL map, and reject + this case. + 2018-10-29 Iain Buclaw * config.gcc (xstormy16-*-elf): Set tm_d_file. diff --git a/gcc/input.c b/gcc/input.c index 57a1a3c42d31..a94a010f3537 100644 --- a/gcc/input.c +++ b/gcc/input.c @@ -1463,6 +1463,8 @@ get_substring_ranges_for_loc (cpp_reader *pfile, const line_map_ordinary *final_ord_map; linemap_resolve_location (line_table, src_range.m_finish, LRK_SPELLING_LOCATION, &final_ord_map); + if (start_ord_map == NULL || final_ord_map == NULL) + return "failed to get ordinary maps"; /* Bulletproofing. We ought to only have different ordinary maps for start vs finish due to line-length jumps. */ if (start_ord_map != final_ord_map diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index e6e59050bae0..4fe555cd624f 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,13 @@ +2018-10-29 David Malcolm + + PR c++/87721 + * c-c++-common/substring-location-PR-87721.c: New test. + * gcc.dg/plugin/diagnostic-test-string-literals-1.c: Add test for + PR 87721. + * gcc.dg/plugin/diagnostic_plugin_test_string_literals.c + (test_string_literals): Fold the index arguments before checking + for INTEGER_CST. + 2018-10-29 David Malcolm * c-c++-common/spellcheck-reserved.c: Update expected output for diff --git a/gcc/testsuite/c-c++-common/substring-location-PR-87721.c b/gcc/testsuite/c-c++-common/substring-location-PR-87721.c new file mode 100644 index 000000000000..ba99f1baaa5b --- /dev/null +++ b/gcc/testsuite/c-c++-common/substring-location-PR-87721.c @@ -0,0 +1,11 @@ +# define DBG_ERROR(dbg_logger, format, args...) if (1){\ + char dbg_buffer[256]; \ + __builtin_snprintf(dbg_buffer, sizeof(dbg_buffer)-1,\ + __FILE__":%5d: " format , __LINE__ , ## args); \ +}; + +void testPasswordStore1(int argc, char **argv) { + const char *pw1="Secret1"; + char pw[256]; + DBG_ERROR(0, "Bad password, expected [%s], got [%s].", pw1, pw); +} diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-string-literals-1.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-string-literals-1.c index b7350955e991..36324fda43c1 100644 --- a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-string-literals-1.c +++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-string-literals-1.c @@ -319,3 +319,33 @@ pr87652 (const char *stem, int counter) ^~ { dg-end-multiline-output "" } */ } + +/* Reproducer for PR 87721. */ + +# define OFFSET __builtin_strlen (__FILE__) + __builtin_strlen(":%5d: ") + +# define DBG_ERROR(format, caret_idx, start_idx, end_idx) \ + do { \ + __emit_string_literal_range(__FILE__":%5d: " format, \ + OFFSET + caret_idx, \ + OFFSET + start_idx, \ + OFFSET + end_idx); \ + } while (0) + +/* { dg-error "unable to read substring location: failed to get ordinary maps" "" { target *-*-* } 329 } */ +/* { dg-begin-multiline-output "" } + __emit_string_literal_range(__FILE__":%5d: " format, \ + ^~~~~~~~ + { dg-end-multiline-output "" { target c } } */ +/* { dg-begin-multiline-output "" } + __emit_string_literal_range(__FILE__":%5d: " format, \ + ^ + { dg-end-multiline-output "" { target c++ } } */ + +void pr87721 (void) { + DBG_ERROR("Bad password, expected [%s], got [%s].", 24, 24, 25); /* { dg-message "in expansion of macro 'DBG_ERROR'" } */ + /* { dg-begin-multiline-output "" } + DBG_ERROR("Bad password, expected [%s], got [%s].", 24, 24, 25); + ^~~~~~~~~ + { dg-end-multiline-output "" } */ +} diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_string_literals.c b/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_string_literals.c index 99a504dc5415..cf99697a4170 100644 --- a/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_string_literals.c +++ b/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_string_literals.c @@ -140,7 +140,7 @@ test_string_literals (gimple *stmt) return; } - tree t_caret_idx = gimple_call_arg (call, 1); + tree t_caret_idx = fold (gimple_call_arg (call, 1)); if (TREE_CODE (t_caret_idx) != INTEGER_CST) { error_at (call->location, "integer constant required for arg 2"); @@ -148,7 +148,7 @@ test_string_literals (gimple *stmt) } int caret_idx = TREE_INT_CST_LOW (t_caret_idx); - tree t_start_idx = gimple_call_arg (call, 2); + tree t_start_idx = fold (gimple_call_arg (call, 2)); if (TREE_CODE (t_start_idx) != INTEGER_CST) { error_at (call->location, "integer constant required for arg 3"); @@ -156,7 +156,7 @@ test_string_literals (gimple *stmt) } int start_idx = TREE_INT_CST_LOW (t_start_idx); - tree t_end_idx = gimple_call_arg (call, 3); + tree t_end_idx = fold (gimple_call_arg (call, 3)); if (TREE_CODE (t_end_idx) != INTEGER_CST) { error_at (call->location, "integer constant required for arg 4"); -- 2.39.2