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.

Sign up for GitHub

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

Jump to bottom

cmd/compile: bad debug_line stmt selection for if statement involving string comparison #31138

Closed
aarzilli opened this issue Mar 29, 2019 · 4 comments
Closed

cmd/compile: bad debug_line stmt selection for if statement involving string comparison #31138

aarzilli opened this issue Mar 29, 2019 · 4 comments
Labels
Debugging FrozenDueToAge

Comments

@aarzilli
Copy link
Contributor

aarzilli commented Mar 29, 2019 •
edited 

$ go version
go version devel +ddef157813 Fri Mar 29 07:17:49 2019 +0000 linux/amd64

Given the following program:

package main

type Struct struct {
	Name string
	Func func()
}

func NewStruct(name string) Struct {
	return Struct{Name:name}
}

func FindIndex2(structs []Struct, s string) int {
	for i, st := range structs { // breakpoint here, then step
		// fmt.Println(i) // uncomment this and it will work
		if st.Name == s { // starts with i = 3
			return i
		}
	}
	return -1
}

func main() {
	structs := []Struct{
		NewStruct("a"),
		NewStruct("b"),
		NewStruct("c"),
		NewStruct("dddddddd1"),
		NewStruct("dddddddd2"),
	}

	FindIndex2(structs, "dddddddd2")
}

for line 15 (if st.Name == s) the generated code is:

test.go:15 |   | 0x450bea | 48894c2440 | MOVQ CX, 0x40(SP)
test.go:15 |   | 0x450bef | 4889542448 | MOVQ DX, 0x48(SP)
test.go:15 |   | 0x450bf4 | 48399424c8000000 | CMPQ DX, 0xc8(SP)
test.go:15 |   | 0x450bfc | 7405 | JE 0x450c03
test.go:15 |   | 0x450bfe | e98c000000 | JMP 0x450c8f
test.go:15 | S | 0x450c03 | 48890c24 | MOVQ CX, 0(SP)
test.go:15 |   | 0x450c07 | 488b8424c0000000 | MOVQ 0xc0(SP), AX
test.go:15 |   | 0x450c0f | 4889442408 | MOVQ AX, 0x8(SP)
test.go:15 |   | 0x450c14 | 4889542410 | MOVQ DX, 0x10(SP)
test.go:15 | S | 0x450c19 | e8f215fbff | CALL runtime.memequal(SB)
test.go:15 |   | 0x450c1e | 807c241800 | CMPB $0x0, 0x18(SP)
test.go:15 |   | 0x450c23 | 7502 | JNE 0x450c27
test.go:15 |   | 0x450c25 | eb1d | JMP 0x450c44

Both instructions selected as statements (0x450c03 and 0x450c19) happen after a check on string length, something like 0x450bea or 0x450bf4 would be more appropriate.

Original report: go-delve/delve#1527
cc @heschik @dr2chase
@gopherbot please add label Debugging
@cuonglm
Copy link
Member

cuonglm commented Apr 3, 2019

It's only happen if you disable optimization, which dlv debug does by default. In this case, the compiler seems to be smart enough to ignore element, which has len(Name) != len(s).

If you move NewStruct("dddddddd1") to first, you got i == 0, second, got i == 1.

For me, the compiler does reverse logic here. I expect the behavior of non-optimization happens in optimization mode.

@cuonglm
Copy link
Member

cuonglm commented Apr 3, 2019

git bisect points to 2578ac5

It seems to be reasonable, because with optimization, NewStruct is inlined:

go build -gcflags "-m -m"      
# _/home/cuonglm/bin/go/31138
./main.go:8:6: can inline NewStruct as: func(string) Struct { return Struct literal }
./main.go:12:6: cannot inline FindIndex2: unhandled op RANGE
./main.go:22:6: cannot inline main: function too complex: cost 102 exceeds budget 80
./main.go:24:12: inlining call to NewStruct func(string) Struct { return Struct literal }
./main.go:25:12: inlining call to NewStruct func(string) Struct { return Struct literal }
./main.go:26:12: inlining call to NewStruct func(string) Struct { return Struct literal }
./main.go:27:12: inlining call to NewStruct func(string) Struct { return Struct literal }
./main.go:28:12: inlining call to NewStruct func(string) Struct { return Struct literal }
./main.go:8:16: leaking param: name to result ~r1 level=0
./main.go:8:16: 	from Struct literal (struct literal element) at ./main.go:9:15
./main.go:8:16: 	from ~r1 (return) at ./main.go:9:2
./main.go:12:17: FindIndex2 structs does not escape
./main.go:12:35: FindIndex2 s does not escape
./main.go:23:21: main []Struct literal does not escape

What should we do in this case? cc @josharian

@dr2chase
Copy link
Contributor

What tool are you using to print that?
I think this problem may have gotten fixed as a side effect from some other change, what I now see in ssa.html (GOSSAFUNC=FindIndex2 go build -gcflags='-N -l' bug31138.go) looks okay (+15 for v140)

v54   00059 (13) PCDATA	$0, $1
v54   00060 (13) MOVQ	AX, "".st-56(SP)
v140  00061 (+15) MOVQ	CX, ""..autotmp_10-88(SP)
v60   00062 (15) MOVQ	DX, ""..autotmp_10-80(SP)
v126  00063 (15) CMPQ	"".s+32(SP), DX
b5    00064 (15) JEQ	66
b5    00065 (15) JMP	108
v74   00066 (15) PCDATA	$0, $0
v74   00067 (15) MOVQ	CX, (SP)
v154  00068 (15) PCDATA	$0, $2
v154  00069 (15) MOVQ	"".s+24(SP), AX
v78   00070 (15) PCDATA	$0, $0
v78   00071 (15) MOVQ	AX, 8(SP)
v84   00072 (15) MOVQ	DX, 16(SP)
v85   00073 (15) CALL	runtime.memequal(SB)
v138  00074 (15) CMPB	24(SP), $0
b10   00075 (15) JNE	77
b10   00076 (15) JMP	81

@aarzilli
Copy link
Contributor Author

You are correct, this problem is fixed on master. I'm using diexplorer, I haven't found anything else that prints the stmt flag in a satisfactory way.

@golang golang locked and limited conversation to collaborators May 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Debugging FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@aarzilli @dr2chase @cuonglm @gopherbot