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

Fix find pattern vs peephole optimization bug #4094

Closed
wants to merge 0 commits into from

Conversation

palkan
Copy link
Contributor

@palkan palkan commented Jan 18, 2021

Ref https://bugs.ruby-lang.org/issues/17534

It turned out (see the ticket above) that iseq_peephole_optimize incorrectly optimizes find pattern iseq.
Consider the following example:

case [1, 2, 3]
  in a
    puts "branch1"
  in [*, 2, *]
    puts "branch2"
  end

And that's the crash dump:

// first pattern
  trace: 1
   0000 putnil                                                           (   2)
   0001 duparray             <hidden>                                    (   1)
   0003 dup                                                              (   2)
   0004 setlocal_WC_0        3                                           (   2)
   0006 jump                 <L002>                                      (   2)
// while_begin chunk
 <L010> [sp: 2]
   0008 dup                                                              (   4)
   0009 topn                 2                                           (   4)
   0011 opt_le               <calldata:<=, 1>                            (   4)
   0013 branchunless         <L013>                                      (   4)
   0015 topn                 3                                           (   4)
   0017 topn                 1                                           (   4)
   0019 opt_aref             <calldata:[], 1>                            (   4)
   0021 putobject            2                                           (   4)
   0023 checkmatch           2                                           (   4)
   0025 branchif             <L012>                                      (   4)
   0027 putobject_INT2FIX_1_                                             (   4)
   0028 opt_plus             <calldata:+, 1>                             (   4)
   0030 jump                 <L010>                                      (   4)
// find_failed chunk
 <L013> [sp: 2]
   0032 pop                                                              (   4)
   0033 pop                                                              (   4)
*  0034 pop                                                              (   4)
   0035 jump                 <L006>                                      (   4)
// find_succeed chunk
 <L012> [sp: 2]
   0037 pop                                                              (   4)
   0038 pop                                                              (   4)
   0039 pop                                                              (   4)
   0040 pop                                                              (   4)
   0041 jump                 <L004>                                      (   4)
// match_failed chunk
 <L006> [sp: -1]
   0043 pop                                                              (   4)
 <L001> [sp: -1]
   0044 putspecialobject     1                                           (   1)
   0046 putobject            NoMatchingPatternError                      (   1)
   0048 topn                 2                                           (   1)
   0050 opt_send_without_block <calldata:core#raise, 2>                  (   1)
   0052 pop                                                              (   1)
   0053 pop                                                              (   1)
   0054 pop                                                              (   1)
   0055 putnil                                                           (   1)
   0056 leave                                                            (   5)
 <L002> [sp: 2]
   0057 pop                                                              (   2)
   0058 pop                                                              (   2)
   trace: 1
   0059 putself                                                          (   3)
   0060 putstring            "branch1"                                   (   3)
   0062 opt_send_without_block <calldata:puts, 1>                        (   3)
   0064 leave                                                            (   5)
 <L004> [sp: -1]
   0065 pop                                                              (   4)
   0066 pop                                                              (   4)
   trace: 1
   0067 putself                                                          (   5)
   0068 putstring            "branch2"                                   (   5)
   0070 opt_send_without_block <calldata:puts, 1>                        (   5)
   0072 leave                                                            (   5)

The instructions added before while_begin chunk were eliminated, others left, thus making the stack state invalid.

The proposed solution is to mark patterns as unremoveable. As far as I understand, there is no other case for unreachable patterns except from trivial lasgn/dasgn from the example above (which shouldn't happen in real-life).

@palkan palkan force-pushed the fix/find-pattern-optimization-bug branch 2 times, most recently from da0deb8 to 22a2e06 Compare January 19, 2021 11:58
@palkan palkan closed this Jan 19, 2021
@palkan palkan force-pushed the fix/find-pattern-optimization-bug branch from 22a2e06 to eeacdcb Compare January 19, 2021 12:00
@palkan palkan deleted the fix/find-pattern-optimization-bug branch January 19, 2021 12:00
@palkan
Copy link
Contributor Author

palkan commented Jan 19, 2021

Has been merged 1b89b99

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
1 participant