Bug 224565 - Defer TerminationExceptions when evaluating ASSERT in HashMapIml::addNormalized().
Summary: Defer TerminationExceptions when evaluating ASSERT in HashMapIml::addNormaliz...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-04-14 11:31 PDT by Mark Lam
Modified: 2021-04-14 14:53 PDT (History)
7 users (show)

See Also:


Attachments
proposed patch. (8.79 KB, patch)
2021-04-14 11:49 PDT, Mark Lam
no flags Details | Formatted Diff | Diff
proposed patch. (9.16 KB, patch)
2021-04-14 14:24 PDT, Mark Lam
ysuzuki: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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>.