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.

Sign up for GitHub

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

Jump to bottom

Assert cc cme nonull #5122

Merged
merged 6 commits into from
Nov 17, 2021
Merged

Assert cc cme nonull #5122

merged 6 commits into from
Nov 17, 2021

Conversation

ko1
Copy link
Contributor

@ko1 ko1 commented Nov 16, 2021 •
edited

Recently cc->cme_ (rb_callcache::cme_) points NULL, but cached cme_ should not be NULL (when it is markable).
This PR fixes the invariant violation.

@ko1 ko1 mentioned this pull request Nov 16, 2021
Closed
@ko1 ko1 force-pushed the assert_cc_cme_nonull branch 4 times, most recently from 2415795 to bbbce9a Compare November 16, 2021 21:48
@ko1
Copy link
Contributor Author

ko1 commented Nov 17, 2021

on windows:

https://github.com/ruby/ruby/runs/4230922396?check_suite_focus=true#step:17:917

    1) Error:
  TestRefinement#test_refine_recursion:
  NoMethodError: undefined method `recursive_length' for "oo":String
      D:/a/ruby/ruby/src/test/ruby/test_refinement.rb:683:in `recursive_length'
      (eval):1:in `<module:Sandbox>'
      D:/a/ruby/ruby/src/test/ruby/test_refinement.rb:2678:in `eval'
      D:/a/ruby/ruby/src/test/ruby/test_refinement.rb:2678:in `eval_using'
      D:/a/ruby/ruby/src/test/ruby/test_refinement.rb:690:in `test_refine_recursion'

@ko1
a variable is not needed.
47b216a
@ko1
assert cc->call_ != NULL
450ed13
@ko1
vm_empty_cc_for_super
fcecb91 
Same as `vm_empty_cc`, introduce a global variable which has
`.call_ = vm_call_super_method`. Use it if the `cme == NULL` on
`vm_search_super_method`.
@ko1
assert cc->cme_ != NULL
44f88e7 
when `vm_cc_markable(cc)`.
@ko1
add VM_CALLCACHE_ON_STACK
13bd711 
check if iseq refers to on stack CC (it shouldn't).
@ko1
no need to check cme == NULL
92796a5 
Now `cc->cme_` is not NULL.
@ko1
Copy link
Contributor Author

ko1 commented Nov 17, 2021

https://github.com/ruby/ruby/pull/5122/files#diff-f8c174347e6ea8889b5036064a1ff4fe5e7c53a821befa9bdc5ccbf17800a649R3717 may fix win32 issue.

@ko1 ko1 marked this pull request as ready for review November 17, 2021 04:29
@ko1 ko1 merged commit b95d7d2 into ruby:master Nov 17, 2021
peterzhu2118 added a commit to Shopify/ruby that referenced this pull request Nov 17, 2021
@peterzhu2118
Add tests for cme NULL crash
7fb7d34 
Tests for GitHub PR ruby#5122. Originally in GitHub PR ruby#5121.
@peterzhu2118 peterzhu2118 mentioned this pull request Nov 17, 2021
Merged
peterzhu2118 added a commit to Shopify/ruby that referenced this pull request Nov 17, 2021
@peterzhu2118
Add tests for cme NULL crash
89a41a1 
Tests for GitHub PR ruby#5122. Originally in GitHub PR ruby#5121.
peterzhu2118 added a commit that referenced this pull request Nov 17, 2021
@peterzhu2118
Add tests for cme NULL crash
1454906 
Tests for GitHub PR #5122. Originally in GitHub PR #5121.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
1 participant
@ko1