cmd/compile: nonsensical DWARF location lists produced for function argument

[andreimatei]

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

$ go version
go version go1.21rc3 linux/amd64

Does this issue reproduce with the latest release?

Yes.
It also reproduces with 1.20, and maybe older versions too.

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

go env Output

$ go env
GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/andrei/.cache/go-build'
GOENV='/home/andrei/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/andrei/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/andrei/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/andrei/sdk/go1.21rc3'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/andrei/sdk/go1.21rc3/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.21rc3'
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-build1118470848=/tmp/go-build -gno-record-gcc-switches'

What did you do?

In the CockroachDB (optimized) binary, I very commonly see broken location lists that contain duplicate pieces - so, for example, a 128-bit interface is described as a 256-bit struct, where the two halves are repeated. Playing by hand with small repros, I could also produce a case where the pieces are not even repeated, making the debug information even more confusing / nonsensical - which leads me to believe that the bug is not simply a "mechanical" one. Here's an example that shows all these problems:
https://goplay.tools/snippet/UfS5J4cYGy5

Consider this function in the snippet:

28:func myfunc(ctx context.Context) {
29:	dummy()
30:	foo(ctx)
31:	if gx == 0 {
32:		ctx = makeCtx1()
33:		dummy()
34:	} else {
35:		ctx = makeCtx2()
36:	}
37:	foo(ctx)
38:}

The debug info for the ctx context.Context function argument is the following:

0x000021c9:     DW_TAG_formal_parameter
                  DW_AT_name    ("ctx")
                  DW_AT_variable_parameter      (0x00)
                  DW_AT_decl_line       (28)
                  DW_AT_type    (0x0000000000019374 "context.Context")
                  DW_AT_location        (0x000016ff: 
                     [0x0000000000463da0, 0x0000000000463dbd): DW_OP_reg0 RAX, DW_OP_piece 0x8, DW_OP_reg0 RAX, DW_OP_piece 0x8, DW_OP_reg3 RBX, DW_OP_piece 0x8, DW_OP_reg3 RBX, DW_OP_piece 0x8
                     [0x0000000000463dbd, 0x0000000000463de0): DW_OP_call_frame_cfa, DW_OP_piece 0x8, DW_OP_call_frame_cfa, DW_OP_piece 0x8, DW_OP_fbreg +8, DW_OP_piece 0x8, DW_OP_fbreg +8, DW_OP_piece 0x8
                     [0x0000000000463de0, 0x0000000000463de5): DW_OP_fbreg -32, DW_OP_piece 0x8, DW_OP_call_frame_cfa, DW_OP_piece 0x8, DW_OP_fbreg +8, DW_OP_piece 0x8, DW_OP_fbreg +8, DW_OP_piece 0x8
                     [0x0000000000463de5, 0x0000000000463dea): DW_OP_fbreg -32, DW_OP_piece 0x8, DW_OP_call_frame_cfa, DW_OP_piece 0x8, DW_OP_fbreg -24, DW_OP_piece 0x8, DW_OP_fbreg +8, DW_OP_piece 0x8
                     [0x0000000000463dea, 0x0000000000463df6): DW_OP_fbreg -32, DW_OP_piece 0x8, DW_OP_call_frame_cfa, DW_OP_piece 0x8, DW_OP_fbreg -24, DW_OP_piece 0x8, DW_OP_fbreg +8, DW_OP_piece 0x8
                     [0x0000000000463df6, 0x0000000000463e00): DW_OP_call_frame_cfa, DW_OP_piece 0x8, DW_OP_call_frame_cfa, DW_OP_piece 0x8, DW_OP_fbreg +8, DW_OP_piece 0x8, DW_OP_fbreg +8, DW_OP_piece 0x8
                     [0x0000000000463e00, 0x0000000000463e05): DW_OP_reg0 RAX, DW_OP_piece 0x8, DW_OP_call_frame_cfa, DW_OP_piece 0x8, DW_OP_reg3 RBX, DW_OP_piece 0x8, DW_OP_fbreg +8, DW_OP_piece 0x8
                     [0x0000000000463e05, 0x0000000000463e29): DW_OP_piece 0x8, DW_OP_call_frame_cfa, DW_OP_piece 0x8, DW_OP_piece 0x8, DW_OP_fbreg +8, DW_OP_piece 0x8)

