523348 - Vector and Array functions wrongly assume args won't be modified by callee

Status: RESOLVED FIXED

(Tamarin Graveyard :: Virtual Machine, defect)

Description

[Edwin Smith]

In the course of implementing in-place unboxing in coerceUnboxEnter, I ran into call sites which wrongly assume that calling a function will not modify the args passed in.

Our calling protocol requires that the callee can modify the arguments in place, to avoid extra copies and stack allocation.

ArrayClass::
  generic_every()
  generic_filter()
  generic_forEach()
  generic_some()
  generic_map()

VectorBaseObject::
  map()
  filter()

Comment 1

[Edwin Smith]

Attachment: never reuse args after passing them to callee

Comment 2

[Tom Harwood]

Comment on attachment 407283 [details] [diff] [review]
never reuse args after passing them to callee

The change to VectorBaseClass::filter() that returns "element" allows the callback to change the property, but puts the previous value into the result.  The docs for Vector are not explicit on this point, unsurprisingly.  It'd be good to have them updated, too.

Comment 3

[Edwin Smith]

The previous code assumed that coerceEnter() made a copy of args[] before calling the callback (which it always does, currently, but is not guaranteed to do).  so, it always preserved the old value of element, and the patch preserves this behavior.

its true that callback() can mutate the array we're working on, but i haven't intended to change anything in that area.

Comment 4

[Steven Johnson]

Comment on attachment 407283 [details] [diff] [review]
never reuse args after passing them to callee

would be useful to add an arg reminding why the args are done that way, so someone doesn't try to "improve" it in the future

Comment 5

[Edwin Smith]

will do.  (s/arg/comment)

Comment 6

[Edwin Smith]

pushed
http://hg.mozilla.org/tamarin-redux/rev/0d678217dcef