104085 2012-12-04 21:43:33 -0800 Fix broken WinCE build due to call to add64 in JIT.cpp 2012-12-11 19:37:03 -0800 1 1 1 Unclassified WebKit JavaScriptCore 528+ (Nightly build) Unspecified Unspecified CLOSED DUPLICATE 104103 P2 Normal --- 1 vivek.vg vivek.vg andersca barraclough fpizlo gaborb ggaren laszlo.gombos oliver paroga zherczeg oldest_to_newest 783379 0 vivek.vg 2012-12-04 21:43:33 -0800 The method add64() is called inside JIT::privateCompileMainPass(). This call should be under the #if USE(JSVALUE64) guard as its not implemented in WinCE MacroAssemblerARM.cpp Patch follows. 783381 1 177666 vivek.vg 2012-12-04 21:46:44 -0800 Created attachment 177666 Patch 784755 2 177666 paroga 2012-12-06 01:36:27 -0800 Comment on attachment 177666 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=177666&action=review > Source/JavaScriptCore/ChangeLog:9 > + This call should be under the #if USE(JSVALUE64) guard as its not implemented in WinCE MacroAssemblerARM.cpp This has _nothing_ to do with WinCE. It's a "problem" of the traditional ARM MacroAssembler. Correct way to fix this is to implement add64() in the MacroAssembler. Usually the szeged guys maintain the traditional ARM (e.g. https://trac.webkit.org/changeset/135610). 784830 3 vivek.vg 2012-12-06 04:00:28 -0800 (In reply to comment #2) > (From update of attachment 177666 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=177666&action=review > > > Source/JavaScriptCore/ChangeLog:9 > > + This call should be under the #if USE(JSVALUE64) guard as its not implemented in WinCE MacroAssemblerARM.cpp > > This has _nothing_ to do with WinCE. It's a "problem" of the traditional ARM MacroAssembler. Thank you Patrick, I wasn't aware of this. > Correct way to fix this is to implement add64() in the MacroAssembler. > Usually the szeged guys maintain the traditional ARM (e.g. https://trac.webkit.org/changeset/135610). I added the macro guard as currently all the calls to add64 are under the #if USE(JSVALUE64) except this one. Hence thought this would fix the issue for the time being until a real add64 is implemented. But may be I am wrong with the approach. Thanks for pointing this. 784976 4 fpizlo 2012-12-06 09:10:58 -0800 (In reply to comment #3) > (In reply to comment #2) > > (From update of attachment 177666 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=177666&action=review > > > > > Source/JavaScriptCore/ChangeLog:9 > > > + This call should be under the #if USE(JSVALUE64) guard as its not implemented in WinCE MacroAssemblerARM.cpp > > > > This has _nothing_ to do with WinCE. It's a "problem" of the traditional ARM MacroAssembler. > > Thank you Patrick, I wasn't aware of this. > > > Correct way to fix this is to implement add64() in the MacroAssembler. > > Usually the szeged guys maintain the traditional ARM (e.g. https://trac.webkit.org/changeset/135610). > > I added the macro guard as currently all the calls to add64 are under the #if USE(JSVALUE64) except this one. Hence thought this would fix the issue for the time being until a real add64 is implemented. But may be I am wrong with the approach. Thanks for pointing this. I don't recommend using this kind of thinking in the future. Your change breaks a new JSC feature on all 32-bit platforms just because one particular kind of 32-bit platform has a broken assembler. We don't usually like to fix bugs by introducing new ones. 785291 5 vivek.vg 2012-12-06 14:40:02 -0800 (In reply to comment #4) > I don't recommend using this kind of thinking in the future. Your change breaks a new JSC feature on all 32-bit platforms just because one particular kind of 32-bit platform has a broken assembler. We don't usually like to fix bugs by introducing new ones. I apologize for not considering the consequences of my changes. I will make sure to consider all these scenarios before submitting the patches going forward. Thanks for your feedback, really appreciated. 789126 6 vivek.vg 2012-12-11 19:37:03 -0800 *** This bug has been marked as a duplicate of bug 104103 *** 177666 2012-12-04 21:46:44 -0800 2012-12-06 01:36:27 -0800 Patch bug-104085-20121205111451.patch text/plain 1773 vivek.vg U3VidmVyc2lvbiBSZXZpc2lvbjogMTM2NjM3CmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlw dENvcmUvQ2hhbmdlTG9nIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwppbmRleCAx MWQxNmU2NDEzNjAyYTk0OWM5NWJlYTg1NTMyN2JiZjQ1MjBmNDYyLi5mZTJkMWQ5ZTg2YWIwNzFj ZDFkN2UzZWYzNmRjMDA4NmFjMWM0NDU1IDEwMDY0NAotLS0gYS9Tb3VyY2UvSmF2YVNjcmlwdENv cmUvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwxNiBAQAorMjAxMi0xMi0wNCAgVml2ZWsgR2FsYXRhZ2UgIDx2aXZlay52Z0BzYW1zdW5n LmNvbT4KKworICAgICAgICBGaXggYnJva2VuIFdpbkNFIGJ1aWxkIGR1ZSB0byBjYWxsIHRvIGFk ZDY0IGluIEpJVC5jcHAKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcu Y2dpP2lkPTEwNDA4NQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgIFRoZSBtZXRob2QgYWRkNjQoKSBpcyBjYWxsZWQgaW5zaWRlIEpJVDo6cHJpdmF0ZUNv bXBpbGVNYWluUGFzcygpLgorICAgICAgICBUaGlzIGNhbGwgc2hvdWxkIGJlIHVuZGVyIHRoZSAj aWYgVVNFKEpTVkFMVUU2NCkgZ3VhcmQgYXMgaXRzIG5vdCBpbXBsZW1lbnRlZCBpbiBXaW5DRSBN YWNyb0Fzc2VtYmxlckFSTS5jcHAKKworICAgICAgICAqIGppdC9KSVQuY3BwOgorICAgICAgICAo SlNDOjpKSVQ6OnByaXZhdGVDb21waWxlTWFpblBhc3MpOgorCiAyMDEyLTEyLTA0ICBGaWxpcCBQ aXpsbyAgPGZwaXpsb0BhcHBsZS5jb20+CiAKICAgICAgICAgZGlzcGxheS1wcm9maWxlci1vdXRw dXQgc2hvdWxkIGJlIGFibGUgdG8gc2hvdyBzb3VyY2UgY29kZQpkaWZmIC0tZ2l0IGEvU291cmNl L0phdmFTY3JpcHRDb3JlL2ppdC9KSVQuY3BwIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL2ppdC9K SVQuY3BwCmluZGV4IDRlY2VkMmY0NjkxZGZkMmQ3Y2UyNDE2MDM2YTY2MTQ5MzdmYzNlZmMuLjcz ZmE0ZTVmY2NkZjYwZjE2YWM5MTE5YjI3NTc1ZTY0MzIwOWE0OTggMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9KYXZhU2NyaXB0Q29yZS9qaXQvSklULmNwcAorKysgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUv aml0L0pJVC5jcHAKQEAgLTIzNCwxMiArMjM0LDE0IEBAIHZvaWQgSklUOjpwcml2YXRlQ29tcGls ZU1haW5QYXNzKCkKICAgICAgICAgZGF0YUxvZ0YoIk9sZCBKSVQgZW1pdHRpbmcgY29kZSBmb3Ig YmMjJXUgYXQgb2Zmc2V0IDB4JWx4LlxuIiwgbV9ieXRlY29kZU9mZnNldCwgKGxvbmcpZGVidWdP ZmZzZXQoKSk7CiAjZW5kaWYKICAgICAgICAgCisjaWYgVVNFKEpTVkFMVUU2NCkKICAgICAgICAg aWYgKG1fY29tcGlsYXRpb24pIHsKICAgICAgICAgICAgIGFkZDY0KAogICAgICAgICAgICAgICAg IFRydXN0ZWRJbW0zMigxKSwKICAgICAgICAgICAgICAgICBBYnNvbHV0ZUFkZHJlc3MobV9jb21w aWxhdGlvbi0+ZXhlY3V0aW9uQ291bnRlckZvcihQcm9maWxlcjo6T3JpZ2luU3RhY2soUHJvZmls ZXI6Ok9yaWdpbigKICAgICAgICAgICAgICAgICAgICAgbV9jb21waWxhdGlvbi0+Ynl0ZWNvZGVz KCksIG1fYnl0ZWNvZGVPZmZzZXQpKSktPmFkZHJlc3MoKSkpOwogICAgICAgICB9CisjZW5kaWYK IAogICAgICAgICBzd2l0Y2ggKG1faW50ZXJwcmV0ZXItPmdldE9wY29kZUlEKGN1cnJlbnRJbnN0 cnVjdGlvbi0+dS5vcGNvZGUpKSB7CiAgICAgICAgIERFRklORV9CSU5BUllfT1Aob3BfZGVsX2J5 X3ZhbCkK