Bug 158118 - ScopedLambda should have a lifetime story that makes sense to the compiler
Summary: ScopedLambda should have a lifetime story that makes sense to the compiler
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-05-26 08:06 PDT by Filip Pizlo
Modified: 2016-05-26 14:58 PDT (History)
5 users (show)

See Also:

Attachments
the patch (7.73 KB, patch)
2016-05-26 08:12 PDT, Filip Pizlo 		no flags Details | Formatted Diff | Diff
performance (77.37 KB, text/plain)
2016-05-26 09:17 PDT, Filip Pizlo 		no flags Details
the patch (15.26 KB, patch)
2016-05-26 12:02 PDT, Filip Pizlo 		mark.lam: review+
Details | Formatted Diff | Diff
Show Obsolete (1) 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 Filip Pizlo 2016-05-26 08:06:47 PDT 
ScopedLambdaFunctor currently gets a reference to the passed lambda, and if RVO gets disabled, it will lead to the creation of a ScopedLambda whose m_arg points at dead memory.
Comment 1 Filip Pizlo 2016-05-26 08:12:55 PDT 
Created attachment 279884 [details]
the patch

I'm testing this now.  I'll put it up for review once I know that it actually works.  Also, I'm going to add a test case for this.  I think I know how to do that.
Comment 2 Filip Pizlo 2016-05-26 09:17:26 PDT 
Created attachment 279890 [details]
performance

Looks good.
Comment 3 WebKit Commit Bot 2016-05-26 11:07:05 PDT 
Attachment 279884 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/ScopedLambda.h:78:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/ScopedLambda.h:84:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 2 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Filip Pizlo 2016-05-26 12:02:05 PDT 
Created attachment 279901 [details]
the patch
Comment 5 WebKit Commit Bot 2016-05-26 12:03:45 PDT 
Attachment 279901 [details] did not pass style-queue:


ERROR: Source/WTF/wtf/ScopedLambda.h:78:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WTF/wtf/ScopedLambda.h:84:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 2 in 8 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 6 Mark Lam 2016-05-26 12:33:19 PDT 
Comment on attachment 279901 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=279901&action=review

r=me

> Source/WTF/wtf/ScopedLambda.h:115
> +// We expected scopedLambda to be valid for its entire lifetime, but if it camputed the lambda by reference

typo: /camputed/computed/
Comment 7 Filip Pizlo 2016-05-26 12:47:51 PDT 
Those test failures look pretty scary, I will wait before landing.
Comment 8 Filip Pizlo 2016-05-26 14:58:30 PDT 
Landed in http://trac.webkit.org/changeset/201433