Bug 139724 - AsyncRequestImpl casting in AsyncRequest::completeRequest() is incorrect
Summary: AsyncRequestImpl casting in AsyncRequest::completeRequest() is incorrect
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zan Dobersek
URL:
Keywords:
Depends on:
Blocks: 139491
  Show dependency tree / graph
 
Reported: 2014-12-17 01:50 PST by Zan Dobersek
Modified: 2015-01-21 11:14 PST (History)
5 users (show)

See Also:

Attachments
Patch (11.60 KB, patch)
2014-12-17 02:13 PST, Zan Dobersek 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.