188699 2018-08-17 10:01:26 -0700 Adjust CMAKE_MODULE_LINKER_FLAGS for asan 2018-08-18 08:03:18 -0700 1 1 1 Unclassified WebKit Tools / Tests Other PC Linux RESOLVED FIXED InRadar P2 Normal --- 1 mcatanzaro mcatanzaro annulen commit-queue don.olmstead lforschler mcatanzaro webkit-bug-importer oldest_to_newest 1451133 0 mcatanzaro 2018-08-17 10:01:26 -0700 As suggested by Konstantin, it's good form to set CMAKE_MODULE_LINKER_FLAGS for when enabling asan. 1451134 1 347362 mcatanzaro 2018-08-17 10:02:02 -0700 Created attachment 347362 Patch 1451139 2 347362 mcatanzaro 2018-08-17 10:09:06 -0700 Comment on attachment 347362 Patch Hm, maybe should also set CMAKE_STATIC_LINKER_FLAGS 1451140 3 347362 annulen 2018-08-17 10:10:51 -0700 Comment on attachment 347362 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=347362&action=review > Source/cmake/WebKitCompilerFlags.cmake:177 > + set(CMAKE_MODULE_LINKER_FLAGS "-lpthread ${CMAKE_MODULE_LINKER_FLAGS} -fsanitize=address") It would be better to avoid duplication of flags, e.g. set(CMAKE_EXE_LINKER_FLAGS "-lpthread ${CMAKE_EXE_LINKER_FLAGS} -fsanitize=address") set(CMAKE_SHARED_LINKER_FLAGS ${CMAKE_EXE_LINKER_FLAGS}) set(CMAKE_MODULE_LINKER_FLAGS ${CMAKE_EXE_LINKER_FLAGS}) 1451141 4 annulen 2018-08-17 10:11:24 -0700 (In reply to Michael Catanzaro from comment #2) > Hm, maybe should also set CMAKE_STATIC_LINKER_FLAGS No, it's not needed 1451159 5 mcatanzaro 2018-08-17 11:06:10 -0700 (In reply to Konstantin Tokarev from comment #3) > It would be better to avoid duplication of flags, e.g. > > set(CMAKE_EXE_LINKER_FLAGS "-lpthread ${CMAKE_EXE_LINKER_FLAGS} > -fsanitize=address") > set(CMAKE_SHARED_LINKER_FLAGS ${CMAKE_EXE_LINKER_FLAGS}) > set(CMAKE_MODULE_LINKER_FLAGS ${CMAKE_EXE_LINKER_FLAGS}) Well that's no good, what if the user wants to set different flags? I checked what ECMEnableSanitizers is doing and it boils down to link_libraries("asan") 1451161 6 annulen 2018-08-17 11:09:07 -0700 (In reply to Michael Catanzaro from comment #5) > (In reply to Konstantin Tokarev from comment #3) > > It would be better to avoid duplication of flags, e.g. > > > > set(CMAKE_EXE_LINKER_FLAGS "-lpthread ${CMAKE_EXE_LINKER_FLAGS} > > -fsanitize=address") > > set(CMAKE_SHARED_LINKER_FLAGS ${CMAKE_EXE_LINKER_FLAGS}) > > set(CMAKE_MODULE_LINKER_FLAGS ${CMAKE_EXE_LINKER_FLAGS}) > > Well that's no good, what if the user wants to set different flags? Oops. Just wanted to get away without additional variable, but it was surely wrong 1451163 7 annulen 2018-08-17 11:10:41 -0700 (In reply to Michael Catanzaro from comment #5) > I checked what ECMEnableSanitizers is doing and it boils down to > link_libraries("asan") Yep, it should be fine, at least ECMEnableSanitizers works fine with QtWebKit 1451168 8 347362 mcatanzaro 2018-08-17 11:19:04 -0700 Comment on attachment 347362 Patch Bringing back this original patch... I was hoping to fix asan under flatpak here, but it's too much work for this bug. 1451385 9 347362 commit-queue 2018-08-18 08:00:19 -0700 Comment on attachment 347362 Patch Clearing flags on attachment: 347362 Committed r235010: <https://trac.webkit.org/changeset/235010> 1451386 10 commit-queue 2018-08-18 08:00:20 -0700 All reviewed patches have been landed. Closing bug. 1451387 11 webkit-bug-importer 2018-08-18 08:03:18 -0700 <rdar://problem/43461813> 347362 2018-08-17 10:02:02 -0700 2018-08-18 08:00:19 -0700 Patch bug-188699-20180817120202.patch text/plain 1290 mcatanzaro U3VidmVyc2lvbiBSZXZpc2lvbjogMjM0OTgxCmRpZmYgLS1naXQgYS9Tb3VyY2UvY21ha2UvV2Vi S2l0Q29tcGlsZXJGbGFncy5jbWFrZSBiL1NvdXJjZS9jbWFrZS9XZWJLaXRDb21waWxlckZsYWdz LmNtYWtlCmluZGV4IGFmMDRlOGQ4ZGI5YzJiMDZmMDg5ZWMxODU3NGUyZThjYTNjZDA1OTEuLjcy ZGM1Yzk2Nzc5MjhkZGJkODVjMjEyMDY5OTIzMjE3ZjI5ZjQ1NmIgMTAwNjQ0Ci0tLSBhL1NvdXJj ZS9jbWFrZS9XZWJLaXRDb21waWxlckZsYWdzLmNtYWtlCisrKyBiL1NvdXJjZS9jbWFrZS9XZWJL aXRDb21waWxlckZsYWdzLmNtYWtlCkBAIC0xNzQsNiArMTc0LDcgQEAgaWYgKENPTVBJTEVSX0lT X0dDQ19PUl9DTEFORykKICAgICAgICAgc2V0KENNQUtFX0NYWF9GTEFHUyAiJHtDTUFLRV9DWFhf RkxBR1N9IC1mc2FuaXRpemU9YWRkcmVzcyIpCiAgICAgICAgIHNldChDTUFLRV9FWEVfTElOS0VS X0ZMQUdTICItbHB0aHJlYWQgJHtDTUFLRV9FWEVfTElOS0VSX0ZMQUdTfSAtZnNhbml0aXplPWFk ZHJlc3MiKQogICAgICAgICBzZXQoQ01BS0VfU0hBUkVEX0xJTktFUl9GTEFHUyAiLWxwdGhyZWFk ICR7Q01BS0VfU0hBUkVEX0xJTktFUl9GTEFHU30gLWZzYW5pdGl6ZT1hZGRyZXNzIikKKyAgICAg ICAgc2V0KENNQUtFX01PRFVMRV9MSU5LRVJfRkxBR1MgIi1scHRocmVhZCAke0NNQUtFX01PRFVM RV9MSU5LRVJfRkxBR1N9IC1mc2FuaXRpemU9YWRkcmVzcyIpCiAgICAgZW5kaWYgKCkKIGVuZGlm ICgpCiAKZGlmZiAtLWdpdCBhL0NoYW5nZUxvZyBiL0NoYW5nZUxvZwppbmRleCAyMmRmZWRkYmI1 OGIzYTNiNGI1ZGJkMThjNTRlNjMxZTM0OTIwNjc0Li5iMjIzYjljYWI5YTU0OGQwYjJmMzY5NzJh OThkMGMxMTg1NTUwMWYwIDEwMDY0NAotLS0gYS9DaGFuZ2VMb2cKKysrIGIvQ2hhbmdlTG9nCkBA IC0xLDMgKzEsMTIgQEAKKzIwMTgtMDgtMTcgIE1pY2hhZWwgQ2F0YW56YXJvICA8bWNhdGFuemFy b0BpZ2FsaWEuY29tPgorCisgICAgICAgIEFkanVzdCBDTUFLRV9NT0RVTEVfTElOS0VSX0ZMQUdT IGZvciBhc2FuCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9p ZD0xODg2OTkKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAg ICAqIFNvdXJjZS9jbWFrZS9XZWJLaXRDb21waWxlckZsYWdzLmNtYWtlOgorCiAyMDE4LTA3LTMw ICBUaGliYXVsdCBTYXVuaWVyICA8dHNhdW5pZXJAaWdhbGlhLmNvbT4KIAogICAgICAgICBbR1N0 cmVhbWVyXSBNYWtlIGNvZGVjcGFyc2VycyBvcHRpb25uYWwK