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

Description Zan Dobersek 2014-12-17 01:50:50 PST
AsyncRequestImpl casting in AsyncRequest::completeRequest() is incorrect
Comment 1 Zan Dobersek 2014-12-17 02:13:46 PST
Created attachment 243431 [details]
Patch
Comment 2 WebKit Commit Bot 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.
Comment 3 Darin Adler 2015-01-20 09:06:51 PST
Comment on attachment 243431 [details]
Patch

Why no test? What was the incorrect symptom before?
Comment 4 Carlos Garcia Campos 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
Comment 5 Zan Dobersek 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.
Comment 6 Zan Dobersek 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>
Comment 7 Zan Dobersek 2015-01-21 11:14:11 PST
All reviewed patches have been landed.  Closing bug.