193043 2018-12-27 03:33:24 -0800 [CMake][WTF] Finesse i386 architecture detection 2018-12-28 05:02:32 -0800 1 1 1 Unclassified WebKit CMake WebKit Nightly Build Unspecified Unspecified RESOLVED INVALID P2 Normal --- 1 jmason webkit-unassigned ews-watchlist oldest_to_newest 1490892 0 jmason 2018-12-27 03:33:24 -0800 WTF processor architecture is being determined via CMAKE_SYSTEM_PROCESSOR. On systems which support uname, this is initialized to `uname -p`. However, the tricky bit is that on some OSes, `uname -p` returns 'i386' for both 32-bit and 64-bit members of the Intel processor family. Thus, in this case, we must check the pointer size to determine the actual architecture. 1490895 1 358107 jmason 2018-12-27 04:18:49 -0800 Created attachment 358107 Patch 1490896 2 ews-watchlist 2018-12-27 04:21:18 -0800 Attachment 358107 did not pass style-queue: ERROR: CMakeLists.txt:105: No trailing spaces [whitespace/trailing] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style. 1490897 3 358108 jmason 2018-12-27 04:23:11 -0800 Created attachment 358108 Patch Fixed style fault 1490906 4 358108 ap 2018-12-27 11:27:48 -0800 Comment on attachment 358108 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=358108&action=review > CMakeLists.txt:98 > + # On some OSes, `uname -p` always reports i386 for all members How will we know in the future that this hack can be removed? Can you leave any pointers at all? 1490909 5 jmason 2018-12-27 13:34:29 -0800 (In reply to Alexey Proskuryakov from comment #4) > How will we know in the future that this hack can be removed? Can you leave > any pointers at all? There is no intention that this should ever be removed; it is necessary because of the semantics of uname -p: https://en.wikipedia.org/wiki/Uname As can be observed from the link above, many operating systems return 'i386' for `uname -p` although they are in fact 64-bit. For correct operation, it must be checked in this case whether the target is actually 32- or 64-bits. See for example: https://cmake.org/pipermail/cmake/2007-March/013134.html 1490929 6 jmason 2018-12-28 05:02:18 -0800 I am going to withdraw this patch for now. CMAKE_SYSTEM_PROCESSOR is not ideal, as i386 is ambiguous; however, I am not sure there is a best-practices alternative for architecture determination in CMake. As for the patch, my doubt is whether CMAKE_SIZEOF_VOID_P will yield the expected result if the target architecture is different to the build architecture. For now, users who are affected by the i386 ambiguity can override the value of CMAKE_SYSTEM_PROCESSOR to work within the existing semantics. That is not ideal, but now I can think of no better way that does not introduce some other problem. 358107 2018-12-27 04:18:49 -0800 2018-12-27 04:23:11 -0800 Patch Patch text/plain 1344 jmason SW5kZXg6IENoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBDaGFuZ2VMb2cJKHJldmlzaW9uIDIzOTU1 NCkKKysrIENoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEyIEBACisyMDE4LTEy LTI3ICBKaW0gTWFzb24gIDxqbWFzb25AaWJpbnguY29tPgorCisgICAgICAgIFtDTWFrZV1bV1RG XSBGaW5lc3NlIGkzODYgYXJjaGl0ZWN0dXJlIGRldGVjdGlvbgorICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTkzMDQzCisKKyAgICAgICAgUmV2aWV3ZWQg YnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBDTWFrZUxpc3RzLnR4dDoKKwogMjAxOC0x Mi0xOSAgQWRyaWFuIFBlcmV6IGRlIENhc3RybyAgPGFwZXJlekBpZ2FsaWEuY29tPgogCiAgICAg ICAgIFtHVEtdIENhbm5vdCBidWlsZCB3aXRoIENNYWtlIDwzLjcKSW5kZXg6IENNYWtlTGlzdHMu dHh0Cj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT0KLS0tIENNYWtlTGlzdHMudHh0CShyZXZpc2lvbiAyMzk1NTQpCisrKyBD TWFrZUxpc3RzLnR4dAkod29ya2luZyBjb3B5KQpAQCAtOTUsNyArOTUsMTQgQEAKIGVsc2VpZiAo TE9XRVJDQVNFX0NNQUtFX1NZU1RFTV9QUk9DRVNTT1IgTUFUQ0hFUyAiKHg2NHx4ODZfNjR8YW1k NjQpIikKICAgICBzZXQoV1RGX0NQVV9YODZfNjQgMSkKIGVsc2VpZiAoTE9XRVJDQVNFX0NNQUtF X1NZU1RFTV9QUk9DRVNTT1IgTUFUQ0hFUyAiKGlbMy02XTg2fHg4NikiKQotICAgIHNldChXVEZf Q1BVX1g4NiAxKQorICAgICMgT24gc29tZSBPU2VzLCBgdW5hbWUgLXBgIGFsd2F5cyByZXBvcnRz IGkzODYgZm9yIGFsbCBtZW1iZXJzCisgICAgIyBvZiB0aGUgSW50ZWwgcHJvY2Vzc29yIGZhbWls eSwgd2hldGhlciAzMi0gb3IgNjQtYml0LiAgV2UgbXVzdAorICAgICMgaW5zcGVjdCB0aGUgcG9p bnRlciBzaXplIHRvIGRldGVybWluZSB0aGUgYWN0dWFsIGFyY2hpdGVjdHVyZS4KKyAgICBpZiAo Q01BS0VfU0laRU9GX1ZPSURfUCBNQVRDSEVTIDgpCisgICAgICAgIHNldChXVEZfQ1BVX1g4Nl82 NCAxKQorICAgIGVsc2UgKCkKKyAgICAgICAgc2V0KFdURl9DUFVfWDg2IDEpCisgICAgZW5kaWYg KCkgICAgCiBlbHNlaWYgKExPV0VSQ0FTRV9DTUFLRV9TWVNURU1fUFJPQ0VTU09SIE1BVENIRVMg InBwYyIpCiAgICAgc2V0KFdURl9DUFVfUFBDIDEpCiBlbHNlaWYgKExPV0VSQ0FTRV9DTUFLRV9T WVNURU1fUFJPQ0VTU09SIE1BVENIRVMgInBwYzY0IikK 358108 2018-12-27 04:23:11 -0800 2018-12-28 05:02:32 -0800 Patch Patch text/plain 1340 jmason SW5kZXg6IENoYW5nZUxvZwo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBDaGFuZ2VMb2cJKHJldmlzaW9uIDIzOTU1 NCkKKysrIENoYW5nZUxvZwkod29ya2luZyBjb3B5KQpAQCAtMSwzICsxLDEyIEBACisyMDE4LTEy LTI3ICBKaW0gTWFzb24gIDxqbWFzb25AaWJpbnguY29tPgorCisgICAgICAgIFtDTWFrZV1bV1RG XSBGaW5lc3NlIGkzODYgYXJjaGl0ZWN0dXJlIGRldGVjdGlvbgorICAgICAgICBodHRwczovL2J1 Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MTkzMDQzCisKKyAgICAgICAgUmV2aWV3ZWQg YnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiBDTWFrZUxpc3RzLnR4dDoKKwogMjAxOC0x Mi0xOSAgQWRyaWFuIFBlcmV6IGRlIENhc3RybyAgPGFwZXJlekBpZ2FsaWEuY29tPgogCiAgICAg ICAgIFtHVEtdIENhbm5vdCBidWlsZCB3aXRoIENNYWtlIDwzLjcKSW5kZXg6IENNYWtlTGlzdHMu dHh0Cj09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT0KLS0tIENNYWtlTGlzdHMudHh0CShyZXZpc2lvbiAyMzk1NTQpCisrKyBD TWFrZUxpc3RzLnR4dAkod29ya2luZyBjb3B5KQpAQCAtOTUsNyArOTUsMTQgQEAKIGVsc2VpZiAo TE9XRVJDQVNFX0NNQUtFX1NZU1RFTV9QUk9DRVNTT1IgTUFUQ0hFUyAiKHg2NHx4ODZfNjR8YW1k NjQpIikKICAgICBzZXQoV1RGX0NQVV9YODZfNjQgMSkKIGVsc2VpZiAoTE9XRVJDQVNFX0NNQUtF X1NZU1RFTV9QUk9DRVNTT1IgTUFUQ0hFUyAiKGlbMy02XTg2fHg4NikiKQotICAgIHNldChXVEZf Q1BVX1g4NiAxKQorICAgICMgT24gc29tZSBPU2VzLCBgdW5hbWUgLXBgIGFsd2F5cyByZXBvcnRz IGkzODYgZm9yIGFsbCBtZW1iZXJzCisgICAgIyBvZiB0aGUgSW50ZWwgcHJvY2Vzc29yIGZhbWls eSwgd2hldGhlciAzMi0gb3IgNjQtYml0LiAgV2UgbXVzdAorICAgICMgaW5zcGVjdCB0aGUgcG9p bnRlciBzaXplIHRvIGRldGVybWluZSB0aGUgYWN0dWFsIGFyY2hpdGVjdHVyZS4KKyAgICBpZiAo Q01BS0VfU0laRU9GX1ZPSURfUCBNQVRDSEVTIDgpCisgICAgICAgIHNldChXVEZfQ1BVX1g4Nl82 NCAxKQorICAgIGVsc2UgKCkKKyAgICAgICAgc2V0KFdURl9DUFVfWDg2IDEpCisgICAgZW5kaWYg KCkKIGVsc2VpZiAoTE9XRVJDQVNFX0NNQUtFX1NZU1RFTV9QUk9DRVNTT1IgTUFUQ0hFUyAicHBj IikKICAgICBzZXQoV1RGX0NQVV9QUEMgMSkKIGVsc2VpZiAoTE9XRVJDQVNFX0NNQUtFX1NZU1RF TV9QUk9DRVNTT1IgTUFUQ0hFUyAicHBjNjQiKQo=