Bug 139724

Summary: AsyncRequestImpl casting in AsyncRequest::completeRequest() is incorrect
Product: WebKit Reporter: Zan Dobersek <zan>
Component: New BugsAssignee: Zan Dobersek <zan>
Status: RESOLVED FIXED    
Severity: Normal CC: andersca, beidson, cgarcia, commit-queue, darin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 139491    
Attachments:
Description Flags
Patch none

Zan Dobersek
Reported 2014-12-17 01:50:50 PST
AsyncRequestImpl casting in AsyncRequest::completeRequest() is incorrect
Attachments
Patch (11.60 KB, patch)
2014-12-17 02:13 PST, Zan Dobersek 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Zan Dobersek
Comment 1 2014-12-17 02:13:46 PST
Created attachment 243431 [details] Patch
WebKit Commit Bot
Comment 2 2014-12-17 02:16:13 PST
Attachment 243431 [details] did not pass style-queue: ERROR: Source/WebKit2/Shared/AsyncRequest.h:62: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/Shared/AsyncRequest.h:67: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/Shared/AsyncRequest.h:85: Extra space before ( in function call [whitespace/parens] [4] ERROR: Source/WebKit2/Shared/AsyncRequest.h:97: Extra space before ( in function call [whitespace/parens] [4] Total errors found: 4 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Darin Adler
Comment 3 2015-01-20 09:06:51 PST
Comment on attachment 243431 [details] Patch Why no test? What was the incorrect symptom before?
Carlos Garcia Campos
Comment 4 2015-01-20 09:31:19 PST
(In reply to comment #3) > Comment on attachment 243431 [details] > Patch > > Why no test? What was the incorrect symptom before? I don't know why it works with clang, but with gcc I got a lot of layout tests failures when I implemented the database process sopport for GTK port. So, at the beginning I thought it was a compiler bug, see the discussion here https://bugs.webkit.org/show_bug.cgi?id=139491
Zan Dobersek
Comment 5 2015-01-21 10:57:53 PST
(In reply to comment #3) > Comment on attachment 243431 [details] > Patch > > Why no test? What was the incorrect symptom before? Because of incorrect casting of the std::function<> object (through AsyncRequestImpl) the argument for the uint32_t parameter was passed in as a reference when invoking the wrapped function, while the function expected an argument that was passed in by value. Because of that the uint32_t argument actually held the address of the relevant uint32_t object, instead of the value. This was only occurring when compiling with GCC. Getting the IDB tests running in GCC-compiled builds would show that this works as intended.
Zan Dobersek
Comment 6 2015-01-21 11:13:55 PST
Comment on attachment 243431 [details] Patch Clearing flags on attachment: 243431 Committed r178850: <http://trac.webkit.org/changeset/178850>
Zan Dobersek
Comment 7 2015-01-21 11:14:11 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.