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

cmd/go: confusing error when using cgo with Go assembly code #19448

Closed
FiloSottile opened this issue Mar 8, 2017 · 5 comments
Closed

cmd/go: confusing error when using cgo with Go assembly code #19448

FiloSottile opened this issue Mar 8, 2017 · 5 comments
Milestone

Comments

@FiloSottile
Copy link
Contributor

FiloSottile commented Mar 8, 2017

Currently if a package has a import "C" all .s files will be compiled by the C compiler, which of course rejects Go assembly.

When the Go tool sees that one or more Go files use the special import "C", it will look for other non-Go files in the directory and compile them as part of the Go package. Any .c, .s, or .S files will be compiled with the C compiler.

clang -I . -fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=$WORK=/tmp/go-build -gno-record-gcc-switches -fno-common -I $WORK/github.com/FiloSottile/cgo-assembly/_obj/ -g -O2 -o $WORK/github.com/FiloSottile/cgo-assembly/_obj/add_amd64.o -c ./add_amd64.s
# github.com/FiloSottile/cgo-assembly
./add_amd64.s:1:24: error: unexpected token in memory operand
DATA p256const0<>+0x00(SB)/8, $0x00000000ffffffff
                       ^

I suspect this is asking too much, but it would be useful if there was a way to still compile assembly files with the Go assembler in a cgo package.

As a lower bar, it would be useful to get more meaningful error messages, as syntax errors appearing when adding import "C" in a different file are pretty obscure.

@ianlancetaylor ianlancetaylor changed the title cmd/cgo: no way of compiling Go assembly cmd/go: confusing error when using cgo with Go assembly code Mar 8, 2017
@ianlancetaylor
Copy link
Contributor

In retrospect perhaps we should have always assembled .s files with the Go assembler and .S files with the system assembler, but I think it's too late now. Changing things now would be too likely to break too many existing packages.

Generating a more meaningful error message sounds good, but I'm not sure how. I'm open to suggestions.

@ianlancetaylor ianlancetaylor added this to the Go1.9 milestone Mar 8, 2017
@FiloSottile
Copy link
Contributor Author

What do you think of adding a new suffix for code to be always assembled by the Go assembler? Like _go.s. It's unlikely to break existing packages, and can be made even more unlikely by picking something more complex like _goasm.s.

@ianlancetaylor
Copy link
Contributor

I can't say that I'm particularly fond of it, but I suggest that you send a message to the golang-dev mailing list and see what people think about the idea.

@rsc
Copy link
Contributor

rsc commented Jun 22, 2017

I'm loathe to introduce new semantics here when we've gotten this far without them. But I will add a heuristic test to produce a better error in this case.

@gopherbot
Copy link

CL https://golang.org/cl/46423 mentions this issue.

@golang golang locked and limited conversation to collaborators Jun 23, 2018
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

4 participants