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

x/telemetry: crashes on windows-386 #65447

Closed
adonovan opened this issue Feb 2, 2024 · 4 comments
Closed

x/telemetry: crashes on windows-386 #65447

adonovan opened this issue Feb 2, 2024 · 4 comments
Assignees
Labels
arch-386 Issues solely affecting the 32-bit x86 architecture NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows telemetry x/telemetry issues
Milestone

Comments

@adonovan
Copy link
Member

adonovan commented Feb 2, 2024

Tests of https://go.dev/cl/558256 are failing spuriously due to crashes in the go command on windows-386 (decoding the hash table in the shared counter page). Example:

https://ci.chromium.org/ui/p/golang/builders/try/x_tools-gotip-windows-386/b8757239534114088753/test-results?q=ExactID%3Agolang.org%2Fx%2Ftools%2Fcmd%2Fcallgraph.TestCallgraph+VHash%3A36b5a8c990697350&clean=&sortby=&groupby=

Is this perhaps a file format version skew? Or a bug that has already been fixed in x/telemetry?

main_test.go:81: err: exit status 2: stderr: unexpected fault address 0x222700c0
        fatal error: fault
        [signal 0xc0000005 code=0x0 addr=0x222700c0 pc=0x7268b4]
        
        goroutine 1 gp=0x204a008 m=0 mp=0x12096a0 [running]:
        runtime.throw({0xdb5593, 0x5})
        	runtime/panic.go:1011 +0x4d fp=0x20b5b34 sp=0x20b5b20 pc=0x75eedd
        runtime.sigpanic()
        	runtime/signal_windows.go:414 +0x100 fp=0x20b5b58 sp=0x20b5b34 pc=0x775c50
        runtime/internal/atomic.Load(0x222700c0)
        	runtime/internal/atomic/atomic_386.go:19 +0x4 fp=0x20b5b5c sp=0x20b5b58 pc=0x7268b4
        sync/atomic.(*Uint32).Load(...)
        	sync/atomic/type.go:121
        cmd/vendor/golang.org/x/telemetry/internal/counter.(*mappedFile).load32(...)
        	cmd/vendor/golang.org/x/telemetry/internal/counter/file.go:534
        cmd/vendor/golang.org/x/telemetry/internal/counter.(*mappedFile).newCounter(0x213c060, {0x2106400, 0x19})
        	cmd/vendor/golang.org/x/telemetry/internal/counter/file.go:632 +0x43b fp=0x20b5bf0 sp=0x20b5b5c pc=0xce215b
        cmd/vendor/golang.org/x/telemetry/internal/counter.(*file).newCounter1(0x12085a0, {0x2106400, 0x19})
        	cmd/vendor/golang.org/x/telemetry/internal/counter/file.go:342 +0x194 fp=0x20b5c4c sp=0x20b5bf0 pc=0xce09a4
        cmd/vendor/golang.org/x/telemetry/internal/counter.(*file).newCounter(0x12085a0, {0x2106400, 0x19})
        	cmd/vendor/golang.org/x/telemetry/internal/counter/file.go:325 +0x2f fp=0x20b5c68 sp=0x20b5c4c pc=0xce07df
        cmd/vendor/golang.org/x/telemetry/internal/counter.(*file).lookup(0x12085a0, {0x2106400, 0x19})
        	cmd/vendor/golang.org/x/telemetry/internal/counter/file.go:107 +0x50 fp=0x20b5c8c sp=0x20b5c68 pc=0xcdee40
        cmd/vendor/golang.org/x/telemetry/internal/counter.(*Counter).releaseLock(0x2129dc0, 0xffffffff)
        	cmd/vendor/golang.org/x/telemetry/internal/counter/counter.go:224 +0x1d7 fp=0x20b5d00 sp=0x20b5c8c pc=0xcde107
        cmd/vendor/golang.org/x/telemetry/internal/counter.(*Counter).Add(0x2129dc0, 0x1)
        	cmd/vendor/golang.org/x/telemetry/internal/counter/counter.go:178 +0x4fd fp=0x20b5d80 sp=0x20b5d00 pc=0xcdd9fd
        cmd/vendor/golang.org/x/telemetry/internal/counter.(*Counter).Inc(0x2129dc0)
        	cmd/vendor/golang.org/x/telemetry/internal/counter/counter.go:130 +0x2f fp=0x20b5d90 sp=0x20b5d80 pc=0xcdd4ef
        main.invoke.CountFlags.func3(0x2112ee0)
        	cmd/vendor/golang.org/x/telemetry/counter/counter.go:92 +0x5e fp=0x20b5db0 sp=0x20b5d90 pc=0xce49ee
        flag.(*FlagSet).Visit(0x20b5e50, 0x20b5eb0)
        	flag/flag.go:472 +0x4a fp=0x20b5dd0 sp=0x20b5db0 pc=0x83f54a
        cmd/vendor/golang.org/x/telemetry/counter.CountFlags(...)
        	cmd/vendor/golang.org/x/telemetry/counter/counter.go:91
        main.invoke(0x1201a20, {0x2076008, 0xb, 0xf})
        	cmd/go/main.go:244 +0x468 fp=0x20b5ed8 sp=0x20b5dd0 pc=0xce4688
        main.main()
        	cmd/go/main.go:180 +0x8e7 fp=0x20b5fac sp=0x20b5ed8 pc=0xce3ea7
