python/cpython

Regression in 3.12.5 causing non-deterministic failures in Black

hauntsaninja opened this issue · 8 comments

hauntsaninja commented

Bug report

Bug description:

#122063 causes Black to fail non-deterministically.

To reproduce:

printf 'import argparse
import string


S1 = f"{argparse.OPTIONAL[:9]=}"
S2 = (string.ascii_uppercase + string.ascii_lowercase)
' > test.py

python3.12 -m pip install --no-cache-dir --upgrade black --target tmpenv
for i in $(seq 1 10); do PYTHONPATH=tmpenv ./python.exe -m black --check --diff test.py; done

You now non-deterministically get:

error: cannot format test.py: INTERNAL ERROR: Black produced code that is not equivalent to the source.  Please report a bug on https://github.com/psf/black/issues.  This diff might be helpful: /var/folders/0y/.../T/....log

Note I think you need a non-debug build and probably a Mac

cc @pablogsal

CPython versions tested on:

3.12

hauntsaninja commented

Repro that doesn't depend on Black:

import ast
import sys

PROG = """import argparse
import string


S1 = f"{argparse.OPTIONAL[:9]=}"
S2 = (string.ascii_uppercase + string.ascii_lowercase)
"""

outputs = set()
i = 0
while len(outputs) < 2:
    output = ast.dump(ast.parse(PROG), indent=4)
    outputs.add(output)
    i += 1
    print(f"{i} iterations...")

outputs = list(outputs)
print(len(outputs), "different outputs")

print(outputs[0])
print("\n\n\n")
print(outputs[1])
print("\n\n\n")

import difflib
sys.stdout.writelines(
    difflib.unified_diff(
        outputs[0].splitlines(keepends=True),
        outputs[1].splitlines(keepends=True),
        fromfile="output1", tofile="output2"
    )
)

On Mac I can reproduce the error^ on ff3bc82 (v3.12.5) and a9daa4f, but on Linux repro is much rarer (I think I did get one, but maybe I've confused myself)

hauntsaninja commented

Okay, was getting some confusing bisect results, but I think this is fixed on latest 3.12 by #123265

pablogsal commented

Okay, was getting some confusing bisect results, but I think this is fixed on latest 3.12 by #123265

I'm a bit confused. That should not be able to affect black at all. It's just initializing the tokeniser structure consistently with zeros

JelleZijlstra commented

The bug affected Black's AST safety check, which parses the code before and after formatting to see if it's the same. So if the uninitialized memory in the tokenizer got used somehow, it's plausible that it could affect Black. I'd also expect other code that does ast.parse to occasionally behave erratically.

pablogsal commented

Ahhhhh, I misunderstood that this PR was the regression instead that this PR was the fix!

pablogsal commented

Man time to setup a valgrind Buildbot 👀

  • 🚀3