216746 2020-09-20 10:47:50 -0700 Enable AddressSanitizer in C++ std library templates 2021-09-10 14:36:22 -0700 1 1 1 Unclassified WebKit Tools / Tests Other Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=230100 InRadar P2 Normal --- 216318 1 ddkilzer ddkilzer ap darin dino kkinnunen peng.liu6 ryanhaddad webkit-bug-importer youennf oldest_to_newest 1690176 0 ddkilzer 2020-09-20 10:47:50 -0700 Enable AddressSanitizer in C++ std library templates. This amounts to undefining the _LIBCPP_HAS_NO_ASAN macro when building. Although most of WebKit doesn't used C++ std library types like std::vector or std::hash, ANGLE and libwebrtc do use these types, so it's beneficial to enable AddressSanitizer in these cases when compiling with Asan enabled for the rest of WebKit. 1690182 1 409239 ddkilzer 2020-09-20 11:10:18 -0700 Created attachment 409239 Patch v1 1690185 2 ddkilzer 2020-09-20 11:11:25 -0700 (In reply to David Kilzer (:ddkilzer) from comment #0) > Although most of WebKit doesn't used C++ std library types like std::vector > or std::hash, ANGLE and libwebrtc do use these types, so it's beneficial to > enable AddressSanitizer in these cases when compiling with Asan enabled for > the rest of WebKit. Actually, I think this is mostly affects std::vector and related classes, not std::hash. 1690187 3 ddkilzer 2020-09-20 11:19:59 -0700 I CCed folks that work on ANGLE and libwebrtc since there is a small chance that enabling this might find some latent issues in those libraries when running layout tests. 1690206 4 409239 ap 2020-09-20 13:03:27 -0700 Comment on attachment 409239 Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=409239&action=review > Tools/sanitizer/asan.xcconfig:9 > +WK_SANITIZER_OTHER_CPLUSPLUSFLAGS_YES = $(inherited) -U_LIBCPP_HAS_NO_ASAN; Nice. Did you run tests locally to confirm that nothing terrible was going to happen? EWS doesn't have ASan. > Tools/sanitizer/sanitizer.xcconfig:7 > WK_SANITIZER_OTHER_CFLAGS_YES = -fno-omit-frame-pointer -g; > +WK_SANITIZER_OTHER_CPLUSPLUSFLAGS_YES = ; Does "-fno-omit-frame-pointer -g" get added for C++ in some other way, or do we not need it? 1690245 5 409239 ddkilzer 2020-09-20 20:45:48 -0700 Comment on attachment 409239 Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=409239&action=review >> Tools/sanitizer/asan.xcconfig:9 >> +WK_SANITIZER_OTHER_CPLUSPLUSFLAGS_YES = $(inherited) -U_LIBCPP_HAS_NO_ASAN; > > Nice. Did you run tests locally to confirm that nothing terrible was going to happen? EWS doesn't have ASan. Yes. No new ASan-related crashes in any std C++ objects. >> Tools/sanitizer/sanitizer.xcconfig:7 >> +WK_SANITIZER_OTHER_CPLUSPLUSFLAGS_YES = ; > > Does "-fno-omit-frame-pointer -g" get added for C++ in some other way, or do we not need it? As noted in the ChangeLog, OTHER_CFLAGS applies to C++ sources, so it doesn't need to be listed twice. I verified this by reviewing build log output. 1690383 6 ap 2020-09-21 12:01:20 -0700 > As noted in the ChangeLog, OTHER_CFLAGS applies to C++ sources, so it doesn't need to be listed twice. My understanding was that OTHER_CFLAGS was the default value for OTHER_CPLUSPLUSFLAGS, but it had no effect when OTHER_CPLUSPLUSFLAGS was defined. > I verified this by reviewing build log output. Surprising. 1690431 7 webkit-bug-importer 2020-09-21 12:25:34 -0700 <rdar://problem/69319755> 1690456 8 ews-feeder 2020-09-21 13:02:29 -0700 Committed r267358: <https://trac.webkit.org/changeset/267358> All reviewed patches have been landed. Closing bug and clearing flags on attachment 409239. 409239 2020-09-20 11:10:18 -0700 2020-09-21 13:02:30 -0700 Patch v1 bug-216746-20200920111111.patch text/plain 2501 ddkilzer U3VidmVyc2lvbiBSZXZpc2lvbjogMjY3MTQ0CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggNGZhNzA5NDdjMzgzODJmYzAzMGMyMWFlN2U2YmUyNWUx YjI2ZWUzZS4uZDMzN2JkNmZkMTVmMmQyYTU5ZDQ2MDBlOWU3ZjE1ZTUwNzE4N2E1YyAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDI2 IEBACisyMDIwLTA5LTIwICBEYXZpZCBLaWx6ZXIgIDxkZGtpbHplckBhcHBsZS5jb20+CisKKyAg ICAgICAgRW5hYmxlIEFkZHJlc3NTYW5pdGl6ZXIgaW4gQysrIHN0ZCBsaWJyYXJ5IHRlbXBsYXRl cworICAgICAgICA8aHR0cHM6Ly93ZWJraXQub3JnL2IvMjE2NzQ2PgorCisgICAgICAgIFJldmll d2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRoaXMgY2hhbmdlIGFsc28gcHJldmVu dHMgZHVwbGljYXRlIGNvbW1hbmQtbGluZSBzd2l0Y2hlcyBmcm9tCisgICAgICAgIGJlaW5nIHBh c3NlZCB0byBjbGFuZyBzaW5jZSBPVEhFUl9DRkxBR1MgaXMgdXNlZCB3aGVuIGNvbXBpbGluZwor ICAgICAgICBDKysgc291cmNlcy4KKworICAgICAgICAqIHNhbml0aXplci9hc2FuLnhjY29uZmln OgorICAgICAgICAoV0tfU0FOSVRJWkVSX09USEVSX0NQTFVTUExVU0ZMQUdTX1lFUyk6IEFkZC4K KyAgICAgICAgLSBVbmRlZmluZXMgX0xJQkNQUF9IQVNfTk9fQVNBTiBtYWNybyB0byBlbmFibGUg QVNhbiBmb3IgQysrIHN0ZAorICAgICAgICAgIGxpYnJhcnkgdGVtcGxhdGVzLgorCisgICAgICAg ICogc2FuaXRpemVyL3Nhbml0aXplci54Y2NvbmZpZzoKKyAgICAgICAgKFdLX1NBTklUSVpFUl9P VEhFUl9DUExVU1BMVVNGTEFHU19ZRVMpOiBBZGQuCisgICAgICAgIC0gUHJvdmlkZSBwbGFjZWhv bGRlci4KKyAgICAgICAgKE9USEVSX0NQTFVTUExVU0ZMQUdTKToKKyAgICAgICAgLSBTd2l0Y2gg ZnJvbSBXS19TQU5JVElaRVJfT1RIRVJfQ0ZMQUdTXyBwcmVmaXggdG8KKyAgICAgICAgICBXS19T QU5JVElaRVJfT1RIRVJfQ1BMVVNQTFVTRkxBR1NfIHByZWZpeC4KKwogMjAyMC0wOS0xNiAgUGhp bGlwcGUgTm9ybWFuZCAgPHBub3JtYW5kQGlnYWxpYS5jb20+CiAKICAgICAgICAgW0ZsYXRwYWsg U0RLXSBDaGVycnktcGljayBNZXNhIHVzZS1hZnRlci1mcmVlIGZpeGVzCmRpZmYgLS1naXQgYS9U b29scy9zYW5pdGl6ZXIvYXNhbi54Y2NvbmZpZyBiL1Rvb2xzL3Nhbml0aXplci9hc2FuLnhjY29u ZmlnCmluZGV4IDRiMDcyMDMzNWI5ZGViMTk3NjljODNjM2NmOWM1YmNiOGMwZjI3NGMuLjljZmZm OTcxZmVlOTI3ZDc0NjY1OThiMzM5Y2UxZjU1Y2Q0ZGQ1OTEgMTAwNjQ0Ci0tLSBhL1Rvb2xzL3Nh bml0aXplci9hc2FuLnhjY29uZmlnCisrKyBiL1Rvb2xzL3Nhbml0aXplci9hc2FuLnhjY29uZmln CkBAIC01LDMgKzUsNSBAQCBFTkFCTEVfQUREUkVTU19TQU5JVElaRVJfID0gWUVTOwogRU5BQkxF X0FERFJFU1NfU0FOSVRJWkVSX05PID0gWUVTOwogCiBXS19FTkFCTEVfU0FOSVRJWkVSID0gJChF TkFCTEVfQUREUkVTU19TQU5JVElaRVIpOworCitXS19TQU5JVElaRVJfT1RIRVJfQ1BMVVNQTFVT RkxBR1NfWUVTID0gJChpbmhlcml0ZWQpIC1VX0xJQkNQUF9IQVNfTk9fQVNBTjsKZGlmZiAtLWdp dCBhL1Rvb2xzL3Nhbml0aXplci9zYW5pdGl6ZXIueGNjb25maWcgYi9Ub29scy9zYW5pdGl6ZXIv c2FuaXRpemVyLnhjY29uZmlnCmluZGV4IDk4YzA0NTEyMzZhOWFmN2Q1M2EyZWUyNDEzZGRjMzRl ZjhhMWFkYTAuLjUwYmQ5MTAxZDAwYjgwMGZjNDJjZmJmMTRkOWI0MjBmZjQxNTFlZWEgMTAwNjQ0 Ci0tLSBhL1Rvb2xzL3Nhbml0aXplci9zYW5pdGl6ZXIueGNjb25maWcKKysrIGIvVG9vbHMvc2Fu aXRpemVyL3Nhbml0aXplci54Y2NvbmZpZwpAQCAtNCw5ICs0LDEwIEBAIEdDQ19PUFRJTUlaQVRJ T05fTEVWRUxfUHJvZHVjdGlvbiA9IDE7CiBHQ0NfT1BUSU1JWkFUSU9OX0xFVkVMX1JlbGVhc2Ug PSAxOwogCiBXS19TQU5JVElaRVJfT1RIRVJfQ0ZMQUdTX1lFUyA9IC1mbm8tb21pdC1mcmFtZS1w b2ludGVyIC1nOworV0tfU0FOSVRJWkVSX09USEVSX0NQTFVTUExVU0ZMQUdTX1lFUyA9IDsKIAog T1RIRVJfQ0ZMQUdTID0gJChpbmhlcml0ZWQpICQoV0tfU0FOSVRJWkVSX09USEVSX0NGTEFHU18k KFdLX0VOQUJMRV9TQU5JVElaRVIpKTsKLU9USEVSX0NQTFVTUExVU0ZMQUdTID0gJChpbmhlcml0 ZWQpICQoV0tfU0FOSVRJWkVSX09USEVSX0NGTEFHU18kKFdLX0VOQUJMRV9TQU5JVElaRVIpKTsK K09USEVSX0NQTFVTUExVU0ZMQUdTID0gJChpbmhlcml0ZWQpICQoV0tfU0FOSVRJWkVSX09USEVS X0NQTFVTUExVU0ZMQUdTXyQoV0tfRU5BQkxFX1NBTklUSVpFUikpOwogCiAvLyBXb3JrYXJvdW5k IGZvciByZGFyOi8vcHJvYmxlbS80OTQ5ODA5MgogT1RIRVJfTERGTEFHUyA9ICQoaW5oZXJpdGVk KSAkKFdLX0xERkxBR1NfU0FOSVRJWkVSXyQoV0tfRU5BQkxFX1NBTklUSVpFUikpOwo=