10000 gh-65052: Prevent pdb from crashing when trying to display objects by gaogaotiantian · Pull Request #110578 · python/cpython · GitHub
[go: up one dir, main page]

Skip to content

gh-65052: Prevent pdb from crashing when trying to display objects #110578

10000
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

Merged
merged 3 commits into from
Oct 11, 2023

Conversation

gaogaotiantian
Copy link
Member
@gaogaotiantian gaogaotiantian commented Oct 9, 2023

Currently in pdb we have a couple of explicit/implicit usage of repr when the object is arbitrary - which could potentially cause a pdb crash when the object is not displayable. Even though it's not super common, we should try out best making pdb not crash. Hence _safe_repr is introduced to print objects for:

  • args
  • display
  • retval

(Pdb) args
self = *** AttributeError: 'A' object has no attribute 'a' ***
(Pdb) display self
display self: *** AttributeError: 'A' object has no attribute 'a' ***
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be easier for a user to understand what happened if this printed sometime like what traceback._safe_string prints when str or repr fail (the fact that repr failed is more informative in this context than the precise exception. This could be read as "self is an AttributeError").

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah that makes sense. I added an error message repr({expr}) failed: before the exception. Still think it might be helpful for users to know what actually failed - and now it's less ambiguous with the extra error message (hopefully).

try:
return repr(obj)
except Exception as e:
return _rstr(f"*** repr({expr}) failed: {self._format_exc(e)} ***")
Copy link
Member

Choose a reason for hiding this comment

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

Do we use f-strings or % in this script? Or a mixture of the two?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm using f-strings for all new code. There are plenty of usages with % as pdb is old. Do you think it would be helpful to have a PR to update all strings to f-strings? It would eliminate the mixture.

Copy link
Member

Choose a reason for hiding this comment

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

Probably not worth the risk of breaking something.

@iritkatriel iritkatriel merged commit c523ce0 into python:main Oct 11, 2023
@gaogaotiantian gaogaotiantian deleted the pdb-safe-repr branch October 11, 2023 18:12
@gaogaotiantian
Copy link
Member Author

Should this be backported? In working cases it does not change the output of pdb so it does not break working code. With the additional check, it prevents pdb from crashing and prints a more useful message - I'd consider it as a bug-fix.

@iritkatriel
Copy link
Member

Yes, good point.

@iritkatriel iritkatriel added type-bug An unexpected behavior, bug, or error needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Oct 11, 2023
@miss-islington
Copy link
Contributor

Thanks @gaogaotiantian for the PR, and @iritkatriel for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Thanks @gaogaotiantian for the PR, and @iritkatriel for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@iritkatriel iritkatriel added the stdlib Python modules in the Lib dir label Oct 11, 2023
@miss-islington
Copy link
Contributor

Sorry, @gaogaotiantian and @iritkatriel, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker c523ce0f434582580a3721e15cb7dd6b56ad0236 3.11

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 11, 2023
…cts (pythonGH-110578)

(cherry picked from commit c523ce0)

Co-authored-by: Tian Gao <gaogaotiantian@hotmail.com>
@bedevere-app
Copy link
bedevere-app bot commented Oct 11, 2023

GH-110734 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Oct 11, 2023
iritkatriel pushed a commit that referenced this pull request Oct 11, 2023
…ects (GH-110578) (#110734)

gh-65052: Prevent pdb from crashing when trying to display objects (GH-110578)
(cherry picked from commit c523ce0)

Co-authored-by: Tian Gao <gaogaotiantian@hotmail.com>
@gaogaotiantian
Copy link
Member Author

@iritkatriel , do you need me to cp this to 3.11?

@iritkatriel
Copy link
Member

If you think it should be backported there then yes, it would require a manual PR.

gaogaotiantian added a commit to gaogaotiantian/cpython that referenced this pull request Oct 17, 2023
…ay objects (pythonGH-110578)

(cherry picked from commit c523ce0)

Co-authored-by: Tian Gao <gaogaotiantian@hotmail.com>
@bedevere-app
Copy link
bedevere-app bot commented Oct 17, 2023

GH-111002 is a backport of this pull request to the 3.11 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.11 only security fixes label Oct 17, 2023
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0