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

Bug with nested path and variable-style parameters #186

Closed
thaliaarchi opened this issue Aug 22, 2022 · 2 comments · Fixed by wader/fq#392
Closed

Bug with nested path and variable-style parameters #186

thaliaarchi opened this issue Aug 22, 2022 · 2 comments · Fixed by wader/fq#392

Comments

@thaliaarchi
Copy link

thaliaarchi commented Aug 22, 2022

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:

  • Change the parameter from a filter to a variable: $cond -> cond
  • Desugar the parameter filter: def assert(cond; msg): cond as $cond | … (the jq docs imply that this is equivalent)
  • Inline the call to assert/2 in at/1
  • Use bare array instead of in an object: { stack: [1, 2, 3] } and .stack -> [1, 2, 3] and .

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

@wader
Copy link
Contributor

wader commented Aug 22, 2022

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
Copy link
Owner

itchyny commented Aug 22, 2022

Thank you for reporting. I fixed the bug.

wader added a commit to wader/fq that referenced this issue Aug 22, 2022
Fixes itchyny/gojq#186
skip appending path while variable argument evaluation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants