go: cmd/go: build cache does not check #included headers for changes
go 1.10 linux/amd64
The go build
and go test
commands don’t rebuild packages if included cgo files changed. I guess a solution would be to run the preprocessor first or disable caching for packages that use cgo altogether.
About this issue
- Original URL
- State: open
- Created 6 years ago
- Reactions: 7
- Comments: 21 (15 by maintainers)
Commits related to this issue
- tests: use `go test -a` to work around Go caching Go does not notice when the C code changes, so we have to use `go test -a`. Workaround for https://github.com/golang/go/issues/24355 . — committed to rfjakob/earlyoom by rfjakob 4 years ago
- tests: use `go clean` The behavoir after `go test -a` is somewhat surprising, so add `go clean`, which seems to actuall bring everything up to date. https://github.com/golang/go/issues/24355#issueco... — committed to rfjakob/earlyoom by rfjakob 4 years ago
- testsuite: move Go code to top-level folder As described in https://golang.org/cmd/cgo/ , Cgo only notices changes to .c and .h files in the same folder. Move the testsuite to the top-level folder t... — committed to rfjakob/earlyoom by rfjakob 4 years ago
- Add go clean for install and clean Possibly works around https://github.com/golang/go/issues/24355 Signed-off-by: Jakub Sztandera <kubuxu@protocol.ai> — committed to filecoin-project/filecoin-ffi by Kubuxu 3 years ago
It (may) be nice to have a mode that invalidates all cgo but not proper go.
While true, there’s a hidden security subtlety here. Lets suppose there’s a crypto library called
go-crypto
, which internally wraps thec-crypto
project (random names). The devs ofc-crypto
find a fatal flaw, fix it and notifygo-crypto
, who update their vendored C code and issue a new release too.I - as a user of the
go-crypto
library - see this and do ago get -u
to fetch the new code, sleeping easy that I’m all protected. Except Go didn’t bother to actually recompile anything because only the C code changes, so my binary is still vulnerable, even though I built it with the new code.This same issue will happen arbitrarily high a dependency chain, where anyone forgetting to rebuild with
-a
could potentially be vulnerable.Btw, I’m not saying I know how to fix this or whether it’s even fixable. I just wanted to add a bit of weight behind this issue.
I think so too, a couple of safeguards could save a lot of people a lot of time and confusion.
If you change the underlying C code, or you change the compiler, then you have to rebuild with -a. This is going to cause confusing issues in practice.
After many people update the package code, they don’t even know that the cache of go1.10 caused the bug to not be fixed.
Actually,
go test -a
does not seem to be enough to get the cache up to date. Subsequentgo test
runs without-a
still use some older cached version of the C code.The
!!! enter
message comes from C. No code was changed between the two test runs. First result is up to date, the second result is some older version of the C code:This seems to fix it, so probably better to use this instead of (in addition to?) the
-a
flag:As I have trouble finding what you meant there, I’m gonna copy-paste it from https://golang.org/cmd/go/#hdr-Build_and_test_caching for future readers of this ticket:
In other words, if you use Cgo, you MUST use
go build -a
andgo test -a
, otherwise you’ll never know what ended up in your binary, or what C code you were actually testing.Ah, actually I see that there is already material included about GOCACHE and cgo interaction in the go tool docs; I’ll hold off until the 1.14 cycle is open to poke at fixing this.