cmd/compile: miscompilation in star-tex.org/x/cmd/star-tex #59432

sbinet · 2023-04-04T14:44:07Z

What version of Go are you using (`go version`)?

$ go version
go version devel go1.21-f62c9701b4 Tue Apr 4 07:17:19 2023 +0000 linux/amd64

Does this issue reproduce with the latest release?

yes.

What operating system and processor architecture are you using (`go env`)?

go env Output

$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/binet/.cache/go-build"
GOENV="/home/binet/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/binet/dev/go/gocode/pkg/mod"
GONOPROXY="gitlab.cern.ch/tile-in-one/tio-go,gitlab.cern.ch/tile-in-one/tio-0021/*"
GONOSUMDB="gitlab.cern.ch/tile-in-one/tio-go,gitlab.cern.ch/tile-in-one/tio-0021/*"
GOOS="linux"
GOPATH="/home/binet/dev/go/gocode"
GOPRIVATE="gitlab.cern.ch/tile-in-one/tio-go"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/binet/sdk/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/binet/sdk/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="devel go1.21-f62c9701b4 Tue Apr 4 07:17:19 2023 +0000"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
GOWORK=""
CGO_CFLAGS="-O2 -g"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-O2 -g"
CGO_FFLAGS="-O2 -g"
CGO_LDFLAGS="-O2 -g"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build2001148705=/tmp/go-build -gno-record-gcc-switches"

What did you do?

since 1c783f7, star-tex test is failing:

$> cd star-tex.org/x/tex/cmd/star-tex
$> go test
--- FAIL: TestProcess (0.00s)
    --- FAIL: TestProcess/hello.tex (0.00s)
        main_test.go:43: could not process TeX document: xtex.pasError(-2)
FAIL
exit status 1

ie: it was working with Go-1.18.x, but not anymore starting with Go-1.19.x.

What did you expect to see?

a sucessfully working test.

What did you see instead?

a failing test.

bisecting Go, I've found out the first revision of go test that triggered the failing test.

star-tex is relying on web2go to automatically transpile Donald Knuth's WEB tex.web file to Go.
the result is under x/tex/internal/xtex.
it's machine generated so, not super easy on the eye:

https://git.sr.ht/~sbinet/star-tex/tree/v0.4.0/item/internal/xtex/xtex.go

the failure is actually coming from:

https://git.sr.ht/~sbinet/star-tex/tree/v0.4.0/item/internal/xtex/xtex.go#L1674
which, of course, shouldn't happen by just recompiling the same piece of code but with different go build versions.

looking at 1c783f7 CL, a possible hint at the root cause generating this issue might be in how some of the C-trickeries performed in tex.web are translated into Go, meaning "unsafe":

(perhaps the interplay of unsafe.Pointer and uintptr at L145 and L149 ?)

The text was updated successfully, but these errors were encountered:

mknyszek · 2023-04-04T15:26:22Z

CC @golang/compiler

cherrymui · 2023-04-04T16:51:21Z

Does the test pass with an older version of Go (which is known to work) with go test -race or go test -gcflags=all=-d=checkptr=1 ? So we can see if the unsafe conversions are okay? Thanks.

sbinet · 2023-04-04T17:24:14Z

yes, with 581603c (Go1.18.10) and 7ffc1e4 (one revision before 1c783f7), I get:

$> cd x/tex/cmd/star-tex
$> go version; go test -gcflags=all=-d=checkptr=1; go test -race
go version devel go1.20-7ffc1e47b4 Mon Oct 17 14:53:58 2022 +0000 linux/amd64
PASS
ok  	star-tex.org/x/tex/cmd/star-tex	0.010s
PASS
ok  	star-tex.org/x/tex/cmd/star-tex	0.080s

sbinet · 2023-04-04T17:31:38Z

interestingly, with today's tip (ecc5ba4), I get:

$> go version; go test -gcflags=all=-d=checkptr=1; go test -race
go version devel go1.21-ecc5ba4611 Tue Apr 4 17:04:36 2023 +0000 linux/amd64
--- FAIL: TestProcess (0.00s)
    --- FAIL: TestProcess/hello.tex (0.00s)
        main_test.go:43: could not process TeX document: xtex.pasError(-2)
FAIL
exit status 1
FAIL	star-tex.org/x/tex/cmd/star-tex	0.003s
PASS
ok  	star-tex.org/x/tex/cmd/star-tex	0.080s

(ie: go test -race succeeds)

it's the same for 1c783f7 (the first "bad" revision):

