itchyny / gojq

Pure Go implementation of jq
MIT License
3.3k stars 119 forks source link

Bug with nested path and variable-style parameters #186

Closed thaliaarchi closed 2 years ago

thaliaarchi commented 2 years ago

I wanted to do performance and feature-coverage comparisons between jq and gojq using my wsjq Whitespace interpreter, which is considerably complex for a jq program (and overcomes the limitations of your jq Brainfuck interpreter). However, in the process, I uncovered a bug in gojq. Here's an example, reduced from various instructions in interpret_step:

def assert($cond; msg): if $cond then . else msg | halt_error end;
def at($n): assert(.stack|length > $n; "stack underflow") | .stack[-$n-1];
def top: at(0);

{ stack: [1, 2, 3] } |
top += top
$ jq -ncf minimal.jq
{"stack":[1,2,6]}
$ gojq -nf minimal.jq
gojq: invalid path against: object ({"stack":[1,2,3]})

If any of one these changes are made to the example program, it will run without error:

So, the problem appears to be caused by a nested path generated by a filter that uses variable-style parameters.

wader commented 2 years ago

Spent some time trying to undersand what is going on. I think triggering the error can be minimize to:

{} | path(def f($_): .; f(.s)).

This might be a hint, def f($_) vs def f(c): c as $_. I would have expect them to be very similar? the expbegin difference looks suspicous?

$ GOJQ_DEBUG=1 go run -tags gojq_debug cmd/gojq/main.go -n '{} | path(def f($_): .; f(.s))'
    0   scope                    [1,2,0]
    1   store                    [1,0]  ## $ARGS
    2   const                    {} ## {}
    3   pathbegin                null
    4   store                    [1,1]
    5   jump                     26
    6   scope                    [2,1,0]    ## lambda:6
    7   jump                     17
    8   scope                    [3,3,1]    ## f
    9   store                    [3,0]
    10  store                    [3,1]  ## _
    11  load                     [3,0]
    12  load                     [3,1]
    13  callpc                   null
    14  store                    [3,2]  ## $_
    15  load                     [3,0]
    16  ret                      null   ## end of f
    17  store                    [2,0]
    18  jump                     22
    19  scope                    [4,0,0]    ## lambda:19
    20  index                    "s"    ## .s
    21  ret                      null   ## end of lambda:19
    22  pushpc                   19
    23  load                     [2,0]
    24  call                     8  ## call f
    25  ret                      null   ## end of lambda:6
    26  load                     [1,1]
    27  pushpc                   6
    28  callpc                   null
    29  load                     [1,1]
    30  pathend                  null
    31  ret                      null
    ----------------------------------------+
    0   scope                    [1,2,0]    |   null    {"named":{},"positional":[]}
    1   store                    [1,0]  |   null    {"named":{},"positional":[]}            ## $ARGS
    2   const                    {} |   null            ## {}
    3   pathbegin                null   |   {}
    4   store                    [1,1]  |   {}
    5   jump                     26 |
    26  load                     [1,1]  |
    27  pushpc                   6  |   {}
    28  callpc                   null   |   {}  [6,0]
    6   scope                    [2,1,0]    |   {}          ## lambda:6
    7   jump                     17 |   {}
    17  store                    [2,0]  |   {}
    18  jump                     22 |
    22  pushpc                   19 |
    23  load                     [2,0]  |   [19,1]
    24  call                     8  |   [19,1]  {}          ## call f
    8   scope                    [3,3,1]    |   [19,1]  {}          ## f
    9   store                    [3,0]  |   [19,1]  {}
    10  store                    [3,1]  |   [19,1]          ## _
    11  load                     [3,0]  |
    12  load                     [3,1]  |   {}
    13  callpc                   null   |   {}  [19,1]
    19  scope                    [4,0,0]    |   {}          ## lambda:19
    20  index                    "s"    |   {}          ## .s
    21  ret                      null   |   null            ## end of lambda:19
    14  store                    [3,2]  |   null            ## $_
    15  load                     [3,0]  |
    16  ret                      null   |   {}          ## end of f
    25  ret                      null   |   {}          ## end of lambda:6
    29  load                     [1,1]  |   {}
    30  pathend                  null   |   {}  {}
gojq: invalid path against: object ({})
exit status 5
$ GOJQ_DEBUG=1 go run -tags gojq_debug cmd/gojq/main.go -n '{} | path(def f(c): c as $_ | .; f(.s))'
    0   scope                    [1,2,0]
    1   store                    [1,0]  ## $ARGS
    2   const                    {} ## {}
    3   pathbegin                null
    4   store                    [1,1]
    5   jump                     28
    6   scope                    [2,1,0]    ## lambda:6
    7   jump                     19
    8   scope                    [3,3,1]    ## f
    9   store                    [3,0]
    10  store                    [3,1]  ## c
    11  load                     [3,0]
    12  dup                      null
    13  expbegin                 null
    14  load                     [3,1]
    15  callpc                   null
    16  store                    [3,2]  ## $_
    17  expend                   null
    18  ret                      null   ## end of f
    19  store                    [2,0]
    20  jump                     24
    21  scope                    [4,0,0]    ## lambda:21
    22  index                    "s"    ## .s
    23  ret                      null   ## end of lambda:21
    24  pushpc                   21
    25  load                     [2,0]
    26  call                     8  ## call f
    27  ret                      null   ## end of lambda:6
    28  load                     [1,1]
    29  pushpc                   6
    30  callpc                   null
    31  load                     [1,1]
    32  pathend                  null
    33  ret                      null
    ----------------------------------------+
    0   scope                    [1,2,0]    |   null    {"named":{},"positional":[]}
    1   store                    [1,0]  |   null    {"named":{},"positional":[]}            ## $ARGS
    2   const                    {} |   null            ## {}
    3   pathbegin                null   |   {}
    4   store                    [1,1]  |   {}
    5   jump                     28 |
    28  load                     [1,1]  |
    29  pushpc                   6  |   {}
    30  callpc                   null   |   {}  [6,0]
    6   scope                    [2,1,0]    |   {}          ## lambda:6
    7   jump                     19 |   {}
    19  store                    [2,0]  |   {}
    20  jump                     24 |
    24  pushpc                   21 |
    25  load                     [2,0]  |   [21,1]
    26  call                     8  |   [21,1]  {}          ## call f
    8   scope                    [3,3,1]    |   [21,1]  {}          ## f
    9   store                    [3,0]  |   [21,1]  {}
    10  store                    [3,1]  |   [21,1]          ## c
    11  load                     [3,0]  |
    12  dup                      null   |   {}
    13  expbegin                 null   |   {}  {}
    14  load                     [3,1]  |   {}  {}
    15  callpc                   null   |   {}  {}  [21,1]
    21  scope                    [4,0,0]    |   {}  {}          ## lambda:21
    22  index                    "s"    |   {}  {}          ## .s
    23  ret                      null   |   {}  null            ## end of lambda:21
    16  store                    [3,2]  |   {}  null            ## $_
    17  expend                   null   |   {}
    18  ret                      null   |   {}          ## end of f
    27  ret                      null   |   {}          ## end of lambda:6
    31  load                     [1,1]  |   {}
    32  pathend                  null   |   {}  {}
    33  ret                      null   |   []
[]
    33  ret <backtrack>          null   |
itchyny commented 2 years ago

Thank you for reporting. I fixed the bug.