cmd/asm: referencing a variable from assembly in vendored package fails #28230

kriskwiatkowski · 2018-10-16T13:26:42Z

Please answer these questions before submitting your issue. Thanks!

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

go version go1.10.4 linux/amd64

Does this issue reproduce with the latest release?

Well, not exactly. I still get an error but different

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

GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"

What did you do?

From assembly I reference a variable define in the same package. It works OK as long as package is not vendored. Best way to reproduce is to apply the patch below and run

patch -p1 << 'EOF'
commit 717ea40b6b5455fa3dbab7979ade0818f04b2e0d
Author: Kris Kwiatkowski <kris@amongbytes.com>
Date:   Tue Oct 16 13:59:16 2018 +0100

    ASM vendoring problem

diff --git a/gopath/src/github.com/example/main.go b/gopath/src/github.com/example/main.go
new file mode 100644
index 0000000..52ac11c
--- /dev/null
+++ b/gopath/src/github.com/example/main.go
@@ -0,0 +1,7 @@
+package main
+
+import "github.com/test/test"
+
+func main() {
+	print(test.Add(1)); print("\n")
+}
\ No newline at end of file
diff --git a/gopath/src/github.com/test/test/test.go b/gopath/src/github.com/test/test/test.go
new file mode 100644
index 0000000..22e8d7a
--- /dev/null
+++ b/gopath/src/github.com/test/test/test.go
@@ -0,0 +1,4 @@
+package test
+
+var SixtyNine = 69
+func Add(int) int
\ No newline at end of file
diff --git a/gopath/src/github.com/test/test/test.s b/gopath/src/github.com/test/test/test.s
new file mode 100644
index 0000000..4b00f15
--- /dev/null
+++ b/gopath/src/github.com/test/test/test.s
@@ -0,0 +1,6 @@
+#include "textflag.h"
+TEXT ·Add(SB), NOSPLIT, $16
+	MOVQ github·com∕test∕test·SixtyNine(SB), R9
+	ADDQ x+0(FP), R9
+	MOVQ R9, ret+8(FP)
+	RET
EOF

GOPATH=`pwd`/gopath go run gopath/src/github.com/example/main.go

The result is "70"

Now if I "vendor" the library "github.com/test/test":

mkdir -p gopath/src/github.com/example/vendor/github.com
cp -rf gopath/src/github.com/test gopath/src/github.com/example/vendor/github.com 
GOPATH=`pwd`/gopath go run gopath/src/github.com/example/main.go

I get an error:

# command-line-arguments
github.com/example/vendor/github.com/test/test.Add: relocation target github.com/test/test.SixtyNine not defined
github.com/example/vendor/github.com/test/test.Add: undefined: "github.com/test/test.SixtyNine"

What did you expect to see?

No difference in program behaviour between in vendored and non-vendored library

The text was updated successfully, but these errors were encountered:

ianlancetaylor · 2018-10-16T13:53:00Z

Am I understanding correctly that you are writing assembly code that refers to a variable defined in a different package? And then your code breaks if your packages are vendored in a different directory? As you've discovered, I don't think that is going to work. I don't really see any way that it could ever work.

kriskwiatkowski · 2018-10-16T14:00:34Z

nop, the assembly code is referring variable in the same package. I'm asking myself 2 questions here:

Why it breaks when package is vendored?
Why it works correctly when package is not vendored?

Thanks for providing reference to the spec describing this behavior

ianlancetaylor · 2018-10-16T15:00:22Z

To refer to a variable in the same package, don't write out the package path. Just use a leading ·.

kriskwiatkowski · 2018-10-16T15:40:19Z

What if assembly code is referring to the variable from "internal" package? Is this allowed?

ianlancetaylor · 2018-10-16T22:35:45Z

I'm not sure what you mean by an "internal" package. In general I don't think it's going to work to have assembly code refer to a variable defined in a different package and to then try to vendor that package.

kriskwiatkowski · 2018-10-17T06:29:52Z

I mean package in "internal" folder, but that's unrelated to this issue.

In the spec I don't see why vendored package would be treated in a different way by the compiler than non-vendored, so my understanding is that this is a bug. But maybe there is something I don't see

ianlancetaylor · 2018-10-17T13:54:30Z

The spec doesn't discuss accessing variables in other packages from assembly code. It's not a feature of the language at all. Assembly code can access variables from the package that it is in, but there is no documented or supported way to access variables from other packages. It happens to work in some cases, and you've found a case where it doesn't work. Either way, it's not documented and not supported.

