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

Python/flowgraph.c:1813: void insert_superinstructions(cfg_builder *): Assertion `no_redundant_nops(g)' failed #113054

Closed
alex opened this issue Dec 13, 2023 · 12 comments · Fixed by #113139
Assignees
Labels
release-blocker type-bug An unexpected behavior, bug, or error

Comments

@alex
Copy link
Member

alex commented Dec 13, 2023

Bug report

Bug description:

The fuzz_pycompile identified an assertion failure:


Running: /mnt/scratch0/clusterfuzz/bot/inputs/fuzzer-testcases/crash-09bb1aea9610b3c790c03fc92383fb3d19f08654
--
  | <fuzz input>:1: SyntaxWarning: invalid decimal literal
  | <fuzz input>:1: SyntaxWarning: invalid decimal literal
  | fuzz_pycompile: Python/flowgraph.c:1813: void insert_superinstructions(cfg_builder *): Assertion `no_redundant_nops(g)' failed.
  | MemorySanitizer:DEADLYSIGNAL
  | ==53716==ERROR: MemorySanitizer: ABRT on unknown address 0x05390000d1d4 (pc 0x7eaab279400b bp 0x7eaab2909588 sp 0x7ffd50f56110 T53716)
  | #0 0x7eaab279400b in raise /build/glibc-SzIz7B/glibc-2.31/sysdeps/unix/sysv/linux/raise.c:51:1
  | #1 0x7eaab2773858 in abort /build/glibc-SzIz7B/glibc-2.31/stdlib/abort.c:79:7
  | #2 0x7eaab2773728 in __assert_fail_base /build/glibc-SzIz7B/glibc-2.31/assert/assert.c:92:3
  | #3 0x7eaab2784fd5 in __assert_fail /build/glibc-SzIz7B/glibc-2.31/assert/assert.c:101:3
  | #4 0xc79572 in insert_superinstructions cpython3/Python/flowgraph.c:1813:5
  | #5 0xc79572 in _PyCfg_OptimizeCodeUnit cpython3/Python/flowgraph.c:2424:5
  | #6 0xb388cf in optimize_and_assemble_code_unit cpython3/Python/compile.c:7597:9
  | #7 0xb388cf in optimize_and_assemble cpython3/Python/compile.c:7639:12
  | #8 0xb296b6 in compiler_mod cpython3/Python/compile.c:1802:24
  | #9 0xb296b6 in _PyAST_Compile cpython3/Python/compile.c:555:24
  | #10 0xe531b9 in Py_CompileStringObject cpython3/Python/pythonrun.c:1452:10
  | #11 0xe53554 in Py_CompileStringExFlags cpython3/Python/pythonrun.c:1465:10
  | #12 0x54f518 in fuzz_pycompile cpython3/Modules/_xxtestfuzz/fuzzer.c:550:24
  | #13 0x54f518 in _run_fuzz cpython3/Modules/_xxtestfuzz/fuzzer.c:563:14
  | #14 0x54f518 in LLVMFuzzerTestOneInput cpython3/Modules/_xxtestfuzz/fuzzer.c:704:11
  | #15 0x458603 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15
  | #16 0x443d62 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
  | #17 0x44960c in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:860:9
  | #18 0x472b42 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
  | #19 0x7eaab2775082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/libc-start.c:308:16
  | #20 0x439f2d in _start
  |  


