On 2011/11/24 09:31:57, Lasse Reichstein wrote:
I'm not sure this is the correct place to put the safeguard.
If nothing else, it should be earlier in the function, not after doing
other
tests that expect a string.
If the subject is not a string, it would have kIsNotStringTag set and the
tests
checking for sequential string and cons string would fail. We would fall
through
until we explicitly check for the kIsNotStringTag. Since we do not expect
the
subject not to be a string first place (as this is just a safeguard),
checking
it this late would only affect the performance in the case of sliced string.
http://codereview.chromium.org/8554004/diff/1/src/arm/code-stubs-arm.cc
File src/arm/code-stubs-arm.cc (right):
http://codereview.chromium.org/8554004/diff/1/src/arm/code-stubs-arm.cc#newcode4568
src/arm/code-stubs-arm.cc:4568: // regexp_data: RegExp data (FixedArray)
If r1 contains the instance type, then make a comment about it here with
the
other register invariants.
http://codereview.chromium.org/8554004/diff/1/src/arm/code-stubs-arm.cc#newcode4584
src/arm/code-stubs-arm.cc:4584: __ tst(ebx, Immediate(kIsNotStringMask));
If it should have been guarded against, then make this an Assert check
guarded
by FLAG_debug_code.
And if it there is actually a way to get here with a non-string, it
should be
found and eliminated. We control the arguments to stubs completely, and
are
expected to handle them safely.
If it's called from %_RegExpExec, then the test should be performed there
(all
%-functions must check their arguments thoroughly).
Whenever %_RegExpExec is called from the natives javascript, the subject is
indeed already converted into a string, but I was under the impression that
we
do not completely rely on arguments passed in the natives to be safe. Also,
this
string-check has already been there until it was messed up by one of my
previous
changes related to string slices. Obviously it had no visible effect since
in
fact only strings reach here. Should I turn this into an Assert check?
http://codereview.chromium.org/8554004/
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
