diff options
author | 卜部昌平 <[email protected]> | 2019-10-03 12:26:41 +0900 |
---|---|---|
committer | 卜部昌平 <[email protected]> | 2019-10-03 12:45:24 +0900 |
commit | eb92159d72fc711387f7e17ffbaca1678f23fd47 (patch) | |
tree | 13c5177b80fbd50c7113eee5aca5158652f24f1b /id_table.c | |
parent | ef697388becedf36966a2edcdcf88baca342b9e2 (diff) |
Revert https://github.com/ruby/ruby/pull/2486
This reverts commits: 10d6a3aca7 8ba48c1b85 fba8627dc1 dd883de5ba
6c6a25feca 167e6b48f1 7cb96d41a5 3207979278 595b3c4fdd 1521f7cf89
c11c5e69ac cf33608203 3632a812c0 f56506be0d 86427a3219 .
The reason for the revert is that we observe ABA problem around
inline method cache. When a cache misshits, we search for a
method entry. And if the entry is identical to what was cached
before, we reuse the cache. But the commits we are reverting here
introduced situations where a method entry is freed, then the
identical memory region is used for another method entry. An
inline method cache cannot detect that ABA.
Here is a code that reproduce such situation:
```ruby
require 'prime'
class << Integer
alias org_sqrt sqrt
def sqrt(n)
raise
end
GC.stress = true
Prime.each(7*37){} rescue nil # <- Here we populate CC
class << Object.new; end
# These adjacent remove-then-alias maneuver
# frees a method entry, then immediately
# reuses it for another.
remove_method :sqrt
alias sqrt org_sqrt
end
Prime.each(7*37).to_a # <- SEGV
```
Diffstat (limited to 'id_table.c')
-rw-r--r-- | id_table.c | 85 |
1 files changed, 40 insertions, 45 deletions
diff --git a/id_table.c b/id_table.c index b383fcf81d..f566582479 100644 --- a/id_table.c +++ b/id_table.c @@ -269,62 +269,57 @@ rb_id_table_delete(struct rb_id_table *tbl, ID id) void rb_id_table_foreach_with_replace(struct rb_id_table *tbl, rb_id_table_foreach_func_t *func, rb_id_table_update_callback_func_t *replace, void *data) { - rb_id_table_foreach_with_replace_with_key(tbl, func, replace, data, false); + int i, capa = tbl->capa; + + for (i=0; i<capa; i++) { + if (ITEM_KEY_ISSET(tbl, i)) { + const id_key_t key = ITEM_GET_KEY(tbl, i); + enum rb_id_table_iterator_result ret = (*func)(Qundef, tbl->items[i].val, data); + assert(key != 0); + + if (ret == ID_TABLE_REPLACE) { + VALUE val = tbl->items[i].val; + ret = (*replace)(NULL, &val, data, TRUE); + tbl->items[i].val = val; + } + else if (ret == ID_TABLE_STOP) + return; + } + } } void rb_id_table_foreach(struct rb_id_table *tbl, rb_id_table_foreach_func_t *func, void *data) { - rb_id_table_foreach_with_replace_with_key(tbl, func, 0, data, true); -} - -typedef struct tuple { - rb_id_table_foreach_values_func_t *const func; - void *const data; -} tuple; + int i, capa = tbl->capa; -static enum rb_id_table_iterator_result -cdr(ID car, VALUE cdr, void *data) -{ - const tuple *ptr = data; - return ptr->func(cdr, ptr->data); + for (i=0; i<capa; i++) { + if (ITEM_KEY_ISSET(tbl, i)) { + const id_key_t key = ITEM_GET_KEY(tbl, i); + enum rb_id_table_iterator_result ret = (*func)(key2id(key), tbl->items[i].val, data); + assert(key != 0); + + if (ret == ID_TABLE_DELETE) + hash_delete_index(tbl, i); + else if (ret == ID_TABLE_STOP) + return; + } + } } void rb_id_table_foreach_values(struct rb_id_table *tbl, rb_id_table_foreach_values_func_t *func, void *data) { - rb_id_table_foreach_with_replace( - tbl, cdr, 0, &(tuple) { func, data, }); -} + int i, capa = tbl->capa; -void -rb_id_table_foreach_with_replace_with_key( - struct rb_id_table *tbl, - rb_id_table_foreach_func_t *func, - rb_id_table_update_callback_func_t *replace, - void *data, - bool needkey) -{ - for (int i = 0; i < tbl->capa; i++) { - if (ITEM_KEY_ISSET(tbl, i)) { - const id_key_t key = ITEM_GET_KEY(tbl, i); - assert(key != 0); - ID k = needkey ? key2id(key) : 0; - VALUE v = tbl->items[i].val; - switch (func(k, v, data)) { - case ID_TABLE_DELETE: - hash_delete_index(tbl, i); - /* FALLTHROUGH */ - case ID_TABLE_CONTINUE: - continue; - case ID_TABLE_STOP: - return; - case ID_TABLE_REPLACE: - if (replace) { - replace(&k, &v, data, true); - tbl->items[i].val = v; - } - } - } + for (i=0; i<capa; i++) { + if (ITEM_KEY_ISSET(tbl, i)) { + enum rb_id_table_iterator_result ret = (*func)(tbl->items[i].val, data); + + if (ret == ID_TABLE_DELETE) + hash_delete_index(tbl, i); + else if (ret == ID_TABLE_STOP) + return; + } } } |