RESOLVED FIXED 198271
JITOperations putByVal should mark negative array indices as out-of-bounds 
https://bugs.webkit.org/show_bug.cgi?id=198271
Summary JITOperations putByVal should mark negative array indices as out-of-bounds
Tadeu Zagallo
Reported 2019-05-27 04:45:20 PDT
Like https://bugs.webkit.org/show_bug.cgi?id=198229, but for putByVal.
Attachments
Patch (5.33 KB, patch)
2019-05-27 05:05 PDT, Tadeu Zagallo 		no flags
DetailsFormatted DiffDiff
Patch for landing (4.27 KB, patch)
2019-05-28 08:47 PDT, Tadeu Zagallo 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Tadeu Zagallo
Comment 1 2019-05-27 05:05:26 PDT
Created attachment 370689 [details] Patch
Saam Barati
Comment 2 2019-05-27 11:22:19 PDT
Comment on attachment 370689 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=370689&action=review > Source/JavaScriptCore/jit/JITOperations.cpp:665 > +putProperty: Alternatively, you could have an “else if (subscript.isInt32() && baseValue.isObject()))” then mark as out of bounds
Tadeu Zagallo
Comment 3 2019-05-27 13:02:30 PDT
Comment on attachment 370689 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=370689&action=review >> Source/JavaScriptCore/jit/JITOperations.cpp:665 >> +putProperty: > > Alternatively, you could have an “else if (subscript.isInt32() && baseValue.isObject()))” then mark as out of bounds What about `tookSlowPath`, do I also need to set it if it's int32 but not an object? I guess I don't get why don't we unconditionally set `tookSlowPath`.
Saam Barati
Comment 4 2019-05-27 23:48:10 PDT
Comment on attachment 370689 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=370689&action=review >>> Source/JavaScriptCore/jit/JITOperations.cpp:665 >>> +putProperty: >> >> Alternatively, you could have an “else if (subscript.isInt32() && baseValue.isObject()))” then mark as out of bounds > > What about `tookSlowPath`, do I also need to set it if it's int32 but not an object? I guess I don't get why don't we unconditionally set `tookSlowPath`. I'm also not sure. It probably has to do with normal indexed lookups, but you should look at the code in the DFG (I'm assuming) that reads it
Saam Barati
Comment 5 2019-05-27 23:48:25 PDT
Maybe it affects how we speculate.
Tadeu Zagallo
Comment 6 2019-05-28 08:47:49 PDT
Created attachment 370745 [details] Patch for landing
WebKit Commit Bot
Comment 7 2019-05-28 09:03:08 PDT
Comment on attachment 370745 [details] Patch for landing Clearing flags on attachment: 370745 Committed r245813: <https://trac.webkit.org/changeset/245813>
WebKit Commit Bot
Comment 8 2019-05-28 09:03:10 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 9 2019-05-28 09:04:25 PDT
<rdar://problem/51180527>
Note You need to log in before you can comment on or make changes to this bug.