Fix memory-issue String::fromCharCodes, where a GC could invalidate an address.

Fix memory-issue String::fromCharCodes, where a GC could invalidate an address.

The constructions is now reordered, so the address is not extracted
until after allocation of the new string.

BUG=https://code.google.com/p/dart/issues/detail?id=17602
R=iposva@google.com

Committed: https://code.google.com/p/dart/source/detail?r=34320

[Ivan Posva, 2014-03-24 18:30:43]

LGTMwC

-Ivan

https://codereview.chromium.org/209003009/diff/1/runtime/lib/string.cc
File runtime/lib/string.cc (right):

https://codereview.chromium.org/209003009/diff/1/runtime/lib/string.cc#newcod...
runtime/lib/string.cc:203: return Object::null();
Please put an UNREACHABLE here and drop the others in this function. Returning a
null object from this runtime call is not a valid option.

https://codereview.chromium.org/209003009/diff/1/runtime/vm/object.h
File runtime/vm/object.h (right):

https://codereview.chromium.org/209003009/diff/1/runtime/vm/object.h#newcode5312
runtime/vm/object.h:5312: Heap::Space space);
"Heap::Space space = Heap::kNew" and then you can remove them at the call sites.

I do know that some of the ones here do not have them, we should clean that up
as well. Different CL though.

https://codereview.chromium.org/209003009/diff/1/runtime/vm/object.h#newcode5317
runtime/vm/object.h:5317: Heap::Space space);
ditto

[Anders Johnsen, 2014-03-24 18:48:45]

Thanks, landing. Let me know if there is another way to satisfy the compiler,
and I'll do a followup.

https://codereview.chromium.org/209003009/diff/1/runtime/lib/string.cc
File runtime/lib/string.cc (right):

https://codereview.chromium.org/209003009/diff/1/runtime/lib/string.cc#newcod...
runtime/lib/string.cc:203: return Object::null();
On 2014/03/24 18:30:43, Ivan Posva wrote:
> Please put an UNREACHABLE here and drop the others in this function. Returning
a
> null object from this runtime call is not a valid option.

Done, partially. I need to return something after UNREACHABLE, to satisfy the
compiler. 'Object::null()' seams like the obvious choice.

https://codereview.chromium.org/209003009/diff/1/runtime/vm/object.h
File runtime/vm/object.h (right):

https://codereview.chromium.org/209003009/diff/1/runtime/vm/object.h#newcode5312
runtime/vm/object.h:5312: Heap::Space space);
On 2014/03/24 18:30:43, Ivan Posva wrote:
> "Heap::Space space = Heap::kNew" and then you can remove them at the call
sites.
> 
> I do know that some of the ones here do not have them, we should clean that up
> as well. Different CL though.

Done.

https://codereview.chromium.org/209003009/diff/1/runtime/vm/object.h#newcode5317
runtime/vm/object.h:5317: Heap::Space space);
On 2014/03/24 18:30:43, Ivan Posva wrote:
> ditto

Done.

[Anders Johnsen, 2014-03-24 18:56:57]

Committed patchset #3 manually as r34320 (presubmit successful).