1605591 - Check if bytecode exists before accessing length in JITs

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, task, P2)

Comment 1

[Ted Campbell [:tcampbell]]

In [1,2] we access the ImmutableScriptData::funLength field after checking for the INTERPRETED bit on function. Once this flag is removed, we'll have to handle the lazy case in another way. This can be done by checking if the BaseScript::sharedData() pointer is null before dereferencing it.

Special care must be taken in the bound function init case in Ion so that side-effects don't cause problems.

[1] https://searchfox.org/mozilla-central/rev/8d04c3f5332d470eeae5aa3dc0ed132359a339c1/js/src/jit/CacheIRCompiler.cpp#2975
[2] https://searchfox.org/mozilla-central/rev/8d04c3f5332d470eeae5aa3dc0ed132359a339c1/js/src/jit/CodeGenerator.cpp#13850

Comment 2

[Ted Campbell [:tcampbell]]

Attachment: Bug 1605591 - Factor out BoundFunctionLength in IonMonkey. r?jandem

Factor out the bound-function length computation from Ion code generator.
This is to make register usage and clobbers easier to understand.

Comment 3

[Ted Campbell [:tcampbell]]

Attachment: Bug 1605591 - Factor out BoundFunctionFlags in IonMonkey. r?jandem

Depends on D58082

Comment 4

[Ted Campbell [:tcampbell]]

Attachment: Bug 1605591 - Always check sharedData before reading funLength. r?jandem

In the bound function case, we move the length initialization before the name
is set. This may result in the length being set before the guards are all
finished, but that doesn't affect the slow-path.

Depends on D58083

Comment 5

[Ted Campbell [:tcampbell]]

Attachment: Bug 1605591 - Factor out BoundFunctionName in IonMonkey. r?jandem

Depends on D58084

Comment 6

[Ted Campbell [:tcampbell]]

Attachment: Bug 1605591 - Add MacroAssembler::loadFunctionLength method. r?jandem

Use this to share the lookup code between CacheIR and Ion. Note that in the
CacheIR case we clobber the flags register, while for Ion we preserve it.

Comment 7

[Ted Campbell [:tcampbell]]

I've added another patch to do some cleanup, but will push the main patches to reduce size of lazyscript-removal stack.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/46f0c623959a
Factor out BoundFunctionLength in IonMonkey. r=jandem
https://hg.mozilla.org/integration/autoland/rev/4c34b7791e88
Factor out BoundFunctionFlags in IonMonkey. r=jandem
https://hg.mozilla.org/integration/autoland/rev/9ec61acfd6fc
Always check sharedData before reading funLength. r=jandem
https://hg.mozilla.org/integration/autoland/rev/48159e53bfb8
Factor out BoundFunctionName in IonMonkey. r=jandem

Comment 9

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/46f0c623959a
https://hg.mozilla.org/mozilla-central/rev/4c34b7791e88
https://hg.mozilla.org/mozilla-central/rev/9ec61acfd6fc
https://hg.mozilla.org/mozilla-central/rev/48159e53bfb8

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/44894f33514a
Add MacroAssembler::loadFunctionLength method. r=jandem

Comment 11

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/44894f33514a