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 a crash when using a constant global variable (created via MakeConstantGlobal, such as IsHPCGAP) as index variable of a for loop #3808

Merged
merged 2 commits into from
Dec 31, 2019

Conversation

fingolfin
Copy link
Member

The reader ("parser") did not sufficiently restrict the kind of expression
permitted as index variable in for loops. So one could enter e.g. this:

for x[1] in [1] do od;

The result was a nonsensical error message. Worse, one could specify a
"constant" variable, which caused a segfault; e.g.

for IsHPCGAP in [1] do od;

Text for release notes

Fixed a crash when using a constant global variable (created via MakeConstantGlobal) as index variable of a for loop.

The reader ("parser") did not sufficiently restrict the kind of expression
permitted as index variable in `for` loops. So one could enter e.g. this:

    for x[1] in [1] do od;

The result was a nonsensical error message. Worse, one could specify a
"constant" variable, which caused a segfault; e.g.

    for IsHPCGAP in [1] do od;
@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) topic: kernel release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Dec 29, 2019
Since we now have the type of the index variable readily available in ReadFor,
we can move the calls to Push/PopGlobalForLoopVariable from the coder to the
reader, where they arguably always belonged.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 84.689% when pulling 5024072 on fingolfin:mh/fix-for-loop-var into 55ba181 on gap-system:master.

Copy link
Contributor

@DominikBernhardt DominikBernhardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me.

@fingolfin fingolfin merged commit 400bee4 into gap-system:master Dec 31, 2019
@fingolfin fingolfin deleted the mh/fix-for-loop-var branch December 31, 2019 22:26
@ThomasBreuer ThomasBreuer self-assigned this Feb 17, 2021
@ThomasBreuer ThomasBreuer added release notes: added PRs introducing changes that have since been mentioned in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Feb 17, 2021
@ThomasBreuer ThomasBreuer removed their assignment Feb 17, 2021
@fingolfin fingolfin changed the title kernel: fix a crash and weirdness related to 'for' loops Fix a crash when using a constant global variable (created via MakeConstantGlobal, such as IsHPCGAP) as index variable of a for loop Aug 17, 2022
@fingolfin fingolfin added the release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes label Aug 17, 2022
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: added PRs introducing changes that have since been mentioned in the release notes release notes: use title For PRs: the title of this PR is suitable for direct use in the release notes topic: kernel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants