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

Jump to bottom

cmd/go: -trimpath with -mod=vendor doesn't trim path #36566

Closed
mark-rushakoff opened this issue Jan 15, 2020 · 7 comments
Closed

cmd/go: -trimpath with -mod=vendor doesn't trim path #36566

mark-rushakoff opened this issue Jan 15, 2020 · 7 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone
Go1.14

Comments

@mark-rushakoff
Copy link
Contributor

Given a single file main.go in a module:

package main

import "golang.org/x/text/search"

func main() {
	var p search.Pattern
	x, y := p.Index(nil, search.Backwards) // Deliberate panic here.
	println(x, y)
}

With go version go1.13.6 darwin/amd64, after running go mod vendor, then go run -mod=vendor -trimpath main.go returns the following output:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x10a9cbc]

goroutine 1 [running]:
golang.org/x/text/search.(*Pattern).Index(0xc000076f30, 0x0, 0x0, 0x0, 0xc000076f1f, 0x1, 0x1, 0xc000042750, 0xc000076f50)
	golang.org/x/text@v0.0.0-20170915032832-14c0d48ead0c/search/search.go:177 +0x9c
main.main()
	example.com/trimpath@/main.go:7 +0x7f
exit status 2

Notice that the location is printed as golang.org/x/text@v0....

With go tip at go version devel +71154e061f Tue Jan 14 17:13:34 2020 +0000 darwin/amd64, running go run -mod=vendor -trimpath main.go produces this output:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x10ad40c]

goroutine 1 [running]:
golang.org/x/text/search.(*Pattern).Index(0xc000096f58, 0x0, 0x0, 0x0, 0xc000096f47, 0x1, 0x1, 0xc000044778, 0xc000096f78)
	/tmp/x/vendor/golang.org/x/text/search/search.go:177 +0x9c
main.main()
	example.com/trimpath@/main.go:7 +0x7e
exit status 2

Notice that now, the vendored module has changed to its absolute path on disk, /tmp/x/vendor/....

I bisected to 6cba4db as the commit which introduced this path-based output.

$ gotip version && gotip run -mod=vendor -trimpath main.go
go version devel +1736f3a126 Wed Oct 9 18:12:31 2019 +0000 darwin/amd64
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x10a4acc]

goroutine 1 [running]:
golang.org/x/text/search.(*Pattern).Index(0xc000076f30, 0x0, 0x0, 0x0, 0xc000076f1f, 0x1, 0x1, 0xc000042750, 0xc000076f50)
	golang.org/x/text@v0.0.0-20170915032832-14c0d48ead0c/search/search.go:177 +0x9c
main.main()
	example.com/trimpath@/main.go:7 +0x7e
exit status 2

$ gotip version && gotip run -mod=vendor -trimpath main.go
go version devel +6cba4dbf80 Wed Oct 9 18:39:22 2019 +0000 darwin/amd64
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x8 pc=0x10a4acc]

goroutine 1 [running]:
golang.org/x/text/search.(*Pattern).Index(0xc000076f30, 0x0, 0x0, 0x0, 0xc000076f1f, 0x1, 0x1, 0xc000042750, 0xc000076f50)
	/tmp/x/vendor/golang.org/x/text/search/search.go:177 +0x9c
main.main()
	example.com/trimpath@/main.go:7 +0x7e
exit status 2

With the new behavior since 6cba4db, -trimpath no longer works as documented in go help build ("remove all file system paths from the resulting executable").

/cc @bcmills @jayconrod
@ALTree ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 15, 2020
@ALTree ALTree added this to the Go1.14 milestone Jan 15, 2020
@ALTree
Copy link
Member

ALTree commented Jan 15, 2020

Tentatively putting in the 1.14 milestone since if the bisection is correct this was introduced in this development cycle.

@jayconrod jayconrod self-assigned this Jan 15, 2020
@jayconrod jayconrod added NeedsFix The path to resolution is known, but the work has not been done. release-blocker and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jan 15, 2020
@jayconrod
Copy link
Contributor

cc @bcmills @matloob

It looks like this is happening because we don't set the module root directory with -mod=vendor anymore. We're passing a -trimpath argument to the compiler like this:

-trimpath "$WORK/b002=>;=>golang.org/x/text@v0.3.2"

It should be:

-trimpath "$WORK/b002=>;/tmp/x/vendor/golang.org/x/text=>golang.org/x/text@v0.3.2"

This can also be seen with go list -f '{{.Module.Dir}}' golang.org/x/text/search.

@gopherbot
Copy link

Change https://golang.org/cl/214945 mentions this issue: cmd/go: trim paths from vendored packages with -trimpath

@cmitchell
Copy link

Is there any way this could be considered for a backport to the next go1.13 as well? I wouldn't be able to upgrade to 1.14 for sometime due to that version automatically switching over to TLS 1.3 and removing any switch to fallback to TLS1.2 but man have I hit this issue. I think this may have started with go1.13.5 honestly.

@jayconrod
Copy link
Contributor

@cmitchell This issue was a regression introduced on the 1.14 development branch. I just confirmed it doesn't reproduce in 1.13.6.

Could you open a new issue with steps to reproduce the problem you're seeing?

@gopherbot
Copy link

Change https://golang.org/cl/215940 mentions this issue: cmd/go: unify trimpath logic for -mod=vendor, -mod=mod

gopherbot pushed a commit that referenced this issue Jan 23, 2020
cmd/go: unify trimpath logic for -mod=vendor, -mod=mod
64378c2 
If a package has a module with a version, the package's directory is
replaced with the module path and version, followed by the package's
path within the module.

This is a follow up to CL 214945. We no longer check whether the
module has a directory (with -mod=vendor, it does not).

Updates #36566

Change-Id: I5bc952b13bc7b4659f58ee555bd6c6a087eb7792
Reviewed-on: https://go-review.googlesource.com/c/go/+/215940
Run-TryBot: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
@gopherbot
Copy link

Change https://golang.org/cl/216018 mentions this issue: cmd/go: add a control case to the mod_vendor_trimpath test

gopherbot pushed a commit that referenced this issue Jan 23, 2020
cmd/go: add a control case to the mod_vendor_trimpath test
39ea3d0 
In reviewing CL 215940, it took me a while to find the control
condition for the test, which was located in build_cache_trimpath.txt.

We could consolidate the two tests, but since they check for
regressions of separate issues (with separate root-causes), I think it
makes sense to keep them separate.

However, I would like the control condition to be present in the same
source file, so that we'll be more likely to update both cases if the
behavior of one of them is changed.

Updates #36566

Change-Id: Ic588f1dfb7977dd78d1d5ef61b9841e22bad82e6
Reviewed-on: https://go-review.googlesource.com/c/go/+/216018
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
@golang golang locked and limited conversation to collaborators Jan 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Milestone
Go1.14
Development

No branches or pull requests
5 participants
@cmitchell @jayconrod @mark-rushakoff @ALTree @gopherbot