Project

General

Profile

Actions

Bug #9609

closed

[PATCH] vm_eval.c: fix misplaced RB_GC_GUARDs

Bug #9609: [PATCH] vm_eval.c: fix misplaced RB_GC_GUARDs

Added by normalperson (Eric Wong) over 11 years ago. Updated almost 8 years ago.

Status:
Rejected
Target version:
-
ruby -v:
-
Backport:
[ruby-core:61359]

Description

RB_GC_GUARD needs to be placed after RARRAY_PTR usages to portably
prevent compilers from optimizing the VALUE away.


Files

0001-vm_eval.c-fix-misplaced-GC-guard.patch (1.46 KB) 0001-vm_eval.c-fix-misplaced-GC-guard.patch normalperson (Eric Wong), 03/07/2014 10:06 AM

Updated by ko1 (Koichi Sasada) over 11 years ago Actions #1 [ruby-core:61360]

  • ruby -v changed from ruby 2.2.0dev (2014-03-07 trunk 45281) [x86_64-linux] to -

(2014/03/07 19:09), wrote:

RB_GC_GUARD needs to be placed after RARRAY_PTR usages to portably
prevent compilers from optimizing the VALUE away.

And also it should be RARRAY_CONST_PTR().

--
// SASADA Koichi at atdot dot net

Updated by normalperson (Eric Wong) over 11 years ago Actions #2 [ruby-core:61362]

SASADA Koichi wrote:

And also it should be RARRAY_CONST_PTR().

Thanks (r45283). Btw, I noticed rb_apply uses RARRAY_PTR, too,
but also with OBJ_FREEZE.

Should we ignore WB unprotect for frozen objects?

Updated by ko1 (Koichi Sasada) over 11 years ago Actions #3 [ruby-core:61366]

(2014/03/07 19:38), Eric Wong wrote:

Thanks (r45283). Btw, I noticed rb_apply uses RARRAY_PTR, too,
but also with OBJ_FREEZE.

Should we ignore WB unprotect for frozen objects?

No relation between frozen and wb unprotected flag. We can use
RARRAY_CONST_PTR() for this case. I'll commit it with other patches.

Index: enum.c =================================================================== --- enum.c	(revision 45283) +++ enum.c	(working copy) @@ -1163,7 +1163,7 @@ static void nmin_filter(struct nmin_data *data) { long n; - VALUE *beg; + const VALUE *beg;  int eltsize; long numelts; @@ -1175,7 +1175,7 @@ nmin_filter(struct nmin_data *data)	return; n = data->n; - beg = RARRAY_PTR(data->buf); + beg = RARRAY_CONST_PTR(data->buf);  eltsize = data->by ? 2 : 1; numelts = data->curlen; Index: enumerator.c =================================================================== --- enumerator.c	(revision 45283) +++ enumerator.c	(working copy) @@ -422,13 +422,13 @@ static VALUE enumerator_block_call(VALUE obj, rb_block_call_func *func, VALUE arg) { int argc = 0; - VALUE *argv = 0; + const VALUE *argv = 0;  const struct enumerator *e = enumerator_ptr(obj); ID meth = e->meth;   if (e->args) {	argc = RARRAY_LENINT(e->args); -	argv = RARRAY_PTR(e->args); +	argv = RARRAY_CONST_PTR(e->args);  } return rb_block_call(e->obj, meth, argc, argv, func, arg); } Index: internal.h =================================================================== --- internal.h	(revision 45283) +++ internal.h	(working copy) @@ -963,9 +963,9 @@ VALUE rb_int_positive_pow(long x, unsign /* process.c */ int rb_exec_async_signal_safe(const struct rb_execarg *e, char *errmsg, size_t errmsg_buflen); rb_pid_t rb_fork_async_signal_safe(int *status, int (*chfunc)(void*, char *, size_t), void *charg, VALUE fds, char *errmsg, size_t errmsg_buflen); -VALUE rb_execarg_new(int argc, VALUE *argv, int accept_shell); +VALUE rb_execarg_new(int argc, const VALUE *argv, int accept_shell);  struct rb_execarg *rb_execarg_get(VALUE execarg_obj); /* dangerous. needs GC guard. */ -VALUE rb_execarg_init(int argc, VALUE *argv, int accept_shell, VALUE execarg_obj); +VALUE rb_execarg_init(int argc, const VALUE *argv, int accept_shell, VALUE execarg_obj);  int rb_execarg_addopt(VALUE execarg_obj, VALUE key, VALUE val); void rb_execarg_fixup(VALUE execarg_obj); int rb_execarg_run_options(const struct rb_execarg *e, struct rb_execarg *s, char* errmsg, size_t errmsg_buflen); Index: io.c =================================================================== --- io.c	(revision 45283) +++ io.c	(working copy) @@ -6227,7 +6227,7 @@ rb_io_s_popen(int argc, VALUE *argv, VAL #endif	tmp = rb_ary_dup(tmp);	RBASIC_CLEAR_CLASS(tmp); -	execarg_obj = rb_execarg_new((int)len, RARRAY_PTR(tmp), FALSE); +	execarg_obj = rb_execarg_new((int)len, RARRAY_CONST_PTR(tmp), FALSE); 	rb_ary_clear(tmp); } else { Index: numeric.c =================================================================== --- numeric.c	(revision 45283) +++ numeric.c	(working copy) @@ -1887,7 +1887,7 @@ num_step_size(VALUE from, VALUE args, VA VALUE to, step, hash; int desc; int argc = args ? RARRAY_LENINT(args) : 0; - VALUE *argv = args ? RARRAY_PTR(args) : 0; + const VALUE *argv = args ? RARRAY_CONST_PTR(args) : 0;  NUM_STEP_SCAN_ARGS(argc, argv, to, step, hash, desc); Index: process.c =================================================================== --- process.c	(revision 45283) +++ process.c	(working copy) @@ -2167,7 +2167,7 @@ rb_exec_fillarg(VALUE prog, int argc, VA } VALUE -rb_execarg_new(int argc, VALUE *argv, int accept_shell) +rb_execarg_new(int argc, const VALUE *argv, int accept_shell)  { VALUE execarg_obj; struct rb_execarg *eargp; @@ -2186,7 +2186,7 @@ rb_execarg_get(VALUE execarg_obj) } VALUE -rb_execarg_init(int argc, VALUE *argv, int accept_shell, VALUE execarg_obj) +rb_execarg_init(int argc, const VALUE *argv, int accept_shell, VALUE execarg_obj)  { struct rb_execarg *eargp = rb_execarg_get(execarg_obj); VALUE prog, ret; Index: vm_eval.c =================================================================== --- vm_eval.c	(revision 45283) +++ vm_eval.c	(working copy) @@ -751,7 +751,7 @@ rb_apply(VALUE recv, ID mid, VALUE args)	args = rb_ary_subseq(args, 0, argc);	RBASIC_CLEAR_CLASS(args);	OBJ_FREEZE(args); -	ret = rb_call(recv, mid, argc, RARRAY_PTR(args), CALL_FCALL); +	ret = rb_call(recv, mid, argc, RARRAY_CONST_PTR(args), CALL_FCALL); 	RB_GC_GUARD(args);	return ret; } @@ -1482,7 +1482,7 @@ rb_eval_cmd(VALUE cmd, VALUE arg, int le	rb_set_safe_level_force(level);	if ((state = EXEC_TAG()) == 0) { val = rb_funcall2(cmd, rb_intern("call"), RARRAY_LENINT(arg), - RARRAY_PTR(arg)); + RARRAY_CONST_PTR(arg)); 	}	POP_TAG(); 

// SASADA Koichi at atdot dot net

Updated by ko1 (Koichi Sasada) over 11 years ago Actions #4 [ruby-core:61367]

(2014/03/07 20:03), SASADA Koichi wrote:

Should we ignore WB unprotect for frozen objects?
No relation between frozen and wb unprotected flag. We can use
RARRAY_CONST_PTR() for this case. I'll commit it with other patches.

Ah, I understand what you mean. As you say, we can ignore unprotect.
But I think the checking cost is bigger.

--
// SASADA Koichi at atdot dot net

Updated by normalperson (Eric Wong) almost 8 years ago Actions #5

  • Status changed from Open to Rejected
  • Backport deleted (1.9.3: REQUIRED, 2.0.0: REQUIRED, 2.1: REQUIRED)
Actions

Also available in: PDF Atom