Ref<FetchResponse> use-after-move in DOMCache::put()
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.
<rdar://problem/44969033>
> This is apparently changing in C++17, guaranteeing the function-name > expression being evaluated before the arguments. C++17 seems tempting then!