Bug 70205

Summary: [5 Regression] ICE on valid code on x86_64-linux-gnu: tree check: expected tree_binfo, have error_mark in add_candidates, at cp/call.c:5283
Product: gcc Reporter: Zhendong Su <su>
Component: c++Assignee: Patrick Palka <ppalka>
Severity: normal CC: jakub, paolo, webrown.cpp
Priority: P2 Keywords: ice-checking, ice-on-valid-code
Version: 6.0   
Target Milestone: 6.0   
Host: Target:
Build: Known to work: 4.7.4, 6.1.0
Known to fail: 4.8.5, 5.0, 6.0 Last reconfirmed: 2016-03-13 00:00:00

Description Zhendong Su 2016-03-12 01:36:39 UTC
The following valid code causes an ICE when compiled with the current GCC trunk on x86_64-linux-gnu in both 32-bit and 64-bit modes. 

This is a regression from 5.3.x. 

$ g++-trunk -v
Using built-in specs.
Target: x86_64-pc-linux-gnu
Configured with: ../gcc-source-trunk/configure --enable-languages=c,c++,lto --prefix=/usr/local/gcc-trunk --disable-bootstrap
Thread model: posix
gcc version 6.0.0 20160311 (experimental) [trunk revision 234134] (GCC) 
$ g++-5.3 -c small.cpp
$ g++-trunk -c small.cpp
small.cpp: In member function ‘void D::g()’:
small.cpp:19:11: internal compiler error: tree check: expected tree_binfo, have error_mark in add_candidates, at cp/call.c:5283
     D::f ();
0xfe10bc tree_check_failed(tree_node const*, char const*, int, char const*, ...)
0x61c634 tree_check
0x61c634 add_candidates
0x61ccf6 build_new_method_call_1
0x61ccf6 build_new_method_call(tree_node*, tree_node*, vec<tree_node*, va_gc, vl_embed>**, tree_node*, int, tree_node**, int)
0x7bbddc finish_call_expr(tree_node*, vec<tree_node*, va_gc, vl_embed>**, bool, bool, int)
0x735a49 cp_parser_postfix_expression
0x73edbc cp_parser_unary_expression
0x73f617 cp_parser_cast_expression
0x73fc15 cp_parser_binary_expression
0x740500 cp_parser_assignment_expression
0x742dd9 cp_parser_expression
0x74354f cp_parser_expression_statement
0x73129b cp_parser_statement
0x731f4c cp_parser_statement_seq_opt
0x73203f cp_parser_compound_statement
0x75087f cp_parser_function_body
0x75087f cp_parser_ctor_initializer_opt_and_function_body
0x751321 cp_parser_function_definition_after_declarator
0x756750 cp_parser_late_parsing_for_member
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.


class A
  static void f ();

class B : public A

class C : public A

class D : public C, public B
  void g ()
    D::f ();
Comment 1 Andrew Pinski 2016-03-13 23:02:53 UTC
Confirmed, it ICEs with checking turned on.

Note this might be invalid code as what GCC is iceing on is an error_mark node.
Comment 2 Zhendong Su 2016-03-14 00:39:25 UTC
(In reply to Andrew Pinski from comment #1)
> Confirmed, it ICEs with checking turned on.
> Note this might be invalid code as what GCC is iceing on is an error_mark
> node.

Andrew, this should be valid code. Also see below: 

$ g++-5.3 -Wall -Wextra -pedantic -c small.cpp
$ clang++ -Weverything -pedantic -c small.cpp
Comment 3 Richard Biener 2016-03-14 10:29:43 UTC
Works on the 4.7 branch.
Comment 4 Marek Polacek 2016-03-16 15:30:40 UTC
I suspect this started with r190969.
Comment 5 Marek Polacek 2016-03-16 16:59:32 UTC
This patch makes us compile the testcase and also passes dg.exp testsuite.  But my understanding of BASELINK stuff is too weak to gauge whether this is reasonable approach.

--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -8125,6 +8125,10 @@ build_new_method_call_1 (tree instance, tree fns, vec<tree, va_gc> **args,
   /* Dismantle the baselink to collect all the information we need.  */
   if (!conversion_path)
     conversion_path = BASELINK_BINFO (fns);
+  if (conversion_path == error_mark_node)
+    return error_mark_node;
   access_binfo = BASELINK_ACCESS_BINFO (fns);
   binfo = BASELINK_BINFO (fns);
   optype = BASELINK_OPTYPE (fns);
Comment 6 Paolo Carlini 2016-03-17 14:09:31 UTC
Thanks a lot Marek. The patch makes sense to me, I would recommend fully regression testing it and sending it to the mailing list.
Comment 7 Patrick Palka 2016-03-17 14:40:59 UTC
That would suppress the ICE but in turn it would make us silently generate wrong code since the call to D::f() won't be emitted.

adjust_result_of_qualified_name_lookup() is responsible for clobbering the BASELINK_BINFO of the function.  This seems to work:

--- a/gcc/cp/search.c
+++ b/gcc/cp/search.c
@@ -1751,9 +1751,11 @@ adjust_result_of_qualified_name_lookup (tree decl,
       if (base && base != error_mark_node)
          BASELINK_ACCESS_BINFO (decl) = base;
-         BASELINK_BINFO (decl)
+         tree decl_binfo
            = lookup_base (base, BINFO_TYPE (BASELINK_BINFO (decl)),
                           ba_unique, NULL, tf_none);
+         if (decl_binfo && decl_binfo != error_mark_node)
+           BASELINK_BINFO (decl) = decl_binfo;
Comment 8 Patrick Palka 2016-03-17 15:28:05 UTC
I'll post the above patch.
Comment 9 Paolo Carlini 2016-03-17 16:54:36 UTC
I see...
Comment 10 Patrick Palka 2016-03-18 01:27:22 UTC
Author: ppalka
Date: Fri Mar 18 01:26:50 2016
New Revision: 234317

URL: https://gcc.gnu.org/viewcvs?rev=234317&root=gcc&view=rev
Fix PR c++/70205 (ICE on valid call to qualified static member function)


	PR c++/70205
	* search.c (adjust_result_of_qualified_name_lookup): Don't
	update the BASELINK_BINFO of DECL if the second call
	to lookup_base fails.


	PR c++/70205
	* g++.dg/lookup/pr70205.C: New test.

Comment 11 Patrick Palka 2016-03-18 01:28:13 UTC
Fixed on GCC 6 so far.
Comment 12 Richard Biener 2016-08-03 11:56:45 UTC
GCC 4.9 branch is being closed
Comment 13 Jakub Jelinek 2017-10-10 14:48:05 UTC
GCC 5 branch has been closed, should be fixed in GCC 6 and later.