192491 2018-12-07 02:05:18 -0800 [WTF] Debug build fails due conflicting abort() method 2018-12-08 14:11:24 -0800 1 1 1 Unclassified WebKit Web Template Framework Other Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 aperez aperez ap benjamin cdumez cmarcelo commit-queue dbates ews-watchlist mcatanzaro thorton webkit-bug-importer oldest_to_newest 1485950 0 aperez 2018-12-07 02:05:18 -0800 When building passing -DCMAKE_BUILD_TYPE=Debug: - The XMLHttpRequest class defines an ::abort() method (in Source/WebCore/xml/XMLHttpRequest.h) - The CRASH() and CRASH_UNDER_CONSTEXPR_CONTEXT macros (in Source/WTF/wtf/Assertions.h) call the abort() function, but inside the code for XMLHttpRequest that would implicitly call the method. Ugh. Probably the best solution is to make the macros in Assertions.h use the namespace (so they expand to std::abort), to avoid the method being picked when expandin the macros. Compilation error follows. ---- In file included from DerivedSources/ForwardingHeaders/wtf/StdLibExtras.h:32, from DerivedSources/ForwardingHeaders/wtf/FastMalloc.h:25, from ../Source/WebCore/config.h:51, from CSSValueKeywords.gperf:4, from DerivedSources/WebCore/unified-sources/UnifiedSource1.cpp:1: ../Source/WebCore/xml/XMLHttpRequest.h: In static member function ‘static void* WebCore::XMLHttpRequest::operator new(size_t, NotNullTag, void*)’: DerivedSources/ForwardingHeaders/wtf/Assertions.h:244:23: error: cannot call member function ‘void WebCore::XMLHttpRequest::abort()’ without object #define CRASH() abort() ^ DerivedSources/ForwardingHeaders/wtf/Assertions.h:587:30: note: in expansion of macro ‘CRASH’ #define CRASH_WITH_INFO(...) CRASH() ^~~~~ DerivedSources/ForwardingHeaders/wtf/Assertions.h:323:9: note: in expansion of macro ‘CRASH_WITH_INFO’ CRASH_WITH_INFO(__VA_ARGS__); \ ^~~~~~~~~~~~~~~ DerivedSources/ForwardingHeaders/wtf/FastMalloc.h:288:9: note: in expansion of macro ‘ASSERT’ ASSERT(location); \ ^~~~~~ DerivedSources/ForwardingHeaders/wtf/FastMalloc.h:294:5: note: in expansion of macro ‘WTF_MAKE_FAST_ALLOCATED_IMPL’ WTF_MAKE_FAST_ALLOCATED_IMPL \DerivedSources/ForwardingHeaders/wtf/Assertions.h:244:23: error: cannot call member function ‘void WebCore::XMLHttpRequest::abort()’ without object ^~~~~~~~~~~~~~~~~~~~~~~~~~~~ ../Source/WebCore/xml/XMLHttpRequest.h:53:5: note: in expansion of macro ‘WTF_MAKE_FAST_ALLOCATED’ WTF_MAKE_FAST_ALLOCATED; ^~~~~~~~~~~~~~~~~~~~~~~ 1485953 1 356789 aperez 2018-12-07 02:14:41 -0800 Created attachment 356789 Patch 1486087 2 356789 mcatanzaro 2018-12-07 11:20:49 -0800 Comment on attachment 356789 Patch I wonder why none of the bots noticed this. Or why it doesn't affect me locally. I've done many debug builds. This was good thinking, but unfortunately this is one of a very few headers that get included in C (and Objective C) so we can't use namespaces here. E.g. from EWS: /home/buildbot/ews/WebKit/Source/JavaScriptCore/API/tests/testapi.c: In function ‘createStringWithContentsOfFile’: DerivedSources/ForwardingHeaders/wtf/Assertions.h:244:21: error: expected expression before ‘:’ token #define CRASH() std::abort() ^ I was going to suggest renaming XMLHttpRequest::abort instead, but there are more abort()s in the codebase... NetworkResourceLoader.cpp, testbmalloc.cpp.... We could add a new conditional to check #if __cplusplus? But that would be messier. 1486321 3 aperez 2018-12-08 10:55:28 -0800 (In reply to Michael Catanzaro from comment #2) > Comment on attachment 356789 [details] > Patch > > I wonder why none of the bots noticed this. Or why it doesn't affect me > locally. I've done many debug builds. > > This was good thinking, but unfortunately this is one of a very few headers > that get included in C (and Objective C) so we can't use namespaces here. > E.g. from EWS: > > /home/buildbot/ews/WebKit/Source/JavaScriptCore/API/tests/testapi.c: In > function ‘createStringWithContentsOfFile’: > DerivedSources/ForwardingHeaders/wtf/Assertions.h:244:21: error: expected > expression before ‘:’ token > #define CRASH() std::abort() > ^ > > I was going to suggest renaming XMLHttpRequest::abort instead, but there are > more abort()s in the codebase... NetworkResourceLoader.cpp, > testbmalloc.cpp.... > > We could add a new conditional to check #if __cplusplus? But that would be > messier. I think checking for __cplusplus is reasonable, I'll update the patch. 1486322 4 356878 aperez 2018-12-08 11:09:02 -0800 Created attachment 356878 Patch 1486350 5 356878 mcatanzaro 2018-12-08 13:44:43 -0800 Comment on attachment 356878 Patch :/ 1486353 6 356878 commit-queue 2018-12-08 14:10:45 -0800 Comment on attachment 356878 Patch Clearing flags on attachment: 356878 Committed r239012: <https://trac.webkit.org/changeset/239012> 1486354 7 commit-queue 2018-12-08 14:10:47 -0800 All reviewed patches have been landed. Closing bug. 1486355 8 webkit-bug-importer 2018-12-08 14:11:24 -0800 <rdar://problem/46576939> 356789 2018-12-07 02:14:41 -0800 2018-12-08 11:08:56 -0800 Patch bug-192491-20181207121440.patch text/plain 1386 aperez U3VidmVyc2lvbiBSZXZpc2lvbjogMjM4OTUwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL0NoYW5n ZUxvZyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCmluZGV4IGQ0OTNhNGU5YmViMmQxYTA4YWExZjM2 ODVjNzQwMzAyYmExNmI5NjUuLjhlZTFhZDY3ZDI0ODI0MzM2MGFlYTVjNGZkNzRmYjZiMWJiNjUy MzMgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XVEYvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMTQgQEAKKzIwMTgtMTItMDcgIEFkcmlhbiBQZXJleiBkZSBDYXN0 cm8gIDxhcGVyZXpAaWdhbGlhLmNvbT4KKworICAgICAgICBbV1RGXSBEZWJ1ZyBidWlsZCBmYWls cyBkdWUgY29uZmxpY3RpbmcgYWJvcnQoKSBtZXRob2QKKyAgICAgICAgaHR0cHM6Ly9idWdzLndl YmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE5MjQ5MQorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgICogd3RmL0Fzc2VydGlvbnMuaDogVXNlIG5hbWVzcGFj ZWQgc3RkOjphYm9ydCgpIGluc3RlZCBvZiBwbGFpbgorICAgICAgICBhYm9ydCgpIHRvIGF2b2lk IGNsYXNoZXMgaW5zaWRlIGNsYXNzZXMgd2hpY2ggaGF2ZSBhbiA6OmFib3J0KCkKKyAgICAgICAg bWV0aG9kLgorCiAyMDE4LTEyLTA2ICBBbGV4ZXkgUHJvc2t1cnlha292ICA8YXBAYXBwbGUuY29t PgogCiAgICAgICAgIE1vdmUgVVNFX05FV19USEVNRSBvdXQgb2YgV2ViQ29yZSdzIGNvbmZpZy5o CmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL3d0Zi9Bc3NlcnRpb25zLmggYi9Tb3VyY2UvV1RGL3d0 Zi9Bc3NlcnRpb25zLmgKaW5kZXggZWJhYWEwNTZkNTYzYzcwNzhiMWQxYmUxOTBmYzkxMzllNjVj ZTRlMC4uNGZkYzY3NjUxMzAzMDI4MmJjNjlhZWEyNzdmMTYwOTJmMTU4ZDFhYyAxMDA2NDQKLS0t IGEvU291cmNlL1dURi93dGYvQXNzZXJ0aW9ucy5oCisrKyBiL1NvdXJjZS9XVEYvd3RmL0Fzc2Vy dGlvbnMuaApAQCAtMjQxLDggKzI0MSw4IEBAIFdURl9FWFBPUlRfUFJJVkFURSBib29sIFdURklz RGVidWdnZXJBdHRhY2hlZCh2b2lkKTsKICAgICBfX2J1aWx0aW5fdW5yZWFjaGFibGUoKTsgXAog fSB3aGlsZSAoMCkKICNlbGlmICFFTkFCTEUoREVWRUxPUEVSX01PREUpICYmICFPUyhEQVJXSU4p Ci0jZGVmaW5lIENSQVNIKCkgYWJvcnQoKQotI2RlZmluZSBDUkFTSF9VTkRFUl9DT05TVEVYUFJf Q09OVEVYVCgpIGFib3J0KCkKKyNkZWZpbmUgQ1JBU0goKSBzdGQ6OmFib3J0KCkKKyNkZWZpbmUg Q1JBU0hfVU5ERVJfQ09OU1RFWFBSX0NPTlRFWFQoKSBzdGQ6OmFib3J0KCkKICNlbHNlCiAjZGVm aW5lIENSQVNIKCkgV1RGQ3Jhc2goKQogI2RlZmluZSBDUkFTSF9VTkRFUl9DT05TVEVYUFJfQ09O VEVYVCgpIFdURkNyYXNoKCkK 356878 2018-12-08 11:09:02 -0800 2018-12-08 14:10:45 -0800 Patch bug-192491-20181208210900.patch text/plain 1537 aperez U3VidmVyc2lvbiBSZXZpc2lvbjogMjM5MDA5CmRpZmYgLS1naXQgYS9Tb3VyY2UvV1RGL0NoYW5n ZUxvZyBiL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCmluZGV4IGEyMWRjNTZlNmJmNWJhMjMyZjQ2Njg2 ZmE0YThhYzJiOWNlYmViZjQuLmRiZDkyMjdiZDFhYmYzMDE1ZjQ4MmEzZDA3NjMzNjEwZjQ3ZmMx MDUgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XVEYvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMTQgQEAKKzIwMTgtMTItMDggIEFkcmlhbiBQZXJleiBkZSBDYXN0 cm8gIDxhcGVyZXpAaWdhbGlhLmNvbT4KKworICAgICAgICBbV1RGXSBEZWJ1ZyBidWlsZCBmYWls cyBkdWUgY29uZmxpY3RpbmcgYWJvcnQoKSBtZXRob2QKKyAgICAgICAgaHR0cHM6Ly9idWdzLndl YmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTE5MjQ5MQorCisgICAgICAgIFJldmlld2VkIGJ5IE5P Qk9EWSAoT09QUyEpLgorCisgICAgICAgICogd3RmL0Fzc2VydGlvbnMuaDogVXNlIG5hbWVzcGFj ZWQgc3RkOjphYm9ydCgpIGluc3RlZCBvZiBwbGFpbiBhYm9ydCgpIHRvIGF2b2lkIGNsYXNoZXMK KyAgICAgICAgaW5zaWRlIGNsYXNzZXMgd2hpY2ggaGF2ZSBhbiA6OmFib3J0KCkgbWV0aG9kLCBi dXQgb25seSB3aGVuIF9fY3BsdXNwbHVzIGlzIGRlZmluZWQgdG8KKyAgICAgICAgYWxsb3cgaW5j bHVzaW9uIG9mIHRoZSBoZWFkZXIgaW4gcGxhaW4gQyBzb3VyY2VzLgorCiAyMDE4LTEyLTA3ICBB bmR5IEVzdGVzICA8YWVzdGVzQGFwcGxlLmNvbT4KIAogICAgICAgICBbQ29jb2FdIEFkZCBvcHRp b25hbCB2YXJpYW50cyBvZiBTT0ZUX0xJTktfQ0xBU1NfRk9SX1NPVVJDRQpkaWZmIC0tZ2l0IGEv U291cmNlL1dURi93dGYvQXNzZXJ0aW9ucy5oIGIvU291cmNlL1dURi93dGYvQXNzZXJ0aW9ucy5o CmluZGV4IGViYWFhMDU2ZDU2M2M3MDc4YjFkMWJlMTkwZmM5MTM5ZTY1Y2U0ZTAuLmYxOTM5OTZm MDAyZjQxZmQ2YWEzOGM1YmEyZTE0N2NmNTBjOGRjZTggMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XVEYv d3RmL0Fzc2VydGlvbnMuaAorKysgYi9Tb3VyY2UvV1RGL3d0Zi9Bc3NlcnRpb25zLmgKQEAgLTI0 MSw4ICsyNDEsMTMgQEAgV1RGX0VYUE9SVF9QUklWQVRFIGJvb2wgV1RGSXNEZWJ1Z2dlckF0dGFj aGVkKHZvaWQpOwogICAgIF9fYnVpbHRpbl91bnJlYWNoYWJsZSgpOyBcCiB9IHdoaWxlICgwKQog I2VsaWYgIUVOQUJMRShERVZFTE9QRVJfTU9ERSkgJiYgIU9TKERBUldJTikKKyNpZmRlZiBfX2Nw bHVzcGx1cworI2RlZmluZSBDUkFTSCgpIHN0ZDo6YWJvcnQoKQorI2RlZmluZSBDUkFTSF9VTkRF Ul9DT05TVEVYUFJfQ09OVEVYVCgpIHN0ZDo6YWJvcnQoKQorI2Vsc2UKICNkZWZpbmUgQ1JBU0go KSBhYm9ydCgpCiAjZGVmaW5lIENSQVNIX1VOREVSX0NPTlNURVhQUl9DT05URVhUKCkgYWJvcnQo KQorI2VuZGlmIC8vIF9fY3BsdXNwbHVzCiAjZWxzZQogI2RlZmluZSBDUkFTSCgpIFdURkNyYXNo KCkKICNkZWZpbmUgQ1JBU0hfVU5ERVJfQ09OU1RFWFBSX0NPTlRFWFQoKSBXVEZDcmFzaCgpCg==