Bug 193221

Summary: LLInt put_by_id uses the wrong load instruction for loading flags from the metadata
Product: WebKit Reporter: Tadeu Zagallo <tzagallo>
Component: JavaScriptCoreAssignee: Tadeu Zagallo <tzagallo>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ews-watchlist, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Tadeu Zagallo
Reported 2019-01-07 15:39:39 PST
...
Attachments
Patch (1.53 KB, patch)
2019-01-07 15:41 PST, Tadeu Zagallo 		no flags
DetailsFormatted DiffDiff
Patch (2.79 KB, patch)
2019-01-08 10:07 PST, Tadeu Zagallo 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Tadeu Zagallo
Comment 1 2019-01-07 15:41:14 PST
Created attachment 358546 [details] Patch
Mark Lam
Comment 2 2019-01-07 15:49:49 PST
Comment on attachment 358546 [details] Patch r=me
Keith Miller
Comment 3 2019-01-07 19:23:21 PST
Comment on attachment 358546 [details] Patch Can we get a JSC test?
Tadeu Zagallo
Comment 4 2019-01-08 10:07:44 PST
Created attachment 358607 [details] Patch
WebKit Commit Bot
Comment 5 2019-01-08 10:29:51 PST
Comment on attachment 358607 [details] Patch Clearing flags on attachment: 358607 Committed r239731: <https://trac.webkit.org/changeset/239731>
WebKit Commit Bot
Comment 6 2019-01-08 10:29:53 PST
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 7 2019-01-08 10:30:32 PST
<rdar://problem/47119372>
Saam Barati
Comment 8 2019-01-08 14:48:34 PST
Maybe we should implement a systematic way to prevent such errors as this. Like, what if we had code that compiled: static_assert(sizeof(field) == N) or static_assert(sizeof(field) >= N) Where N is the load size for each static load of `field` in the LLInt. I think the former would probably pass compilation. But if not, we could do the latter. If that doesn't work, we should make anyone explicitly doing a larger load add some kind of syntax for "trust me, I know what I'm doing by emitting a larger load".
Note You need to log in before you can comment on or make changes to this bug.