236365 2022-02-09 07:21:05 -0800 [CMake] REGRESSION(r288994): Setting multiple values in LDFLAGS causes incorrect linker detection 2022-02-09 08:36:17 -0800 1 1 1 Unclassified WebKit Tools / Tests WebKit Local Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=235476 https://bugs.webkit.org/show_bug.cgi?id=236366 InRadar P2 Normal --- 1 aperez aperez annulen aperez cgarcia clopez darin ews-watchlist gyuyoung.kim mcatanzaro mrobinson pnormand ryuan.choi saam sergio webkit-bug-importer ysuzuki oldest_to_newest 1839136 0 aperez 2022-02-09 07:21:05 -0800 After r288994, doing something like the following will cause the linker to be wrongly identified: LDFLAGS='-Wl,-O1 -Wl,--as-needed' Tools/Scripts/build-webkit [...] and CMake will print the following (it should read “BFD”): -- Linker variant in use: UNKNOWN 1839138 1 aperez 2022-02-09 07:26:51 -0800 This is caused by CMake's execute_process() passing arguments verbatim to the command without using the shell nor doing any splitting. Setting LDFLAGS='-Wl,-O1 -Wl,--as-needed' before running CMake results in the variable CMAKE_EXE_LINKER_FLAGS having the literal value of LDFLAGS, and then this: execute_process( COMMAND ${CMAKE_C_COMPILER} ${CMAKE_EXE_LINKER_FLAGS} -Wl,--version OUTPUT_VARIABLE LD_VERSION ERROR_QUIET ) Results in trying to executing (argument index in square brackets): [0]: cc [1]: -Wl,-O1 -Wl,--as-needed [2]: -Wl,--version When what is needed is: [0]: cc [1]: -Wl,-O1 [2]: -Wl,--as-needed [3]: -Wl,--version I have a patch to fix the two usages of execute_process() introduced in r288994 but we might want to audit all the callsites where this CMake command is used to make sure no other similar problems lurk undiscovered. 1839141 2 aperez 2022-02-09 07:30:52 -0800 Note that previous uses of execute_process() would hit the same issue when setting LD/CC/CXX in the environment to some value that includes spaces, so the following would fail even before the patch from r288994: CC='ccache gcc' CXX='ccache g++' Tools/Scripts/build-webkit [...] This explains why I have a vague memory of trying to use “ccache” by adding it as prefix for the compiler command and it failed (of course this is *not* the recommended way of using “ccache”, but the same issue would break the build with any other similar compiler wrappers.) 1839145 3 451375 aperez 2022-02-09 07:33:58 -0800 Created attachment 451375 Patch 1839183 4 ews-feeder 2022-02-09 08:35:47 -0800 Committed r289473 (247016@main): <https://commits.webkit.org/247016@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 451375. 1839184 5 webkit-bug-importer 2022-02-09 08:36:17 -0800 <rdar://problem/88691618> 451375 2022-02-09 07:33:58 -0800 2022-02-09 08:35:51 -0800 Patch bug-236365-20220209173357.patch text/plain 2264 aperez U3VidmVyc2lvbiBSZXZpc2lvbjogMjg5NDY3CmRpZmYgLS1naXQgYS9Tb3VyY2UvY21ha2UvT3B0 aW9uc0NvbW1vbi5jbWFrZSBiL1NvdXJjZS9jbWFrZS9PcHRpb25zQ29tbW9uLmNtYWtlCmluZGV4 IDQwYzI0YmExMDY0YzMxZDdlZjQ0OGI1ZmU3ZmE2Mzc1YzZlYzc4OTEuLjlmNDU1MzY1YjVkMTdl NDM1MzYwYjBjNDRkOTY5ODFmMjM0YmJjZjAgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9jbWFrZS9PcHRp b25zQ29tbW9uLmNtYWtlCisrKyBiL1NvdXJjZS9jbWFrZS9PcHRpb25zQ29tbW9uLmNtYWtlCkBA IC0zNiwxMSArMzYsMTYgQEAgaWYgKFVTRV9MRF9MTEQpCiBlbmRpZiAoKQogCiAjIERldGVybWlu ZSB3aGljaCBsaW5rZXIgaXMgYmVpbmcgdXNlZCB3aXRoIHRoZSBjaG9zZW4gbGlua2VyIGZsYWdz Lgorc2VwYXJhdGVfYXJndW1lbnRzKExEX1ZFUlNJT05fQ09NTUFORCBVTklYX0NPTU1BTkQKKyAg ICAiJHtDTUFLRV9DX0NPTVBJTEVSfSAke0NNQUtFX0VYRV9MSU5LRVJfRkxBR1N9IC1XbCwtLXZl cnNpb24iCispCiBleGVjdXRlX3Byb2Nlc3MoCi0gICAgQ09NTUFORCAke0NNQUtFX0NfQ09NUElM RVJ9ICR7Q01BS0VfRVhFX0xJTktFUl9GTEFHU30gLVdsLC0tdmVyc2lvbgorICAgIENPTU1BTkQg JHtMRF9WRVJTSU9OX0NPTU1BTkR9CiAgICAgT1VUUFVUX1ZBUklBQkxFIExEX1ZFUlNJT04KICAg ICBFUlJPUl9RVUlFVAogKQordW5zZXQoTERfVkVSU0lPTl9DT01NQU5EKQorCiBzZXQoTERfU1VQ UE9SVFNfR0RCX0lOREVYIFRSVUUpCiBzZXQoTERfU1VQUE9SVFNfU1BMSVRfREVCVUcgVFJVRSkK IHNldChMRF9TVVBQT1JUU19USElOX0FSQ0hJVkVTIFRSVUUpCkBAIC02OSwxMiArNzQsMTUgQEAg bWVzc2FnZShTVEFUVVMgIiAgTGlua2VyIHN1cHBvcnRzIC0tZ2RiLWluZGV4IC0gJHtMRF9TVVBQ T1JUU19HREJfSU5ERVh9IikKIG1lc3NhZ2UoU1RBVFVTICIgIExpbmtlciBzdXBwb3J0cyAtLWRp c2FibGUtbmV3LWR0YWdzIC0gJHtMRF9TVVBQT1JUU19ESVNBQkxFX05FV19EVEFHU30iKQogCiAj IERldGVybWluZSB3aGV0aGVyIHRoZSBhcmNoaXZlciBpbiB1c2Ugc3VwcG9ydHMgdGhpbiBhcmNo aXZlcy4KK3NlcGFyYXRlX2FyZ3VtZW50cyhBUl9WRVJTSU9OX0NPTU1BTkQgVU5JWF9DT01NQU5E ICIke0NNQUtFX0FSfSAtViIpCiBleGVjdXRlX3Byb2Nlc3MoCi0gICAgQ09NTUFORCAke0NNQUtF X0FSfSAtVgorICAgIENPTU1BTkQgJHtBUl9WRVJTSU9OX0NPTU1BTkR9CiAgICAgT1VUUFVUX1ZB UklBQkxFIEFSX1ZFUlNJT04KICAgICBSRVNVTFRfVkFSSUFCTEUgQVJfU1RBVFVTCiAgICAgRVJS T1JfUVVJRVQKICkKK3Vuc2V0KEFSX1ZFUlNJT05fQ09NTUFORCkKKwogc2V0KEFSX1NVUFBPUlRT X1RISU5fQVJDSElWRVMgRkFMU0UpCiBpZiAoQVJfU1RBVFVTIEVRVUFMIDApCiAgICAgaWYgKEFS X1ZFUlNJT04gTUFUQ0hFUyAiXkdOVSBhciAiKQpkaWZmIC0tZ2l0IGEvQ2hhbmdlTG9nIGIvQ2hh bmdlTG9nCmluZGV4IDdjNWU5MDgxMmIyYjllMmFiYmM1NTllNDNkZDNiMzhlYWNhNGNlZTguLjY0 OGM3OTFlNTBiMGI1ZDJlOGU0ZWJiYWNiOWVhNTNmM2IzNjgyYjggMTAwNjQ0Ci0tLSBhL0NoYW5n ZUxvZworKysgYi9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNCBAQAorMjAyMi0wMi0wOSAgQWRyaWFu IFBlcmV6IGRlIENhc3RybyAgPGFwZXJlekBpZ2FsaWEuY29tPgorCisgICAgICAgIFtDTWFrZV0g UkVHUkVTU0lPTihyMjg4OTk0KTogU2V0dGluZyBtdWx0aXBsZSB2YWx1ZXMgaW4gTERGTEFHUyBj YXVzZXMgaW5jb3JyZWN0IGxpbmtlciBkZXRlY3Rpb24KKyAgICAgICAgaHR0cHM6Ly9idWdzLndl YmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIzNjM2NQorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgICogU291cmNlL2NtYWtlL09wdGlvbnNDb21tb24uY21h a2U6IFVzZSBzZXBhcmF0ZV9hcmd1bWVudHMoKSB0byB0dXJuIHBsYWluIGNvbW1hbmQgc3RyaW5n cworICAgICAgICBpbnRvIGxpc3RzIG9mIHN0cmluZ3MsIHdoaWNoIGNhbiB0aGVuIGJlIHBhc3Nl ZCBkb3duIHRvIGV4ZWN1dGVfcHJvY2VzcygpIGFzIGl0IGtub3dzIGhvdworICAgICAgICB0byBo YW5kbGUgbGlzdHMgcHJvcGVybHkuCisKIDIwMjItMDItMDkgIENhcmxvcyBHYXJjaWEgQ2FtcG9z ICA8Y2dhcmNpYUBpZ2FsaWEuY29tPgogCiAgICAgICAgIFVucmV2aWV3ZWQuIFVwZGF0ZSBPcHRp b25zR1RLLmNtYWtlIGFuZCBORVdTIGZvciAyLjM1LjMgcmVsZWFzZQo=