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
cmd/link: improve error for missing method body #48780
Comments
Good point. I guess one difficulty is that for an undefined symbol the linker doesn't know if it is supposed to be a function or not. Maybe we could make some guess, like assuming all ABIInternal references are functions (which will probably help this one), or name-based guessing. |
Or we could say "undefined function or variable". |
@ianlancetaylor @cherrymui Hi, I'd like to pick this issue up? Can you please assign it, if it isn't being worked on already? Also could you point me to the relevant files for the error definitions? |
@deepto98 thanks. The error is emitted at https://cs.opensource.google/go/go/+/master:src/cmd/link/internal/ld/errors.go;l=63 |
@cherrymui @ianlancetaylor I'm trying to reproduce this issue, but I get a different error for the code snippet given: |
@deepto98 Please include plain text messages as plain text, not as an image. Images are very hard to read. Thanks. The error in the image appears to be from the external linker. Unfortunately I don't know that there is much that we can do about this when in external linking mode. But I also don't know why you didn't see the same error that I did. What operating system are you on? I am on amd64-linux. |
Okay, sure, will keep this in mind about error messages. I'm also on linux (Ubuntu 20.04) and the go version I'm using is go1.12.2 gccgo (Ubuntu 9.3.0-17ubuntu1~20.04) 9.3.0 linux/amd64 . I added the code to a file (foo.go) and ran go build foo.go |
@ianlancetaylor I wasn't aware about external and internal linking, and just read up on it. Do you know how I can change the config to switch to internal linking? |
This issue is not about gccgo. This is about the Go toolchain distributed from https://golang.org/dl/ , also known as "the gc toolchain". |
@cherrymui okay, got it, my bad. I just installed go 1.16.8 and was able to reproduce the error you'd mentioned initially |
@cherrymui Should I change the error message format to say type.main.S: undefined function or variable? |
I don't know. I think it is important to print the undefined symbol name, i.e. the |
okay |
Does the linker have any way of knowing that an undefined symbol is a function? I can imagine various ways to determining that but I don't know if the linker currently does any of them. |
okay, I'm looking into this |
@ianlancetaylor I don't think the linker has a very definitive answer. I posted some thoughts in #48780 (comment) |
@cherrymui @ianlancetaylor from what I found out for empty variable definitions, they're already handled in /go/src/go/parser/parser.go with messages missing variable type or initialization or missing constant value, so maybe we can assume that ABI reference symbols are functions, as @cherrymui had suggested? |
We may have to consider references from assembly code. |
@ianlancetaylor Okay, I'm looking into this. Can please you point me to some resources so I can test this with assembly? |
An example of assembly code referring to a variable is runtime/asm_amd64.s. These lines assign to the variables
Those variables are defined in the same assembly file using:
If those |
The language permits omitting the method body. However, the error message for this could be clearer.
Test case (uses
import "C"
to force the compiler to assume that method bodies are defined elsewhere):Running
go build foo.go
producesThis error message is accurate but conveys little information to someone unfamiliar with linker jargon like "relocation target." We should instead say "missing function body", though unfortunately it may be difficult to give a good file/line position for the declaration with the missing body.
The text was updated successfully, but these errors were encountered: