Bug 68473 - ICE: in contains_point, at diagnostic-show-locus.c:340 after error
Summary: ICE: in contains_point, at diagnostic-show-locus.c:340 after error
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c (show other bugs)
Version: 6.0
: P3 normal
Target Milestone: ---
Assignee: David Malcolm
URL:
Keywords:
: 68839 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-11-21 19:09 UTC by Zdenek Sojka
Modified: 2017-01-16 20:03 UTC (History)
10 users (show)

See Also:
Host: x86_64-pc-linux-gnu
Target: x86_64-pc-linux-gnu
Build: x86_64-pc-linux-gnu
Known to work: 5.2.1
Known to fail: 6.0
Last reconfirmed: 2015-11-23 00:00:00


Attachments
reduced testcase (142 bytes, text/x-csrc)
2015-11-21 19:09 UTC, Zdenek Sojka
Details
Another testcase, (19.30 KB, text/plain)
2015-12-18 17:33 UTC, Jeffrey A. Law
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Zdenek Sojka 2015-11-21 19:09:48 UTC
Created attachment 36799 [details]
reduced testcase

Compiler output:
$ gcc -mno-fp-ret-in-387 testcase.c 
testcase.c: In function 'foo':
testcase.c:11:12: error: x87 register return with x87 disabled
 
