Bug 87906 - [9 Regression] ICE in tree check: expected block, have function_decl in block_ultimate_origin, at tree.c:12326 since r264734
Summary: [9 Regression] ICE in tree check: expected block, have function_decl in block...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: lto (show other bugs)
Version: 9.0
: P3 normal
Target Milestone: 9.0
Assignee: Richard Biener
URL:
Keywords: ice-on-valid-code
Depends on:
Blocks:
 
Reported: 2018-11-06 14:44 UTC by Martin Liška
Modified: 2018-11-07 08:25 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work: 8.2.0
Known to fail: 9.0
Last reconfirmed: 2018-11-06 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Liška 2018-11-06 14:44:17 UTC
Following 2 files cause an ICE:

$ cat 1.ii
namespace com {
namespace sun {
namespace star {}
} // namespace sun
} // namespace com
namespace a = com::sun::star;
namespace com {
namespace sun {
namespace star {
namespace uno {
class b {
public:
  ~b();
};
class c {
  b e;
};
class RuntimeException : c {};
} // namespace uno
} // namespace star
} // namespace sun
} // namespace com
template <typename> void d(int) { throw a::uno::RuntimeException(); }
int f;
void g() { d<a::uno::b>(f); }

$ cat 2.ii
namespace com {
namespace sun {
namespace star {
namespace uno {
class a {
public:
  ~a();
};
class b {
  a c;
};
class RuntimeException : b {};
} // namespace uno
class C : uno::RuntimeException {};
} // namespace star
} // namespace sun
} // namespace com
using com::sun::star::C;
using com::sun::star::uno::RuntimeException;
void d() { throw RuntimeException(); }
void e() { C(); }

$ g++ [12].ii -O -shared -fPIC -flto
1.ii:11:7: warning: type ‘struct b’ violates the C++ One Definition Rule [-Wodr]
   11 | class b {
      |       ^
1.ii:11:7: note: a different type is defined in another translation unit
2.ii:10:5: note: the first difference of corresponding definitions is field ‘c’
   10 |   a c;
      |     ^
1.ii:11:7: note: a type with different number of fields is defined in another translation unit
   11 | class b {
      |       ^
1.ii:18:7: warning: type ‘struct RuntimeException’ violates the C++ One Definition Rule [-Wodr]
   18 | class RuntimeException : c {};
      |       ^
1.ii:18:7: note: a type with different bases is defined in another translation unit
during GIMPLE pass: optimized
2.ii: In function ‘e’:
2.ii:21:6: internal compiler error: tree check: expected block, have function_decl in block_ultimate_origin, at tree.c:12326
   21 | void e() { C(); }
      |      ^
0x7d56a6 tree_check_failed(tree_node const*, char const*, int, char const*, ...)
	/home/marxin/Programming/gcc/gcc/tree.c:9544
0x7dca31 tree_check(tree_node*, char const*, int, char const*, tree_code)
	/home/marxin/Programming/gcc/gcc/tree.h:3118
0x7dca31 block_ultimate_origin(tree_node const*)
	/home/marxin/Programming/gcc/gcc/tree.c:12324
0x184709b inlined_polymorphic_ctor_dtor_block_p(tree_node*, bool)
	/home/marxin/Programming/gcc/gcc/ipa-polymorphic-call.c:516
0x2a06f91 remove_unused_scope_block_p
	/home/marxin/Programming/gcc/gcc/tree-ssa-live.c:420
0x2a074a5 remove_unused_scope_block_p
	/home/marxin/Programming/gcc/gcc/tree-ssa-live.c:516
0x2a074a5 remove_unused_scope_block_p
	/home/marxin/Programming/gcc/gcc/tree-ssa-live.c:516
0x2a0f17e remove_unused_locals()
	/home/marxin/Programming/gcc/gcc/tree-ssa-live.c:896
0x1e65964 execute_function_todo
	/home/marxin/Programming/gcc/gcc/passes.c:1920
0x1e6709e execute_todo
	/home/marxin/Programming/gcc/gcc/passes.c:1996
Comment 1 Martin Liška 2018-11-06 15:13:22 UTC
Better test-cases that do not violate ODR:

$ cat 1.ii
namespace com {
namespace sun {
namespace star {}
} // namespace sun
} // namespace com
namespace a = com::sun::star;
namespace com {
namespace sun {
namespace star {
namespace uno {
class a {
public:
  ~a();
};

class b {
public:
  ~b();
  a c;
};
class c {
  b e;
};
class RuntimeException : b {};
} // namespace uno
} // namespace star
} // namespace sun
} // namespace com
template <typename> void d(int) { throw a::uno::RuntimeException(); }
int f;
void g() { d<a::uno::b>(f); }

$ cat 2.ii
namespace com {
namespace sun {
namespace star {
namespace uno {
class a {
public:
  ~a();
};
class b {
  a c;
};
class RuntimeException : b {};
} // namespace uno
class C : uno::RuntimeException {};
} // namespace star
} // namespace sun
} // namespace com
using com::sun::star::C;
using com::sun::star::uno::RuntimeException;
void d() { throw RuntimeException(); }
void e() { C(); }
Comment 2 Richard Biener 2018-11-06 15:33:57 UTC
Ah, so the assert is somewhat bogus if it triggers:

12324         gcc_checking_assert ((DECL_P (origin)
12325                               && DECL_ORIGIN (origin) == origin)
12326                              || BLOCK_ORIGIN (origin) == origin);

when it's a DECL_P but DECL_ORIGIN is wrecked then we access a DECL as BLOCK.

(gdb) p origin
$3 = <function_decl 0x7ffff6a74f00 __dt_base >
(gdb) p origin->decl_common.abstract_origin 
$4 = <function_decl 0x7ffff6a74e00 __dt >

what this means is that when we do tree/decl merging we end up merging
sth with DECL_ORIGIN (x) == x to a prevailing node which does not
honor this property.  This makes

      BLOCK_ABSTRACT_ORIGIN (id->block) = DECL_ORIGIN (fn);

as set by the inliner not the ultimate origin and thus we ICE.

Indeed.

(gdb) commands 8
Type commands for breakpoint(s) 8, one per line.
End with a line saying just "end".
>p decl->decl_common.abstract_origin
>p prevailing->decl_common.abstract_origin
>end

Breakpoint 8, lto_symtab_prevail_decl (
    prevailing=<function_decl 0x7ffff6a74c00 __dt_base >, 
    decl=<function_decl 0x7ffff667a600 __dt_base >)
    at /space/rguenther/src/gcc-slpcost/gcc/lto/lto-symtab.h:34
34        gcc_checking_assert (! DECL_LANG_FLAG_0 (decl));
$3 = <tree 0x0>
$4 = <function_decl 0x7ffff6a74b00 __dt >

And that probably happens because of the ODR violation.
Comment 3 Richard Biener 2018-11-06 15:43:51 UTC
Iff we are sure abstract origin and BLOCK are never in the same SCC we can fixup
during stream-in, but it's still somewhat ugly and it feels that we are losing some debug info here (which clone we inlined).

diff --git a/gcc/tree-streamer-in.c b/gcc/tree-streamer-in.c
index bd98ed0b128..50433bf925c 100644
--- a/gcc/tree-streamer-in.c
+++ b/gcc/tree-streamer-in.c
@@ -915,6 +915,8 @@ lto_input_ts_block_tree_pointers (struct lto_input_block *ib,
 
   BLOCK_SUPERCONTEXT (expr) = stream_read_tree (ib, data_in);
   BLOCK_ABSTRACT_ORIGIN (expr) = stream_read_tree (ib, data_in);
+  if (DECL_P (BLOCK_ORIGIN (expr)))
+    BLOCK_ABSTRACT_ORIGIN (expr) = DECL_ORIGIN (BLOCK_ABSTRACT_ORIGIN (expr));
   /* Do not stream BLOCK_NONLOCALIZED_VARS.  We cannot handle debug information
      for early inlined BLOCKs so drop it on the floor instead of ICEing in
      dwarf2out.c.  */

when we'd stream this reference as DIE we would get away.  But of course
esp. the FUNCTION_DECLs referenced as part of the inline BLOCKs are very
hard to get rid of (BLOCKs as abstract origin not so).  And this isn't
stage1 material anymore I fear.
Comment 4 Richard Biener 2018-11-07 08:07:29 UTC
Author: rguenth
Date: Wed Nov  7 08:06:57 2018
New Revision: 265861

URL: https://gcc.gnu.org/viewcvs?rev=265861&root=gcc&view=rev
Log:
2018-11-07  Richard Biener  <rguenther@suse.de>

	PR lto/87906
	* tree-streamer-in.c (lto_input_ts_block_tree_pointers): Fixup
	BLOCK_ABSTRACT_ORIGIN to be the ultimate origin.

	* g++.dg/lto/pr87906_0.C: New testcase.
	* g++.dg/lto/pr87906_1.C: Likewise.

Added:
    trunk/gcc/testsuite/g++.dg/lto/pr87906_0.C
    trunk/gcc/testsuite/g++.dg/lto/pr87906_1.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-streamer-in.c
Comment 5 Richard Biener 2018-11-07 08:25:18 UTC
Fixed.