Bug 188331

Summary: REGRESSION (r208953): TemplateObjectDescriptor constructor calculates m_hash on use-after-move variable
Product: WebKit Reporter: David Kilzer (:ddkilzer) <ddkilzer>
Component: JavaScriptCoreAssignee: Yusuke Suzuki <ysuzuki>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ews-watchlist, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 164898    
Bug Blocks:    
Attachments:
Description Flags
Patch v1 none

Description David Kilzer (:ddkilzer) 2018-08-04 12:22:19 PDT
TemplateObjectDescriptor constructor calculates m_hash on use-after-move variable:

inline TemplateObjectDescriptor::TemplateObjectDescriptor(StringVector&& rawStrings, OptionalStringVector&& cookedStrings)
    : m_rawStrings(WTFMove(rawStrings))
    , m_cookedStrings(WTFMove(cookedStrings))
    , m_hash(calculateHash(rawStrings))
{
}

It should probably use m_rawstrings instead.
Comment 1 David Kilzer (:ddkilzer) 2018-08-04 12:23:25 PDT
Regressed in r208953:

Bug 164898: Crash in com.apple.JavaScriptCore: WTF::ThreadSpecific<WTF::WTFThreadData, + 142
​<https://bugs.webkit.org/show_bug.cgi?id=164898>
<https://trac.webkit.org/changeset/208953/webkit>
Comment 2 Yusuke Suzuki 2018-08-04 12:24:18 PDT
Oops!
Comment 3 David Kilzer (:ddkilzer) 2018-08-04 12:27:00 PDT
Created attachment 346593 [details]
Patch v1

Not sure how to write a test for this.
Comment 4 Radar WebKit Bug Importer 2018-08-04 12:27:26 PDT
<rdar://problem/42935448>
Comment 5 David Kilzer (:ddkilzer) 2018-08-04 12:28:15 PDT
(In reply to David Kilzer (:ddkilzer) from comment #3)
> Created attachment 346593 [details]
> Patch v1
> 
> Not sure how to write a test for this.

Also not sure of the user visible effect.  Just poor performance when looking up items in the hash?
Comment 6 Yusuke Suzuki 2018-08-04 12:28:34 PDT
Comment on attachment 346593 [details]
Patch v1

r=me
Comment 7 David Kilzer (:ddkilzer) 2018-08-04 12:29:15 PDT
<rdar://problem/42882339>
Comment 8 Yusuke Suzuki 2018-08-04 12:29:31 PDT
(In reply to David Kilzer (:ddkilzer) from comment #5)
> (In reply to David Kilzer (:ddkilzer) from comment #3)
> > Created attachment 346593 [details]
> > Patch v1
> > 
> > Not sure how to write a test for this.
> 
> Also not sure of the user visible effect.  Just poor performance when
> looking up items in the hash?

I think so. Calculating a hash on empty vector (moved).
Comment 9 WebKit Commit Bot 2018-08-04 13:07:21 PDT
Comment on attachment 346593 [details]
Patch v1

Clearing flags on attachment: 346593

Committed r234580: <https://trac.webkit.org/changeset/234580>
Comment 10 WebKit Commit Bot 2018-08-04 13:07:22 PDT
All reviewed patches have been landed.  Closing bug.