RHEL Engineering is moving the tracking of its product development work on RHEL 6 through RHEL 9 to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "RHEL project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs in the statuses "NEW", "ASSIGNED", and "POST" are being migrated throughout September 2023. Bugs of Red Hat partners with an assigned Engineering Partner Manager (EPM) are migrated in late September as per pre-agreed dates. Bugs against components "kernel", "kernel-rt", and "kpatch" are only migrated if still in "NEW" or "ASSIGNED". If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "RHEL project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/RHEL-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.
Bug 810847 - Python problem with nested IFs
Summary: Python problem with nested IFs
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Red Hat Enterprise Linux 6
Classification: Red Hat
Component: python
Version: 6.2
Hardware: Unspecified
OS: Unspecified
unspecified
medium
Target Milestone: rc
: ---
Assignee: Dave Malcolm
QA Contact: Petr Šplíchal
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-04-09 11:50 UTC by Zied Fakhfakh
Modified: 2016-06-01 01:42 UTC (History)
1 user (show)

Fixed In Version: python-2.6.6-35.el6
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2013-02-21 10:31:42 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)
Proposed patch to change list comprehensions/generator expressions to simply emit a single POP_TOP in the cleanup block (if there is one) (872 bytes, patch)
2012-04-09 19:20 UTC, Dave Malcolm
no flags Details | Diff


Links
System ID Private Priority Status Summary Last Updated
Red Hat Product Errata RHBA-2013:0437 0 normal SHIPPED_LIVE python bug fix update 2013-02-20 20:48:31 UTC

Description Zied Fakhfakh 2012-04-09 11:50:48 UTC
Description of problem:
A code of 4 or more nested IFs fails to compile

Version-Release number of selected component (if applicable):
python-2.6.6-29


How reproducible:
always

Steps to Reproduce:
1.python -c "[1 for x in () if 1 if 1 if 1 if 1]"
  
Actual results:
python: Python/compile.c:3437: stackdepth_walk: Assertion `depth >= 0' failed.
Aborted (core dumped)

Expected results:


Additional info:
It's very serious to openerp as we cannot build it on RHEL6 because of this bug.
on other distros it seems to be working fine.

Comment 2 Dave Malcolm 2012-04-09 19:15:25 UTC
Thanks for filing this bug report.

I checked the latest version of the upstream hg.python.org 2.6 code, configured
  --with-pydebug
and the assertion failure *does* affect that code (that upstream branch is closed for all changes but security fixes).

You're seeing this symptom on RHEL 6's Python because we ship with C-level assertions enabled.

This affects list comprehensions with numerous "if" clauses (like the reproducer you given in the initial report)

This also affects generator expressions: as per your reproducer, but with parentheses, giving a generator:

  $ python -c "(1 for x in () if 1 if 1 if 1 if 1)"
  python: Python/compile.c:3437: stackdepth_walk: Assertion `depth >= 0' failed.
  Aborted (core dumped)

However I believe it does *not* affect the "if" statement.

