x/sys/windows/mkwinsyscall: IsStdRepo can be wrong when Go tree is inside a symlinked directory

[dmitshur]

The Source.IsStdRepo method tries to report whether src is part of standard library. It can return an incorrect result when src is a part of standard library but the Go tree is inside a symlinked directory.

To reproduce:

$ mkdir -p $HOME/dir1/dir2 && cd $HOME/dir1/dir2
$ git clone https://go.googlesource.com/go && cd go/src
$ ./make.bash

$ ln -s $HOME/dir1/dir2 $HOME/aaa && cd $HOME/aaa/go/src
$ export GOROOT="$HOME/aaa/go"
$ export PATH="$HOME/aaa/go/bin:$PATH"
$ go generate syscall
$ git status
[...]
	modified:   syscall/zsyscall_windows.go
[...]
$ git diff
[...]
 import (
-       "internal/syscall/windows/sysdll"
        "unsafe"
+
+       "golang.org/x/sys/windows"
 )
[...]

Filing this issue to determine if this can be fixed in the general case. There is a real possibility that it's infeasible due to the properties of symlinks.

It may also be relevant that go generate does not pass-through the PWD env var to the process it invokes, which can help with preserving a consistent os.Getwd result (but I haven't checked if changing that would be sufficient to handle the general case).

CC @zx2c4, @alexbrainman, @bcmills.

[bcmills]

It may also be relevant that go generate does not pass-through the PWD env var to the process it invokes

FWIW, that's #43862.

[bcmills]

I note that (*Source).IsStdRepo is using a strictly lexical comparison, whereas the go command makes an attempt to evaluate symlinks for the equivalent comparison (see search.InDir).

If propagating PWD doesn't work, it may be possible to fix this issue by copying (or using a similar approach to) search.InDir.

[bcmills]

A more robust alternative might be to give mkwinsyscall an explicit flag to indicate whether the package is in std, instead of (or in addition to) having it try to infer that information.

[dmitshur]

Thank you for analyzing this Bryan.

From local testing I've done earlier, I expect CL 287152 should be enough to fix the issue.

[alexbrainman]

@dmitshur and @bcmills thank you for taking care of this. I am at my deep end here. So I trust you guys whatever decision you make.

Alex

[dmitshur]

From local testing I've done earlier, I expect CL 287152 should be enough to fix the issue.

That CL has been submitted, and I can confirm the reproduce steps I've provided in the original issue no longer reproduce it.

However, I'm still running into this issue in a different context. I'll try to understand what's different and update this issue.

[dmitshur]

Ok, removing WaitingForInfo, the problem was just that I was confused.

This issue is now fixed (by the go generate fix in CL 287152) and the reproduce instructions don't trigger at Go tip (yet they do trigger at Go 1.16.2 since that doesn't have the fix).

Closing because I don't see a need to do more at this point. (I agree that it would be more robust to give mkwinsyscall an explicit flag to indicate whether the package is in std, but we can defer that until there is a new reproducible problem and this needs more attention.)

[gopherbot]

Change https://golang.org/cl/304449 mentions this issue: cmd/internal/moddeps: fix false positive when $TMPDIR is symlinked