Please ignore this one. Sergey fixed this the right way. :)

Thanks Sergey.

-- Mads

On Thu, May 6, 2010 at 11:35 AM,  <[email protected]> wrote:
> Reviewers: SeRya,
>
> Description:
> Remove smi check in regexp entry stub. In principle, a non-smi index
> can be passed into the regexp entry stub if someone gets hold of the
> builtins object. However, there is little harm in that since the only
> thing that can happen is that we will perform a regexp match with a
> strange index. This is okay since the call would make no sense in any
> case.
>
>
> Please review this at http://codereview.chromium.org/2034001/show
>
> SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/
>
> Affected files:
>  M     src/arm/codegen-arm.cc
>  M     src/ia32/codegen-ia32.cc
>  M     src/x64/codegen-x64.cc
>
>
> Index: src/ia32/codegen-ia32.cc
> ===================================================================
> --- src/ia32/codegen-ia32.cc    (revision 4597)
> +++ src/ia32/codegen-ia32.cc    (working copy)
> @@ -2702,7 +2702,7 @@
>       } else {
>         __ mov(temp2.reg(),
>                FieldOperand(left_side.reg(), String::kLengthOffset));
> -        __ SmiUntag(temp2.reg());
> +        ASSERT(kSmiTag == 0);
>         __ sub(Operand(temp2.reg()), Immediate(1));
>         Label comparison;
>         // If the length is 0 then the subtraction gave -1 which compares
> less
> @@ -10952,11 +10952,12 @@
>   // ebx: Length of subject string as a smi
>   // ecx: RegExp data (FixedArray)
>   // edx: Number of capture registers
> -  // Check that the third argument is a positive smi less than the subject
> -  // string length. A negative value will be greater (unsigned comparison).
> +  // Check that the third argument (assumed to be a smi) is less than
> +  // the subject string length. In principle, if someone got hold of
> +  // the builtins object they could pass in a heap pointer here which
> +  // would lead to a strange index but there seems to be no harm in
> +  // that. A negative value will be greater (unsigned comparison).
>   __ mov(eax, Operand(esp, kPreviousIndexOffset));
> -  __ test(eax, Immediate(kSmiTagMask));
> -  __ j(zero, &runtime);
>   __ cmp(eax, Operand(ebx));
>   __ j(above_equal, &runtime);
>
> Index: src/x64/codegen-x64.cc
> ===================================================================
> --- src/x64/codegen-x64.cc      (revision 4597)
> +++ src/x64/codegen-x64.cc      (working copy)
> @@ -8133,10 +8133,12 @@
>   // rbx: Length of subject string as smi
>   // rcx: RegExp data (FixedArray)
>   // rdx: Number of capture registers
> -  // Check that the third argument is a positive smi less than the string
> -  // length. A negative value will be greater (unsigned comparison).
> +  // Check that the third argument (assumed to be a smi) is less than
> +  // the subject string length. In principle, if someone got hold of
> +  // the builtins object they could pass in a heap pointer here which
> +  // would lead to a strange index but there seems to be no harm in
> +  // that. A negative value will be greater (unsigned comparison).
>   __ movq(rax, Operand(rsp, kPreviousIndexOffset));
> -  __ JumpIfNotSmi(rax, &runtime);
>   __ SmiCompare(rax, rbx);
>   __ j(above_equal, &runtime);
>
> Index: src/arm/codegen-arm.cc
> ===================================================================
> --- src/arm/codegen-arm.cc      (revision 4597)
> +++ src/arm/codegen-arm.cc      (working copy)
> @@ -8497,13 +8497,14 @@
>   // r3: Length of subject string as a smi
>   // subject: Subject string
>   // regexp_data: RegExp data (FixedArray)
> -  // Check that the third argument is a positive smi less than the subject
> -  // string length. A negative value will be greater (unsigned comparison).
> +  // Check that the third argument (assumed to be a smi) is less than
> +  // the subject string length. In principle, if someone got hold of
> +  // the builtins object they could pass in a heap pointer here which
> +  // would lead to a strange index but there seems to be no harm in
> +  // that. A negative value will be greater (unsigned comparison).
>   __ ldr(r0, MemOperand(sp, kPreviousIndexOffset));
> -  __ tst(r0, Operand(kSmiTagMask));
> -  __ b(eq, &runtime);
> -  __ cmp(r3, Operand(r0));
> -  __ b(le, &runtime);
> +  __ cmp(r0, Operand(r3));
> +  __ b(hs, &runtime);
>
>   // r2: Number of capture registers
>   // subject: Subject string
>
>
>

-- 
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to