Set GC_DEBUG for debug builds of the interpreter

[pablogsal]

BPO	38437
Nosy	@tim-one, @vstinner, @pablogsal
PRs	bpo-38437: Activate GC_DEBUG when PY_DEBUG is set #16707

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2019-10-10.21:49:20.915>
created_at = <Date 2019-10-10.19:39:02.352>
labels = ['3.9']
title = 'Set GC_DEBUG for debug builds of the interpreter'
updated_at = <Date 2019-10-10.21:49:32.342>
user = 'https://github.com/pablogsal'

bugs.python.org fields:

activity = <Date 2019-10-10.21:49:32.342>
actor = 'pablogsal'
assignee = 'none'
closed = True
closed_date = <Date 2019-10-10.21:49:20.915>
closer = 'pablogsal'
components = []
creation = <Date 2019-10-10.19:39:02.352>
creator = 'pablogsal'
dependencies = []
files = []
hgrepos = []
issue_num = 38437
keywords = ['patch']
message_count = 6.0
messages = ['354401', '354403', '354405', '354406', '354408', '354409']
nosy_count = 3.0
nosy_names = ['tim.peters', 'vstinner', 'pablogsal']
pr_nums = ['16707']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue38437'
versions = ['Python 3.9']

[pablogsal]

While working on bpo-38379 I had to manually set the GC_DEBUG macro to 1 to activate the extra checks that 'validate_list' does. These checks are super useful to make sure all the gc lists used are consistent and in the expected state with the expected masks.

For this reason, I propose to always activate GC_DEBUG for debug builds of the interpreter. It will have a performance impact, but the debugging benefits are substantial.

[vstinner]

Under which condition can such list be corrupted?

[pablogsal]

Under which condition can such list be corrupted?

If someone is adding/modifiying the gc and calls any of the functions that set the gc flags like (PREV_MASK_COLLECTING).

One example is that after calling move_unreachable(), the unreachable set has NEXT_MASK_UNREACHABLE and therefore is an invalid list (until move_legacy_finalizers removes the flag and makes it valid again). If you mess the order or violate any of the contract, the lists will be invalid. It helps a lot to know that you are not leaving the lists in invalid state by mistake or on the other hand, to check that they are valid when they are supposed to be valid.

[tim-one]

+1. This code got quite brittle when they decided to fit two pointers, a fat integer, and 3 flags into a struct with room for only the two pointers ;-) It's a mine field now. Enabling one of the few automated mine detectors is thoroughly sensible.

[pablogsal]

New changeset 320dd50 by Pablo Galindo in branch 'master':
bpo-38437: Activate GC_DEBUG when PY_DEBUG is set (GH-16707)
320dd50

[pablogsal]

Thanks Victor and Tim!