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

bpo-45494: Fix parser crash when reporting errors involving invalid continuation characters #28993

Merged
merged 1 commit into from
Oct 19, 2021

Conversation

pablogsal
Copy link
Member

@pablogsal pablogsal commented Oct 16, 2021

There are two errors that this commit fixes:

  • The parser was not correctly computing the offset and the string
    source for E_LINECONT errors due to the incorrect usage of strtok().
  • The parser was not correctly unwinding the call stack when a tokenizer
    exception happened in rules involving optionals ('?', [...]) as we
    always make them return valid results by using the comma operator. We
    need to check first if we don't have an error before continuing.

https://bugs.python.org/issue45494

@pablogsal pablogsal changed the title Fix parser crash when reporting errors involving invalid continuation characters bpo-45494: Fix parser crash when reporting errors involving invalid continuation characters Oct 16, 2021
@pablogsal pablogsal added needs backport to 3.9 only security fixes needs backport to 3.10 only security fixes labels Oct 16, 2021
@pablogsal pablogsal force-pushed the bpo-45494 branch 3 times, most recently from 60578dd to 4023afe Compare October 16, 2021 16:38
…ontinuation characters

There are two errors that this commit fixes:

* The parser was not correctly computing the offset and the string
  source for E_LINECONT errors due to the incorrect usage of strtok().
* The parser was not correctly unwinding the call stack when a tokenizer
  exception happened in rules involving optionals ('?', [...]) as we
  always make them return valid results by using the comma operator. We
  need to check first if we don't have an error before continuing.
@pablogsal
Copy link
Member Author

Man, strtok is an evil function

@ambv ambv merged commit a106343 into python:main Oct 19, 2021
@miss-islington
Copy link
Contributor

Thanks @pablogsal for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry @pablogsal and @ambv, I had trouble checking out the 3.10 backport branch.
Please backport using cherry_picker on command line.
cherry_picker a106343f632a99c8ebb0136fa140cf189b4a6a57 3.10

@miss-islington
Copy link
Contributor

Sorry, @pablogsal and @ambv, I could not cleanly backport this to 3.9 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker a106343f632a99c8ebb0136fa140cf189b4a6a57 3.9

@ambv
Copy link
Contributor

ambv commented Oct 19, 2021

regen-pegen fun is endless

ambv pushed a commit to ambv/cpython that referenced this pull request Oct 19, 2021
…valid continuation characters (pythonGH-28993)

There are two errors that this commit fixes:

* The parser was not correctly computing the offset and the string
  source for E_LINECONT errors due to the incorrect usage of strtok().
* The parser was not correctly unwinding the call stack when a tokenizer
  exception happened in rules involving optionals ('?', [...]) as we
  always make them return valid results by using the comma operator. We
  need to check first if we don't have an error before continuing..
(cherry picked from commit a106343)

Co-authored-by: Pablo Galindo Salgado <[email protected]>
@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Oct 19, 2021
@bedevere-bot
Copy link

GH-29070 is a backport of this pull request to the 3.10 branch.

@pablogsal
Copy link
Member Author

regen-pegen fun is endless

Wait, this pr should not need any regen-pegen

ambv pushed a commit to ambv/cpython that referenced this pull request Oct 19, 2021
…alid continuation characters (pythonGH-28993)

There are two errors that this commit fixes:

* The parser was not correctly computing the offset and the string
  source for E_LINECONT errors due to the incorrect usage of strtok().
* The parser was not correctly unwinding the call stack when a tokenizer
  exception happened in rules involving optionals ('?', [...]) as we
  always make them return valid results by using the comma operator. We
  need to check first if we don't have an error before continuing..
(cherry picked from commit a106343)

Co-authored-by: Pablo Galindo Salgado <[email protected]>
@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label Oct 19, 2021
@bedevere-bot
Copy link

GH-29071 is a backport of this pull request to the 3.9 branch.

@ambv
Copy link
Contributor

ambv commented Oct 19, 2021

| Wait, this pr should not need any regen-pegen

Yeah, it doesn't. I just tend to always run regen-pegen (which in turn runs regen-parser which is what we want in this PR) since it covers all bases.

ambv added a commit that referenced this pull request Oct 19, 2021
…valid continuation characters (GH-28993) (GH-29070)

There are two errors that this commit fixes:

* The parser was not correctly computing the offset and the string
  source for E_LINECONT errors due to the incorrect usage of strtok().
* The parser was not correctly unwinding the call stack when a tokenizer
  exception happened in rules involving optionals ('?', [...]) as we
  always make them return valid results by using the comma operator. We
  need to check first if we don't have an error before continuing..
(cherry picked from commit a106343)

Co-authored-by: Pablo Galindo Salgado <[email protected]>
ambv added a commit that referenced this pull request Oct 20, 2021
…alid continuation characters (GH-28993) (#29071)

There are two errors that this commit fixes:

* The parser was not correctly computing the offset and the string
  source for E_LINECONT errors due to the incorrect usage of strtok().
* The parser was not correctly unwinding the call stack when a tokenizer
  exception happened in rules involving optionals ('?', [...]) as we
  always make them return valid results by using the comma operator. We
  need to check first if we don't have an error before continuing..
(cherry picked from commit a106343)

Co-authored-by: Pablo Galindo Salgado <[email protected]>

NOTE: unlike the cherry-picked original, this commit points at a crazy location
due to a bug in the tokenizer that required a big refactor in 3.10 to fix.
We are leaving as-is for 3.9.
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.

5 participants