<br class="Apple-interchange-newline">Running: /mnt/scratch0/clusterfuzz/bot/inputs/fuzzer-testcases/crash-09bb1aea9610b3c790c03fc92383fb3d19f08654
<fuzz input>:1: SyntaxWarning: invalid decimal literal
<fuzz input>:1: SyntaxWarning: invalid decimal literal
fuzz_pycompile: Python/flowgraph.c:1813: void insert_superinstructions(cfg_builder *): Assertion `no_redundant_nops(g)' failed.
MemorySanitizer:DEADLYSIGNAL
==53716==ERROR: MemorySanitizer: ABRT on unknown address 0x05390000d1d4 (pc 0x7eaab279400b bp 0x7eaab2909588 sp 0x7ffd50f56110 T53716)
    #0 0x7eaab279400b in raise /build/glibc-SzIz7B/glibc-2.31/sysdeps/unix/sysv/linux/raise.c:51:1
    #1 0x7eaab2773858 in abort /build/glibc-SzIz7B/glibc-2.31/stdlib/abort.c:79:7
    #2 0x7eaab2773728 in __assert_fail_base /build/glibc-SzIz7B/glibc-2.31/assert/assert.c:92:3
    #3 0x7eaab2784fd5 in __assert_fail /build/glibc-SzIz7B/glibc-2.31/assert/assert.c:101:3
    #4 0xc79572 in insert_superinstructions [cpython3/Python/flowgraph.c:1813](https://github.com/python/cpython/blob/e0fb7004ede71389c9dd462cd03352cc3c3a4d8c/Python/flowgraph.c#L1813):5
    #5 0xc79572 in _PyCfg_OptimizeCodeUnit [cpython3/Python/flowgraph.c:2424](https://github.com/python/cpython/blob/e0fb7004ede71389c9dd462cd03352cc3c3a4d8c/Python/flowgraph.c#L2424):5
    #6 0xb388cf in optimize_and_assemble_code_unit [cpython3/Python/compile.c:7597](https://github.com/python/cpython/blob/e0fb7004ede71389c9dd462cd03352cc3c3a4d8c/Python/compile.c#L7597):9
    #7 0xb388cf in optimize_and_assemble [cpython3/Python/compile.c:7639](https://github.com/python/cpython/blob/e0fb7004ede71389c9dd462cd03352cc3c3a4d8c/Python/compile.c#L7639):12
    #8 0xb296b6 in compiler_mod [cpython3/Python/compile.c:1802](https://github.com/python/cpython/blob/e0fb7004ede71389c9dd462cd03352cc3c3a4d8c/Python/compile.c#L1802):24
    #9 0xb296b6 in _PyAST_Compile [cpython3/Python/compile.c:555](https://github.com/python/cpython/blob/e0fb7004ede71389c9dd462cd03352cc3c3a4d8c/Python/compile.c#L555):24
    #10 0xe531b9 in Py_CompileStringObject [cpython3/Python/pythonrun.c:1452](https://github.com/python/cpython/blob/e0fb7004ede71389c9dd462cd03352cc3c3a4d8c/Python/pythonrun.c#L1452):10
    #11 0xe53554 in Py_CompileStringExFlags [cpython3/Python/pythonrun.c:1465](https://github.com/python/cpython/blob/e0fb7004ede71389c9dd462cd03352cc3c3a4d8c/Python/pythonrun.c#L1465):10
    #12 0x54f518 in fuzz_pycompile [cpython3/Modules/_xxtestfuzz/fuzzer.c:550](https://github.com/python/cpython/blob/e0fb7004ede71389c9dd462cd03352cc3c3a4d8c/Modules/_xxtestfuzz/fuzzer.c#L550):24
    #13 0x54f518 in _run_fuzz [cpython3/Modules/_xxtestfuzz/fuzzer.c:563](https://github.com/python/cpython/blob/e0fb7004ede71389c9dd462cd03352cc3c3a4d8c/Modules/_xxtestfuzz/fuzzer.c#L563):14
    #14 0x54f518 in LLVMFuzzerTestOneInput [cpython3/Modules/_xxtestfuzz/fuzzer.c:704](https://github.com/python/cpython/blob/e0fb7004ede71389c9dd462cd03352cc3c3a4d8c/Modules/_xxtestfuzz/fuzzer.c#L704):11
    #15 0x458603 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:611:15
    #16 0x443d62 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6
    #17 0x44960c in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:860:9
    #18 0x472b42 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10
    #19 0x7eaab2775082 in __libc_start_main /build/glibc-SzIz7B/glibc-2.31/csu/libc-start.c:308:16
    #20 0x439f2d in _start

Reproducer (note that the first two bytes are metadata for the fuzzer):

00000000: 2020 6966 2035 6966 2035 656c 7365 2054    if 5if 5else T
00000010: 3a79                                     :y

cc: @bradlarsen

CPython versions tested on:

CPython main branch

Operating systems tested on:

No response

Linked PRs

@alex alex added the type-bug An unexpected behavior, bug, or error label Dec 13, 2023
@iritkatriel
Copy link
Member

How can I reproduce this?

@alex
Copy link
Member Author

alex commented Dec 13, 2023

I believe compile('if 5if 5else T:y', '<eval>', 'exec') should reproduce this on a debug build of Python.

@iritkatriel
Copy link
Member

Also:

./python.exe -c "if 5if 5else T:y"

@iritkatriel
Copy link
Member

@pablogsal Is it ok that this even gets past the parser?

>>> ast.parse("if 5if 5else T:pass")
<unknown>:1: SyntaxWarning: invalid decimal literal
<unknown>:1: SyntaxWarning: invalid decimal literal
<ast.Module object at 0x110c2e4f0>

@pablogsal
Copy link
Member

@pablogsal Is it ok that this even gets past the parser?


>>> ast.parse("if 5if 5else T:pass")

<unknown>:1: SyntaxWarning: invalid decimal literal

<unknown>:1: SyntaxWarning: invalid decimal literal

<ast.Module object at 0x110c2e4f0>

Unfortunately yes, we discussed this in the past and we resolved to show a warning but technically disallowing it it's backwards incompatible :(

I will try to find the issue when I am back home if someone doesn't find it first

@pablogsal
Copy link
Member

In the docs we say:

https://docs.python.org/3/reference/lexical_analysis.html#whitespace-between-tokens

Except at the beginning of a logical line or in string literals, the whitespace characters space, tab and formfeed can be used interchangeably to separate tokens. Whitespace is needed between two tokens only if their concatenation could otherwise be interpreted as a different token (e.g., ab is one token, but a b is two tokens).

@iritkatriel
Copy link
Member

If we put aside the issue of whitespace, is this valid?

>>> ast.dump(ast.parse('if 5 if 5 else T:   pass'))
"Module(body=[If(test=IfExp(test=Constant(value=5), body=Constant(value=5), orelse=Name(id='T', ctx=Load())), body=[Pass()], orelse=[])], type_ignores=[])"

@iritkatriel
Copy link
Member

I guess that's

if (5 if 5 else T): pass

@AlexWaygood
Copy link
Member

AlexWaygood commented Dec 14, 2023

It looks like you don't need any of the weird (lack of) whitespace stuff to trigger an assertion:

Running Debug|x64 interpreter...
Python 3.13.0a2+ (heads/main:cf6110ba13, Dec  7 2023, 20:00:55) [MSC v.1932 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> if 5 if 5 else T: 0
...
Assertion failed: no_redundant_nops(g), file C:\Users\alexw\coding\cpython\Python\flowgraph.c, line 1813

(And it still triggers with the parens, FWIW):

>>> if (5 if 5 else T): 0
...
Assertion failed: no_redundant_nops(g), file C:\Users\alexw\coding\cpython\Python\flowgraph.c, line 1813

@iritkatriel
Copy link
Member

The issue is that this code produces at some point these three basic blocks:

1:
  NOP [line=1]
2:
  NOP [no line number]
3:
  NOP [line=1]

2 is redundant because it has no line number. Once it is removed, 1 becomes redundant because it's a NOP that has the same line number as the next instruction. But the way the algorithm is implemented, this is checked before 2 is removed, so we don't remove 1. Then the assertion fails.

This is not a correctness bug (without the assertion we would just have another NOP in the code).

I'll think how to fix this.

@iritkatriel
Copy link
Member

The NOP with no line number came from a redundant jump with no line number. So I made a change to remove such jumps instead of replacing them by NOPs that will be removed later. #113139

@iritkatriel
Copy link
Member

3.12 doesn't have this issue as far as I can tell.

iritkatriel added a commit to iritkatriel/cpython that referenced this issue Dec 19, 2023
ryan-duve pushed a commit to ryan-duve/cpython that referenced this issue Dec 26, 2023
iritkatriel added a commit to iritkatriel/cpython that referenced this issue Jan 1, 2024
aisk pushed a commit to aisk/cpython that referenced this issue Feb 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-blocker type-bug An unexpected behavior, bug, or error
Projects
4 participants