$> go version; go test -gcflags=all=-d=checkptr=1; go test -race
go version devel go1.20-1c783f7c68 Mon Oct 17 15:11:16 2022 +0000 linux/amd64
--- FAIL: TestProcess (0.00s)
    --- FAIL: TestProcess/hello.tex (0.00s)
        main_test.go:43: could not process TeX document: xtex.pasError(-2)
FAIL
exit status 1
FAIL	star-tex.org/x/tex/cmd/star-tex	0.002s
PASS
ok  	star-tex.org/x/tex/cmd/star-tex	0.060s

dr2chase · 2023-04-05T18:28:44Z

Can confirm that undoing that single CL "fixes" the bug. As they say, hmmmmmmm.

dr2chase · 2023-04-05T20:03:15Z

I wasn't able to write a test case, so shall include the debugging debris here:
This line (1666 in github.com/go-latex/star-tex/internal/xtex/xtex.go)

a = (10 * a) + int32(tex.xord[n]) - 48

compiles to (just before late lower)

b109: ← b107
  v1495 (+1667) = CMPBconst <flags> [9] v2107
  v1895 (+1666) = LEAL4 <int32> v2103 v2103
  v878 (+1666) = LEAL2 <int32> v675 v1895
  v879 (1666) = LEAL2 <int32> [-48] v675 v1895 (a[int32])
EQ v1495 → b137 b144 (1667)

late lower transforms this to

b109: ← b107-
  v1495 (+1667) = CMPBconst <flags> [9] v2107
  v1895 (+1666) = LEAL4 <int32> v2103 v2103
  v1953 (1666) = LEAL2 <byte> v675 v1895
  v878 (+1666) = LEAL2 <int32> v675 v1895
  v879 (1666) = ADDLconst <int32> [-48] v1953 (a[int32])
EQ v1495 → b137 b144 (1667)

After additional optimization, v879 is moved to another block, and its input (v1953) is spilled as a byte:

b109: ← b107-
  v1895 (+1666) = LEAL4 <int32> v2103 v2103 : SI
  v878 (+1666) = LEAL2 <int32> v675 v1895 : R8
  v1953 (1666) = LEAL2 <byte> v675 v1895 : SI
  v1495 (+1667) = CMPBconst <flags> [9] v2107
EQ v1495 → b137 b144 (unlikely) (1667)
b144: ← b109-
  v1810 (1666) = StoreReg <byte> v1953 : .autotmp_185[byte]
...
b105: ← b230-
...
  v1684 (1666) = LoadReg <byte> v1810 : SI
  v879 (1666) = ADDLconst <int32> [-48] v1684 : SI (a[int32])

and the root cause of this is the wrong type used to create the optimized LEA instructions.

gopherbot · 2023-04-05T20:27:06Z

Change https://go.dev/cl/482536 mentions this issue: cmd/compile: use correct type in amd64 late-lower rules

dr2chase · 2023-04-06T14:02:22Z

@gopherbot please open the backport tracking issues. This is a severe compiler bug.

gopherbot · 2023-04-06T14:02:53Z

Backport issue(s) opened: #59467 (for 1.19), #59468 (for 1.20).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

gopherbot · 2023-04-06T14:29:27Z

Change https://go.dev/cl/482815 mentions this issue: [release-branch.go1.20] cmd/compile: use correct type in amd64 late-lower rules

dr2chase · 2023-04-06T18:12:20Z

Fix CL had a problem, going to just remove the offending rewrite rules instead.

gopherbot · 2023-04-06T18:53:49Z

Change https://go.dev/cl/482820 mentions this issue: cmd/compile: remove broken LEA "optimization"

gopherbot · 2023-04-10T14:50:30Z

Change https://go.dev/cl/482164 mentions this issue: cmd/compile: remove broken LEA "optimization"

CL 440035 added rewrite rules to simplify "costly" LEA instructions, but the types in the rewrites were wrong and the code would go bad if the wrong-typed register was spilled. CL 482536 attempted to fix this by correcting the type in the rewrite, but that "fix" broke something on windows-amd64-race. Instead / for-now, remove the offending rewrite rules. Updates #21735. Updates #59432. Fixes #59468. Change-Id: I0497c42db414f2055e1378e0a53e2bceee9cd5d9 Reviewed-on: https://go-review.googlesource.com/c/go/+/482820 Run-TryBot: David Chase <[email protected]> Reviewed-by: Cherry Mui <[email protected]> TryBot-Result: Gopher Robot <[email protected]> (cherry picked from commit 6a97a60) Reviewed-on: https://go-review.googlesource.com/c/go/+/482164 Auto-Submit: Dmitri Shuralyov <[email protected]> Reviewed-by: Keith Randall <[email protected]> Reviewed-by: Keith Randall <[email protected]>

CL 440035 added rewrite rules to simplify "costly" LEA instructions, but the types in the rewrites were wrong and the code would go bad if the wrong-typed register was spilled. CL 482536 attempted to fix this by correcting the type in the rewrite, but that "fix" broke something on windows-amd64-race. Instead / for-now, remove the offending rewrite rules. Updates golang#21735. Updates golang#59432. Fixes golang#59468. Change-Id: I0497c42db414f2055e1378e0a53e2bceee9cd5d9 Reviewed-on: https://go-review.googlesource.com/c/go/+/482820 Run-TryBot: David Chase <[email protected]> Reviewed-by: Cherry Mui <[email protected]> TryBot-Result: Gopher Robot <[email protected]> (cherry picked from commit 6a97a60) Reviewed-on: https://go-review.googlesource.com/c/go/+/482164 Auto-Submit: Dmitri Shuralyov <[email protected]> Reviewed-by: Keith Randall <[email protected]> Reviewed-by: Keith Randall <[email protected]>

gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Apr 4, 2023

mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 4, 2023

mknyszek added this to the Backlog milestone Apr 4, 2023

mknyszek added this to Go Compiler / Runtime Apr 5, 2023

mknyszek assigned dr2chase Apr 5, 2023

mknyszek moved this to In Progress in Go Compiler / Runtime Apr 5, 2023

gopherbot closed this as completed in f9cf2c4 Apr 6, 2023

github-project-automation bot moved this from In Progress to Done in Go Compiler / Runtime Apr 6, 2023

This was referenced Apr 6, 2023

cmd/compile: miscompilation in star-tex.org/x/cmd/star-tex [1.19 backport] #59467

Closed

cmd/compile: miscompilation in star-tex.org/x/cmd/star-tex [1.20 backport] #59468

Closed

dr2chase reopened this Apr 6, 2023

github-project-automation bot moved this from Done to In Progress in Go Compiler / Runtime Apr 6, 2023

gopherbot closed this as completed in 6a97a60 Apr 7, 2023

github-project-automation bot moved this from In Progress to Done in Go Compiler / Runtime Apr 7, 2023

mknyszek removed this from Go Compiler / Runtime Apr 19, 2023

dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Apr 24, 2023

dmitshur modified the milestones: Backlog, Go1.21 Apr 24, 2023

golang locked and limited conversation to collaborators Apr 23, 2024

gopherbot added the FrozenDueToAge label Apr 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/compile: miscompilation in star-tex.org/x/cmd/star-tex #59432

cmd/compile: miscompilation in star-tex.org/x/cmd/star-tex #59432

sbinet commented Apr 4, 2023

mknyszek commented Apr 4, 2023

cherrymui commented Apr 4, 2023

sbinet commented Apr 4, 2023

sbinet commented Apr 4, 2023 •

edited

Loading

dr2chase commented Apr 5, 2023

dr2chase commented Apr 5, 2023

gopherbot commented Apr 5, 2023

dr2chase commented Apr 6, 2023

gopherbot commented Apr 6, 2023

gopherbot commented Apr 6, 2023

dr2chase commented Apr 6, 2023

gopherbot commented Apr 6, 2023

gopherbot commented Apr 10, 2023

cmd/compile: miscompilation in star-tex.org/x/cmd/star-tex #59432

cmd/compile: miscompilation in star-tex.org/x/cmd/star-tex #59432

Comments

sbinet commented Apr 4, 2023

What version of Go are you using (go version)?

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

What did you do?

What did you expect to see?

What did you see instead?

mknyszek commented Apr 4, 2023

cherrymui commented Apr 4, 2023

sbinet commented Apr 4, 2023

sbinet commented Apr 4, 2023 • edited Loading

dr2chase commented Apr 5, 2023

dr2chase commented Apr 5, 2023

gopherbot commented Apr 5, 2023

dr2chase commented Apr 6, 2023

gopherbot commented Apr 6, 2023

gopherbot commented Apr 6, 2023

dr2chase commented Apr 6, 2023

gopherbot commented Apr 6, 2023

gopherbot commented Apr 10, 2023

What version of Go are you using (`go version`)?

What operating system and processor architecture are you using (`go env`)?

sbinet commented Apr 4, 2023 •

edited

Loading