Bug 139724 – AsyncRequestImpl casting in AsyncRequest::completeRequest() is incorrect

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Zan Dobersek]

AsyncRequestImpl casting in AsyncRequest::completeRequest() is incorrect

[Zan Dobersek]

Created attachment 243431 [details]
Patch

[WebKit Commit Bot]

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 on attachment 243431 [details]
Patch

Why no test? What was the incorrect symptom before?

[Carlos Garcia Campos]

(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]

(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 on attachment 243431 [details]
Patch

Clearing flags on attachment: 243431

Committed r178850: <http://trac.webkit.org/changeset/178850>

[Zan Dobersek]

All reviewed patches have been landed.  Closing bug.