Summary: | Ref<FetchResponse> use-after-move in DOMCache::put() | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Zan Dobersek <zan> | ||||
Component: | New Bugs | Assignee: | Zan Dobersek <zan> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | fred.wang, mcatanzaro, webkit-bug-importer, youennf | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Zan Dobersek
2018-10-03 01:22:38 PDT
Created attachment 351492 [details]
Patch
Comment on attachment 351492 [details]
Patch
What a footgun :(
Comment on attachment 351492 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=351492&action=review > Source/WebCore/Modules/cache/DOMCache.cpp:354 > + responseRef.consumeBodyReceivedByChunk([promise = WTFMove(promise), request = WTFMove(request), response = WTFMove(response), data = SharedBuffer::create(), pendingActivity = makePendingActivity(*this), this](auto&& result) mutable { Is it really a bug on compilers? Shouldn't they all do: 1. Call -> operator on the Ref<FetchResponse> 2. Call consumeBodyReceivedByChunk or move the Ref<FetchResponse> in the lambda 3. Move the Ref<FetchResponse> in the lambda or call consumeBodyReceivedByChunk (In reply to youenn fablet from comment #3) > Comment on attachment 351492 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=351492&action=review > > > Source/WebCore/Modules/cache/DOMCache.cpp:354 > > + responseRef.consumeBodyReceivedByChunk([promise = WTFMove(promise), request = WTFMove(request), response = WTFMove(response), data = SharedBuffer::create(), pendingActivity = makePendingActivity(*this), this](auto&& result) mutable { > > Is it really a bug on compilers? > Shouldn't they all do: > 1. Call -> operator on the Ref<FetchResponse> > 2. Call consumeBodyReceivedByChunk or move the Ref<FetchResponse> in the > lambda > 3. Move the Ref<FetchResponse> in the lambda or call > consumeBodyReceivedByChunk It's a bug in our code, but compilers are free to choose the desired order of evaluating the call expression as well as each argument: https://en.cppreference.com/w/cpp/language/operator_other#Built-in_function_call_operator This is apparently changing in C++17, guaranteeing the function-name expression being evaluated before the arguments. Comment on attachment 351492 [details] Patch Clearing flags on attachment: 351492 Committed r236789: <https://trac.webkit.org/changeset/236789> All reviewed patches have been landed. Closing bug. > This is apparently changing in C++17, guaranteeing the function-name
> expression being evaluated before the arguments.
C++17 seems tempting then!
|