Bug 224565

Summary: Defer TerminationExceptions when evaluating ASSERT in HashMapIml::addNormalized().
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: ews-watchlist, keith_miller, msaboff, saam, tzagallo, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed patch.
none
proposed patch. ysuzuki: review+

Description Mark Lam 2021-04-14 11:31:40 PDT 
rdar://76645980
Comment 1 Mark Lam 2021-04-14 11:49:05 PDT 
Created attachment 426026 [details]
proposed patch.
Comment 2 Yusuke Suzuki 2021-04-14 12:20:41 PDT 
Comment on attachment 426026 [details]
proposed patch.

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

> Source/JavaScriptCore/runtime/ExceptionHelpers.h:72
> +#define DEFER_TERMINATION_AND_ASSERT(vm, assertion, ...) do { \
> +        JSC::DeferTerminationForAWhile deferScope(vm); \
> +        ASSERT(assertion, __VA_ARGS__); \
> +    } while (false)
> +
> +#define DEFER_TERMINATION_AND_ASSERT_WITH_MESSAGE(vm, assertion, ...) do { \
> +        JSC::DeferTerminationForAWhile deferScope(vm); \
> +        ASSERT_WITH_MESSAGE(assertion, __VA_ARGS__); \
> +    } while (false)

Let's make them nop for Release builds. JSC::DeferTerminationForAWhile is calling deferTermination / undoDeferTermination functions. And I think addNormalized is really critical enough so that we should not insert these calls.
Comment 3 Mark Lam 2021-04-14 14:24:11 PDT 
Created attachment 426046 [details]
proposed patch.
Comment 4 Yusuke Suzuki 2021-04-14 14:28:32 PDT 
Comment on attachment 426046 [details]
proposed patch.

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

r=me

> Source/JavaScriptCore/runtime/VMTraps.h:194
> +    enum DeferAction {

Let’s use enum class.
Comment 5 Mark Lam 2021-04-14 14:53:04 PDT 
Thanks for the review.  I've made the enum class change.

Landed in r275969: <http://trac.webkit.org/r275969>.