Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

gccgo: bug in type descriptor equality #32901

Closed
cherrymui opened this issue Jul 2, 2019 · 4 comments
Closed

gccgo: bug in type descriptor equality #32901

cherrymui opened this issue Jul 2, 2019 · 4 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Gccgo

Comments

@cherrymui
Copy link
Member

What version of Go are you using (go version)?

gccgo (GCC) 10.0.0 20190702 (experimental)

What did you do?

Build and run the program below, which has four packages: main imports p, which imports q, which imports r.

main.go

package main
  
import "p"
import "reflect"

func main() {
        x := p.F()
        pp := p.P()
        t := reflect.PtrTo(reflect.TypeOf(x))
        tp := reflect.TypeOf(pp)
        println(t == tp) // should print true
}

p.go

package p
  
import "q"

func F() interface{} {
        go func(){}() // make it non-inlineable
        return q.F()
}

func P() interface{} {
        go func(){}() // make it non-inlineable
        return q.P()
}

q.go

package q
  
import "r"

func F() interface{} {
        return r.F()
}

func P() interface{} {
        return r.P()
}

r.go

package r
  
type T struct { x int }

func F() interface{} {
        return [2]T{}
}

func P() interface{} {
        return &[2]T{}
}

What did you expect to see?

Print true.

What did you see instead?

Print false.

The problem is that in CL 179598 we were using Gogo::packages_ (when compiling the main package) as the list of packages to register type descriptors. But if I understand correctly, this list only includes the direct imports of main, and (one level) indirect imports. It does not include all the packages transitively imported. In the example above, "r" is not included, therefore the type descriptors there are not registered, and not deduplicated with reflect-created types.
@gopherbot gopherbot added this to the Gccgo milestone Jul 2, 2019
@gopherbot
Copy link

Change https://golang.org/cl/184718 mentions this issue: test: add a test for gccgo bug #32901

@gopherbot
Copy link

Change https://golang.org/cl/184717 mentions this issue: compiler: include transitive imports in the type descriptor list

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 2, 2019
gopherbot pushed a commit that referenced this issue Jul 3, 2019
@cherrymui
test: add a test for gccgo bug #32901
1c1e517 
This CL adds a test for gccgo bug #32901: not all the type
descriptors are registered and thus deduplicated with types
created by reflection. It needs a few levels of indirect imports
to trigger this bug.

Updates #32901.

Change-Id: Idbd89bedd63fea746769f2687f3f31c9767e5ec0
Reviewed-on: https://go-review.googlesource.com/c/go/+/184718
Reviewed-by: Ian Lance Taylor <iant@golang.org>
kraj pushed a commit to kraj/gcc that referenced this issue Jul 3, 2019
compiler: include transitive imports in the type descriptor list
f825856 
    
    In CL 179598, we were using Gogo::packages_, when compiling the
    main package, as the list of packages of which we need to
    register the type descriptors. This is not complete. It only
    includes main's direct import and one-level indirect imports. It
    does not include all the packages transitively imported.
    
    To fix that, we need to track all the transitive imports. We
    have almost already done that, for init functions. However, there
    may be packages that don't need init functions but do need to
    register type descriptors. For them, we add a dummy init function
    to its export data. So when we compile the main package we will
    see all the transitive imports. The dummy init functions are not
    real functions and are not called.
    
    Fixes golang/go#32901.
    
    Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/184717


git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@273009 138bc75d-0d04-0410-961f-82ee72b054a4
@cherrymui
Copy link
Member Author

Fixed in https://golang.org/cl/184717. Not sure why this isn't auto closed...

@cherrymui
Copy link
Member Author

Not sure why this isn't auto closed...

Probably due to #32931.

gopherbot pushed a commit to golang/gofrontend that referenced this issue Jul 3, 2019
@cherrymui @ianlancetaylor
compiler: include transitive imports in the type descriptor list
ae7d7e0 
In CL 179598, we were using Gogo::packages_, when compiling the
main package, as the list of packages of which we need to
register the type descriptors. This is not complete. It only
includes main's direct import and one-level indirect imports. It
does not include all the packages transitively imported.

To fix that, we need to track all the transitive imports. We
have almost already done that, for init functions. However, there
may be packages that don't need init functions but do need to
register type descriptors. For them, we add a dummy init function
to its export data. So when we compile the main package we will
see all the transitive imports. The dummy init functions are not
real functions and are not called.

Fixes golang/go#32901.

Change-Id: I683045273fb1b2ac7cf41617e641d8b4340b85a9
Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/184717
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@golang golang locked and limited conversation to collaborators Jul 2, 2020
asiekierka pushed a commit to WonderfulToolchain/gcc-ia16 that referenced this issue May 16, 2022
compiler: include transitive imports in the type descriptor list
a35d6e9 
    
    In CL 179598, we were using Gogo::packages_, when compiling the
    main package, as the list of packages of which we need to
    register the type descriptors. This is not complete. It only
    includes main's direct import and one-level indirect imports. It
    does not include all the packages transitively imported.
    
    To fix that, we need to track all the transitive imports. We
    have almost already done that, for init functions. However, there
    may be packages that don't need init functions but do need to
    register type descriptors. For them, we add a dummy init function
    to its export data. So when we compile the main package we will
    see all the transitive imports. The dummy init functions are not
    real functions and are not called.
    
    Fixes golang/go#32901.
    
    Reviewed-on: https://go-review.googlesource.com/c/gofrontend/+/184717

From-SVN: r273009
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Gccgo
Development

No branches or pull requests
3 participants
@bcmills @gopherbot @cherrymui