gccgo: assert in "implements_interface", types.cc:9145 with inlinable function #33013
Comments
ianlancetaylor
added
the
NeedsInvestigation
|
What would happen if we changed |
|
I am OK with that I suppose. Input::read_type gets called quite early, but then again finalize_methods happens pretty early as well. I will give it a try. |
|
It looks like an eager call to finalize_methods_for_type from Import::read_type has some problems -- what seems to happen is that we read a named type NT (but not its underlying type), create a forward declaration type for NT, then read another type T2 whose methods refer to NT. When we try to finalize methods for T2, we get an assert, e.g. #1 Forward_declaration_type::warn () at ../../gcc-trunk/gcc/go/gofrontend/types.cc:12278 #2 Forward_declaration_type::real_type () at ../../gcc-trunk/gcc/go/gofrontend/types.cc:12327 #3 Forward_declaration_type::do_hash_for_method () at ../../gcc-trunk/gcc/go/gofrontend/types.h:3760 #4 Type::hash_for_method () at ../../gcc-trunk/gcc/go/gofrontend/types.cc:943 ... I'm going to propose an alternative, which is to change the importer to make a second pass through the types array after exported types are read in and call finalize_methods_for_type then. |
|
Change https://golang.org/cl/185517 mentions this issue: |
|
Change https://golang.org/cl/185518 mentions this issue: |
Updates #33013 Change-Id: I3db062b37860bb0c6c99a553408b47cf0313531e Reviewed-on: https://go-review.googlesource.com/c/go/+/185517 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Ian Lance Taylor <iant@golang.org>
This patch changes the compiler to be more aggressive about finalizing
methods on imported types, to avoid problems with interface types that
are imported but remain unreachable until a later stage in the compilation.
The normal pattern prior to this change was that the import process would
leave imported interface types alone, and rely on Gogo::finalize_methods
to locate and finalize all interface types at a later point. This way
of doing things was not working in all cases due to the fact that we can
import an interface type that is only reachable from the body of an
inlinable function, meaning that we can't "find" the type during
the methods finalize phase.
The importer's Import::read_types() now makes a pass over all imported
types to finalize methods on any newly imported type, which takes care
of the issue.
New test case for this problem in CL 185517.
Fixes golang/go#33013
Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/185518
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@273364 138bc75d-0d04-0410-961f-82ee72b054a4
This patch changes the compiler to be more aggressive about finalizing
methods on imported types, to avoid problems with interface types that
are imported but remain unreachable until a later stage in the compilation.
The normal pattern prior to this change was that the import process would
leave imported interface types alone, and rely on Gogo::finalize_methods
to locate and finalize all interface types at a later point. This way
of doing things was not working in all cases due to the fact that we can
import an interface type that is only reachable from the body of an
inlinable function, meaning that we can't "find" the type during
the methods finalize phase.
The importer's Import::read_types() now makes a pass over all imported
types to finalize methods on any newly imported type, which takes care
of the issue.
New test case for this problem in CL 185517.
Fixes golang/go#33013
Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/185518
From-SVN: r273364
What version of Go are you using (
go version)?Using gccgo tip
What operating system and processor architecture are you using (
go env)?linux/amd64
What did you do?
Build the following three packages: first "a", then "b", and finally "c".
Package "a":
Package "b":
Package "c":
What did you expect to see?
Clean compile
What did you see instead?
Assert as follows:
What appears to be happening is as follows:
types 30 3 ... type 1 "Service" type 2 "ServiceDesc" type 3 ".a.uc" type 4 "context.Context" ... type 22 struct { X ; ? ; } type 27 interface { } type 28 interface { .a.f () ; } type 29 interface { Deadline () (deadline , ok ); Done () ; Err () ; Value (key ) ; }Note that there is a chain of type references from one of the exported types ("ServiceDesc") to the empty interface type corresponding to "type 27", which gets created eagerly (since it is needed by an exported type).
types 33 2 ... type 1 "BI" type 2 ".a.uc" type 3 "a.Service" type 4 "a.ServiceDesc" type 5 "context.Context" type 29 interface { } type 30 interface { .a.f () ; } type 31 interface { Another (pxp ) ; Something (s ) ; } type 32 interface { Deadline () (deadline , ok ); Done () ; Err () ; Value (key ) ; } func BRS (sd , server , xyz ) ($ret0 ) // /ssd2/go1/src/b/b.go:13 { //14 $ret0 = RS(sd, $convert(, server), $convert(, 7 )) //14 return //14 } //0Key thing to note is that we have an exported type 'BI', which refers to context.Context. Since context.Context was encountered previously, we discover this fact at the point where we import the named type, and so we reuse the previously established Type object. See import.cc line 1091.
However we continue on to read the methods (import.cc line 1103, meaning that we create a new empty interface corresponding to "type 29" (which again gets created eagerly).
The key difference here is that there is no reference to this type from an exported type in the package (we broke the link by using the previous definition of conext.Context).
Compiler calls Gogo::finalize_methods. This visits all of the various methods reachable from package bindings, but it skips "type 29" (as described above) since there is no way to reach it.
Compiler calls Gogo::lower_parse_tree(). This has the side effect of bringing in the body of the inlineable function "BRS" from package "b". We now have a live reference to type 29.
Compiler calls check_types() -- now we encounter type 29, and assert since it wasn't exposed to finalize_methods().
Possible approaches to fixing this.
One possibility would be to change Import::read_named_type so that if it encounters a previously defined type it doesn't read it at all -- just returns the previously defined type. I prototyped this, but I don't think it will work in this specific case: there is still a dangling reference to 'type 29' via the prototype for the "a.RS" declaration, so the phantom type still gets created on import. We'd need some way to skip it as well.
Another hacky solution would be to set an interface finalized on creation if its parse_methods are empty.
The text was updated successfully, but these errors were encountered: