cue-lang / cue

The home of the CUE language! Validate and define text-based and dynamic configuration
https://cuelang.org
Apache License 2.0
5.07k stars 288 forks source link

evaluator: endless recursion panic when a list comprehension unifies with its own list #2229

Open verdverm opened 1 year ago

verdverm commented 1 year ago

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

cue version v0.5.0-beta.2

       -compiler gc
        -ldflags -s -w
     CGO_ENABLED 0
          GOARCH amd64
            GOOS linux
         GOAMD64 v1

Go 1.19.3

What did you do?

exec cue eval ci.cue
cmp stdout golden.stdout

-- ci.cue --
ci: {
    build: bool | *true
    test:  bool | *true

    make: [ for k,v in ci if (v & true) != _|_ { k } ]
}

-- golden.stdout --
ci: {
    build: true
    test:  true
    make: ["build", "test"]
}

Also happens with export, def, and other commands

What did you expect to see?

PASS

What did you see instead?

runtime: goroutine stack exceeds 1000000000-byte limit
runtime: sp=0xc020640768 stack=[0xc020640000, 0xc040640000]
fatal error: stack overflow

... lots'o'lines

It does work if _make is hidden, but I need this value in export through an import, so this workaround will not work for my case

```cue
exec cue eval ci.cue -H
cmp stdout golden.stdout

-- ci.cue --
ci: {
    build: bool | *true
    test:  bool | *true

    _make: [ for k,v in ci if (v & true) != _|_ { k } ]
}

-- golden.stdout --
ci: {
    build: true
    test:  true
    _make: ["build", "test"]
}
mvdan commented 1 year ago

Thanks for filing this bug. I can confirm the stack limit panic on f23e3d565c499c1a220b94edcbc18f83c1438836.

It does not appear to be a v0.5 regression, as v0.4.3 also fails, but it's worth noting that v0.4.3 seems to burn CPU and use an ever-increasing amount of memory. I had to kill it before it grabbed all of my machine's available memory in ten to fifteen seconds.

mvdan commented 1 year ago

Smaller repro:

exec cue eval root.cue
cmp stdout golden.stdout

-- root.cue --
root: {
    x: true
    y: [ for _, v in root if v & true { _ } ]
}

-- golden.stdout --
root: {
    x: true
    y: [_]
}

This configuration could be considered cyclic: we try to unify root.y & true to fill the list root.y. You and I know that root.y & true will never evaluate to true, since root.y is a list, but perhaps the evaluator isn't seeing that.

Avoiding the unification, like y: [ for _, v in root if v != _|_ { _ } ], appears to avoid the endless recursion.

verdverm commented 1 year ago

I think the reason the hidden version works is because hidden fields are not included in iteration, which would align with your idea of cyclic evaluation.

My work around was to put the list comp in the -e flag, we are using it to extract make rules to run in a CI job. It would be nice to have this in CUE, and perhaps it would be cleaner to nest the make rules in a struct anyway...

myitcv commented 1 year ago

This appears to be a clear bug (per discussion with @mvdan and @mpvl) but pushing to v0.7.0 in the interests of not delaying v0.6.0 any further, but also because this isn't a recent regression. Thanks for the report, @verdverm!