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

internal/cmd/cue-ast-print: fix panic when printing the ast tree #2771

Closed
wants to merge 1 commit into from

Conversation

rudifa
Copy link
Contributor

@rudifa rudifa commented Jan 20, 2024

Panic was observed when running cue-ast-print 2567.slice.1.cue or similar cue samples.

This commit fixes the problem by moving the test if !v.IsValid(){...} a few lines up, to prevent referencing a nil pointer.

The script issue-panic.txtar verifies that cue-ast-print works after this fix.

Running the script with a build of cue-ast-print before the fix demonstrates the panic about half-way into the printing.

Panic was observed when running cue-ast-print 2567.slice.1.cue or similar cue samples.

This commit fixes the problem by moving the test `if !v.IsValid(){...}` a few lines up,
to prevent referencing a nil pointer.

The script issue-panic.txtar verifies that cue-ast-print works after this fix.

Running the script with a build of cue-ast-print before the fix demonstrates the panic
about half-way into the printing.

Signed-off-by: Rudi Farkas <rudi.farkas@gmail.com>
@rudifa rudifa requested a review from cueckoo as a code owner January 20, 2024 21:40
@mvdan
Copy link
Member

mvdan commented May 14, 2024

Thanks! Importing as https://review.gerrithub.io/c/cue-lang/cue/+/1194721.

cueckoo pushed a commit that referenced this pull request May 14, 2024
For example, in SliceExpr, the High field can be a nil interface,
and that caused us to panic as reflect.Value.Elem returns a zero Value
in such a scenario. Thanks to Rudolf Farkas for the report and fix.

Note that we don't have any tests right now, as this tool
is used only for debugging purposes.

Closes #2771 as merged.

Signed-off-by: Daniel Martí <mvdan@mvdan.cc>
Change-Id: Id41797ea500b75bbc7a09f7f40cc9e51f5c0931a
@rudifa
Copy link
Contributor Author

rudifa commented May 14, 2024 via email

@cueckoo cueckoo closed this in c789d3e May 15, 2024
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 this pull request may close these issues.

None yet

2 participants