Panic during cue eval

[cueckoo]

Originally opened by @verdverm in https://github.com/cuelang/cue/issues/598

What version of CUE are you using (cue version)?

master @ 6c49cf0

the previous commit(s) do not have this panic

panic: runtime error: invalid memory address or nil pointer dereference [recovered]
        panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x6d7017]

goroutine 1 [running]:
cuelang.org/go/cmd/cue/cmd.recoverError(0xc000723ec0)
        /home/tony/cue/cue/cmd/cue/cmd/root.go:221 +0x95
panic(0xbcfac0, 0x11a5f00)
        /usr/local/go/src/runtime/panic.go:969 +0x1b9
cuelang.org/go/internal/core/adt.(*Vertex).Default(0x0, 0xd9e0e0)
        /home/tony/cue/cue/internal/core/adt/default.go:46 +0x37
cuelang.org/go/internal/core/adt.(*OpContext).node(0xc0002cd6c0, 0xd99420, 0xc0003d0cd0, 0x40bc01, 0xc000020000)
        /home/tony/cue/cue/internal/core/adt/context.go:676 +0x1cc
cuelang.org/go/internal/core/adt.(*SelectorExpr).resolve(0xc00044f0e0, 0xc0002cd6c0, 0xc000390c30)
        /home/tony/cue/cue/internal/core/adt/expr.go:588 +0x54
cuelang.org/go/internal/core/adt.(*OpContext).Resolve(0xc0002cd6c0, 0xc0001df9f0, 0xd93660, 0xc00044f0e0, 0xc00044f0e0, 0x1)
        /home/tony/cue/cue/internal/core/adt/context.go:331 +0xdb
cuelang.org/go/internal/core/eval.(*nodeContext).evalExpr(0xc0000cf980, 0xc0001df9f0, 0xd8bc40, 0xc00044f100, 0x0)
        /home/tony/cue/cue/internal/core/eval/eval.go:1144 +0x49e
cuelang.org/go/internal/core/eval.(*nodeContext).addExprConjunct(0xc0000cf980, 0xc0001df9f0, 0xd8bc40, 0xc00044f100, 0x7ea800000000)
        /home/tony/cue/cue/internal/core/eval/eval.go:1114 +0xaa
cuelang.org/go/internal/core/eval.(*Evaluator).evalVertex(0xc000345e60, 0xc0002cd6c0, 0xc000340360, 0x4, 0x0, 0x0, 0x0)
        /home/tony/cue/cue/internal/core/eval/eval.go:408 +0x2d8
cuelang.org/go/internal/core/eval.(*Evaluator).UnifyAccept(0xc000345e60, 0xc0002cd6c0, 0xc000340360, 0x719804, 0x0, 0x0)
        /home/tony/cue/cue/internal/core/eval/eval.go:283 +0xa7
cuelang.org/go/internal/core/eval.(*Evaluator).Unify(0xc000345e60, 0xc0002cd6c0, 0xc000340360, 0xc0001d4a04)
        /home/tony/cue/cue/internal/core/eval/eval.go:257 +0x50
cuelang.org/go/internal/core/eval.(*nodeContext).completeArcs(0xc0000cf380, 0xc000130004)
        /home/tony/cue/cue/internal/core/eval/eval.go:620 +0xc8
cuelang.org/go/internal/core/eval.(*nodeContext).postDisjunct(0xc0000cf380, 0xbdb004)
        /home/tony/cue/cue/internal/core/eval/eval.go:599 +0x470
cuelang.org/go/internal/core/eval.(*nodeContext).updateResult(0xc0000cf380, 0xc0005b6904, 0x3)
        /home/tony/cue/cue/internal/core/eval/disjunct.go:132 +0x4c
cuelang.org/go/internal/core/eval.(*nodeContext).tryDisjuncts(0xc0000cf380, 0xc000510704, 0xd8bc40)
        /home/tony/cue/cue/internal/core/eval/disjunct.go:203 +0x1a7
cuelang.org/go/internal/core/eval.(*Evaluator).evalVertex(0xc000345e60, 0xc0002cd6c0, 0xc000132240, 0xc000785704, 0x0, 0x0, 0x0)
        /home/tony/cue/cue/internal/core/eval/eval.go:438 +0x3b5
...

fmt.Printf("nil node: %!v(MISSING)\n", v)