It does not affect Python 2.7, which appears to be due to the rewrite of list comprehensions to use the new POP_JUMP_IF_{TRUE,FALSE} opcodes (was http://bugs.python.org/issue4715 ; see http://hg.python.org/cpython/rev/51920)

Comment 3 Dave Malcolm 2012-04-09 19:16:07 UTC
Upon removing the failing assert, dis.dis() shows that this is what would be assembled:
$ ./python -c "import dis; dis.dis(lambda: [1 for x in () if 1 if 1 if 1 if 1])"
  1           0 BUILD_LIST               0
              3 DUP_TOP             
              4 STORE_FAST               0 (_[1])
              7 LOAD_CONST               1 (())
             10 GET_ITER            
        >>   11 FOR_ITER                29 (to 43)
             14 STORE_FAST               1 (x)
             17 LOAD_FAST                0 (_[1])
             20 LOAD_CONST               0 (1)
             23 LIST_APPEND         
             24 JUMP_ABSOLUTE           32
             27 POP_TOP             
             28 JUMP_ABSOLUTE           36
             31 POP_TOP             
        >>   32 JUMP_ABSOLUTE           40
             35 POP_TOP             
        >>   36 JUMP_ABSOLUTE           11
             39 POP_TOP             
        >>   40 JUMP_ABSOLUTE           11
        >>   43 DELETE_FAST              0 (_[1])
             46 RETURN_VALUE        
[16859 refs]

Turning off the peephole optimizer shows that this bytecode would be emitted:
$ ./python -c "import dis; dis.dis(lambda: [1 for x in () if 1 if 1 if 1 if 1])"
  1           0 BUILD_LIST               0
              3 DUP_TOP             
              4 STORE_FAST               0 (_[1])
              7 BUILD_TUPLE              0
             10 GET_ITER            
        >>   11 FOR_ITER                57 (to 71)
             14 STORE_FAST               1 (x)
             17 LOAD_CONST               0 (1)
             20 JUMP_IF_FALSE           32 (to 55)
             23 POP_TOP             
             24 LOAD_CONST               0 (1)
             27 JUMP_IF_FALSE           25 (to 55)
             30 POP_TOP             
             31 LOAD_CONST               0 (1)
             34 JUMP_IF_FALSE           18 (to 55)
             37 POP_TOP             
             38 LOAD_CONST               0 (1)
             41 JUMP_IF_FALSE           11 (to 55)
             44 POP_TOP             
             45 LOAD_FAST                0 (_[1])
             48 LOAD_CONST               0 (1)
             51 LIST_APPEND         
             52 JUMP_FORWARD             1 (to 56)
        >>   55 POP_TOP             
        >>   56 JUMP_FORWARD             1 (to 60)
             59 POP_TOP             
        >>   60 JUMP_FORWARD             1 (to 64)
             63 POP_TOP             
        >>   64 JUMP_FORWARD             1 (to 68)
             67 POP_TOP             
        >>   68 JUMP_ABSOLUTE           11
        >>   71 DELETE_FAST              0 (_[1])
             74 RETURN_VALUE        
[16871 refs]

It appears to me that Python/compiler.c:compiler_listcomp_generator (and the analogous code in compiler_genexp_generator) has a bug:  the loop

  2653      for (i = 0; i < n; i++) {
  2654          ADDOP_I(c, JUMP_FORWARD, 1);
  2655          if (i == 0)
  2656              compiler_use_next_block(c, if_cleanup);
  2657          ADDOP(c, POP_TOP);
  2658      }

for the case where there is 1 or more "if" within the list comprehension generates the instructions that becomes these bytecodes:

52     JUMP_FORWARD 1
    if_cleanup:
55     POP_TOP
56     JUMP_FORWARD 1
59     POP_TOP
60     JUMP_FORWARD 1
63     POP_TOP
64     JUMP_FORWARD 1
67     POP_TOP

However, within if_cleanup, only 55 and 56, and the chain of subsequent JUMP_FORWARD instructions are reachable: none of the other POP_TOP commands can actually be reached (and only a single POP_TOP is needed, for popping the expression used by the failed conditional from the stack).

It's this "basic block" that confuses the stack depth calculator: it does not take the JUMP_FORWARD expressions into account, and mistakenly calculates the stack depth as if all of the POP_TOP instructions are executed: popping 4 values from a stack of depth 3 would crash the interpreter, and hence the assertion fires.

Comment 4 Dave Malcolm 2012-04-09 19:16:34 UTC
Some possible approaches:
  (a) fix compiler_listcomp_generator/compiler_genexp_generator to avoid generating the redundant POP_TOP operations (am about to attach a patch)
  (b) remove the assertion
  (c) turn off C-level assertions

Comment 5 Dave Malcolm 2012-04-09 19:20:35 UTC
Created attachment 576290 [details]
Proposed patch to change list comprehensions/generator expressions to simply emit a single POP_TOP in the cleanup block (if there is one)

Proposed patch to change list comprehensions/generator expressions to simply emit a single POP_TOP in the cleanup block.

With this patch, with the peephole optimizer disabled, the "raw" disassembly looks like this:
$ ./python -c "import dis; dis.dis(lambda: [1 for x in () if 1 if 1 if 1 if 1])"
  1           0 BUILD_LIST               0
              3 DUP_TOP             
              4 STORE_FAST               0 (_[1])
              7 BUILD_TUPLE              0
             10 GET_ITER            
        >>   11 FOR_ITER                45 (to 59)
             14 STORE_FAST               1 (x)
             17 LOAD_CONST               0 (1)
             20 JUMP_IF_FALSE           32 (to 55)
             23 POP_TOP             
             24 LOAD_CONST               0 (1)
             27 JUMP_IF_FALSE           25 (to 55)
             30 POP_TOP             
             31 LOAD_CONST               0 (1)
             34 JUMP_IF_FALSE           18 (to 55)
             37 POP_TOP             
             38 LOAD_CONST               0 (1)
             41 JUMP_IF_FALSE           11 (to 55)
             44 POP_TOP             
             45 LOAD_FAST                0 (_[1])
             48 LOAD_CONST               0 (1)
             51 LIST_APPEND         
             52 JUMP_FORWARD             1 (to 56)
        >>   55 POP_TOP             
        >>   56 JUMP_ABSOLUTE           11
        >>   59 DELETE_FAST              0 (_[1])
             62 RETURN_VALUE        
[16859 refs]

and with the peephole optimizer re-enabled, the optimized disassembly looks like this:
$ ./python -c "import dis; dis.dis(lambda: [1 for x in () if 1 if 1 if 1 if 1])"
  1           0 BUILD_LIST               0
              3 DUP_TOP             
              4 STORE_FAST               0 (_[1])
              7 LOAD_CONST               1 (())
             10 GET_ITER            
        >>   11 FOR_ITER                17 (to 31)
             14 STORE_FAST               1 (x)
             17 LOAD_FAST                0 (_[1])
             20 LOAD_CONST               0 (1)
             23 LIST_APPEND         
             24 JUMP_ABSOLUTE           11
             27 POP_TOP             
             28 JUMP_ABSOLUTE           11
        >>   31 DELETE_FAST              0 (_[1])
             34 RETURN_VALUE        
[16859 refs]

(Compare with comment #3.  Both with and without the patch, the peephole optimizer entirely eliminates the "if 1" conditionals, but with this patch it does a better job of eliminating the redundant cleanup code).

The patch passes a full "make test" of the upstream test suite, although that appears to contain few examples of multiple conditionals.

As a more concrete example, the following compiles correctly:
$ ./python -c "import dis; dis.dis(lambda: [x for x in (1, 2, 3, 4) if x>=1 if x<=4 if x>1 if x<4])"
  1           0 BUILD_LIST               0
              3 DUP_TOP             
              4 STORE_FAST               0 (_[1])
              7 LOAD_CONST               4 ((1, 2, 3, 4))
             10 GET_ITER            
        >>   11 FOR_ITER                69 (to 83)
             14 STORE_FAST               1 (x)
             17 LOAD_FAST                1 (x)
             20 LOAD_CONST               0 (1)
             23 COMPARE_OP               5 (>=)
             26 JUMP_IF_FALSE           50 (to 79)
             29 POP_TOP             
             30 LOAD_FAST                1 (x)
             33 LOAD_CONST               3 (4)
             36 COMPARE_OP               1 (<=)
             39 JUMP_IF_FALSE           37 (to 79)
             42 POP_TOP             
             43 LOAD_FAST                1 (x)
             46 LOAD_CONST               0 (1)
             49 COMPARE_OP               4 (>)
             52 JUMP_IF_FALSE           24 (to 79)
             55 POP_TOP             
             56 LOAD_FAST                1 (x)
             59 LOAD_CONST               3 (4)
             62 COMPARE_OP               0 (<)
             65 JUMP_IF_FALSE           11 (to 79)
             68 POP_TOP             
             69 LOAD_FAST                0 (_[1])
             72 LOAD_FAST                1 (x)
             75 LIST_APPEND         
             76 JUMP_ABSOLUTE           11
        >>   79 POP_TOP             
             80 JUMP_ABSOLUTE           11
        >>   83 DELETE_FAST              0 (_[1])
             86 RETURN_VALUE        
[16876 refs]

Comment 7 Dave Malcolm 2012-04-09 19:30:53 UTC
(In reply to comment #0)
> It's very serious to openerp as we cannot build it on RHEL6 because of this
> bug.
> on other distros it seems to be working fine.

Thanks again for filing this bug.

Note that bugzilla is merely a bug-reporting mechanism.  

Have you filed a support ticket with Red Hat Support about this yet?  See http://access.redhat.com/

Comment 9 Zied Fakhfakh 2012-04-10 18:00:50 UTC
(In reply to comment #4)
> Some possible approaches:
>   (a) fix compiler_listcomp_generator/compiler_genexp_generator to avoid
> generating the redundant POP_TOP operations (am about to attach a patch)
>   (b) remove the assertion
>   (c) turn off C-level assertions

I changed the problematic code by a patched file from Upstream openerp and it compiles perfectly.

Indeed at 2.7 this no more a problem.

Thank you for your prompt reactivity.

Comment 10 RHEL Program Management 2012-07-10 08:39:43 UTC
This request was not resolved in time for the current release.
Red Hat invites you to ask your support representative to
propose this request, if still desired, for consideration in
the next release of Red Hat Enterprise Linux.

Comment 11 RHEL Program Management 2012-07-11 01:57:53 UTC
This request was erroneously removed from consideration in Red Hat Enterprise Linux 6.4, which is currently under development.  This request will be evaluated for inclusion in Red Hat Enterprise Linux 6.4.

Comment 12 RHEL Program Management 2012-08-14 22:01:00 UTC
This request was evaluated by Red Hat Product Management for
inclusion in a Red Hat Enterprise Linux release.  Product
Management has requested further review of this request by
Red Hat Engineering, for potential inclusion in a Red Hat
Enterprise Linux release for currently deployed products.
This request is not yet committed for inclusion in a release.

Comment 19 errata-xmlrpc 2013-02-21 10:31:42 UTC
Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory, and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

http://rhn.redhat.com/errata/RHBA-2013-0437.html


Note You need to log in before you can comment on or make changes to this bug.