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

Fix crash in LocationFunc when the input has incomplete location information #4508

Merged
merged 1 commit into from
May 28, 2021

Conversation

fingolfin
Copy link
Member

Also change it to error if the argument is not a function, and to
return fail (instead of an empty string) if the location can not
be determined.

Finally, make the underlying kernel function GET_LOCATION_BODY resilient
about function bodies that store no location information at all,
as produced by the code which turns syntax tree object back into
functions. This previously crashed.

Fixes #4507

Note: we could also try to improve the syntax tree code to store "complete" location information, but I am not sure what would be appropriate, so I opted to not tackle this in this PR. But if @zickgraf is interested in having that, PRs and discussions are welcome :-)

Also change it to error if the argument is not a function, and to
return fail (instead of an empty string) if the location can not
be determined.

Finally, make the underlying kernel function `GET_LOCATION_BODY` resilient
about function bodies that store no location information at all,
as produced by the code which turns syntax tree object back into
functions. This previously crashed.
@fingolfin fingolfin added kind: bug Issues describing general bugs, and PRs fixing them kind: bug: crash Issues describing bugs that cause GAP to crash, and PRs fixing them (used for release notes) release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes labels May 26, 2021
@zickgraf
Copy link
Contributor

Thanks for the fix!

Note: we could also try to improve the syntax tree code to store "complete" location information, but I am not sure what would be appropriate, so I opted to not tackle this in this PR. But if @zickgraf is interested in having that, PRs and discussions are welcome :-)

Of course that would be nice to have, but I currently do not need it: I try to do all my plausibility checks right after calling SYNTAX_TREE, where I still have a reference to the original function. And once I have transformed the syntax tree, a reference to the original line would not be helpful in most cases. So basically I'm fine with any output, as long as there is no segfault :D

@fingolfin fingolfin merged commit 4496362 into gap-system:master May 28, 2021
@fingolfin fingolfin deleted the mh/fix-LocationFunc branch May 28, 2021 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug: crash Issues describing bugs that cause GAP to crash, and PRs fixing them (used for release notes) kind: bug Issues describing general bugs, and PRs fixing them release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Segfault when applying LocationFunc to a function created with SYNTAX_TREE_CODE
3 participants