...
nil node: &adt.BasicType{Src:(*ast.Ident)(nil), K: 0xff}
nil node: &adt.BasicType{Src:(*ast.Ident)(nil), K: 0xff}
nil node: &adt.BasicType{Src:(*ast.Ident)(nil), K: 0xff}
nil node: &adt.BasicType{Src:(*ast.Ident)(nil), K: 0xff}
nil node: &adt.Bottom{Src:(*ast.SelectorExpr)( 0xc000343260), Err:errors.list{(*adt.ValueError)( 0xc000674d20), (*adt.ValueError)( 0xc000674d90)}, Code: 4, HasRecursive:false, ChildError:false, Value:adt.Value(nil)}
nil node: &adt.Bottom{Src:ast.Node(nil), Err:(*errors.posError)( 0xc000094660), Code: 4, HasRecursive:false, ChildError:false, Value:adt.Value(nil)}
nil node: &adt.BasicType{Src:(*ast.Ident)(nil), K: 0xff}
nil node: &adt.BasicType{Src:(*ast.Ident)(nil), K: 0xff}
nil node: &adt.BasicType{Src:(*ast.Ident)(nil), K: 0xff}
nil node: &adt.BasicType{Src:(*ast.Ident)(nil), K: 0xff}
nil node: &adt.Bottom{Src:(*ast.SelectorExpr)( 0xc000343260), Err:errors.list{(*adt.ValueError)( 0xc000675340), (*adt.ValueError)( 0xc0006753b0)}, Code: 4, HasRecursive:false, ChildError:false, Value:adt.Value(nil)}
nil node: &adt.BasicType{Src:(*ast.Ident)(nil), K: 0xff}
nil node: &adt.BasicType{Src:(*ast.Ident)(nil), K: 0xff}
nil node: &adt.BasicType{Src:(*ast.Ident)(nil), K: 0xff}
nil node: &adt.BasicType{Src:(*ast.Ident)(nil), K: 0xff}
nil node: &adt.Bottom{Src:(*ast.SelectorExpr)( 0xc000343260), Err:errors.list{(*adt.ValueError)( 0xc0006758f0), (*adt.ValueError)( 0xc000675960)}, Code: 4, HasRecursive:false, ChildError:false, Value:adt.Value(nil)}
nil node: &adt.Bottom{Src:ast.Node(nil), Err:(*errors.posError)( 0xc000094660), Code: 4, HasRecursive:false, ChildError:false, Value:adt.Value(nil)}
nil node: &adt.BasicType{Src:(*ast.Ident)(nil), K: 0xff}
nil node: &adt.BasicType{Src:(*ast.Ident)(nil), K: 0xff}
nil node: &adt.BasicType{Src:(*ast.Ident)(nil), K: 0xff}
nil node: &adt.BasicType{Src:(*ast.Ident)(nil), K: 0xff}
nil node: &adt.Bottom{Src:(*ast.SelectorExpr)( 0xc000343260), Err:errors.list{(*adt.ValueError)( 0xc000675f10), (*adt.ValueError)( 0xc000675f80)}, Code: 4, HasRecursive:false, ChildError:false, Value:adt.Value(nil)}
nil node: &adt.BasicType{Src:(*ast.Ident)(nil), K: 0xff}
nil node: &adt.BasicType{Src:(*ast.Ident)(nil), K: 0xff}
nil node: &adt.BasicType{Src:(*ast.Ident)(nil), K: 0xff}
nil node: &adt.BasicType{Src:(*ast.Ident)(nil), K: 0xff}
nil node: &adt.Bottom{Src:(*ast.SelectorExpr)( 0xc000343260), Err:errors.list{(*adt.ValueError)( 0xc0006804d0), (*adt.ValueError)( 0xc000680540)}, Code: 4, HasRecursive:false, ChildError:false, Value:adt.Value(nil)}
nil node: &adt.StructMarker{NeedClose:false}

[cueckoo]

Original reply by @verdverm in https://github.com/cuelang/cue/issues/598#issuecomment-733422505

at https://github.com/cuelang/cue/blob/master/internal/core/adt/context.go#L660

adding the following to the empty switch case resolves the issue and does not seem to break my usage / output. Not sure if that is correct or if node should really never be nil at this point...

if node == nil {
    return emptyNode
}

[cueckoo]

Original reply by @mpvl in https://github.com/cuelang/cue/issues/598#issuecomment-733659160

I'm not entirely clear what you did to produce this panic.

[cueckoo]

Original reply by @verdverm in https://github.com/cuelang/cue/issues/598#issuecomment-733873637

I cue eval'd one of my mega configs at that commit. https://github.com/hofstadter-io/hofmod-server or _saas repo repro. Haven't found a minimal example yet...

[cueckoo]

Original reply by @mpvl in https://github.com/cuelang/cue/issues/598#issuecomment-733898623

Could you try it with tip again? There have been some significant changes in the code so the stack trace won't say much anymore and the problem may have been fixed.

Thanks!

[cueckoo]

Original reply by @verdverm in https://github.com/cuelang/cue/issues/598#issuecomment-733899599

Sure, was just reading some of them on Gerrit that seem to be the ones your merged. I think I'm actually following those CL's a bit!

[cueckoo]

Original reply by @mpvl in https://github.com/cuelang/cue/issues/598#issuecomment-733906045

I somehow think these won't fix it though. There is something fishy. The marker should only be obtainable from node, which then cannot be nil. So if that is the case, it must have been returned by getDefault, which should never happen. So I must be missing something. (Or evalState, which is even worse.)

[cueckoo]

Original reply by @mpvl in https://github.com/cuelang/cue/issues/598#issuecomment-733917584

So would be good where this StructMarker is coming from, assuming it is not node.Value. That is the issue.

Also, are you using it through the API or is this produced by the cue tool?

[cueckoo]

Original reply by @verdverm in https://github.com/cuelang/cue/issues/598#issuecomment-733919843

produced by both, repro'd with cue exclusively

[cueckoo]

Original reply by @mpvl in https://github.com/cuelang/cue/issues/598#issuecomment-733921638

Interesting. Looked at the code but will be hard to figure it out unless I know where the StructMarker is coming from. So if you can trace that back that would be great. Basically, a StructMarker should never be returned by any function.

[cueckoo]

Original reply by @verdverm in https://github.com/cuelang/cue/issues/598#issuecomment-733962241

Something like the check here for the StructMarker case? Is that the condition we want to find the source of? ( I am seeing them both print many times, always together )

(also happy to run through delve and the input with a screen schare, ping me on slack)

func (c *OpContext) node(x Expr, scalar bool) *Vertex {
        v := c.evalState(x, EvaluatingArcs)

        v, ok := c.getDefault(v, scalar)
        if !ok {
                // Error already generated by getDefault.
                return emptyNode
        }

        node, ok := v.(*Vertex)
        if ok {
                v = node.Value
        }
        switch nv := v.(type) {
        case nil:
                c.addErrf(IncompleteError, pos(x), "incomplete value %!s(MISSING)", c.Str(x))
                return emptyNode

        case *Bottom:
                c.AddBottom(nv)
                return emptyNode

        case *ListMarker:
                // fmt.Println("case: *ListMarker")
        case *StructMarker:
                if !ok {
                        fmt.Println("case: *StructMarker", )
                        if node == nil {
                                fmt.Println("nil StructMarker", )
                                return emptyNode
                        }
                }

        default:
                if v.Kind()&StructKind != 0 {
                        c.addErrf(IncompleteError, pos(x),
                                "incomplete feed source value %!s(MISSING) (type %!s(MISSING))",
                                x.Source(), v.Kind())
                        return emptyNode

                } else if !ok {
                        c.addErrf(0, pos(x), // TODO(error): better message.
                                "invalid operand %!s(MISSING) (found %!s(MISSING), want list or struct)",
                                x.Source(), v.Kind())
                        return emptyNode
                }
        }
        return node.Default()
}

[cueckoo]

Original reply by @verdverm in https://github.com/cuelang/cue/issues/598#issuecomment-733969355

adding a case to getDefault...

func (c *OpContext) getDefault(v Value, scalar bool) (result Value, ok bool) {
        var d *Disjunction
        switch x := v.(type) {
        default:
                return v, true

        // This always prints with the two prints in the last comment
        case *StructMarker:
                fmt.Println("STRUCT")
                return v, true

        case *Vertex:
                // TODO: return vertex if not disjunction.
                switch t := x.Value.(type) {
                case *Disjunction:
                        d = t

                case *StructMarker, *ListMarker:
                        return v, true

                case *Vertex:
                        return c.getDefault(t, scalar)

                default:
                        if !scalar {
                                return v, true
                        }
                        return t, true
                }

        case *Disjunction:
                d = x
        }

        if d.NumDefaults != 1 {
                c.addErrf(IncompleteError, c.pos(),
                        "unresolved disjunction %!s(MISSING) (type %!s(MISSING))", c.Str(d), d.Kind())
                return nil, false
        }
        return c.getDefault(d.Values[0], scalar)
}

[cueckoo]

Original reply by @verdverm in https://github.com/cuelang/cue/issues/598#issuecomment-733970792

This is always printing as well

func (c *OpContext) node(x Expr, scalar bool) *Vertex {
        v := c.evalState(x, EvaluatingArcs)

        switch v.(type) {
                case *StructMarker:
                        fmt.Println("evalState return StructMarker")
        }

        v, ok := c.getDefault(v, scalar)
        if !ok {
                // Error already generated by getDefault.
                return emptyNode
        }
        ....

[cueckoo]

Original reply by @mpvl in https://github.com/cuelang/cue/issues/598#issuecomment-734276544

Good to know. So evalState is returning a StructMarker when it shouldn't. Will take a look.

[cueckoo]

Original reply by @mpvl in https://github.com/cuelang/cue/issues/598#issuecomment-734390391

I will have a solution: I'm making the markers different types so that the compiler will enforce correctness.

[cueckoo]

Original reply by @mpvl in https://github.com/cuelang/cue/issues/598#issuecomment-734452060

See these three CLs: remote: https://cue-review.googlesource.com/c/cue/+/7764 internal/core/adt: add methods for concreteness [NEW] remote: https://cue-review.googlesource.com/c/cue/+/7765 internal/core/adt: introduce base value type [NEW] remote: https://cue-review.googlesource.com/c/cue/+/7766 internal/core/adt: automatic renaming

These make ListMarker and StructMarker a separate type, to the point that functions that return or pass them to the adt.Expr or adt.Value types. There are some casts left, but these are much more localized.

I ran into some funky stuff. Didn't seem too critical, but I can imagine it could have led to your issue. So hopefully it is gone now.