69403 2011-10-05 00:26:28 -0700 [WK2] [GTK] WebKitTestRunner crashes with heap corruption 2011-10-24 00:53:09 -0700 1 1 1 Unclassified WebKit WebKitGTK 528+ (Nightly build) Unspecified Linux RESOLVED FIXED P2 Normal --- 69523 1 alex webkit-unassigned andersca mrobinson pnormand sam webkit.review.bot oldest_to_newest 478195 0 alex 2011-10-05 00:26:28 -0700 WebKitTestRunner is crashing randomly due to heap memory corruption. The stacks of the crash does not give much information about the issue. The valgrind log shows we have problems with Mutex size in some situations: ==962== Invalid write of size 8 ==962== at 0x37FC808963: pthread_mutex_init (pthread_mutex_init.c:83) ==962== by 0x8EC5C53: WTF::Mutex::Mutex() (ThreadingPthreads.cpp:228) ==962== by 0xD8BC1B0: WTF::HashTable<unsigned int, std::pair<unsigned int, WebKit::WKRetainPtr<OpaqueWKBundleScriptWorld const*> >, WTF::PairFirstExtractor<std::pair<unsigned int, WebKit::WKRetainPtr<OpaqueWKBundleScriptWorld const*> > >, WTF::IntHash<unsigned int>, $ ==962== by 0xD8BC061: WTF::HashMap<unsigned int, WebKit::WKRetainPtr<OpaqueWKBundleScriptWorld const*>, WTF::IntHash<unsigned int>, WTF::HashTraits<unsigned int>, WTF::HashTraits<WebKit::WKRetainPtr<OpaqueWKBundleScriptWorld const*> > >::HashMap() (HashMap.h:32) ==962== by 0xD8BBB97: WTR::worldMap() (LayoutTestController.cpp:461) ==962== by 0xD8BBBD5: WTR::LayoutTestController::worldIDForWorld(OpaqueWKBundleScriptWorld const*) (LayoutTestController.cpp:467) ==962== by 0xD8B4E6F: WTR::InjectedBundlePage::didClearWindowForFrame(OpaqueWKBundleFrame const*, OpaqueWKBundleScriptWorld const*) (InjectedBundlePage.cpp:639) ==962== by 0xD8B3F16: WTR::InjectedBundlePage::didClearWindowForFrame(OpaqueWKBundlePage const*, OpaqueWKBundleFrame const*, OpaqueWKBundleScriptWorld const*, void const*) (InjectedBundlePage.cpp:358) ... ==962== Address 0xdc76b48 is 0 bytes after a block of size 40 alloc'd ==962== at 0x4A05E46: malloc (vg_replace_malloc.c:195) ==962== by 0x8EA9E22: WTF::fastMalloc(unsigned long) (FastMalloc.cpp:264) ==962== by 0xD8BC135: WTF::HashMap<unsigned int, WebKit::WKRetainPtr<OpaqueWKBundleScriptWorld const*>, WTF::IntHash<unsigned int>, WTF::HashTraits<unsigned int>, WTF::HashTraits<WebKit::WKRetainPtr<OpaqueWKBundleScriptWorld const*> > >::operator new(unsigned long) ($ ==962== by 0xD8BBB89: WTR::worldMap() (LayoutTestController.cpp:461) ==962== by 0xD8BBBD5: WTR::LayoutTestController::worldIDForWorld(OpaqueWKBundleScriptWorld const*) (LayoutTestController.cpp:467) ... 478196 1 109753 alex 2011-10-05 00:30:05 -0700 Created attachment 109753 Proposed patch 478437 2 109753 mrobinson 2011-10-05 09:44:12 -0700 Comment on attachment 109753 Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=109753&action=review Would it be sufficient to simply add #if defined (BUILDING_GTK__) #include "autotoolsconfig.h" #endif /* defined (BUILDING_GTK__) */ to the prefix header? I'm wondering if there is some way we can #error out if the includes are in the wrong order. What do you think? > Tools/WebKitTestRunner/PixelDumpSupport.h:32 > +#include <stdlib.h> Hrm...why is this necessary? 478463 3 alex 2011-10-05 10:07:12 -0700 (In reply to comment #2) > (From update of attachment 109753 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=109753&action=review > > Would it be sufficient to simply add > #if defined (BUILDING_GTK__) > #include "autotoolsconfig.h" > #endif /* defined (BUILDING_GTK__) */ > > to the prefix header? > Probably it would work, but I was trying to get the same structure we have in all the other config.h files in the repository, with the autotoolsconfig.h before the Platform.h. And I thought it is a better option to have the WebKit2.h included in the files that requires it instead of adding it to all the compilations. Actually I was wondering if we could even remove the Prefix.h and just have the config.h. > I'm wondering if there is some way we can #error out if the includes are in the wrong order. What do you think? > Never tried it, I can check it. > > Tools/WebKitTestRunner/PixelDumpSupport.h:32 > > +#include <stdlib.h> > > Hrm...why is this necessary? Apparently it is needed for the size_t now that WebKit2.h is not included in every compilation. 488321 4 111946 alex 2011-10-21 05:05:23 -0700 Created attachment 111946 Proposed patch Used the solution of adding the include to the prefix file. I've checked the option to add an #error in the Platform.h but we are not respecting the rule in JSC. Currently it is not a problem because those files do not use the DEFINES in Platform.h, so the code would be a little bit overkill in the my opinion to be added in this patch. I think we can check it in the future if we find this situation is common. 488439 5 111946 mrobinson 2011-10-21 09:37:39 -0700 Comment on attachment 111946 Proposed patch Nice! 489209 6 alex 2011-10-24 00:53:09 -0700 Thanks! Landed: http://trac.webkit.org/changeset/98221 109753 2011-10-05 00:30:05 -0700 2011-10-21 05:05:23 -0700 Proposed patch random_crashes.patch text/plain 3518 alex ZGlmZiAtLWdpdCBhL1Rvb2xzL0NoYW5nZUxvZyBiL1Rvb2xzL0NoYW5nZUxvZwppbmRleCA1MWY5 ZmVjLi5iOTQxZTNhIDEwMDY0NAotLS0gYS9Ub29scy9DaGFuZ2VMb2cKKysrIGIvVG9vbHMvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMjAgQEAKKzIwMTEtMTAtMDUgIEFsZWphbmRybyBHLiBDYXN0cm8g IDxhbGV4QGlnYWxpYS5jb20+CisKKyAgICAgICAgW1dLMl0gW0dUS10gV2ViS2l0VGVzdFJ1bm5l ciBjcmFzaGVzIHdpdGggaGVhcCBjb3JydXB0aW9uCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD02OTQwMworCisgICAgICAgIFdlIGRpZCBub3QgYWRkZWQg YXV0b3Rvb2xzY29uZmlnLmggdG8gdGhlIGNvbmZpZy5oIGZpbGUgYW5kIHdlCisgICAgICAgIHdl cmUgYWRkaW5nIFBsYXRmb3JtLmggd2l0aG91dCB0aGUgY29uZmlnLmggaW4gdGhlCisgICAgICAg IFdlYktpdFRlc3RSdW5uZXJQcmVmaXguaC4gUmVtb3ZlZCB0aGUgaW5jbHVkZXMgaW4gdGhlIFBy ZWZpeCBmaWxlCisgICAgICAgIGFuZCBhZGRlZCB0aGVtIHRvIHRoZSByZXF1aXJlZCBsb2NhdGlv bnMuCisKKyAgICAgICAgKiBXZWJLaXRUZXN0UnVubmVyL1BpeGVsRHVtcFN1cHBvcnQuY3BwOgor ICAgICAgICAqIFdlYktpdFRlc3RSdW5uZXIvUGl4ZWxEdW1wU3VwcG9ydC5oOgorICAgICAgICAq IFdlYktpdFRlc3RSdW5uZXIvUGxhdGZvcm1XZWJWaWV3Lmg6CisgICAgICAgICogV2ViS2l0VGVz dFJ1bm5lci9UZXN0Q29udHJvbGxlci5oOgorICAgICAgICAqIFdlYktpdFRlc3RSdW5uZXIvV2Vi S2l0VGVzdFJ1bm5lclByZWZpeC5oOgorICAgICAgICAqIFdlYktpdFRlc3RSdW5uZXIvY29uZmln Lmg6CisKIDIwMTEtMTAtMDQgIEtlbnQgVGFtdXJhICA8dGtlbnRAY2hyb21pdW0ub3JnPgogCiAg ICAgICAgIEludHJvZHVjZSBmZWF0dXJlIGZsYWdzIGZvciBpbmNvbXBsZXRlIGlucHV0IHR5cGVz CmRpZmYgLS1naXQgYS9Ub29scy9XZWJLaXRUZXN0UnVubmVyL1BpeGVsRHVtcFN1cHBvcnQuY3Bw IGIvVG9vbHMvV2ViS2l0VGVzdFJ1bm5lci9QaXhlbER1bXBTdXBwb3J0LmNwcAppbmRleCA3Njk3 NDczLi45YmE2ZjFkIDEwMDY0NAotLS0gYS9Ub29scy9XZWJLaXRUZXN0UnVubmVyL1BpeGVsRHVt cFN1cHBvcnQuY3BwCisrKyBiL1Rvb2xzL1dlYktpdFRlc3RSdW5uZXIvUGl4ZWxEdW1wU3VwcG9y dC5jcHAKQEAgLTMwLDYgKzMwLDcgQEAKICNpbmNsdWRlICJQaXhlbER1bXBTdXBwb3J0LmgiCiAK ICNpbmNsdWRlICJDeWNsaWNSZWR1bmRhbmN5Q2hlY2suaCIKKyNpbmNsdWRlIDxXZWJLaXQyL1dl YktpdDIuaD4KICNpbmNsdWRlIDxjc3RkaW8+CiAKIHN0YXRpYyB2b2lkIGFwcGVuZEludFRvVmVj dG9yKHVuc2lnbmVkIG51bWJlciwgVmVjdG9yPHVuc2lnbmVkIGNoYXI+JiB2ZWN0b3IpCmRpZmYg LS1naXQgYS9Ub29scy9XZWJLaXRUZXN0UnVubmVyL1BpeGVsRHVtcFN1cHBvcnQuaCBiL1Rvb2xz L1dlYktpdFRlc3RSdW5uZXIvUGl4ZWxEdW1wU3VwcG9ydC5oCmluZGV4IDQxYzZlMzguLjAyNGI4 YjkgMTAwNjQ0Ci0tLSBhL1Rvb2xzL1dlYktpdFRlc3RSdW5uZXIvUGl4ZWxEdW1wU3VwcG9ydC5o CisrKyBiL1Rvb2xzL1dlYktpdFRlc3RSdW5uZXIvUGl4ZWxEdW1wU3VwcG9ydC5oCkBAIC0yOSw2 ICsyOSw4IEBACiAjaWZuZGVmIFBpeGVsRHVtcFN1cHBvcnRfaAogI2RlZmluZSBQaXhlbER1bXBT dXBwb3J0X2gKIAorI2luY2x1ZGUgPHN0ZGxpYi5oPgorCiB2b2lkIHByaW50UE5HKGNvbnN0IHVu c2lnbmVkIGNoYXIqIGRhdGEsIGNvbnN0IHNpemVfdCBkYXRhTGVuZ3RoLCBjb25zdCBjaGFyKiBj aGVja3N1bSk7CiAKICNlbmRpZiAvLyBQaXhlbER1bXBTdXBwb3J0X2gKZGlmZiAtLWdpdCBhL1Rv b2xzL1dlYktpdFRlc3RSdW5uZXIvUGxhdGZvcm1XZWJWaWV3LmggYi9Ub29scy9XZWJLaXRUZXN0 UnVubmVyL1BsYXRmb3JtV2ViVmlldy5oCmluZGV4IDg4ODY2MDUuLjc5YmZjMGQgMTAwNjQ0Ci0t LSBhL1Rvb2xzL1dlYktpdFRlc3RSdW5uZXIvUGxhdGZvcm1XZWJWaWV3LmgKKysrIGIvVG9vbHMv V2ViS2l0VGVzdFJ1bm5lci9QbGF0Zm9ybVdlYlZpZXcuaApAQCAtMjYsNiArMjYsOCBAQAogI2lm bmRlZiBQbGF0Zm9ybVdlYlZpZXdfaAogI2RlZmluZSBQbGF0Zm9ybVdlYlZpZXdfaAogCisjaW5j bHVkZSA8V2ViS2l0Mi9XZWJLaXQyLmg+CisKICNpZiBkZWZpbmVkKEJVSUxESU5HX1FUX18pCiBj bGFzcyBRRGVza3RvcFdlYlZpZXc7CiB0eXBlZGVmIFFEZXNrdG9wV2ViVmlldyogUGxhdGZvcm1X S1ZpZXc7CmRpZmYgLS1naXQgYS9Ub29scy9XZWJLaXRUZXN0UnVubmVyL1Rlc3RDb250cm9sbGVy LmggYi9Ub29scy9XZWJLaXRUZXN0UnVubmVyL1Rlc3RDb250cm9sbGVyLmgKaW5kZXggNmFiODk4 MS4uYjE5NTk1NSAxMDA2NDQKLS0tIGEvVG9vbHMvV2ViS2l0VGVzdFJ1bm5lci9UZXN0Q29udHJv bGxlci5oCisrKyBiL1Rvb2xzL1dlYktpdFRlc3RSdW5uZXIvVGVzdENvbnRyb2xsZXIuaApAQCAt MjYsNiArMjYsNyBAQAogI2lmbmRlZiBUZXN0Q29udHJvbGxlcl9oCiAjZGVmaW5lIFRlc3RDb250 cm9sbGVyX2gKIAorI2luY2x1ZGUgPFdlYktpdDIvV2ViS2l0Mi5oPgogI2luY2x1ZGUgPFdlYktp dDIvV0tSZXRhaW5QdHIuaD4KICNpbmNsdWRlIDxzdHJpbmc+CiAjaW5jbHVkZSA8dmVjdG9yPgpk aWZmIC0tZ2l0IGEvVG9vbHMvV2ViS2l0VGVzdFJ1bm5lci9XZWJLaXRUZXN0UnVubmVyUHJlZml4 LmggYi9Ub29scy9XZWJLaXRUZXN0UnVubmVyL1dlYktpdFRlc3RSdW5uZXJQcmVmaXguaAppbmRl eCAxZTU0MGNjLi4xYzI3YTllIDEwMDY0NAotLS0gYS9Ub29scy9XZWJLaXRUZXN0UnVubmVyL1dl YktpdFRlc3RSdW5uZXJQcmVmaXguaAorKysgYi9Ub29scy9XZWJLaXRUZXN0UnVubmVyL1dlYktp dFRlc3RSdW5uZXJQcmVmaXguaApAQCAtMzMsNiArMzMsMyBAQAogI2RlZmluZSBtYXggbWF4CiAj ZGVmaW5lIG1pbiBtaW4KICNlbmRpZgotCi0jaW5jbHVkZSA8d3RmL1BsYXRmb3JtLmg+Ci0jaW5j bHVkZSA8V2ViS2l0Mi9XZWJLaXQyLmg+CmRpZmYgLS1naXQgYS9Ub29scy9XZWJLaXRUZXN0UnVu bmVyL2NvbmZpZy5oIGIvVG9vbHMvV2ViS2l0VGVzdFJ1bm5lci9jb25maWcuaAppbmRleCAzODU2 OGFlLi5mZWUxNDczIDEwMDY0NAotLS0gYS9Ub29scy9XZWJLaXRUZXN0UnVubmVyL2NvbmZpZy5o CisrKyBiL1Rvb2xzL1dlYktpdFRlc3RSdW5uZXIvY29uZmlnLmgKQEAgLTI1LDYgKzI1LDEwIEBA CiAjaWZuZGVmIFdlYktpdFRlc3RSdW5uZXJfY29uZmlnX2gKICNkZWZpbmUgV2ViS2l0VGVzdFJ1 bm5lcl9jb25maWdfaAogCisjaWYgZGVmaW5lZCAoQlVJTERJTkdfR1RLX18pCisjaW5jbHVkZSAi YXV0b3Rvb2xzY29uZmlnLmgiCisjZW5kaWYgLyogZGVmaW5lZCAoQlVJTERJTkdfR1RLX18pICov CisKICNpbmNsdWRlIDx3dGYvUGxhdGZvcm0uaD4KIAogLyogU2VlIG5vdGUgaW4gd3RmL1BsYXRm b3JtLmggZm9yIG1vcmUgaW5mbyBvbiBFWFBPUlRfTUFDUk9TLiAqLwo= 111946 2011-10-21 05:05:23 -0700 2011-10-21 09:37:38 -0700 Proposed patch random_crash.patch text/plain 1178 alex ZGlmZiAtLWdpdCBhL1Rvb2xzL0NoYW5nZUxvZyBiL1Rvb2xzL0NoYW5nZUxvZwppbmRleCAwZjk5 YjdkLi42ZjQwYmE2IDEwMDY0NAotLS0gYS9Ub29scy9DaGFuZ2VMb2cKKysrIGIvVG9vbHMvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMTcgQEAKKzIwMTEtMTAtMjEgIEFsZWphbmRybyBHLiBDYXN0cm8g IDxhbGV4QGlnYWxpYS5jb20+CisKKyAgICAgICAgW1dLMl0gW0dUS10gV2ViS2l0VGVzdFJ1bm5l ciBjcmFzaGVzIHdpdGggaGVhcCBjb3JydXB0aW9uCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJr aXQub3JnL3Nob3dfYnVnLmNnaT9pZD02OTQwMworCisgICAgICAgIFdlIGRpZCBub3QgYWRkZWQg YXV0b3Rvb2xzY29uZmlnLmggdG8gdGhlIGNvbmZpZy5oIGZpbGUgYW5kIHdlCisgICAgICAgIHdl cmUgYWRkaW5nIFBsYXRmb3JtLmggd2l0aG91dCB0aGUgY29uZmlnLmggaW4gdGhlCisgICAgICAg IFdlYktpdFRlc3RSdW5uZXJQcmVmaXguaC4gQWRkZWQgdGhlIGF1dG90b29sc2NvbmZpZy5oIGlu Y2x1ZGUgaW4KKyAgICAgICAgdGhlIFByZWZpeCBmaWxlLgorCisgICAgICAgIFJldmlld2VkIGJ5 IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgICogV2ViS2l0VGVzdFJ1bm5lci9XZWJLaXRUZXN0 UnVubmVyUHJlZml4Lmg6CisKIDIwMTEtMTAtMjAgIEVyaWMgU2VpZGVsICA8ZXJpY0B3ZWJraXQu b3JnPgogCiAgICAgICAgIFN3aXRjaCB3ZWJraXQyIGJvdCB0byBOUldUCmRpZmYgLS1naXQgYS9U b29scy9XZWJLaXRUZXN0UnVubmVyL1dlYktpdFRlc3RSdW5uZXJQcmVmaXguaCBiL1Rvb2xzL1dl YktpdFRlc3RSdW5uZXIvV2ViS2l0VGVzdFJ1bm5lclByZWZpeC5oCmluZGV4IDFlNTQwY2MuLmY2 OWZhM2UgMTAwNjQ0Ci0tLSBhL1Rvb2xzL1dlYktpdFRlc3RSdW5uZXIvV2ViS2l0VGVzdFJ1bm5l clByZWZpeC5oCisrKyBiL1Rvb2xzL1dlYktpdFRlc3RSdW5uZXIvV2ViS2l0VGVzdFJ1bm5lclBy ZWZpeC5oCkBAIC0zNCw1ICszNCw5IEBACiAjZGVmaW5lIG1pbiBtaW4KICNlbmRpZgogCisjaWYg ZGVmaW5lZChCVUlMRElOR19HVEtfXykKKyNpbmNsdWRlICJhdXRvdG9vbHNjb25maWcuaCIKKyNl bmRpZiAvKiBkZWZpbmVkIChCVUlMRElOR19HVEtfXykgKi8KKwogI2luY2x1ZGUgPHd0Zi9QbGF0 Zm9ybS5oPgogI2luY2x1ZGUgPFdlYktpdDIvV2ViS2l0Mi5oPgo=