@adonovan adonovan added the telemetry x/telemetry issues label Feb 2, 2024
@bcmills
Copy link
Contributor

bcmills commented Feb 2, 2024

Quim sent a revert for this in https://go.dev/cl/560655.

@bcmills bcmills added NeedsFix The path to resolution is known, but the work has not been done. GoCommand cmd/go FixPending Issues that have a fix which has not yet been reviewed or submitted. labels Feb 2, 2024
@bcmills bcmills added this to the Go1.23 milestone Feb 2, 2024
@bcmills bcmills added OS-Windows release-blocker Soon This needs action soon. (recent regressions, service outages, unusual time-sensitive situations) arch-386 Issues solely affecting the 32-bit x86 architecture labels Feb 2, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/560655 mentions this issue: Revert "cmd/go: add telemetry counters for flag names and subcommand"

@findleyr
Copy link
Member

findleyr commented Feb 7, 2024

Let's reopen this to track fixing the underlying issue on windows/386. CC @pjweinb

@findleyr findleyr reopened this Feb 7, 2024
@findleyr findleyr changed the title cmd/go: vendored x/telemetry crashes on windows-386 x/telemetry: crashes on windows-386 Feb 7, 2024
@findleyr findleyr removed GoCommand cmd/go release-blocker Soon This needs action soon. (recent regressions, service outages, unusual time-sensitive situations) labels Feb 7, 2024
@hyangah hyangah assigned pjweinb and unassigned qmuntal Feb 7, 2024
@findleyr findleyr added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed NeedsFix The path to resolution is known, but the work has not been done. FixPending Issues that have a fix which has not yet been reviewed or submitted. labels Feb 7, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/561995 mentions this issue: telemetry: extend to 386 systems

ezz-no pushed a commit to ezz-no/go-ezzno that referenced this issue Feb 18, 2024
This reverts CL 559519.

Reason for revert: Broke windows/386. See https://build.golang.org/log/03594b706c425bd61fb3c65495aae6dd01b4a81b.

Fixes golang#65447.

Change-Id: I567bca0368168dbfb256fadba37bce3cd31aceb2
Reviewed-on: https://go-review.googlesource.com/c/go/+/560655
Reviewed-by: Bryan Mills <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Hyang-Ah Hana Kim <[email protected]>
Pinkle-pash added a commit to Pinkle-pash/telemetry that referenced this issue Dec 4, 2024
Modify the way mmap is invoked on Windows systems to support our
technique for extending the size of counter files.
The existing code was overelaborate and failed on 386.

Also, stop generating empty file names for uploading.

Fixes golang/go#65447
Fixes golang/go#60692
Fixes golang/go#60615

Change-Id: I8349d250fb516c188850557d3d099378248fb17b
Reviewed-on: https://go-review.googlesource.com/c/telemetry/+/561995
Run-TryBot: Peter Weinberger <[email protected]>
Reviewed-by: Robert Findley <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-386 Issues solely affecting the 32-bit x86 architecture NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Windows telemetry x/telemetry issues
Projects
None yet
Development

No branches or pull requests

6 participants