135287 2014-07-25 07:31:49 -0700 BuildFix: JavaScriptCore/bytecode/StructureSet.h:262:77: warning 2014-07-28 19:12:07 -0700 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) Unspecified Unspecified RESOLVED FIXED P2 Normal --- 1 tgergely.u-szeged webkit-unassigned commit-queue darin fpizlo oldest_to_newest 1024813 0 tgergely.u-szeged 2014-07-25 07:31:49 -0700 I got a compile warning: Source/JavaScriptCore/bytecode/StructureSet.h:262:77: warning: '<anonymous>.JSC::StructureSet::m_pointer' is used uninitialized in this function [-Wuninitialized] This is because the set() method (that is used to initialize the StructureSet object from the constructors directly or through setEmpty() or copyFrom() methods) preserves the reservedFlag from the old value which is not defined when the object is constructed. 1024815 1 235514 tgergely.u-szeged 2014-07-25 07:47:30 -0700 Created attachment 235514 patch 1025108 2 235514 darin 2014-07-27 23:34:07 -0700 Comment on attachment 235514 patch View in context: https://bugs.webkit.org/attachment.cgi?id=235514&action=review I think the new version of StructureSet was rolled out of the tree; this patch isn’t relevant until we bring it back in. Best would be to make sure Filip fixes this before he re-lands the new code. > Source/JavaScriptCore/bytecode/StructureSet.h:43 > + : m_pointer(0) Please use nullptr. > Source/JavaScriptCore/bytecode/StructureSet.h:49 > + : m_pointer(0) Please use nullptr. > Source/JavaScriptCore/bytecode/StructureSet.h:55 > + : m_pointer(0) Not sure it’s needed in this case. I didn’t carefully study copyFrom, but I don’t think it uses the old m_pointer value. 1025134 3 fpizlo 2014-07-28 08:03:32 -0700 (In reply to comment #2) > (From update of attachment 235514 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=235514&action=review > > I think the new version of StructureSet was rolled out of the tree; this patch isn’t relevant until we bring it back in. It's rolled back in. There are more changes now so this does need to be rebased. But r=me too. This change is fine. > > Best would be to make sure Filip fixes this before he re-lands the new code. > > > Source/JavaScriptCore/bytecode/StructureSet.h:43 > > + : m_pointer(0) > > Please use nullptr. It's a uintptr, so that would be weird. Would it even work? I think it would be weird even if it did work since the point of explicitly setting it is to clear the low tag bits. > > > Source/JavaScriptCore/bytecode/StructureSet.h:49 > > + : m_pointer(0) > > Please use nullptr. Ditto. > > > Source/JavaScriptCore/bytecode/StructureSet.h:55 > > + : m_pointer(0) > > Not sure it’s needed in this case. I didn’t carefully study copyFrom, but I don’t think it uses the old m_pointer value. I think that copyFrom leaves the low bits alone, so we need this change here also. 1025356 4 235514 darin 2014-07-28 18:38:25 -0700 Comment on attachment 235514 patch OK. 1025361 5 235514 commit-queue 2014-07-28 19:12:04 -0700 Comment on attachment 235514 patch Clearing flags on attachment: 235514 Committed r171719: <http://trac.webkit.org/changeset/171719> 1025362 6 commit-queue 2014-07-28 19:12:07 -0700 All reviewed patches have been landed. Closing bug. 235514 2014-07-25 07:47:30 -0700 2014-07-28 19:12:04 -0700 patch b135287.patch text/plain 1440 tgergely.u-szeged ZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvSmF2 YVNjcmlwdENvcmUvQ2hhbmdlTG9nCmluZGV4IDZiNzEyNjAuLjdlNmVjNWMgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL0phdmFTY3JpcHRD b3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDE0LTA3LTI1ICBUYW1hcyBHZXJnZWx5 ICA8dGdlcmdlbHkudS1zemVnZWRAcGFydG5lci5zYW1zdW5nLmNvbT4KKworICAgICAgICBCdWls ZEZpeDogSmF2YVNjcmlwdENvcmUvYnl0ZWNvZGUvU3RydWN0dXJlU2V0Lmg6MjYyOjc3OiB3YXJu aW5nLgorICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTM1 Mjg3CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgVGhl IHNldCgpIG1ldGhvZCB0cmllcyB0byB1c2UgYSBwYXJ0IG9mIHRoZSBvbGQgdmFsdWUgKHRoZSBy ZXNlcnZlZEZsYWcgYml0KSB3aGljaAorICAgICAgICB3YXMgbm90IGRlZmluZWQgd2hlbiB0aGUg Y29uc3RydWN0b3IgaXMgY2FsbGVkLiBJbml0aWFsaXplIG1fcG9pbnRlciB0byAwIGV4cGxpY2l0 ZWx5LgorCisgICAgICAgICogYnl0ZWNvZGUvU3RydWN0dXJlU2V0Lmg6CisgICAgICAgIChKU0M6 OlN0cnVjdHVyZVNldDo6U3RydWN0dXJlU2V0KToKKwogMjAxNC0wNy0yMyAgQnJlbnQgRnVsZ2hh bSAgPGJmdWxnaGFtQGFwcGxlLmNvbT4KIAogICAgICAgICBCdWlsZCBmaXggYWZ0ZXIgcjE3MTQ4 Mi4KZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9ieXRlY29kZS9TdHJ1Y3R1cmVT ZXQuaCBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9ieXRlY29kZS9TdHJ1Y3R1cmVTZXQuaAppbmRl eCAwZTlhNDY3Li4zZDljZTQwIDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvYnl0 ZWNvZGUvU3RydWN0dXJlU2V0LmgKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL2J5dGVjb2Rl L1N0cnVjdHVyZVNldC5oCkBAIC00MCwxNiArNDAsMTkgQEAgY2xhc3MgU3RydWN0dXJlQWJzdHJh Y3RWYWx1ZTsKIGNsYXNzIFN0cnVjdHVyZVNldCB7CiBwdWJsaWM6CiAgICAgU3RydWN0dXJlU2V0 KCkKKyAgICAgICAgOiBtX3BvaW50ZXIoMCkKICAgICB7CiAgICAgICAgIHNldEVtcHR5KCk7CiAg ICAgfQogICAgIAogICAgIFN0cnVjdHVyZVNldChTdHJ1Y3R1cmUqIHN0cnVjdHVyZSkKKyAgICAg ICAgOiBtX3BvaW50ZXIoMCkKICAgICB7CiAgICAgICAgIHNldChzdHJ1Y3R1cmUpOwogICAgIH0K ICAgICAKICAgICBBTFdBWVNfSU5MSU5FIFN0cnVjdHVyZVNldChjb25zdCBTdHJ1Y3R1cmVTZXQm IG90aGVyKQorICAgICAgICA6IG1fcG9pbnRlcigwKQogICAgIHsKICAgICAgICAgY29weUZyb20o b3RoZXIpOwogICAgIH0K