Bug 203282 – [JSC] Remove LLInt's Callee size assumption

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Yusuke Suzuki]

331     loadp Callee[cfr], t3
   332     andp MarkedBlockMask, t3
   333     loadp MarkedBlockFooterOffset + MarkedBlock::Footer::m_vm[t3], t3

This will be wrong.

[Yusuke Suzuki]

Created attachment 381784 [details]
Patch

[EWS Watchlist]

Comment on attachment 381784 [details]
Patch

Attachment 381784 [details] did not pass jsc-ews (mac):
Output: https://webkit-queues.webkit.org/results/13173398

New failing tests:
mozilla-tests.yaml/js1_5/Array/regress-101964.js.mozilla-ftl-eager-no-cjit-validate-phases

[EWS Watchlist]

Comment on attachment 381784 [details]
Patch

Attachment 381784 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/13173450

Number of test failures exceeded the failure limit.

[EWS Watchlist]

Created attachment 381799 [details]
Archive of layout-test-results from ews213 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews213  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit

[Yusuke Suzuki]

Created attachment 381826 [details]
Patch

[Yusuke Suzuki]

Created attachment 381829 [details]
Patch

[Mark Lam]

Comment on attachment 381829 [details]
Patch

r=me

[Keith Miller]

Comment on attachment 381829 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=381829&action=review

r=me with comments.

> Source/JavaScriptCore/ChangeLog:15
> +           exception unwinding happens, and it is already taking some time.

Nit: maybe change "and it is already taking some time" => "which is an expensive operation anyway"

> Source/JavaScriptCore/heap/LargeAllocation.h:155
> +    static constexpr unsigned headerSize()
> +    {
> +        return ((sizeof(LargeAllocation) + halfAlignment - 1) & ~(halfAlignment - 1)) | halfAlignment;
> +    }

Nit: Can we put this on one line?

> Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:2045
> +    if X86 or X86_WIN
> +        addp 8, sp
> +    elsif MIPS
> +        addp 24, sp
> +    else
> +        addp 8, sp
> +    end

Can this just be:
if MIPS 
    addp 24, sp
else
    addp 8, sp
end

[Yusuke Suzuki]

Comment on attachment 381829 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=381829&action=review

Thanks!

>> Source/JavaScriptCore/ChangeLog:15
>> +           exception unwinding happens, and it is already taking some time.
> 
> Nit: maybe change "and it is already taking some time" => "which is an expensive operation anyway"

OK, changed.

>> Source/JavaScriptCore/heap/LargeAllocation.h:155
>> +    }
> 
> Nit: Can we put this on one line?

Done.

>> Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm:2045
>> +    end
> 
> Can this just be:
> if MIPS 
>     addp 24, sp
> else
>     addp 8, sp
> end

Sounds nice, fixed.

[Yusuke Suzuki]

Committed r251556: <https://trac.webkit.org/changeset/251556>

[Radar WebKit Bug Importer]

<rdar://problem/56591933>