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: -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
Assignees
Milestone

Comments

@mark-rushakoff
Copy link
Contributor

@mark-rushakoff mark-rushakoff commented Jan 15, 2020

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 this to the Go1.14 milestone Jan 15, 2020
@ALTree

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Contributor

@jayconrod jayconrod commented Jan 15, 2020

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

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jan 15, 2020

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

@cmitchell

This comment has been minimized.

Copy link

@cmitchell cmitchell commented Jan 16, 2020

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.

@gopherbot gopherbot closed this in 574c286 Jan 16, 2020
@jayconrod

This comment has been minimized.

Copy link
Contributor

@jayconrod jayconrod commented Jan 16, 2020

@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

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jan 22, 2020

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

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jan 23, 2020

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
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.