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

go/types: Implements function returns wrong result #53156

Closed
hoyhbx opened this issue May 31, 2022 · 2 comments
Closed

go/types: Implements function returns wrong result #53156

hoyhbx opened this issue May 31, 2022 · 2 comments

Comments

@hoyhbx
Copy link

hoyhbx commented May 31, 2022

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

$ go version
go version go1.18.2 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GOHOSTARCH="amd64"
GOHOSTOS="linux"
...

What did you do?

We found that the func Implements(V [Type], T *[Interface]) [bool] does not return correct result.
We initially tried to find all the types in the go/ssa package that implements the *ssa.Instruction interface using the Implement function, but we found that the Implements function returns wrong results.

To reproduce

We wrote a minimal program to reproduce this bug.
In the sample.go program, there is one interface Bar which has two methods: String() and Name(), there is another struct Foo that implements this Bar interface.

Inside main.go, we call types.Implements(Bar, Foo) and expect it to return true, as the Struct Foo clearly implements the interface Bar.
However, the Implements function returns false. We include our investigation result below.

reproduce.zip

What did you expect to see?

We expect to see the Implements returns correctly.

What did you see instead?

In our reproduce code, the named type Foo clearly implements the interface Bar, but the Implements function returns false.

Root cause

After some investigation, we found that the logic of the inner function missingMethod is wrong. On the line https://cs.opensource.google/go/go/+/refs/tags/go1.18.2:src/go/types/lookup.go;drc=52f68efa45a34e60e8fc5a2ad5fc124a865ac2a4;l=359 , the function missingMethod returns a method if the variable found is false. The variable found is set here if lookupFieldOrMethod returns nil. However, since the code retries lookupFieldOrMethod with pointer type and fold case later, the variable found should be updated if lookupFieldOrMethod returns some non-nil result after the line.

Additional notes

We initially found this wrong result when we were using go1.17, and we found that the original issue has been partially fixed in 1.18. However, as described above, the 1.18.2 is still not fixed.
Considering severity, we recommend to backport the fix to previous versions.

@dominikh
Copy link
Member

In your example code, *Foo implements Bar, but Foo does not.

@hoyhbx
Copy link
Author

hoyhbx commented May 31, 2022

Hmmm, I see the alt return value now, I probably misunderstood the spec of this function. It seems that the alternative method is returned when the Pointer type implements the method.

@hoyhbx hoyhbx closed this as completed May 31, 2022
@dominikh dominikh closed this as not planned Won't fix, can't repro, duplicate, stale May 31, 2022
@golang golang locked and limited conversation to collaborators May 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants