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

Jump to bottom

cmd/compile: internal compiler error: not lowered: v108, Zero SSA PTR SSA #59174

Closed
ALTree opened this issue Mar 21, 2023 · 8 comments
Closed

cmd/compile: internal compiler error: not lowered: v108, Zero SSA PTR SSA #59174

ALTree opened this issue Mar 21, 2023 · 8 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Go1.21

Comments

@ALTree
Copy link
Member

ALTree commented Mar 21, 2023 

$ gotip version
go version devel go1.21-9eba17f Tue Mar 21 19:14:14 2023 +0000 linux/amd64

package main

func main() {
	s := make([]int, copy([]byte{' '}, "")-1)
	_ = append([]int{}, make([]int, len(s))...)
}

$ gotip build crash.go 
# command-line-arguments
./crash.go:6:1: internal compiler error: 'main': not lowered: v108, Zero SSA PTR SSA

goroutine 21 [running]:
runtime/debug.Stack()
	./gotip/src/runtime/debug/stack.go:24 +0x5e
cmd/compile/internal/base.FatalfAt({0x3dfcf0?, 0xc0?}, {0xc0003dfcc8, 0x8}, {0xc0003d8300, 0x2, 0x2})
	./gotip/src/cmd/compile/internal/base/print.go:234 +0x1d7
cmd/compile/internal/base.Fatalf(...)
	./gotip/src/cmd/compile/internal/base/print.go:203
cmd/compile/internal/ssagen.(*ssafn).Fatalf(0xc0003dfd70?, {0x9a78a868?, 0x7f76?}, {0xd35cde, 0x2}, {0xc00009f810, 0x1, 0x0?})
	./gotip/src/cmd/compile/internal/ssagen/ssa.go:7891 +0x16a
cmd/compile/internal/ssa.(*Func).Fatalf(0xc0003851e0, {0xd35cde, 0x2}, {0xc00009f810, 0x1, 0x1})
	./gotip/src/cmd/compile/internal/ssa/func.go:716 +0x279
cmd/compile/internal/ssa.checkLower(0xc0003851e0)
	./gotip/src/cmd/compile/internal/ssa/lower.go:49 +0x405
cmd/compile/internal/ssa.Compile(0xc0003851e0)
	./gotip/src/cmd/compile/internal/ssa/compile.go:97 +0x950
cmd/compile/internal/ssagen.buildssa(0xc0003b58c0, 0x3)
	./gotip/src/cmd/compile/internal/ssagen/ssa.go:567 +0x1fc7
cmd/compile/internal/ssagen.Compile(0xc0003b58c0, 0x0?)
	./gotip/src/cmd/compile/internal/ssagen/pgen.go:187 +0x45
cmd/compile/internal/gc.compileFunctions.func5.1(0xc0003a9980?)
	./gotip/src/cmd/compile/internal/gc/compile.go:184 +0x34
cmd/compile/internal/gc.compileFunctions.func3.1()
	./gotip/src/cmd/compile/internal/gc/compile.go:166 +0x30
created by cmd/compile/internal/gc.compileFunctions.func3 in goroutine 20
	./gotip/src/cmd/compile/internal/gc/compile.go:165 +0x23a

Doesn't crash with go1.20.2.
@ALTree ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 21, 2023
@ALTree ALTree added this to the Go1.21 milestone Mar 21, 2023
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Mar 21, 2023
@erifan
Copy link

erifan commented Mar 22, 2023

The culprit found by git bisect CL 454255 @jake-ciolek

@jake-ciolek
Copy link
Contributor

jake-ciolek commented Mar 22, 2023 •
edited

Thanks for the report.

I just had a quick look and it's because the clear length ends up being negative, in this case it's -8:

v108 = Zero {uint8} [-8] v101 v106

The arch-specific lowering rules don't match negative Zero arguments. I think my patch just surfaced the issue because I don't think passing negative integers to the memclrNoHeapPointers is the expected behavior.

I'll look more later, but I think there's another bug here. The memclrNoHeapPointers doesn't run because a panic gets raised by panicmakeslicelen but I don't think we should ever call it with negative lengths.

1.19 assembly for reference:

    00000 (3) TEXT main.main(SB), ABIInternal
    00001 (3) FUNCDATA $0, gclocals·g2BeySu+wFnoycgXfElmcg==(SB)
    00002 (3) FUNCDATA $1, gclocals·g2BeySu+wFnoycgXfElmcg==(SB)
v16
    00003 (+4) LEAQ type.int(SB), AX
v107
    00004 (4) MOVQ $-1, BX
v71
    00005 (4) MOVQ BX, CX
v49
    00006 (4) PCDATA $1, $0
v49
    00007 (4) CALL runtime.makeslice(SB)
v70
    00008 (+5) LEAQ type.int(SB), AX
v53
    00009 (5) LEAQ main..autotmp_3(SP), BX
v48
    00010 (5) XORL CX, CX
v44
    00011 (5) MOVQ CX, DI
v77
    00012 (5) MOVQ $-1, SI
v84
    00013 (+5) CALL runtime.growslice(SB)
v100
    00014 (5) MOVQ $-8, BX
v108
    00015 (5) CALL runtime.memclrNoHeapPointers(SB)
b12
    00016 (6) RET
    00017 (?) END

To summarize, I think we should panic on any clears to memclrNoHeapPointers with size < 0 , I don't think it's safe to use negative-sized clears as the routines don't seem to be adapted to that.

@cuonglm
Copy link
Member

cuonglm commented Mar 22, 2023 •
edited

Even if we make memclrNoHeapPointers panics on negative size, the right fix here is applying the optimization only if size is not negative IMHO.

@jake-ciolek
Copy link
Contributor

@cuonglm Perhaps we can turn it into a zero sized clear? Then it should elide the unreachable memclrNoHeapPointers call.

@cuonglm
Copy link
Member

cuonglm commented Mar 22, 2023

@cuonglm Perhaps we can turn it into a zero sized clear? Then it should elide the unreachable memclrNoHeapPointers call.

I'm not sure it's correct behavior. The program must panic at runtime. If you turn it into zero size, it does not panic!

@jake-ciolek
Copy link
Contributor

jake-ciolek commented Mar 22, 2023 •
edited

So I tried clamping negative clears to 0 and the program doesn't miscompile anymore. Now the assembly looks like:

    00000 (3) TEXT main.main(SB), ABIInternal
    00001 (3) FUNCDATA $0, gclocals·g2BeySu+wFnoycgXfElmcg==(SB)
    00002 (3) FUNCDATA $1, gclocals·g2BeySu+wFnoycgXfElmcg==(SB)
v68
    00003 (+4) LEAQ type:int(SB), AX
v106
    00004 (4) MOVQ $-1, BX
v15
    00005 (4) MOVQ BX, CX
v48
    00006 (4) PCDATA $1, $0
v48
    00007 (4) CALL runtime.makeslice(SB)
v10
    00008 (+5) LEAQ main..autotmp_3(SP), AX
v36
    00009 (5) MOVQ $-1, BX
v78
    00010 (5) XORL CX, CX
v108
    00011 (5) MOVQ BX, DI
v101
    00012 (5) LEAQ type:int(SB), SI
v92
    00013 (+5) CALL runtime.growslice(SB)
b12
    00014 (6) RET
    00015 (?) END

The progam still panics on runtime:

panic: runtime error: makeslice: len out of range

If the branch is never taken (due to the panicmakeslicelen call) then we might as well remove the memcrlNoHeapPointers call as it should never fire - this reduces the code size. We probably don't ever want to call memcrlNoHeapPointers with negative sized clears so it also deals with that.

@cuonglm
Copy link
Member

cuonglm commented Mar 22, 2023

@jake-ciolek ah cool, I thought that the call to makeslice would be elided.

@gopherbot
Copy link

Change https://go.dev/cl/478375 mentions this issue: cmd/compile: mark negative size memclr non-inlineable

@golang golang locked and limited conversation to collaborators Mar 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Go1.21
Development

No branches or pull requests
5 participants
@ALTree @cuonglm @gopherbot @erifan @jake-ciolek