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

A few backports #195

Closed
wants to merge 4 commits into from
Closed

A few backports #195

wants to merge 4 commits into from

Conversation

djensenius
Copy link

A few backported fixes.

For https://github.com/github/ruby-architecture/issues/915

peterzhu2118 and others added 4 commits January 31, 2024 09:01
[Bug #20145]

Before this commit, both copy_compare_by_id and hash_copy will create a
copy of the ST table, so the ST table created in copy_compare_by_id will
be leaked.

    h = { 1 => 2 }.compare_by_identity

    10.times do
      1_000_000.times do
        h.select { false }
      end

      puts `ps -o rss= -p #{$$}`
    end

Before:

    110736
    204352
    300272
    395520
    460704
    476736
    542000
    604704
    682624
    770528

After:

    15504
    16048
    16144
    16256
    16320
    16320
    16752
    16752
    16752
    16752
Previously on builds with optimizations disabled, this could result in
an out of bounds read. When we had all of:
* built with -O0
* Leaf builtin
* Primitive.mandatory_only
* "no args builtin", called by vm_call_single_noarg_inline_builti
* The stack is escaped to the heap via binding or a proc

This is because mk_builtin_loader generated reads for all locals
regardless of whether they were used and in the case we generated a
mandatory_only iseq that would include more variables than were actually
available.

On optimized builds, the invalid accesses would be optimized away, and
this also was often unnoticed as the invalid access would just hit
another part of the stack unless it had been escaped to the heap.

The fix here is imperfect, as this could have false positives, but since
Primitive.cexpr! is only available within the cruby codebase itself
that's probably fine as a proper fix would be much more challenging (the
only false positives we found were in rjit.rb).

Fixes [Bug #20178]

Co-authored-by: Adam Hess <[email protected]>
It looks like `sched_getcpu(3)` returns a strange number on some
(virtual?) environments.

I decided to remove the setaffinity mechanism because the performance
does not appear to degrade on a quick benchmark even if removed.

[Bug #20172]
@jhawthorn jhawthorn closed this May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants