Bug 188331 - REGRESSION (r208953): TemplateObjectDescriptor constructor calculates m_hash on use-after-move variable
Summary: REGRESSION (r208953): TemplateObjectDescriptor constructor calculates m_hash ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Yusuke Suzuki
URL:
Keywords: InRadar
Depends on: 164898
Blocks:
  Show dependency treegraph
 
Reported: 2018-08-04 12:22 PDT by David Kilzer (:ddkilzer)
Modified: 2018-08-04 13:07 PDT (History)
8 users (show)

See Also:


Attachments
Patch v1 (1.53 KB, patch)
2018-08-04 12:27 PDT, David Kilzer (:ddkilzer)
no flags Details | Formatted Diff | Diff

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