Bug 202379 - Make assertion in JSObject::putOwnDataProperty more precise
Summary: Make assertion in JSObject::putOwnDataProperty more precise
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tadeu Zagallo
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-09-30 16:12 PDT by Tadeu Zagallo
Modified: 2019-09-30 21:06 PDT (History)
8 users (show)

See Also:


Attachments
Patch (4.84 KB, patch)
2019-09-30 16:34 PDT, Tadeu Zagallo
no flags Details | Formatted Diff | Diff
Patch (5.94 KB, patch)
2019-09-30 16:37 PDT, Tadeu Zagallo
no flags Details | Formatted Diff | Diff
Patch (6.45 KB, patch)
2019-09-30 17:02 PDT, Tadeu Zagallo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tadeu Zagallo 2019-09-30 16:12:06 PDT
<rdar://problem/49515980>
Comment 1 Tadeu Zagallo 2019-09-30 16:34:34 PDT
Created attachment 379854 [details]
Patch
Comment 2 Tadeu Zagallo 2019-09-30 16:37:32 PDT
Created attachment 379855 [details]
Patch

Include test
Comment 3 Yusuke Suzuki 2019-09-30 16:42:12 PDT
Comment on attachment 379855 [details]
Patch

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

> Source/JavaScriptCore/runtime/JSObjectInlines.h:482
> +#if !ASSERT_DISABLED
> +#define VALIDATE_PUT_OWN_DATA_PROPERTY() \
> +    do { \
> +        ASSERT(value); \
> +        ASSERT(!Heap::heap(value) || Heap::heap(value) == Heap::heap(this)); \
> +        unsigned attributes; \
> +        PropertyOffset offset = structure(vm)->get(vm, propertyName, attributes); \
> +        if (isValidOffset(offset)) \
> +            ASSERT(!(attributes & (PropertyAttribute::Accessor | PropertyAttribute::CustomAccessor | PropertyAttribute::ReadOnly))); \
> +        else if (TypeInfo::hasStaticPropertyTable(inlineTypeFlags())) { \
> +            if (auto entry = findPropertyHashEntry(vm, propertyName)) { \
> +                ASSERT(!(entry->value->attributes() & (PropertyAttribute::Accessor | PropertyAttribute::CustomAccessor | PropertyAttribute::ReadOnly))); \
> +            } \
> +        } \
> +    } while (false)
> +#else
> +#define VALIDATE_PUT_OWN_DATA_PROPERTY()
> +#endif

Why not making it as an inline function?
Comment 4 Tadeu Zagallo 2019-09-30 16:43:55 PDT
Comment on attachment 379855 [details]
Patch

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

Thanks for the review.

>> Source/JavaScriptCore/runtime/JSObjectInlines.h:482
>> +#endif
> 
> Why not making it as an inline function?

I just didn't want to pass all the values through, but that does seem better. I'll update.
Comment 5 Tadeu Zagallo 2019-09-30 17:02:00 PDT
Created attachment 379861 [details]
Patch

Refactor validation into inline function
Comment 6 WebKit Commit Bot 2019-09-30 21:06:31 PDT
Comment on attachment 379861 [details]
Patch

Clearing flags on attachment: 379861

Committed r250543: <https://trac.webkit.org/changeset/250543>
Comment 7 WebKit Commit Bot 2019-09-30 21:06:33 PDT
All reviewed patches have been landed.  Closing bug.