Bug 201495 - [JSC] Do not use FTLOutput::weakPointer directly
Summary: [JSC] Do not use FTLOutput::weakPointer directly
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: InRadar
Depends on:
Blocks:
 
Reported: 2019-09-04 23:56 PDT by Yusuke Suzuki
Modified: 2019-09-05 16:21 PDT (History)
9 users (show)

See Also:


Attachments
Patch (4.42 KB, patch)
2019-09-05 00:00 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 2019-09-04 23:56:19 PDT
[JSC] Do not use FTLOutput::weakPointer directly
Comment 1 Yusuke Suzuki 2019-09-05 00:00:23 PDT
Created attachment 378051 [details]
Patch
Comment 2 Yusuke Suzuki 2019-09-05 00:01:21 PDT
Comment on attachment 378051 [details]
Patch

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

> Source/JavaScriptCore/ChangeLog:12
> +        For FrozenValue, we should use frozenPointer helper function.

Note that DFG implementation of CreatePromise/NewPromise are using DFGSpeculativeJIT::TrustedImmPtr::weakPointer correctly.
And I checked NewPromise and it was OK.
Comment 3 Yusuke Suzuki 2019-09-05 00:02:15 PDT
<rdar://problem/55055463>
Comment 4 Yusuke Suzuki 2019-09-05 00:09:40 PDT
Ensures this patch fixes the debug assertion in jsc-stress-tests.
https://build.webkit.org/builders/Apple%20High%20Sierra%20Debug%20JSC%20%28Tests%29/builds/3589
Comment 5 Filip Pizlo 2019-09-05 08:08:15 PDT
Comment on attachment 378051 [details]
Patch

Can you do a follow-up that renames FTL::Output::weakPointer so that folks don't make the same mistake?  Maybe Output::alreadyRegisteredWeakPointer or something that makes it clear what the assumptions are.
Comment 6 WebKit Commit Bot 2019-09-05 08:51:53 PDT
Comment on attachment 378051 [details]
Patch

Clearing flags on attachment: 378051

Committed r249530: <https://trac.webkit.org/changeset/249530>
Comment 7 WebKit Commit Bot 2019-09-05 08:51:54 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Yusuke Suzuki 2019-09-05 11:14:44 PDT
(In reply to Filip Pizlo from comment #5)
> Comment on attachment 378051 [details]
> Patch
> 
> Can you do a follow-up that renames FTL::Output::weakPointer so that folks
> don't make the same mistake?  Maybe Output::alreadyRegisteredWeakPointer or
> something that makes it clear what the assumptions are.

Sounds nice! I'll change this.
Comment 9 Yusuke Suzuki 2019-09-05 16:21:27 PDT
Committed r249552: <https://trac.webkit.org/changeset/249552>