Bug 186920 - [WTF] Use Ref<> for the result type of non-failing factory functions
Summary: [WTF] Use Ref<> for the result type of non-failing factory functions
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: 2018-06-22 07:39 PDT by Yusuke Suzuki
Modified: 2018-06-27 19:38 PDT (History)
14 users (show)

See Also:


Attachments
Patch (17.09 KB, patch)
2018-06-22 07:40 PDT, Yusuke Suzuki
darin: review+
ews-watchlist: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews202 for win-future (12.85 MB, application/zip)
2018-06-23 02:22 PDT, EWS Watchlist
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Yusuke Suzuki 2018-06-22 07:39:42 PDT
[WTF] Use Ref<> for the result type of non-failing factory functions
Comment 1 Yusuke Suzuki 2018-06-22 07:40:35 PDT
Created attachment 343324 [details]
Patch
Comment 2 Darin Adler 2018-06-22 17:43:09 PDT
Comment on attachment 343324 [details]
Patch

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

> Source/WTF/wtf/AutomaticThread.cpp:38
> +    return adoptRef(*new AutomaticThreadCondition());

Another thing I like to do at call sites like this one is leave out the optional ().
Comment 3 EWS Watchlist 2018-06-23 02:22:09 PDT
Comment on attachment 343324 [details]
Patch

Attachment 343324 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8302312

New failing tests:
http/tests/preload/onload_event.html
Comment 4 EWS Watchlist 2018-06-23 02:22:21 PDT
Created attachment 343427 [details]
Archive of layout-test-results from ews202 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews202  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment 5 Yusuke Suzuki 2018-06-23 02:59:09 PDT
Committed r233123: <https://trac.webkit.org/changeset/233123>
Comment 6 Radar WebKit Bug Importer 2018-06-23 03:02:03 PDT
<rdar://problem/41395636>
Comment 7 Truitt Savell 2018-06-26 11:07:18 PDT
It looks like after this revision we are getting JSC failures:

https://build.webkit.org/builders/Apple%20High%20Sierra%20Debug%20JSC%20%28Tests%29/builds/1180/steps/jscore-test/logs/stdio

stress/dfg-put-getter-by-val-class.js.default: ASSERTION FAILED: !getDirect(offset) || !JSValue::encode(getDirect(offset))
stress/dfg-put-getter-by-val-class.js.default: /Volumes/Data/slave/highsierra-debug/build/Source/JavaScriptCore/runtime/JSObjectInlines.h(335) : bool JSC::JSObject::putDirectInternal(JSC::VM &, JSC::PropertyName, JSC::JSValue, unsigned int, JSC::PutPropertySlot &)
stress/dfg-put-getter-by-val-class.js.default: 1   0x10ed66a69 WTFCrash
stress/dfg-put-getter-by-val-class.js.default: 2   0x10eea5766 bool JSC::JSObject::putDirectInternal<(JSC::JSObject::PutMode)1>(JSC::VM&, JSC::PropertyName, JSC::JSValue, unsigned int, JSC::PutPropertySlot&)
stress/dfg-put-getter-by-val-class.js.default: 3   0x10eea4580 JSC::JSObject::putDirect(JSC::VM&, JSC::PropertyName, JSC::JSValue, unsigned int)
stress/dfg-put-getter-by-val-class.js.default: 4   0x110008bf2 JSC::JSFunction::reifyName(JSC::VM&, JSC::ExecState*, WTF::String)
stress/dfg-put-getter-by-val-class.js.default: 5   0x1100088b9 JSC::JSFunction::setFunctionName(JSC::ExecState*, JSC::JSValue)
stress/dfg-put-getter-by-val-class.js.default: 6   0x10fd17395 operationSetFunctionName
stress/dfg-put-getter-by-val-class.js.default: 7   0x547d9a5f728
stress/dfg-put-getter-by-val-class.js.default: 8   0x10ee58ddc llint_entry
stress/dfg-put-getter-by-val-class.js.default: 9   0x10ee50842 vmEntryToJavaScript
stress/dfg-put-getter-by-val-class.js.default: 10  0x10fcacfaa JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
stress/dfg-put-getter-by-val-class.js.default: 11  0x10fcac551 JSC::Interpreter::executeProgram(JSC::SourceCode const&, JSC::ExecState*, JSC::JSObject*)
stress/dfg-put-getter-by-val-class.js.default: 12  0x10ff63947 JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&)
stress/dfg-put-getter-by-val-class.js.default: 13  0x10ec9fd00 runWithOptions(GlobalObject*, CommandLine&, bool&)
stress/dfg-put-getter-by-val-class.js.default: 14  0x10ec7748c jscmain(int, char**)::$_3::operator()(JSC::VM&, GlobalObject*, bool&) const
stress/dfg-put-getter-by-val-class.js.default: 15  0x10ec5ecb4 int runJSC<jscmain(int, char**)::$_3>(CommandLine, bool, jscmain(int, char**)::$_3 const&)
stress/dfg-put-getter-by-val-class.js.default: 16  0x10ec5d79f jscmain(int, char**)
stress/dfg-put-getter-by-val-class.js.default: 17  0x10ec5d6fe main
stress/dfg-put-getter-by-val-class.js.default: 18  0x7fff6e830015 start
stress/dfg-put-getter-by-val-class.js.default: test_script_7860: line 2: 10510 Segmentation fault: 11  ( "$@" ../../.vm/JavaScriptCore.framework/Resources/jsc --useFTLJIT\=false --useFunctionDotArguments\=true --validateExceptionChecks\=true --useDollarVM\=true --maxPerThreadStackUsage\=1572864 --useFTLJIT\=true dfg-put-getter-by-val-class.js )
stress/dfg-put-getter-by-val-class.js.default: ERROR: Unexpected exit code: 139
FAIL: stress/dfg-put-getter-by-val-class.js.default
Running stress/dfg-put-getter-by-val-class.js.dfg-maximal-flush-validate-no-cjit
Comment 8 Ryan Haddad 2018-06-26 11:16:13 PDT
(In reply to Truitt from comment #7)
> It looks like after this revision we are getting JSC failures:

I don't think this is the right change to blame for the assertion failure.
Comment 9 Yusuke Suzuki 2018-06-26 17:20:08 PDT
(In reply to Ryan Haddad from comment #8)
> (In reply to Truitt from comment #7)
> > It looks like after this revision we are getting JSC failures:
> 
> I don't think this is the right change to blame for the assertion failure.

Yeah, I don't think this change affects on that issue.
Comment 10 Yusuke Suzuki 2018-06-27 19:38:29 PDT
(In reply to Truitt Savell from comment #7)
> It looks like after this revision we are getting JSC failures:
> 
> https://build.webkit.org/builders/
> Apple%20High%20Sierra%20Debug%20JSC%20%28Tests%29/builds/1180/steps/jscore-
> test/logs/stdio
> 
> stress/dfg-put-getter-by-val-class.js.default: ASSERTION FAILED:
> !getDirect(offset) || !JSValue::encode(getDirect(offset))
> stress/dfg-put-getter-by-val-class.js.default:
> /Volumes/Data/slave/highsierra-debug/build/Source/JavaScriptCore/runtime/
> JSObjectInlines.h(335) : bool JSC::JSObject::putDirectInternal(JSC::VM &,
> JSC::PropertyName, JSC::JSValue, unsigned int, JSC::PutPropertySlot &)
> stress/dfg-put-getter-by-val-class.js.default: 1   0x10ed66a69 WTFCrash
> stress/dfg-put-getter-by-val-class.js.default: 2   0x10eea5766 bool
> JSC::JSObject::putDirectInternal<(JSC::JSObject::PutMode)1>(JSC::VM&,
> JSC::PropertyName, JSC::JSValue, unsigned int, JSC::PutPropertySlot&)
> stress/dfg-put-getter-by-val-class.js.default: 3   0x10eea4580
> JSC::JSObject::putDirect(JSC::VM&, JSC::PropertyName, JSC::JSValue, unsigned
> int)
> stress/dfg-put-getter-by-val-class.js.default: 4   0x110008bf2
> JSC::JSFunction::reifyName(JSC::VM&, JSC::ExecState*, WTF::String)
> stress/dfg-put-getter-by-val-class.js.default: 5   0x1100088b9
> JSC::JSFunction::setFunctionName(JSC::ExecState*, JSC::JSValue)
> stress/dfg-put-getter-by-val-class.js.default: 6   0x10fd17395
> operationSetFunctionName
> stress/dfg-put-getter-by-val-class.js.default: 7   0x547d9a5f728
> stress/dfg-put-getter-by-val-class.js.default: 8   0x10ee58ddc llint_entry
> stress/dfg-put-getter-by-val-class.js.default: 9   0x10ee50842
> vmEntryToJavaScript
> stress/dfg-put-getter-by-val-class.js.default: 10  0x10fcacfaa
> JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*)
> stress/dfg-put-getter-by-val-class.js.default: 11  0x10fcac551
> JSC::Interpreter::executeProgram(JSC::SourceCode const&, JSC::ExecState*,
> JSC::JSObject*)
> stress/dfg-put-getter-by-val-class.js.default: 12  0x10ff63947
> JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue,
> WTF::NakedPtr<JSC::Exception>&)
> stress/dfg-put-getter-by-val-class.js.default: 13  0x10ec9fd00
> runWithOptions(GlobalObject*, CommandLine&, bool&)
> stress/dfg-put-getter-by-val-class.js.default: 14  0x10ec7748c jscmain(int,
> char**)::$_3::operator()(JSC::VM&, GlobalObject*, bool&) const
> stress/dfg-put-getter-by-val-class.js.default: 15  0x10ec5ecb4 int
> runJSC<jscmain(int, char**)::$_3>(CommandLine, bool, jscmain(int,
> char**)::$_3 const&)
> stress/dfg-put-getter-by-val-class.js.default: 16  0x10ec5d79f jscmain(int,
> char**)
> stress/dfg-put-getter-by-val-class.js.default: 17  0x10ec5d6fe main
> stress/dfg-put-getter-by-val-class.js.default: 18  0x7fff6e830015 start
> stress/dfg-put-getter-by-val-class.js.default: test_script_7860: line 2:
> 10510 Segmentation fault: 11  ( "$@"
> ../../.vm/JavaScriptCore.framework/Resources/jsc --useFTLJIT\=false
> --useFunctionDotArguments\=true --validateExceptionChecks\=true
> --useDollarVM\=true --maxPerThreadStackUsage\=1572864 --useFTLJIT\=true
> dfg-put-getter-by-val-class.js )
> stress/dfg-put-getter-by-val-class.js.default: ERROR: Unexpected exit code:
> 139
> FAIL: stress/dfg-put-getter-by-val-class.js.default
> Running
> stress/dfg-put-getter-by-val-class.js.dfg-maximal-flush-validate-no-cjit

This is fixed in https://bugs.webkit.org/show_bug.cgi?id=187091