Bug 214067 – Add a way to return early from detected infinite loops to aid the fuzzer

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

[Saam Barati]

...

[Saam Barati]

Created attachment 403753 [details]
patch

[Yusuke Suzuki]

Comment on attachment 403753 [details]
patch

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

r=me

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:14718
> +        patchpoint->effects.writesLocalState = true;

patchpoint->clobber(RegisterSet::macroScratchRegisters()); is required.

> Source/JavaScriptCore/runtime/VM.cpp:1562
> +        uint64_t* ptr = static_cast<uint64_t*>(fastMalloc(sizeof(uint64_t)));
> +        *ptr = 0;
> +        addResult.iterator->value.second = ptr;

How about using `std::unique_ptr<uint64_t>` & `makeUnique<uint64_t>()`?

> Source/JavaScriptCore/runtime/VM.cpp:1582
> +        fastFree(iter->value.second);

Then, we can remove this.

[Saam Barati]

Comment on attachment 403753 [details]
patch

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

>> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:14718
>> +        patchpoint->effects.writesLocalState = true;
> 
> patchpoint->clobber(RegisterSet::macroScratchRegisters()); is required.

Actually I tried to not make it so that we use them. I added AllowMacroScratxh RAII before I refactored the code. I will remove the RAII

>> Source/JavaScriptCore/runtime/VM.cpp:1562
>> +        addResult.iterator->value.second = ptr;
> 
> How about using `std::unique_ptr<uint64_t>` & `makeUnique<uint64_t>()`?

Sounds good

[Mark Lam]

Comment on attachment 403753 [details]
patch

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

> Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:468
> +        *ptr += codeBlock->llintExecuteCounter().m_activeThreshold;

Why inc by m_activeThreshold here and only by 1 in the JITs?

[Saam Barati]

Comment on attachment 403753 [details]
patch

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

>> Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:468
>> +        *ptr += codeBlock->llintExecuteCounter().m_activeThreshold;
> 
> Why inc by m_activeThreshold here and only by 1 in the JITs?

this is just a heuristic of how much we've executed. This is actually slightly higher than what we've executed. It's because I don't want to change the LLInt's runtime code. But changing the JITs is trivial, since it's controlled behind a runtime option.

[Saam Barati]

Created attachment 403757 [details]
patch for landing

[Mark Lam]

Comment on attachment 403753 [details]
patch

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

>>> Source/JavaScriptCore/llint/LLIntSlowPaths.cpp:468
>>> +        *ptr += codeBlock->llintExecuteCounter().m_activeThreshold;
>> 
>> Why inc by m_activeThreshold here and only by 1 in the JITs?
> 
> this is just a heuristic of how much we've executed. This is actually slightly higher than what we've executed. It's because I don't want to change the LLInt's runtime code. But changing the JITs is trivial, since it's controlled behind a runtime option.

OK, I understand now.  The detail to note is that this function (llint_loop_osr) is only called once after the loop_hint has been visited m_activeThreshold times.  It is not called on every loop_hint.

[Saam Barati]

Created attachment 403758 [details]
patch for landing

Now skipping this for builtin functions, since we rely on some of those not early returning out of loops for the integrity of data structures we create.

[EWS]

Committed r264105: <https://trac.webkit.org/changeset/264105>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 403758 [details].

[Radar WebKit Bug Importer]

<rdar://problem/65227843>