testcase.c:11:12: internal compiler error: in contains_point, at diagnostic-show-locus.c:340
testcase.c:4:13: note: in definition of macro 'TEST_EQ'
   if ((long)FUNC##l(xl,xl) != (long)xl) \
             ^~~~

0x14fc73f contains_point
        /mnt/svn/gcc-trunk/gcc/diagnostic-show-locus.c:340
0x14fc73f get_state_at_point
        /mnt/svn/gcc-trunk/gcc/diagnostic-show-locus.c:684
0x14fcc70 print_source_line
        /mnt/svn/gcc-trunk/gcc/diagnostic-show-locus.c:538
0x14fcc70 diagnostic_show_locus(diagnostic_context*, diagnostic_info const*)
        /mnt/svn/gcc-trunk/gcc/diagnostic-show-locus.c:803
0x6a1690 c_diagnostic_finalizer
        /mnt/svn/gcc-trunk/gcc/c-family/c-opts.c:167
0x14fab08 diagnostic_report_diagnostic(diagnostic_context*, diagnostic_info*)
        /mnt/svn/gcc-trunk/gcc/diagnostic.c:800
0x14fb95d error(char const*, ...)
        /mnt/svn/gcc-trunk/gcc/diagnostic.c:1157
0xeaed98 construct_container
        /mnt/svn/gcc-trunk/gcc/config/i386/i386.c:8435
0xeb8929 function_value_64
        /mnt/svn/gcc-trunk/gcc/config/i386/i386.c:9547
0xeb8929 ix86_function_value_1
        /mnt/svn/gcc-trunk/gcc/config/i386/i386.c:9607
0x81b30f hard_function_value(tree_node const*, tree_node const*, tree_node const*, int)
        /mnt/svn/gcc-trunk/gcc/explow.c:1854
0x89926c aggregate_value_p(tree_node const*, tree_node const*)
        /mnt/svn/gcc-trunk/gcc/function.c:2081
0x8fa2ed gimplify_modify_expr_rhs
        /mnt/svn/gcc-trunk/gcc/gimplify.c:4352
0x8fad94 gimplify_modify_expr
        /mnt/svn/gcc-trunk/gcc/gimplify.c:4643
0x8f0602 gimplify_expr(tree_node**, gimple**, gimple**, bool (*)(tree_node*), int)
        /mnt/svn/gcc-trunk/gcc/gimplify.c:9887
0x8f2e26 gimplify_stmt(tree_node**, gimple**)
        /mnt/svn/gcc-trunk/gcc/gimplify.c:5591
0x8f4c12 gimplify_and_add(tree_node*, gimple**)
        /mnt/svn/gcc-trunk/gcc/gimplify.c:417
0x8f4c12 internal_get_tmp_var
        /mnt/svn/gcc-trunk/gcc/gimplify.c:562
0x8edb7c gimplify_expr(tree_node**, gimple**, gimple**, bool (*)(tree_node*), int)
        /mnt/svn/gcc-trunk/gcc/gimplify.c:10812
0x8edeff gimplify_expr(tree_node**, gimple**, gimple**, bool (*)(tree_node*), int)
        /mnt/svn/gcc-trunk/gcc/gimplify.c:9827
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.

Tested revisions:
r230703 - ICE
5-branch r229305 - OK
Comment 1 David Malcolm 2015-11-23 15:29:39 UTC
Thanks for filing; I can reproduce.

Failing assertion at line 340:

337	bool
338	layout_range::contains_point (int row, int column) const
339	{
340	  gcc_assert (m_start.m_line <= m_finish.m_line);
Comment 2 David Malcolm 2015-11-23 15:40:36 UTC
It's attempting to print this source range:
(gdb) p *range
$8 = {m_start = {m_line = 13, m_column = 12},
      m_finish = {m_line = 6, m_column = 26},
      m_show_caret_p = true,
      m_caret = {m_line = 13, m_column = 12}}

i.e. (after dealing with tabs):

        0000000001111111111222222222233333333334444444444555555555566666666667
        1234567890123456879012345687901234568790123456879012345687901234568790
     1  /* { dg-options "-fdiagnostics-show-caret -mno-fp-ret-in-387" } */
     2
     3  #include <math.h>
     4
     5  #define TEST_EQ(FUNC) do { \
     6  if ((long)FUNC##l(xl,xl) != (long)xl) \
                                 ^FINISH
     7      return; \
     8    } while (0)
     9
    10  void
    11  foo (long double xl)
    12  {
    13    TEST_EQ (fmin);
                  ^
                  START&CARET
    14  }
Comment 3 David Malcolm 2015-11-23 15:42:25 UTC
Oops; I think line 6 above should read:

     6    if ((long)FUNC##l(xl,xl) != (long)xl) \
                                 ^FINISH
Comment 4 David Malcolm 2015-11-23 15:57:03 UTC
No, I was wrong again.

The locus and start of the range are here:

(gdb) call inform(line_table->location_adhoc_data_map.data[0x17].src_range.m_start, "m_start")

pr68473-1.c:13:12: note: m_start
   TEST_EQ (fmin); /* { dg-error "x87 register return with x87 disabled" } */
            ^

pr68473-1.c:6:13: note: in definition of macro ‘TEST_EQ’
   if ((long)FUNC##l(xl,xl) != (long)xl) \
             ^~~~

and the finish of the range is here:

(gdb) call inform(line_table->location_adhoc_data_map.data[0x17].src_range.m_finish, "m_finish")

pr68473-1.c:6:26: note: m_finish
   if ((long)FUNC##l(xl,xl) != (long)xl) \
                          ^

pr68473-1.c:13:3: note: in expansion of macro ‘TEST_EQ’
   TEST_EQ (fmin); /* { dg-error "x87 register return with x87 disabled" } */
   ^~~~~~~

via:

Breakpoint 2, error (gmsgid=0x1897930 "x87 register return with x87 disabled") at ../../src/gcc/diagnostic.c:1153
1153	  rich_location richloc (line_table, input_location);
(gdb) p /x input_location
$2 = 0x80000017
(gdb) p line_table->location_adhoc_data_map.data[0x17]
$3 = {locus = 2147442627, src_range = {m_start = 2147442627, m_finish = 2147442633}, data = 0x0}
(gdb) p /x line_table->location_adhoc_data_map.data[0x17].src_range.m_start
$5 = 0x7fff5fc3
(gdb) p /x line_table->location_adhoc_data_map.data[0x17].src_range.m_finish
$6 = 0x7fff5fc9
Comment 5 David Malcolm 2015-11-23 17:54:17 UTC
I've posted a candidate patch here:
  https://gcc.gnu.org/ml/gcc-patches/2015-11/msg02771.html
Comment 6 Jay Foad 2015-11-24 11:48:42 UTC
FYI here's another test case that seems to trigger the same bug. Your candidate patch fixes it.

$ cat x.c
#define P(b) b&&4
int a[]=0;
int f() { X||P(d); }
$ ~/gcc/build/gcc/cc1 -quiet -Wall x.c
[...]
x.c:3:1: internal compiler error: in contains_point, at diagnostic-show-locus.c:335
 int f() { X||P(d); }
 ^~~

0x1268fc9 contains_point
	/home/jay/svn/gcc/trunk/gcc/diagnostic-show-locus.c:335
0x1268fc9 get_state_at_point
	/home/jay/svn/gcc/trunk/gcc/diagnostic-show-locus.c:612
0x12696e2 print_source_line
	/home/jay/svn/gcc/trunk/gcc/diagnostic-show-locus.c:533
0x12696e2 diagnostic_show_locus(diagnostic_context*, diagnostic_info const*)
	/home/jay/svn/gcc/trunk/gcc/diagnostic-show-locus.c:710
0x69b210 c_diagnostic_finalizer
	/home/jay/svn/gcc/trunk/gcc/c-family/c-opts.c:167
0x1267220 diagnostic_report_diagnostic(diagnostic_context*, diagnostic_info*)
	/home/jay/svn/gcc/trunk/gcc/diagnostic.c:800
0x1267b07 warning_at(unsigned int, int, char const*, ...)
	/home/jay/svn/gcc/trunk/gcc/diagnostic.c:1029
0x607e58 parser_build_binary_op(unsigned int, tree_code, c_expr, c_expr)
	/home/jay/svn/gcc/trunk/gcc/c/c-typeck.c:3514
0x61855a c_parser_binary_expression
	/home/jay/svn/gcc/trunk/gcc/c/c-parser.c:6539
0x618a18 c_parser_conditional_expression
	/home/jay/svn/gcc/trunk/gcc/c/c-parser.c:6182
0x619100 c_parser_expr_no_commas
	/home/jay/svn/gcc/trunk/gcc/c/c-parser.c:6099
0x6198d2 c_parser_expression
	/home/jay/svn/gcc/trunk/gcc/c/c-parser.c:8230
0x61a3a9 c_parser_expression_conv
	/home/jay/svn/gcc/trunk/gcc/c/c-parser.c:8263
0x633431 c_parser_statement_after_labels
	/home/jay/svn/gcc/trunk/gcc/c/c-parser.c:5172
0x635065 c_parser_compound_statement_nostart
	/home/jay/svn/gcc/trunk/gcc/c/c-parser.c:4757
0x6358ae c_parser_compound_statement
	/home/jay/svn/gcc/trunk/gcc/c/c-parser.c:4594
0x6314e3 c_parser_declaration_or_fndef
	/home/jay/svn/gcc/trunk/gcc/c/c-parser.c:2015
0x63d31d c_parser_external_declaration
	/home/jay/svn/gcc/trunk/gcc/c/c-parser.c:1459
0x63dbe9 c_parser_translation_unit
	/home/jay/svn/gcc/trunk/gcc/c/c-parser.c:1346
0x63dbe9 c_parse_file()
	/home/jay/svn/gcc/trunk/gcc/c/c-parser.c:17622
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.
Comment 7 Martin Sebor 2015-12-10 16:18:34 UTC
*** Bug 68839 has been marked as a duplicate of this bug. ***
Comment 8 David Malcolm 2015-12-10 19:12:01 UTC
(In reply to Jay Foad from comment #6)
> FYI here's another test case that seems to trigger the same bug. Your
> candidate patch fixes it.
> 
> $ cat x.c
> #define P(b) b&&4
> int a[]=0;
> int f() { X||P(d); }
> $ ~/gcc/build/gcc/cc1 -quiet -Wall x.c
> [...]
> x.c:3:1: internal compiler error: in contains_point, at
> diagnostic-show-locus.c:335
>  int f() { X||P(d); }
>  ^~~

Thanks.

For this one, it's attempting to issue:
  "suggest parentheses around %<&&%> within %<||%>"

and all three parts of the location are within macro expansions, as it happens, within the same macro expansion:

(gdb) p /x line_table->location_adhoc_data_map.data[0x1]
$12 = {locus = 0x7ffffffb, src_range = {m_start = 0x7ffffffa, m_finish = 0x7ffffffc}, data = 0x0}

(gdb) call inform (0x7ffffffb, "locus")
pr68473-2.c:2:15: note: locus
 #define P(b) b&&4
               ^

pr68473-2.c:4:14: note: in expansion of macro ‘P’
 int f() { X||P(d); }
              ^

(gdb) call inform (0x7ffffffa, "m_start")
pr68473-2.c:4:16: note: m_start
 int f() { X||P(d); }
                ^

pr68473-2.c:2:14: note: in definition of macro ‘P’
 #define P(b) b&&4
              ^

(gdb) call inform (0x7ffffffc, "m_finish")
pr68473-2.c:2:17: note: m_finish
 #define P(b) b&&4
                 ^

pr68473-2.c:4:14: note: in expansion of macro ‘P’
 int f() { X||P(d); }
              ^

i.e. it's the: "b&&4"

The patch from comment #5 makes it emit:

pr68473-2.c:2:15: warning: suggest parentheses around '&&' within '||' [-Wparentheses]
 #define P(b) b&&4
               ^

pr68473-2.c:4:14: note: in expansion of macro 'P'
 int f() { X||P(d); }
              ^

Perhaps ideally it should emit:

pr68473-2.c:2:15: warning: suggest parentheses around '&&' within '||' [-Wparentheses]
 #define P(b) b&&4
              ~^~~

pr68473-2.c:4:14: note: in expansion of macro 'P'
 int f() { X||P(d); }
              ^

given that all parts of the range are within the same expansion (a much more involved patch, I think)
Comment 9 David Malcolm 2015-12-11 18:27:46 UTC
Updated patch kit posted here:
  https://gcc.gnu.org/ml/gcc-patches/2015-12/msg01291.html
Comment 10 Jeffrey A. Law 2015-12-18 17:33:46 UTC
Created attachment 37089 [details]
Another testcase,

Attaching another testcase.  Hopefully it's fixed by the pending patch from David.
Comment 11 David Malcolm 2015-12-22 22:28:17 UTC
Author: dmalcolm
Date: Tue Dec 22 22:27:45 2015
New Revision: 231919

URL: https://gcc.gnu.org/viewcvs?rev=231919&root=gcc&view=rev
Log:
PR c/68473: sanitize source range-printing within certain macro expansions

gcc/ChangeLog:
	PR c/68473
	* diagnostic-show-locus.c (layout::layout): Make loc_range const.
	Sanitize the layout_range against ranges that finish before they
	start.

gcc/testsuite/ChangeLog:
	PR c/68473
	* gcc.dg/plugin/diagnostic-test-expressions-1.c (fminl): New decl.
	(TEST_EQ): New macro.
	(test_macro): New function.
	* gcc.target/i386/pr68473-1.c: New test case.


Added:
    trunk/gcc/testsuite/gcc.target/i386/pr68473-1.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/diagnostic-show-locus.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c
Comment 12 David Malcolm 2015-12-22 22:30:53 UTC
(In reply to David Malcolm from comment #11)
> Author: dmalcolm
> Date: Tue Dec 22 22:27:45 2015
> New Revision: 231919
> 
> URL: https://gcc.gnu.org/viewcvs?rev=231919&root=gcc&view=rev
> Log:
> PR c/68473: sanitize source range-printing within certain macro expansions

This commit corresponds to the patch from comment #5, rather than the one from comment #9.  Keeping this open in the hope of doing a more thorough fix for this, but this may need to wait for gcc 7.
Comment 13 Richard Biener 2016-01-14 11:22:30 UTC
Removing the regression marker then.
Comment 14 David Malcolm 2016-03-09 18:15:15 UTC
Author: dmalcolm
Date: Wed Mar  9 18:14:43 2016
New Revision: 234087

URL: https://gcc.gnu.org/viewcvs?rev=234087&root=gcc&view=rev
Log:
PR c++/70105: Defer location expansion until diagnostic_show_locus

gcc/ChangeLog:
	PR c/68473
	PR c++/70105
	* diagnostic-show-locus.c (layout_range::layout_range): Replace
	location_range param with three const expanded_locations * and a
	bool.
	(layout::layout): Replace call to
	rich_location::lazily_expand_location with get_expanded_location.
	Extract the range and perform location expansion here, passing
	the results to the layout_range ctor.
	* diagnostic.c (source_range::debug): Delete.
	* diagnostic.h (diagnostic_expand_location): Reimplement in terms
	of rich_location::get_expanded_location.
	* gcc-rich-location.c (get_range_for_expr): Delete.
	(gcc_rich_location::add_expr): Reimplement to avoid the
	rich_location::add_range overload that took a location_range,
	passing a location_t instead.

gcc/testsuite/ChangeLog:
	PR c/68473
	PR c++/70105
	* gcc.dg/plugin/diagnostic_plugin_show_trees.c (show_tree):
	Drop range information from call to inform_at_rich_loc.
	* gcc.dg/plugin/diagnostic_plugin_test_show_locus.c (add_range):
	New.
	(test_show_locus): Replace calls to rich_location::add_range with
	calls to add_range.  Rewrite the tests that used the now-defunct
	rich_location ctor taking a source_range.  Simplify other tests
	by replacing calls to COMBINE_LOCATION_DATA with calls to
	make_location.

libcpp/ChangeLog:
	PR c/68473
	PR c++/70105
	* include/line-map.h (source_range::debug): Delete.
	(struct location_range): Update comment.  Replace
	expanded_location fields "m_start", "m_finish", and "m_caret" with
	a source_location field: "m_loc".
	(class rich_location): Reword comment.
	(rich_location::get_loc): Reimplement in terms of a new overloaded
	variant which takes an unsigned int.
	(rich_location::get_loc_addr): Delete.
	(rich_location::add_range): Drop params "start" and "finish" in
	favor of param "loc".  Drop overloaded variants taking a
	source_range or location_range *.
	(rich_location::lazily_expand_location): Delete in favor of...
	(rich_location::get_expanded_location): New decl.
	(rich_location::m_loc): Delete field.
	(rich_location::m_column_override): New field.
	* line-map.c (rich_location::rich_location):  Drop name of
	line_maps * param.  Update initializations for deletion of field
	"m_loc" and addition of field "m_column_override".  Reimplement
	body as a call to add_range.  Delete overloaded variant taking a
	source_range.
	(rich_location::get_loc): New function.
	(rich_location::lazily_expand_location): Delete in favor of...
	(rich_location::get_expanded_location): New function.
	(rich_location::override_column): Reimplement.
	(rich_location::add_range): Drop params "start" and "finish" in
	favor of param "loc".  Eliminate location expansion in favor of
	simply storing loc.  Drop overloaded variants taking a
	source_range or location_range *.
	(rich_location::set_range): Eliminate location expansion.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/diagnostic-show-locus.c
    trunk/gcc/diagnostic.c
    trunk/gcc/diagnostic.h
    trunk/gcc/gcc-rich-location.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_show_trees.c
    trunk/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_show_locus.c
    trunk/libcpp/ChangeLog
    trunk/libcpp/include/line-map.h
    trunk/libcpp/line-map.c
Comment 15 David Malcolm 2016-03-09 18:23:59 UTC
Author: dmalcolm
Date: Wed Mar  9 18:23:27 2016
New Revision: 234088

URL: https://gcc.gnu.org/viewcvs?rev=234088&root=gcc&view=rev
Log:
PR c++/70105: prevent nonsensical underline spew for macro expansions

diagnostic_show_locus can sometimes do the wrong thing when handling
expressions built up from macros.

PR c++/70105 (currently marked as a P3 regression) has an example of
a diagnostic where over 500 lines of irrelevant source are printed,
and underlined, giving >1000 lines of useless spew to stderr.

This patch adds extra sanitization to diagnostic-show-locus.c, so that
we only attempt to print underlines and secondary locations if such
locations are "sufficiently sane" relative to the primary location
of a diagnostic.

This "sufficiently sane" condition is implemented by a new helper
function compatible_locations_p, which requires such locations to
have the same macro expansion hierarchy as the primary location,
using linemap_macro_map_loc_unwind_toward_spelling, effectively
mimicing the expansion performed by LRK_SPELLING_LOCATION.

This may be too strong a condition, but it effectively fixes
PR c++/70105, without removing any underlines in my testing.

Successfully bootstrapped&regrtested in combination with the previous
patch on x86_64-pc-linux-gnu; adds 15 new PASS results to g++.sum
and 4 new PASS results to gcc.sum.

gcc/ChangeLog:
	PR c/68473
	PR c++/70105
	* diagnostic-show-locus.c (compatible_locations_p): New function.
	(layout::layout): Sanitize ranges using compatible_locations_p.

gcc/testsuite/ChangeLog:
	PR c/68473
	PR c++/70105
	* g++.dg/diagnostic/pr70105.C: New test.
	* gcc.dg/plugin/diagnostic-test-expressions-1.c (foo): New decl.
	(test_multiple_ordinary_maps): New test function.

libcpp/ChangeLog:
	PR c/68473
	PR c++/70105
	* line-map.c (linemap_macro_map_loc_unwind_toward_spelling): Move
	decl...
	* include/line-map.h
	(linemap_macro_map_loc_unwind_toward_spelling): ...here,
	converting from static to extern.


Added:
    trunk/gcc/testsuite/g++.dg/diagnostic/pr70105.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/diagnostic-show-locus.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.dg/plugin/diagnostic-test-expressions-1.c
    trunk/libcpp/ChangeLog
    trunk/libcpp/include/line-map.h
    trunk/libcpp/line-map.c
Comment 16 Marek Polacek 2016-07-29 16:08:32 UTC
So is this fixed?
Comment 17 David Malcolm 2017-01-16 20:03:32 UTC
(In reply to David Malcolm from comment #12)
> (In reply to David Malcolm from comment #11)
> > Author: dmalcolm
> > Date: Tue Dec 22 22:27:45 2015
> > New Revision: 231919
> > 
> > URL: https://gcc.gnu.org/viewcvs?rev=231919&root=gcc&view=rev
> > Log:
> > PR c/68473: sanitize source range-printing within certain macro expansions
> 
> This commit corresponds to the patch from comment #5, rather than the one
> from comment #9.  Keeping this open in the hope of doing a more thorough fix
> for this, but this may need to wait for gcc 7.

I believe the commits in comment #14 and comment #15 address this.

I've verified that both reproducers are fixed; marking this one as resolved.