Skip to content

Unnecessary NULL check for code->_co_monitoring in remove_tools #131763

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.

Sign up for GitHub

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

Jump to bottom
Closed
smurav opened this issue Mar 26, 2025 · 4 comments
Closed

Unnecessary NULL check for code->_co_monitoring in remove_tools #131763

smurav opened this issue Mar 26, 2025 · 4 comments
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@smurav
Copy link
Contributor

smurav commented Mar 26, 2025 •
edited by bedevere-app bot
Loading

If monitoring variable is equal to NULL then we'll get segmentation fault in
line 843 since monitoring = code->_co_monitoring.

Otherwise, the if statement is incorrect.

Linked PRs
smurav added a commit to smurav/cpython that referenced this issue Mar 26, 2025
@smurav
Fix incorrect if statement (pythongh-131763)
b962c40
@bedevere-app bedevere-app bot mentioned this issue Mar 26, 2025
Merged
@ZeroIntensity ZeroIntensity added type-bug An unexpected behavior, bug, or error interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Mar 26, 2025
@ZeroIntensity
Copy link
Member

Otherwise, the if statement is incorrect.

I think this is probably the case. I don't think _co_monitoring can be NULL in that code path, but I didn't look too hard.

@smurav
Copy link
Contributor Author

smurav commented Mar 26, 2025

I didn't find such a scenario either. In any case, the statement is incorrect.

@sergey-miryanov
Copy link
Contributor

Your fix doesn't prevent segfault in non-debug build either. If we assume that monitoring never has null value (because assert(monitoring) here) then first part of following condition (monitoring && monitoring->tools) is just a tautology.

@gaogaotiantian
Copy link
Member

gaogaotiantian commented Mar 26, 2025 •
edited
Loading

There won't be a segfault here. code->_co_monitoring should always have some data. That being said, I don't think the change is bad. It's reasonable. Also I wouldn't call the statement incorrect, it's just redundant.

@gaogaotiantian gaogaotiantian changed the title Incorrect if statement Unnecessary NULL check for code->_co_monitoring in remove_tools Mar 26, 2025
gaogaotiantian pushed a commit that referenced this issue Mar 26, 2025
@smurav
gh-131763: Replace the redundant check with assert in remove_tools (#…
151d1bf 
…131765)
diegorusso pushed a commit to diegorusso/cpython that referenced this issue Apr 1, 2025
@smurav @diegorusso
pythongh-131763: Replace the redundant check with assert in remove_to…
550fc82 
…ols (python#131765)
seehwan pushed a commit to seehwan/cpython that referenced this issue Apr 16, 2025
@smurav @seehwan80
pythongh-131763: Replace the redundant check with assert in remove_to…
6732963 
…ols (python#131765)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@sergey-miryanov @smurav @gaogaotiantian @ZeroIntensity