Notice that all the location lists contain 4 8-byte pieces, whereas they should contain 2 8-byte pieces (context.Context is a 16-byte interface).
Some of these location lists contain duplicate information - e.g. the first one:

DW_OP_reg0 RAX, DW_OP_piece 0x8, DW_OP_reg0 RAX, DW_OP_piece 0x8, DW_OP_reg3 RBX, DW_OP_piece 0x8, DW_OP_reg3 RBX, DW_OP_piece 0x8

Notice that the RAX and RBX pieces are each repeated.

But then it gets even more funky. Notice, for example, the 3rd location list:

[0x0000000000463de0, 0x0000000000463de5): DW_OP_fbreg -32, DW_OP_piece 0x8, DW_OP_call_frame_cfa, DW_OP_piece 0x8, DW_OP_fbreg +8, DW_OP_piece 0x8, DW_OP_fbreg +8, DW_OP_piece 0x8

Here, the first piece is no longer repeated.

FWIW, what happens around the 463de0 PC location is the following:

; /home/andrei/src/github.com/andreimatei/stackviz/test_progs/ctx.go:32
  463dd6: e8 45 ff ff ff               	callq	0x463d20 <main.makeCtx1>
; /home/andrei/src/github.com/andreimatei/stackviz/test_progs/ctx.go:37
  463ddb: 48 89 44 24 10               	movq	%rax, 0x10(%rsp)
  463de0: 48 89 5c 24 18               	movq	%rbx, 0x18(%rsp)
; /home/andrei/src/github.com/andreimatei/stackviz/test_progs/ctx.go:33
  463de5: e8 76 ff ff ff               	callq	0x463d60 <main.dummy>

At location 463ddb, the first word of the ctx variable is written to rsp + 16, which corresponds to FBREG - 32. So that explains the update to the first piece. But the 2nd piece (which should not have existed in the first place) is left un-updated.

What did you expect to see?

I expected the location lists for the ctx variable to add up to 128 bits, not 256.

---

If someone is kindly willing to give me some clues and point me in the right direction, I might be inclined to work on some fix.

cc @dr2chase @randall77 @thanm

[ajwerner]

I think this has something to do with the canonicalization of slots (or lack thereof). When I dump the debug logging for the building of debug information for the above function I see this:

at v17: ctx[uintptr] now in AX                                                       
at v17: ctx[unsafe.Pointer] now in AX                                                
at v13: ctx+8[*uint8] now in BX       
at v13: ctx+8[unsafe.Pointer] now in BX                                              
at v8: ctx[uintptr] clobbered out of AX                                              
at v8: ctx[unsafe.Pointer] clobbered out of AX           
at v8: ctx+8[*uint8] clobbered out of BX
at v8: ctx+8[unsafe.Pointer] clobbered out of BX

...

location lists:
Add entry for ctx:      b2v26-b2v-30000 =       ctx[uintptr]@<nil> ctx[unsafe.Pointer]@@+0 ctx+8[*uint8]@<nil> ctx+8[unsafe.Pointer]@@+8

The code and elsewhere in the cache seems to think that there will only be one entry for a part of a variable.

go/src/cmd/compile/internal/ssa/debug.go

Lines 656 to 662 in b25f555

	parts := state.varParts[n]
	state.varSlots[varID] = parts
	for _, slotID := range parts {
	state.slotVars[slotID] = VarID(varID)
	}
	*state.partsByVarOffset.(*partsByVarOffset) = partsByVarOffset{parts, state.slots}
	sort.Sort(state.partsByVarOffset)

I guess the question I have is whether indeed there should be just one Value/slot per piece of a variable or not. My sense is no, that these different values of different types can be independently live and that more logic is needed to merge these differently typed slots together when constructing the location lists. Maybe there's something else clever I'm missing, so any advice on how to proceed would be appreciated.

[gopherbot]

Change https://go.dev/cl/553735 mentions this issue: cmd/compile/internal/ssa: fix location lists bug