Bug #13595
closed
rb_alloc_tmp_buffer2 broken when: elsize % sizeof(VALUE) == 0
Description
Here is the function in full as of current trunk (r58863):
static inline void * rb_alloc_tmp_buffer2(volatile VALUE *store, long count, size_t elsize) { size_t cnt = (size_t)count; if (elsize % sizeof(VALUE) == 0) { if (RB_UNLIKELY(cnt > LONG_MAX / sizeof(VALUE))) { ruby_malloc_size_overflow(cnt, elsize); } } else { size_t size, max = LONG_MAX - sizeof(VALUE) + 1; if (RB_UNLIKELY(rb_mul_size_overflow(cnt, elsize, max, &size))) { ruby_malloc_size_overflow(cnt, elsize); } cnt = (size + sizeof(VALUE) - 1) / sizeof(VALUE); } return rb_alloc_tmp_buffer_with_count(store, cnt * sizeof(VALUE), cnt); } Notice that elsize is completely ignored in the top branch when
"(elsize % sizeof(VALUE) == 0)" is true; this gives me problems
when attempting to use ALLOCV_N.
I am terrible at arithmetic and this function is too complicated for me,
so I'll let naruse or someone else fix this. But please do. Thanks
Updated by normalperson (Eric Wong) over 8 years ago
normalperson@yhbt.net wrote:
Bug #13595: rb_alloc_tmp_buffer2 broken when: elsize % sizeof(VALUE) == 0
https://bugs.ruby-lang.org/issues/13595
Perhaps the following patch is what is needed:
--- a/include/ruby/ruby.h +++ b/include/ruby/ruby.h @@ -1611,7 +1611,7 @@ static inline void * rb_alloc_tmp_buffer2(volatile VALUE *store, long count, size_t elsize) { size_t cnt = (size_t)count; - if (elsize % sizeof(VALUE) == 0) { + if (elsize == sizeof(VALUE)) { if (RB_UNLIKELY(cnt > LONG_MAX / sizeof(VALUE))) { ruby_malloc_size_overflow(cnt, elsize); } ... } /* else {...} */ return rb_alloc_tmp_buffer_with_count(store, cnt * sizeof(VALUE), cnt); Sorry; I am horrible at arithmetic. Honestly I have panic
attacks whenever I try to do it. Any help checking this
function would be greatly appreciated. Thank you.
Updated by nobu (Nobuyoshi Nakada) over 8 years ago
- Description updated (diff)
Maybe like this?
diff --git a/include/ruby/ruby.h b/include/ruby/ruby.h index 0b277dce19..95dab1bf3f 100644 --- a/include/ruby/ruby.h +++ b/include/ruby/ruby.h @@ -1612,9 +1612,10 @@ rb_alloc_tmp_buffer2(volatile VALUE *store, long count, size_t elsize) { size_t cnt = (size_t)count; if (elsize % sizeof(VALUE) == 0) { - if (RB_UNLIKELY(cnt > LONG_MAX / sizeof(VALUE))) { + if (RB_UNLIKELY(cnt > LONG_MAX / elsize)) { ruby_malloc_size_overflow(cnt, elsize); } + cnt *= elsize / sizeof(VALUE); } else { size_t size, max = LONG_MAX - sizeof(VALUE) + 1; Updated by nobu (Nobuyoshi Nakada) over 8 years ago
Reconsidered and yours seems the intended.
Updated by Anonymous over 8 years ago
- Status changed from Open to Closed
Applied in changeset trunk|r58902.
attempt to fix rb_alloc_tmp_buffer2 for ALLOCV_N
This is a confusing function to my arithmetic-challenged mind,
but nobu seems alright with this. Anyways this lets me use
large values of elsize without segfaulting, and "make exam"
passes.
- include/ruby/ruby.h (rb_alloc_tmp_buffer2): attempt to fix
[ruby-core:81388] [ruby-core:81391] [Bug #13595]
Updated by usa (Usaku NAKAMURA) over 8 years ago
- Backport changed from 2.2: UNKNOWN, 2.3: UNKNOWN, 2.4: REQUIRED to 2.2: DONTNEED, 2.3: DONTNEED, 2.4: REQUIRED
Updated by nagachika (Tomoyuki Chikanaga) over 8 years ago
- Backport changed from 2.2: DONTNEED, 2.3: DONTNEED, 2.4: REQUIRED to 2.2: DONTNEED, 2.3: DONTNEED, 2.4: DONE
ruby_2_4 r59408 merged revision(s) 58902.