27060 2009-07-07 19:22:40 -0700 Type cast for first argument in make_pair() 2009-09-17 03:45:03 -0700 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) S60 Emulator S60 3rd edition RESOLVED FIXED P2 Normal --- 27065 0 norbert.leser webkit-unassigned hausmann koshuin laszlo.gombos yongjun.zhang oldest_to_newest 130343 0 32420 norbert.leser 2009-07-07 19:22:40 -0700 Created attachment 32420 Code patch for Structure.cpp Codewarrior compiler (WINSCW) latest build b482 fails with "illegal operand" when first arg of make_pair() method has unqualified type. Proposed patch casts the arguments accordingly. 131089 1 32420 eric 2009-07-10 23:18:01 -0700 Comment on attachment 32420 Code patch for Structure.cpp Why? Also, we use c++ style casts. 139494 2 34683 norbert.leser 2009-08-12 13:18:25 -0700 Created attachment 34683 Updated patch for bug #27060 Symbian compiler for emulator target (WINSCW) fails with "illegal operand" for m_attributesInPrevious in structure.ccp (when calling make_pair functions). This error is apparently due to the compiler not properly resolving the unsigned type of the declared bitfield. Initial patch explicitly casted m_attributesInPrevious to unsigned (at 3 places in structure.cpp), but since bitfield optimization is not critical for the emulator target, this conditional change in header file appears to be least intrusive. 139572 3 34683 darin 2009-08-12 15:02:24 -0700 Comment on attachment 34683 Updated patch for bug #27060 This needs a brief comment, otherwise it's simply too mysterious. I'm going to say review- because I don't think we should land it without a comment. 139824 4 koshuin 2009-08-13 06:10:44 -0700 >This error is apparently due to the compiler not properly resolving the >unsigned type of the declared bitfield. This was my assumption as well, but it doesn't seem to be true. I made a small test program: #include <iostream> #include <utility> class test{ public: unsigned flag:1; unsigned somefield:5; }; using namespace std; int main() { test a; make_pair(a.flag,a.somefield); return 1; } And this one passes with flying colors on WINSCW. I'll try to narrow it down, but it does seem to be related to the curious reoccuring template pattern that is used in class defining the bitfields. 140179 5 koshuin 2009-08-14 01:29:55 -0700 Little more investigations. This isn't about CRTP and neither is about using RefPtr. I even tried the union for accessing as it is used to traverse. It is hard to isolate structure.cpp from surroundings. For what it's worth. I don't see casting to unsigned as a bad solution as it the argument would take 4 bytes anyways on ARM. and storing that to pair has to be 4 bytes as well. Changing the member to be unsigned instead of 7bits is not good solution as that uses more memory and will make the struct different from what it is on other platforms. Can't really think of anything else that might go wrong than unlikely alignment issue or bad alternative make_pair implementation. 140201 6 koshuin 2009-08-14 04:43:38 -0700 Getting there.. It is not the make_pair function that fails, but the HashMap::Contains(). in WTF/StructureTransitionTable.h we define our HashTable to be type: typedef HashMap<StructureTransitionTableHash::Key, Structure*, StructureTransitionTableHash, StructureTransitionTableHashTraits> StructureTransitionTable; where the StructureTransitionTableHashTraits' SecondFirstTraits::TraitType is unsigned. Looks like temporary pair is getting generated with unknown type. From illegal operand we can gather that this is not integer type.I looked at HashTraits.h to see if there was an obvious reason, but cold code reading doesn't help at this point. Unfortunately I don't have src for std so my debugging/preprocessing is limited here. Again casting to unsigned is the easiest way to fix this. 140702 7 norbert.leser 2009-08-17 06:26:09 -0700 (In reply to comment #6) > Getting there.. > > It is not the make_pair function that fails, but the HashMap::Contains(). > > in WTF/StructureTransitionTable.h we define our HashTable to be type: > typedef HashMap<StructureTransitionTableHash::Key, Structure*, > StructureTransitionTableHash, StructureTransitionTableHashTraits> > StructureTransitionTable; > > where the StructureTransitionTableHashTraits' SecondFirstTraits::TraitType is > unsigned. > > Looks like temporary pair is getting generated with unknown type. From illegal > operand we can gather that this is not integer type.I looked at HashTraits.h to > see if there was an obvious reason, but cold code reading doesn't help at this > point. Unfortunately I don't have src for std so my debugging/preprocessing is > limited here. > > Again casting to unsigned is the easiest way to fix this. If unconditional casting to unsigned is acceptable for any other platform, that would be fine with me. However, I am not certain whether that may defeat any possible 7-bit optimization. The rationale for making the single header change to unsigned for COMPILER(WINSCW)was that this is for symbian carbide emulator only, where this space optimization is insignificant, and to avoid 3 #defines in the implementation file. Either solution is fine with me. The question is what's the cleanest without regression on other platforms. 142328 8 34683 darin 2009-08-24 10:42:22 -0700 Comment on attachment 34683 Updated patch for bug #27060 Seems fine. Would be better with a brief comment. 142346 9 34683 eric 2009-08-24 11:28:12 -0700 Comment on attachment 34683 Updated patch for bug #27060 Norbert isn't a committer, and darin is asking for a comment in the comment above, so marking this r- since no one can land this as is. 147344 10 39607 norbert.leser 2009-09-15 11:27:20 -0700 Created attachment 39607 2nd update of patch file for bug #27060 Same patch as before, but included comment, as suggested by Darin Adler. 147361 11 39607 darin 2009-09-15 13:10:04 -0700 Comment on attachment 39607 2nd update of patch file for bug #27060 We can live with this change, but I am concerned about the large number of workarounds needed to work, poorly, with the WINSCW compiler. 147779 12 hausmann 2009-09-17 03:45:03 -0700 Landed in r48461 32420 2009-07-07 19:22:40 -0700 2009-08-12 13:18:25 -0700 Code patch for Structure.cpp patch_Structure_cpp.txt text/plain 2510 norbert.leser SW5kZXg6IENoYW5nZUxvZw0KPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PQ0KLS0tIENoYW5nZUxvZwkocmV2aXNpb24gNDU2 MTIpCisrKyBDaGFuZ2VMb2cJKHdvcmtpbmcgY29weSkKQEAgLTEsMyArMSwxNCBAQAorMjAwOS0w Ny0wNyAgTm9yYmVydCBMZXNlciAgPG5vcmJlcnQubGVzZXJAbm9raWEuY29tPgorCisgICAgICAg IFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFR5cGUgY2FzdCBmb3IgZmly c3QgYXJndW1lbnQgaW4gbWFrZV9wYWlyKCkKKyAgICAgICAgCisgICAgICAgIENvZGV3YXJyaW9y IGNvbXBpbGVyIChXSU5TQ1cpIGxhdGVzdCBiNDgyIGZhaWxzCisgICAgICAgIHdpdGggImlsbGVn YWwgb3BlcmFuZCIKKworICAgICAgICAqIEphdmFTY3JpcHRDb3JlL3J1bnRpbWUvU3RydWN0dXJl LmNwcDoKKwogMjAwOS0wNy0wMyAgSmFuIE1pY2hhZWwgQWxvbnpvICA8am1hbG9uem9Ad2Via2l0 Lm9yZz4KIAogICAgICAgICBSZXZpZXdlZCBieSBYYW4gTG9wZXogYW5kIEd1c3Rhdm8gTm9yb25o YS4KSW5kZXg6IEphdmFTY3JpcHRDb3JlL3J1bnRpbWUvU3RydWN0dXJlLmNwcA0KPT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PQ0KLS0tIEphdmFTY3JpcHRDb3JlL3J1bnRpbWUvU3RydWN0dXJlLmNwcAkocmV2aXNpb24gNDU2 MTIpCisrKyBKYXZhU2NyaXB0Q29yZS9ydW50aW1lL1N0cnVjdHVyZS5jcHAJKHdvcmtpbmcgY29w eSkKQEAgLTE1OSw4ICsxNTksOCBAQCBTdHJ1Y3R1cmU6On5TdHJ1Y3R1cmUoKQogICAgICAgICBp ZiAobV9wcmV2aW91cy0+bV91c2luZ1NpbmdsZVRyYW5zaXRpb25TbG90KSB7CiAgICAgICAgICAg ICBtX3ByZXZpb3VzLT5tX3RyYW5zaXRpb25zLnNpbmdsZVRyYW5zaXRpb24gPSAwOwogICAgICAg ICB9IGVsc2UgewotICAgICAgICAgICAgQVNTRVJUKG1fcHJldmlvdXMtPm1fdHJhbnNpdGlvbnMu dGFibGUtPmNvbnRhaW5zKG1ha2VfcGFpcihtX25hbWVJblByZXZpb3VzLmdldCgpLCBtYWtlX3Bh aXIobV9hdHRyaWJ1dGVzSW5QcmV2aW91cywgbV9zcGVjaWZpY1ZhbHVlSW5QcmV2aW91cykpKSk7 Ci0gICAgICAgICAgICBtX3ByZXZpb3VzLT5tX3RyYW5zaXRpb25zLnRhYmxlLT5yZW1vdmUobWFr ZV9wYWlyKG1fbmFtZUluUHJldmlvdXMuZ2V0KCksIG1ha2VfcGFpcihtX2F0dHJpYnV0ZXNJblBy ZXZpb3VzLCBtX3NwZWNpZmljVmFsdWVJblByZXZpb3VzKSkpOworICAgICAgICAgICAgQVNTRVJU KG1fcHJldmlvdXMtPm1fdHJhbnNpdGlvbnMudGFibGUtPmNvbnRhaW5zKG1ha2VfcGFpcihtX25h bWVJblByZXZpb3VzLmdldCgpLCBtYWtlX3BhaXIodW5zaWduZWQobV9hdHRyaWJ1dGVzSW5QcmV2 aW91cyksIG1fc3BlY2lmaWNWYWx1ZUluUHJldmlvdXMpKSkpOworICAgICAgICAgICAgbV9wcmV2 aW91cy0+bV90cmFuc2l0aW9ucy50YWJsZS0+cmVtb3ZlKG1ha2VfcGFpcihtX25hbWVJblByZXZp b3VzLmdldCgpLCBtYWtlX3BhaXIodW5zaWduZWQobV9hdHRyaWJ1dGVzSW5QcmV2aW91cyksIG1f c3BlY2lmaWNWYWx1ZUluUHJldmlvdXMpKSk7CiAgICAgICAgIH0KICAgICB9CiAKQEAgLTQ1Nyw3 ICs0NTcsNyBAQCBQYXNzUmVmUHRyPFN0cnVjdHVyZT4gU3RydWN0dXJlOjphZGRQcm9wCiAgICAg ICAgIHN0cnVjdHVyZS0+bV91c2luZ1NpbmdsZVRyYW5zaXRpb25TbG90ID0gZmFsc2U7CiAgICAg ICAgIFN0cnVjdHVyZVRyYW5zaXRpb25UYWJsZSogdHJhbnNpdGlvblRhYmxlID0gbmV3IFN0cnVj dHVyZVRyYW5zaXRpb25UYWJsZTsKICAgICAgICAgc3RydWN0dXJlLT5tX3RyYW5zaXRpb25zLnRh YmxlID0gdHJhbnNpdGlvblRhYmxlOwotICAgICAgICB0cmFuc2l0aW9uVGFibGUtPmFkZChtYWtl X3BhaXIoZXhpc3RpbmdUcmFuc2l0aW9uLT5tX25hbWVJblByZXZpb3VzLmdldCgpLCBtYWtlX3Bh aXIoZXhpc3RpbmdUcmFuc2l0aW9uLT5tX2F0dHJpYnV0ZXNJblByZXZpb3VzLCBleGlzdGluZ1Ry YW5zaXRpb24tPm1fc3BlY2lmaWNWYWx1ZUluUHJldmlvdXMpKSwgZXhpc3RpbmdUcmFuc2l0aW9u KTsKKyAgICAgICAgdHJhbnNpdGlvblRhYmxlLT5hZGQobWFrZV9wYWlyKGV4aXN0aW5nVHJhbnNp dGlvbi0+bV9uYW1lSW5QcmV2aW91cy5nZXQoKSwgbWFrZV9wYWlyKHVuc2lnbmVkKGV4aXN0aW5n VHJhbnNpdGlvbi0+bV9hdHRyaWJ1dGVzSW5QcmV2aW91cyksIGV4aXN0aW5nVHJhbnNpdGlvbi0+ bV9zcGVjaWZpY1ZhbHVlSW5QcmV2aW91cykpLCBleGlzdGluZ1RyYW5zaXRpb24pOwogICAgIH0K ICAgICBzdHJ1Y3R1cmUtPm1fdHJhbnNpdGlvbnMudGFibGUtPmFkZChtYWtlX3BhaXIocHJvcGVy dHlOYW1lLnVzdHJpbmcoKS5yZXAoKSwgbWFrZV9wYWlyKGF0dHJpYnV0ZXMsIHNwZWNpZmljVmFs dWUpKSwgdHJhbnNpdGlvbi5nZXQoKSk7CiAgICAgcmV0dXJuIHRyYW5zaXRpb24ucmVsZWFzZSgp Owo= 34683 2009-08-12 13:18:25 -0700 2009-09-15 11:27:20 -0700 Updated patch for bug #27060 structure_h_27060_update_1.txt text/plain 1609 norbert.leser SW5kZXg6IEphdmFTY3JpcHRDb3JlL0NoYW5nZUxvZw0KPT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQ0KLS0tIEphdmFTY3Jp cHRDb3JlL0NoYW5nZUxvZwkocmV2aXNpb24gNDcxNDEpCisrKyBKYXZhU2NyaXB0Q29yZS9DaGFu Z2VMb2cJKHdvcmtpbmcgY29weSkKQEAgLTEsMyArMSwyMCBAQAorMjAwOS0wOC0xMiAgTm9yYmVy dCBMZXNlciAgPG5vcmJlcnQubGVzZXJAbm9raWEuY29tPgorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFN5bWJpYW4gY29tcGlsZXIgZm9yIGVtdWxhdG9y IHRhcmdldCAoV0lOU0NXKSBmYWlscyB3aXRoCisgICAgICAgICJpbGxlZ2FsIG9wZXJhbmQiIGZv ciBtX2F0dHJpYnV0ZXNJblByZXZpb3VzIGluIHN0cnVjdHVyZS5jY3AKKyAgICAgICAgKHdoZW4g Y2FsbGluZyBtYWtlX3BhaXIgZnVuY3Rpb25zKS4KKyAgICAgICAgVGhpcyBlcnJvciBpcyBhcHBh cmVudGx5IGR1ZSB0byB0aGUgY29tcGlsZXIgbm90IHByb3Blcmx5CisgICAgICAgIHJlc29sdmlu ZyB0aGUgdW5zaWduZWQgdHlwZSBvZiB0aGUgZGVjbGFyZWQgYml0ZmllbGQuCisKKyAgICAgICAg SW5pdGlhbCBwYXRjaCBleHBsaWNpdGx5IGNhc3RlZCBtX2F0dHJpYnV0ZXNJblByZXZpb3VzCisg ICAgICAgIHRvIHVuc2lnbmVkLCBidXQgc2luY2UgYml0ZmllbGQgb3B0aW1pemF0aW9uIGlzIG5v dCBjcml0aWNhbCBmb3IKKyAgICAgICAgdGhlIGVtdWxhdG9yIHRhcmdldCwgdGhpcyBjb25kaXRp b25hbCBjaGFuZ2UgaW4gaGVhZGVyIGZpbGUKKyAgICAgICAgYXBwZWFycyB0byBiZSBsZWFzdCBp bnRydXNpdmUuCisKKyAgICAgICAgKiBydW50aW1lL1N0cnVjdHVyZS5oOgorCiAyMDA5LTA4LTEy ICBCYWxhenMgS2VsZW1lbiAgPGtlbGVtZW4uYmFsYXpzLjNAc3R1ZC51LXN6ZWdlZC5odT4KIAog ICAgICAgICBSZXZpZXdlZCBieSBBcml5YSBIaWRheWF0LgpJbmRleDogSmF2YVNjcmlwdENvcmUv cnVudGltZS9TdHJ1Y3R1cmUuaA0KPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQ0KLS0tIEphdmFTY3JpcHRDb3JlL3J1bnRp bWUvU3RydWN0dXJlLmgJKHJldmlzaW9uIDQ3MTA1KQorKysgSmF2YVNjcmlwdENvcmUvcnVudGlt ZS9TdHJ1Y3R1cmUuaAkod29ya2luZyBjb3B5KQpAQCAtMTg0LDcgKzE4NCwxMSBAQCBuYW1lc3Bh Y2UgSlNDIHsKICAgICAgICAgYm9vbCBtX2lzUGlubmVkUHJvcGVydHlUYWJsZSA6IDE7CiAgICAg ICAgIGJvb2wgbV9oYXNHZXR0ZXJTZXR0ZXJQcm9wZXJ0aWVzIDogMTsKICAgICAgICAgYm9vbCBt X3VzaW5nU2luZ2xlVHJhbnNpdGlvblNsb3QgOiAxOworI2lmIENPTVBJTEVSKFdJTlNDVykKKyAg ICAgICAgdW5zaWduZWQgbV9hdHRyaWJ1dGVzSW5QcmV2aW91czsKKyNlbHNlCiAgICAgICAgIHVu c2lnbmVkIG1fYXR0cmlidXRlc0luUHJldmlvdXMgOiA3OworI2VuZGlmICAgICAgICAKICAgICB9 OwogCiAgICAgaW5saW5lIHNpemVfdCBTdHJ1Y3R1cmU6OmdldChjb25zdCBJZGVudGlmaWVyJiBw cm9wZXJ0eU5hbWUpCg== 39607 2009-09-15 11:27:20 -0700 2009-09-15 13:10:03 -0700 2nd update of patch file for bug #27060 structure_h_27060_update_2.txt text/plain 1866 norbert.leser SW5kZXg6IEphdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBKYXZhU2NyaXB0 Q29yZS9DaGFuZ2VMb2cJKHJldmlzaW9uIDQ4MzkzKQorKysgSmF2YVNjcmlwdENvcmUvQ2hhbmdl TG9nCSh3b3JraW5nIGNvcHkpCkBAIC0xLDMgKzEsMjAgQEAKKzIwMDktMDktMTUgIE5vcmJlcnQg TGVzZXIgIDxub3JiZXJ0Lmxlc2VyQG5va2lhLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBO T0JPRFkgKE9PUFMhKS4KKworICAgICAgICBTeW1iaWFuIGNvbXBpbGVyIGZvciBlbXVsYXRvciB0 YXJnZXQgKFdJTlNDVykgZmFpbHMgd2l0aAorICAgICAgICAiaWxsZWdhbCBvcGVyYW5kIiBmb3Ig bV9hdHRyaWJ1dGVzSW5QcmV2aW91cyBpbiBzdHJ1Y3R1cmUuY2NwCisgICAgICAgICh3aGVuIGNh bGxpbmcgbWFrZV9wYWlyIGZ1bmN0aW9ucykuCisgICAgICAgIFRoaXMgZXJyb3IgaXMgYXBwYXJl bnRseSBkdWUgdG8gdGhlIGNvbXBpbGVyIG5vdCBwcm9wZXJseQorICAgICAgICByZXNvbHZpbmcg dGhlIHVuc2lnbmVkIHR5cGUgb2YgdGhlIGRlY2xhcmVkIGJpdGZpZWxkLgorCisgICAgICAgIElu aXRpYWwgcGF0Y2ggZXhwbGljaXRseSBjYXN0ZWQgbV9hdHRyaWJ1dGVzSW5QcmV2aW91cworICAg ICAgICB0byB1bnNpZ25lZCwgYnV0IHNpbmNlIGJpdGZpZWxkIG9wdGltaXphdGlvbiBpcyBub3Qg Y3JpdGljYWwgZm9yCisgICAgICAgIHRoZSBlbXVsYXRvciB0YXJnZXQsIHRoaXMgY29uZGl0aW9u YWwgY2hhbmdlIGluIGhlYWRlciBmaWxlCisgICAgICAgIGFwcGVhcnMgdG8gYmUgbGVhc3QgaW50 cnVzaXZlLgorCisgICAgICAgICogcnVudGltZS9TdHJ1Y3R1cmUuaDoKKwogMjAwOS0wOS0xNSBD c2FiYSBPc3p0cm9nb25hYyA8b3N6aUBpbmYudS1zemVnZWQuaHU+CiAKICAgICAgICAgUmV2aWV3 ZWQgYnkgVG9yIEFybmUgVmVzdGLDuC4KSW5kZXg6IEphdmFTY3JpcHRDb3JlL3J1bnRpbWUvU3Ry dWN0dXJlLmgKPT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PQotLS0gSmF2YVNjcmlwdENvcmUvcnVudGltZS9TdHJ1Y3R1cmUu aAkocmV2aXNpb24gNDgzOTMpCisrKyBKYXZhU2NyaXB0Q29yZS9ydW50aW1lL1N0cnVjdHVyZS5o CSh3b3JraW5nIGNvcHkpCkBAIC0xODgsNyArMTg4LDE0IEBAIG5hbWVzcGFjZSBKU0MgewogICAg ICAgICBib29sIG1faXNEaWN0aW9uYXJ5IDogMTsKICAgICAgICAgYm9vbCBtX2lzUGlubmVkUHJv cGVydHlUYWJsZSA6IDE7CiAgICAgICAgIGJvb2wgbV9oYXNHZXR0ZXJTZXR0ZXJQcm9wZXJ0aWVz IDogMTsKKyNpZiBDT01QSUxFUihXSU5TQ1cpCisgICAgICAgIC8vIFdvcmthcm91bmQgZm9yIFN5 bWJpYW4gV0lOU0NXIGNvbXBpbGVyIHRoYXQgY2Fubm90IHJlc29sdmUgdW5zaWduZWQgdHlwZSBv ZiB0aGUgZGVjbGFyZWQgCisgICAgICAgIC8vIGJpdGZpZWxkLCB3aGVuIHVzZWQgYXMgYXJndW1l bnQgaW4gbWFrZV9wYWlyKCkgZnVuY3Rpb24gY2FsbHMgaW4gc3RydWN0dXJlLmNjcC4KKyAgICAg ICAgLy8gVGhpcyBiaXRmaWVsZCBvcHRpbWl6YXRpb24gaXMgaW5zaWduaWZpY2FudCBmb3IgdGhl IFN5bWJpYW4gZW11bGF0b3IgdGFyZ2V0LgorICAgICAgICB1bnNpZ25lZCBtX2F0dHJpYnV0ZXNJ blByZXZpb3VzOworI2Vsc2UKICAgICAgICAgdW5zaWduZWQgbV9hdHRyaWJ1dGVzSW5QcmV2aW91 cyA6IDc7CisjZW5kaWYgICAgICAgIAogICAgIH07CiAKICAgICBpbmxpbmUgc2l6ZV90IFN0cnVj dHVyZTo6Z2V0KGNvbnN0IElkZW50aWZpZXImIHByb3BlcnR5TmFtZSkK