80566 2012-03-07 21:12:32 -0800 Incorrect tracking of abstract values of variables forced double 2012-03-08 01:03:51 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) All All RESOLVED FIXED InRadar P2 Normal --- 1 fpizlo webkit-unassigned oldest_to_newest 573556 0 fpizlo 2012-03-07 21:12:32 -0800 The issue is that the DFG CFA was incorrectly setting the abstract value of local variables that are forced double. In mergeStateAtTail, it figures out what the state of all locals should be at the tail by looking at the last access to the variable. If it's a SetLocal, it is incorrectly taking the child's abstract value and copying it into the local. Instead, it should be setting the local's abstract value to PredictDouble if the local is shouldUseDoubleFormat(). This has no real effect in most cases, since executing the GetLocal would correctly give you a DataFormatDouble. But If you ever did a GetLocal on the forced-double local, and then immediate a SetLocal to a different local, the SetLocal would think that it's setting the second local to the abstract value of the first local's SetLocal's child. For example, if that first SetLocal had a child that was an int (hence it was performing an int->double cast) then the second local variable would think that it had an int. Subsequent GetLocals would think that it's OK to simply load an Int32, when in fact they were loading the low bits of a double. Example: Block #1: a: SomethingThatProducesInt b: SetLocal(@a, r1) predicted Double, forced double Block #2: c: GetLocal(r1) predicted Double, forced double d: SetLocal(@c, r2) predicted IntDouble Block #3: e: GetLocal(r2) predicted IntDouble @d will store a double into r2, but it will think that its abstract value is (Int, []). Then @e will think that it's loading an (Int, []) and issue a 32-bit load of the payload. And bang, we now have corrupted our numbers. 573557 1 fpizlo 2012-03-07 21:12:41 -0800 <rdar://problem/11001442> 573561 2 130760 fpizlo 2012-03-07 21:14:33 -0800 Created attachment 130760 the patch 573699 3 130760 fpizlo 2012-03-08 01:02:46 -0800 Comment on attachment 130760 the patch Reviewed in person by Gavin. 573700 4 fpizlo 2012-03-08 01:03:51 -0800 Landed in http://trac.webkit.org/changeset/110153 130760 2012-03-07 21:14:33 -0800 2012-03-08 01:02:46 -0800 the patch fixcfa_patch_1.diff text/plain 3217 fpizlo SW5kZXg6IFNvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKPT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gU291 cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gMTEwMTQ0KQorKysgU291cmNl L0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDE0IEBA CisyMDEyLTAzLTA3ICBGaWxpcCBQaXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CisKKyAgICAgICAg SW5jb3JyZWN0IHRyYWNraW5nIG9mIGFic3RyYWN0IHZhbHVlcyBvZiB2YXJpYWJsZXMgZm9yY2Vk IGRvdWJsZQorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9 ODA1NjYKKyAgICAgICAgPHJkYXI6Ly9wcm9ibGVtLzExMDAxNDQyPgorCisgICAgICAgIFJldmll d2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogZGZnL0RGR0Fic3RyYWN0U3RhdGUu Y3BwOgorICAgICAgICAoSlNDOjpERkc6OkFic3RyYWN0U3RhdGU6Om1lcmdlU3RhdGVBdFRhaWwp OgorCiAyMDEyLTAzLTA3ICBTaGVyaWZmIEJvdCAgPHdlYmtpdC5yZXZpZXcuYm90QGdtYWlsLmNv bT4KIAogICAgICAgICBVbnJldmlld2VkLCByb2xsaW5nIG91dCByMTEwMTI3LgpJbmRleDogU291 cmNlL0phdmFTY3JpcHRDb3JlL2RmZy9ERkdBYnN0cmFjdFN0YXRlLmNwcAo9PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0t LSBTb3VyY2UvSmF2YVNjcmlwdENvcmUvZGZnL0RGR0Fic3RyYWN0U3RhdGUuY3BwCShyZXZpc2lv biAxMTAxNDMpCisrKyBTb3VyY2UvSmF2YVNjcmlwdENvcmUvZGZnL0RGR0Fic3RyYWN0U3RhdGUu Y3BwCSh3b3JraW5nIGNvcHkpCkBAIC05OTgsNyArOTk4LDcgQEAgaW5saW5lIGJvb2wgQWJzdHJh Y3RTdGF0ZTo6bWVyZ2VTdGF0ZUF0VAogICAgIGlmIChub2RlSW5kZXggPT0gTm9Ob2RlKQogICAg ICAgICByZXR1cm4gZmFsc2U7CiAgICAgICAgIAotICAgIEFic3RyYWN0VmFsdWUqIHNvdXJjZTsK KyAgICBBYnN0cmFjdFZhbHVlIHNvdXJjZTsKICAgICAgICAgCiAgICAgTm9kZSYgbm9kZSA9IG1f Z3JhcGhbbm9kZUluZGV4XTsKICAgICBpZiAoIW5vZGUucmVmQ291bnQoKSkKQEAgLTEwMTMsNyAr MTAxMyw3IEBAIGlubGluZSBib29sIEFic3RyYWN0U3RhdGU6Om1lcmdlU3RhdGVBdFQKICAgICBj YXNlIFNldEFyZ3VtZW50OgogICAgIGNhc2UgRmx1c2g6CiAgICAgICAgIC8vIFRoZSBibG9jayB0 cmFuc2ZlcnMgdGhlIHZhbHVlIGZyb20gaGVhZCB0byB0YWlsLgotICAgICAgICBzb3VyY2UgPSAm aW5WYXJpYWJsZTsKKyAgICAgICAgc291cmNlID0gaW5WYXJpYWJsZTsKICNpZiBERkdfRU5BQkxF KERFQlVHX1BST1BBR0FUSU9OX1ZFUkJPU0UpCiAgICAgICAgIGRhdGFMb2coIiAgICAgICAgICBU cmFuc2ZlcmluZyBmcm9tIGhlYWQgdG8gdGFpbC5cbiIpOwogI2VuZGlmCkBAIC0xMDIxLDcgKzEw MjEsNyBAQCBpbmxpbmUgYm9vbCBBYnN0cmFjdFN0YXRlOjptZXJnZVN0YXRlQXRUCiAgICAgICAg ICAgICAKICAgICBjYXNlIEdldExvY2FsOgogICAgICAgICAvLyBUaGUgYmxvY2sgcmVmaW5lcyB0 aGUgdmFsdWUgd2l0aCBhZGRpdGlvbmFsIHNwZWN1bGF0aW9ucy4KLSAgICAgICAgc291cmNlID0g JmZvck5vZGUobm9kZUluZGV4KTsKKyAgICAgICAgc291cmNlID0gZm9yTm9kZShub2RlSW5kZXgp OwogI2lmIERGR19FTkFCTEUoREVCVUdfUFJPUEFHQVRJT05fVkVSQk9TRSkKICAgICAgICAgZGF0 YUxvZygiICAgICAgICAgIFJlZmluaW5nLlxuIik7CiAjZW5kaWYKQEAgLTEwMzAsNyArMTAzMCwx MCBAQCBpbmxpbmUgYm9vbCBBYnN0cmFjdFN0YXRlOjptZXJnZVN0YXRlQXRUCiAgICAgY2FzZSBT ZXRMb2NhbDoKICAgICAgICAgLy8gVGhlIGJsb2NrIHNldHMgdGhlIHZhcmlhYmxlLCBhbmQgcG90 ZW50aWFsbHkgcmVmaW5lcyBpdCwgYm90aAogICAgICAgICAvLyBiZWZvcmUgYW5kIGFmdGVyIHNl dHRpbmcgaXQuCi0gICAgICAgIHNvdXJjZSA9ICZmb3JOb2RlKG5vZGUuY2hpbGQxKCkpOworICAg ICAgICBpZiAobm9kZS52YXJpYWJsZUFjY2Vzc0RhdGEoKS0+c2hvdWxkVXNlRG91YmxlRm9ybWF0 KCkpCisgICAgICAgICAgICBzb3VyY2Uuc2V0KFByZWRpY3REb3VibGUpOworICAgICAgICBlbHNl CisgICAgICAgICAgICBzb3VyY2UgPSBmb3JOb2RlKG5vZGUuY2hpbGQxKCkpOwogI2lmIERGR19F TkFCTEUoREVCVUdfUFJPUEFHQVRJT05fVkVSQk9TRSkKICAgICAgICAgZGF0YUxvZygiICAgICAg ICAgIFNldHRpbmcuXG4iKTsKICNlbmRpZgpAQCAtMTAzOCwxMSArMTA0MSwxMCBAQCBpbmxpbmUg Ym9vbCBBYnN0cmFjdFN0YXRlOjptZXJnZVN0YXRlQXRUCiAgICAgICAgIAogICAgIGRlZmF1bHQ6 CiAgICAgICAgIEFTU0VSVF9OT1RfUkVBQ0hFRCgpOwotICAgICAgICBzb3VyY2UgPSAwOwogICAg ICAgICBicmVhazsKICAgICB9CiAgICAgCi0gICAgaWYgKGRlc3RpbmF0aW9uID09ICpzb3VyY2Up IHsKKyAgICBpZiAoZGVzdGluYXRpb24gPT0gc291cmNlKSB7CiAgICAgICAgIC8vIEFic3RyYWN0 IGV4ZWN1dGlvbiBkaWQgbm90IGNoYW5nZSB0aGUgb3V0cHV0IHZhbHVlIG9mIHRoZSB2YXJpYWJs ZSwgZm9yIHRoaXMKICAgICAgICAgLy8gYmFzaWMgYmxvY2ssIG9uIHRoaXMgaXRlcmF0aW9uLgog I2lmIERGR19FTkFCTEUoREVCVUdfUFJPUEFHQVRJT05fVkVSQk9TRSkKQEAgLTEwNTQsNyArMTA1 Niw3IEBAIGlubGluZSBib29sIEFic3RyYWN0U3RhdGU6Om1lcmdlU3RhdGVBdFQKICAgICAvLyBB YnN0cmFjdCBleGVjdXRpb24gcmVhY2hlZCBhIG5ldyBjb25jbHVzaW9uIGFib3V0IHRoZSBzcGVj dWxhdGlvbnMgcmVhY2hlZCBhYm91dAogICAgIC8vIHRoaXMgdmFyaWFibGUgYWZ0ZXIgZXhlY3V0 aW9uIG9mIHRoaXMgYmFzaWMgYmxvY2suIFVwZGF0ZSB0aGUgc3RhdGUsIGFuZCByZXR1cm4KICAg ICAvLyB0cnVlIHRvIGluZGljYXRlIHRoYXQgdGhlIGZpeHBvaW50IG11c3QgZ28gb24hCi0gICAg ZGVzdGluYXRpb24gPSAqc291cmNlOworICAgIGRlc3RpbmF0aW9uID0gc291cmNlOwogI2lmIERG R19FTkFCTEUoREVCVUdfUFJPUEFHQVRJT05fVkVSQk9TRSkKICAgICBkYXRhTG9nKCIgICAgICAg ICAgQ2hhbmdlZCFcbiIpOwogI2VuZGlmCg==