I'm not opposed to changing something so that this works but I don't at present see how it could be done.

ianlancetaylor · 2018-10-17T13:55:23Z

Since there is no plan to do anything I'm going to close this issue. But please feel free to comment if you disagree.

kriskwiatkowski · 2018-10-17T14:25:32Z

Ok, if it simply doesn't work, than fine, I coudln't care less. I just want to get clear statement.
So, in this case variable is in the same package -that's not a problem here, let's put it out of the table. I'm referencing the variable as it would be in different package - that is what's causing a problem. In ASM reference to the variable must not contain root of that source repository.
Correct?

ianlancetaylor · 2018-10-17T14:29:13Z

There simply isn't any supported way for assembly code to refer to a variable defined in a different package.

kriskwiatkowski · 2018-10-17T14:39:40Z

yeah, I think that part was clear since beginning

* Assembly code can't reffer to variables which are not in the same package. * utils/cpu.go keeps flags indicating CPU capabilities. Currently this file keeps flags specific to X86, those flags are set only if program is run on X86, hence on ARM they always be false. Flags are coppied to variables which are local to P503 and P751 (so that assembly code can access them). * we dont need workaround in utils/utils.go anymore as utils/cpu.go is never ignored by the buildtags. See also: golang/go#28230 Fixes #21

fix: fixes vendoring issue * Assembly code can't reffer to variables which are not in the same package. * utils/cpu.go keeps flags indicating CPU capabilities. Currently this file keeps flags specific to X86, those flags are set only if program is run on X86, hence on ARM they always be false. Flags are coppied to variables which are local to P503 and P751 (so that assembly code can access them). * we dont need workaround in utils/utils.go anymore as utils/cpu.go is never ignored by the buildtags. See also: golang/go#28230

kriskwiatkowski changed the title ~~Referencing a variable from vendored package fails~~ Referencing a variable from assemly in vendored package fails Oct 16, 2018

ianlancetaylor changed the title ~~Referencing a variable from assemly in vendored package fails~~ cmd/asm: referencing a variable from assembly in vendored package fails Oct 16, 2018

kriskwiatkowski mentioned this issue Oct 16, 2018

fix: fixes vendoring issue cloudflarearchive/sidh#22

Merged

FiloSottile added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Oct 16, 2018

FiloSottile added this to the Unplanned milestone Oct 16, 2018

ianlancetaylor closed this as completed Oct 17, 2018

golang locked and limited conversation to collaborators Oct 17, 2019

gopherbot added the FrozenDueToAge label Oct 17, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/asm: referencing a variable from assembly in vendored package fails #28230

cmd/asm: referencing a variable from assembly in vendored package fails #28230

kriskwiatkowski commented Oct 16, 2018 •

edited

ianlancetaylor commented Oct 16, 2018

kriskwiatkowski commented Oct 16, 2018 •

edited

ianlancetaylor commented Oct 16, 2018

kriskwiatkowski commented Oct 16, 2018

ianlancetaylor commented Oct 16, 2018

kriskwiatkowski commented Oct 17, 2018 •

edited

ianlancetaylor commented Oct 17, 2018

ianlancetaylor commented Oct 17, 2018

kriskwiatkowski commented Oct 17, 2018

ianlancetaylor commented Oct 17, 2018

kriskwiatkowski commented Oct 17, 2018

cmd/asm: referencing a variable from assembly in vendored package fails #28230

cmd/asm: referencing a variable from assembly in vendored package fails #28230

Comments

kriskwiatkowski commented Oct 16, 2018 • edited

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

Does this issue reproduce with the latest release?

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

What did you do?

What did you expect to see?

ianlancetaylor commented Oct 16, 2018

kriskwiatkowski commented Oct 16, 2018 • edited

ianlancetaylor commented Oct 16, 2018

kriskwiatkowski commented Oct 16, 2018

ianlancetaylor commented Oct 16, 2018

kriskwiatkowski commented Oct 17, 2018 • edited

ianlancetaylor commented Oct 17, 2018

ianlancetaylor commented Oct 17, 2018

kriskwiatkowski commented Oct 17, 2018

ianlancetaylor commented Oct 17, 2018

kriskwiatkowski commented Oct 17, 2018

kriskwiatkowski commented Oct 16, 2018 •

edited

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

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

kriskwiatkowski commented Oct 16, 2018 •

edited

kriskwiatkowski commented Oct 17, 2018 •

edited