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

Comments

Projects
None yet
4 participants
@FiloSottile
Member

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 from cmd/cgo: no way of compiling Go assembly to cmd/go: confusing error when using cgo with Go assembly code Mar 8, 2017

@ianlancetaylor

This comment has been minimized.

Show comment
Hide comment
@ianlancetaylor

ianlancetaylor Mar 8, 2017

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.

Contributor

ianlancetaylor commented Mar 8, 2017

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

This comment has been minimized.

Show comment
Hide comment
@FiloSottile

FiloSottile Mar 8, 2017

Member

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.

Member

FiloSottile commented Mar 8, 2017

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

This comment has been minimized.

Show comment
Hide comment
@ianlancetaylor

ianlancetaylor Mar 8, 2017

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.

Contributor

ianlancetaylor commented Mar 8, 2017

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

This comment has been minimized.

Show comment
Hide comment
@rsc

rsc Jun 22, 2017

Contributor

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.

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

This comment has been minimized.

Show comment
Hide comment
@gopherbot

gopherbot commented Jun 22, 2017

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

@gopherbot gopherbot closed this in a6df299 Jun 23, 2017

@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.