Bug 161427 - [JSC] AbstractValue can contain padding which is not zero-filled
Summary: [JSC] AbstractValue can contain padding which is not zero-filled
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords:
Depends on:
Blocks: 161268 161308
  Show dependency treegraph
 
Reported: 2016-08-30 23:06 PDT by Yusuke Suzuki
Modified: 2016-08-31 10:02 PDT (History)
6 users (show)

See Also:


Attachments
Patch (3.21 KB, patch)
2016-08-30 23:11 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (3.24 KB, patch)
2016-08-30 23:26 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff
Patch (3.31 KB, patch)
2016-08-30 23:31 PDT, Yusuke Suzuki
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2016-08-30 23:06:38 PDT
[JSC][GTK] AbstractValue can contain padding which is not zero-filled
Comment 1 Yusuke Suzuki 2016-08-30 23:11:29 PDT
Created attachment 287489 [details]
Patch
Comment 2 Carlos Garcia Campos 2016-08-30 23:22:25 PDT
Note that this is not GTK+ specific, JSCOnly compiled with GCC also fails, so it's more GCC vs clang issue.
Comment 3 Yusuke Suzuki 2016-08-30 23:26:19 PDT
Created attachment 287493 [details]
Patch
Comment 4 Yusuke Suzuki 2016-08-30 23:26:44 PDT
(In reply to comment #2)
> Note that this is not GTK+ specific, JSCOnly compiled with GCC also fails,
> so it's more GCC vs clang issue.

Renamed the issue & ChangeLog :)
Comment 5 Carlos Garcia Campos 2016-08-30 23:27:17 PDT
Comment on attachment 287489 [details]
Patch

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

Thanks for working on this!

> Source/JavaScriptCore/ChangeLog:12
> +        So debug assertion fails in GTK port.

So debug assertion fails when building with GCC.

> Source/JavaScriptCore/dfg/DFGAbstractValue.cpp:547
> +void AbstractValue::ensureCanInitializeWithZeros()
> +{
> +    std::aligned_storage<sizeof(AbstractValue), alignof(AbstractValue)>::type zeroFilledStorage;
> +    memset(static_cast<void*>(&zeroFilledStorage), 0, sizeof(AbstractValue));
> +    ASSERT(*this == *static_cast<AbstractValue*>(static_cast<void*>(&zeroFilledStorage)));
> +}

This is only used inside a #if USE(JSVALUE64) && !defined(NDEBUG) block, so maybe it should be defined using the same #if.

> Source/JavaScriptCore/dfg/DFGAbstractValue.h:461
> +    void ensureCanInitializeWithZeros();

Ditto.
Comment 6 Yusuke Suzuki 2016-08-30 23:31:06 PDT
Created attachment 287495 [details]
Patch
Comment 7 Yusuke Suzuki 2016-08-30 23:31:18 PDT
Comment on attachment 287489 [details]
Patch

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

>> Source/JavaScriptCore/ChangeLog:12
>> +        So debug assertion fails in GTK port.
> 
> So debug assertion fails when building with GCC.

Thanks. Fixed.

>> Source/JavaScriptCore/dfg/DFGAbstractValue.cpp:547
>> +}
> 
> This is only used inside a #if USE(JSVALUE64) && !defined(NDEBUG) block, so maybe it should be defined using the same #if.

OK, wrapped.

>> Source/JavaScriptCore/dfg/DFGAbstractValue.h:461
>> +    void ensureCanInitializeWithZeros();
> 
> Ditto.

Done.
Comment 8 Saam Barati 2016-08-31 08:19:16 PDT
Comment on attachment 287495 [details]
Patch

r=me
Comment 9 Yusuke Suzuki 2016-08-31 08:45:03 PDT
Comment on attachment 287495 [details]
Patch

Thanks!
Comment 10 WebKit Commit Bot 2016-08-31 10:02:12 PDT
Comment on attachment 287495 [details]
Patch

Clearing flags on attachment: 287495

Committed r205254: <http://trac.webkit.org/changeset/205254>
Comment 11 WebKit Commit Bot 2016-08-31 10:02:21 PDT
All reviewed patches have been landed.  Closing bug.