Fixed regression introduced in r9027.

Fixed regression introduced in r9027.

Committed: http://code.google.com/p/v8/source/detail?r=9029

[Yang, 2011-08-26 13:21:29]

PTAL.

[Sven Panne, 2011-08-26 13:36:21]

LGTM for the one-word patch itself, but the surrounding code has to be clarified
(see my comment).

http://codereview.chromium.org/7741041/diff/1/src/x64/regexp-macro-assembler-...
File src/x64/regexp-macro-assembler-x64.cc (right):

http://codereview.chromium.org/7741041/diff/1/src/x64/regexp-macro-assembler-...
src/x64/regexp-macro-assembler-x64.cc:1188: intptr_t delta = *code_handle -
re_code;
This change looks OK, but...

http://codereview.chromium.org/7741041/diff/1/src/x64/regexp-macro-assembler-...
src/x64/regexp-macro-assembler-x64.cc:1190: *return_address += delta;
This (old) code looks wrong: The delta is in units of Code, but we add it onto a
byte pointer. Is this really correct? I suspect that we should cast the Code
pointers to byte pointers before doing the subtraction. If I am wrong, a comment
why the code is correct should be added.

[Vyacheslav Egorov (Chromium), 2011-08-26 13:41:03]

+lrn

http://codereview.chromium.org/7741041/diff/1/src/x64/regexp-macro-assembler-...
File src/x64/regexp-macro-assembler-x64.cc (right):

http://codereview.chromium.org/7741041/diff/1/src/x64/regexp-macro-assembler-...
src/x64/regexp-macro-assembler-x64.cc:1190: *return_address += delta;
On 2011/08/26 13:36:21, Sven wrote:
> This (old) code looks wrong: The delta is in units of Code, but we add it onto
a
> byte pointer. Is this really correct? I suspect that we should cast the Code
> pointers to byte pointers before doing the subtraction. If I am wrong, a
comment
> why the code is correct should be added.

Yes. Nice catch.

For type safety we need to do code_handle->address() - re_code->address().

sizeof(Code) == 0 so I guess it somehow worked as expected.

[Sven Panne, 2011-08-26 13:57:36]

On 2011/08/26 13:41:03, Vyacheslav Egorov wrote:
> +lrn
> 
>
http://codereview.chromium.org/7741041/diff/1/src/x64/regexp-macro-assembler-...
> File src/x64/regexp-macro-assembler-x64.cc (right):
> 
>
http://codereview.chromium.org/7741041/diff/1/src/x64/regexp-macro-assembler-...
> src/x64/regexp-macro-assembler-x64.cc:1190: *return_address += delta;
> On 2011/08/26 13:36:21, Sven wrote:
> > This (old) code looks wrong: The delta is in units of Code, but we add it
onto
> a
> > byte pointer. Is this really correct? I suspect that we should cast the Code
> > pointers to byte pointers before doing the subtraction. If I am wrong, a
> comment
> > why the code is correct should be added.
> 
> Yes. Nice catch.
> 
> For type safety we need to do code_handle->address() - re_code->address().
> 
> sizeof(Code) == 0 so I guess it somehow worked as expected.

This was a surprise to me as well, so I've just googled a bit:

   http://www2.research.att.com/~bs/bs_faq2.html#sizeof-empty
  
http://stackoverflow.com/questions/3849334/sizeof-empty-structure-is-0-in-c-a...

Somehow I would prefer a compiler warning/error in such cases like ours... :-P

[Lasse Reichstein, 2011-08-26 15:43:48]

LGTM.
You shouldn't need crankshaft, just a compacting GC that moves the calling Code
object. I'm not sure there is a good way to do that consistently, though.