If I introduce a deliberate failure in an API test, like: TEST_F(MetaAllocatorTest, SimpleFullCoalesce32PlusPageLess32ThenPage) { + EXPECT_TRUE(false); testSimpleFullCoalesce(32, pageSize() - 32, pageSize()); } then TestWebKitAPI crashes, rather than just reporting the failure. The crash is: Exception Type: EXC_BAD_ACCESS (SIGSEGV) Exception Codes: KERN_INVALID_ADDRESS at 0x00000000000020d8 VM Regions Near 0x20d8: --> __TEXT 00000001042de000-0000000104359000 [ 492K] r-x/rwx SM=COW /Volumes/VOLUME/* Application Specific Information: objc[91719]: garbage collection is OFF Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.JavaScriptCore 0x0000000104bc3a49 WTF::fastFree(void*) + 137 (TCPageMap.h:234) 1 TestWebKitAPI 0x0000000104320038 testing::Test::Run() + 92 2 TestWebKitAPI 0x00000001043206be testing::internal::TestInfoImpl::Run() + 158 3 TestWebKitAPI 0x0000000104320ae5 testing::TestCase::Run() + 161 4 TestWebKitAPI 0x0000000104323e7a testing::internal::UnitTestImpl::RunAllTests() + 594 5 TestWebKitAPI 0x00000001042dfdde TestWebKitAPI::TestsController::run(int, char**) + 36 (TestsController.cpp:51) 6 TestWebKitAPI 0x00000001042dfcf1 main + 96 (main.mm:35) 7 TestWebKitAPI 0x00000001042df934 start + 52 This looks like malloc/FastMalloc mixing.
Only seems to be an issue in Release builds.
This bug is causing every API test failure to look like a timeout, and therefore be much harder to spot in the output on the bots.
I think this is an interaction between gtest and FastMalloc. One solution is to set USE_SYSTEM_MALLOC=1 for Release builds.
I think that in my tests there are inline destructors that get compiled/inlined when building TestWebKitAPI, with the corresponding constructors being compiled/linked when building JavaScriptCore. Hence setting USE_SYSTEM_MALLOC=1 will only worsen the situation. :-(
<rdar://problem/10607911>
This is trivial to reproduce by changing a StringBuilder test to fail: --- a/Tools/TestWebKitAPI/Tests/WTF/StringBuilder.cpp +++ b/Tools/TestWebKitAPI/Tests/WTF/StringBuilder.cpp @@ -128,7 +128,7 @@ TEST(StringBuilderTest, ToStringPreserveCapacity) StringBuilder builder; builder.append("0123456789"); String string = builder.toStringPreserveCapacity(); - ASSERT_EQ(String("0123456789"), string); + ASSERT_EQ(String("01234567890"), string); ASSERT_EQ(string.impl(), builder.toStringPreserveCapacity().impl()); ASSERT_EQ(string.characters(), builder.characters());
When compiling TestWebKitAPI with "-g -O0" (pardon the hack): --- a/Tools/TestWebKitAPI/Configurations/Base.xcconfig +++ b/Tools/TestWebKitAPI/Configurations/Base.xcconfig @@ -38,1 +38,1 @@ GCC_TREAT_WARNINGS_AS_ERRORS = YES -WARNING_CFLAGS = -Wall -W -Wno-unused-parameter +WARNING_CFLAGS = -Wall -W -Wno-unused-parameter -g -O0 I see this crash when trying to run ./Tools/Scripts/run-api-tests while it's generating a list of tests: Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.JavaScriptCore 0x0000000104c99f69 WTF::fastFree(void*) + 137 (TCPageMap.h:234) 1 TestWebKitAPI 0x0000000104290705 testing::internal::String::~String() + 21 (gtest-string.h:218) 2 libsystem_c.dylib 0x00007fff8fe387c8 __cxa_finalize + 274 3 libsystem_c.dylib 0x00007fff8fe38652 exit + 18 4 TestWebKitAPI 0x000000010428e77b start + 59 This indicates (to me) that gtest is trying to use FastMalloc for its destructors. If gtest is using malloc to allocate this memory, it's not surprising that FastMalloc is crashing when it tries to release memory it's not managing. Since JavaScriptCore, et al, are compiled with FastMalloc on release builds, I don't think that it would work to enable USE_SYSTEM_MALLOC to compile TestWebKitAPI (as Filip notes in Comment #4). We may need to compile gtest using FastMalloc to make TestWebKitAPI not crash.
Hmm... I know that Mitya (dslomov) did these changes to address this issue: http://trac.webkit.org/changeset/93426 http://trac.webkit.org/changeset/92387 But I guess there are still issues.
(In reply to comment #8) > Hmm... I know that Mitya (dslomov) did these changes to address this issue: > http://trac.webkit.org/changeset/93426 Bug 66521 > http://trac.webkit.org/changeset/92387 Bug 61812 > But I guess there are still issues. r93426 was rolled out in r93451. See Bug 66607.
(In reply to comment #9) > (In reply to comment #8) > > Hmm... I know that Mitya (dslomov) did these changes to address this issue: > > http://trac.webkit.org/changeset/93426 > > Bug 66521 > > > http://trac.webkit.org/changeset/92387 > > Bug 61812 > > > But I guess there are still issues. > > r93426 was rolled out in r93451. See Bug 66607. I guess I should work on reintroducing the changes I did related to bug 66521
(In reply to comment #10) > I guess I should work on reintroducing the changes I did related to bug 66521 I have a potential fix for the Mac. Will post a patch shortly.
(In reply to comment #11) > (In reply to comment #10) > > I guess I should work on reintroducing the changes I did related to bug 66521 > > I have a potential fix for the Mac. Will post a patch shortly. Well, I thought I had a fix. What I thought I was seeing in release builds is that FastMalloc.h was redefining the 'new' and 'delete' operators to use fastMalloc() and fastFree() when ENABLE(GLOBAL_FASTMALLOC_NEW) was true. This caused any use of 'new' and 'delete' in gtest headers to start using fastMalloc() and fastFree(), which contracted the use of malloc() and free() in gtest source code, so you ended up with crashes like the one in this bug. My initial solution was to do this in TestWebKitAPI/config.h (thereby turning off the redefining of 'new' and 'delete'): +#define ENABLE_GLOBAL_FASTMALLOC_NEW 0 #include <wtf/Platform.h> This change fixed crashes in Release builds when a WTF::String() comparison fails (and when compiling with "-g -O0"), but I'm still seeing crashes on Debug builds. Investigating.
(In reply to comment #12) > This change fixed crashes in Release builds when a WTF::String() comparison fails (and when compiling with "-g -O0"), but I'm still seeing crashes on Debug builds. Investigating. LOL! Looks like threading isn't being initialized properly in TestWebKitAPI for a new test I wrote (an assertion is firing): Thread 0 Crashed:: Dispatch queue: com.apple.main-thread 0 com.apple.JavaScriptCore 0x000000010792e203 WTF::isMainThread() + 195 (MainThreadMac.mm:144) 1 com.apple.WebCore 0x00000001098a3efd _ZN7WebCoreL22buildBaseTextCodecMapsEv + 13 (TextEncodingRegistry.cpp:213) 2 com.apple.WebCore 0x00000001098a3e2a WebCore::atomicCanonicalTextEncodingName(char const*) + 74 (TextEncodingRegistry.cpp:338) 3 com.apple.WebCore 0x00000001098a2c21 WebCore::TextEncoding::TextEncoding(char const*) + 33 (TextEncoding.cpp:55) 4 com.apple.WebCore 0x00000001098a2bed WebCore::TextEncoding::TextEncoding(char const*) + 29 (TextEncoding.cpp:58) 5 com.apple.WebCore 0x00000001098a3598 WebCore::UTF8Encoding() + 40 (TextEncoding.cpp:250) 6 com.apple.WebCore 0x000000010928e7b4 WebCore::decodeURLEscapeSequences(WTF::String const&) + 36 (KURL.cpp:919) 7 com.apple.WebCore 0x000000010928e77a WebCore::KURL::host() const + 90 (KURL.cpp:586) 8 TestWebKitAPI 0x0000000106073fbf TestWebKitAPI::WebCore_KURLConstCharConstructor_Test::TestBody() + 2015 (KURL.cpp:54) 9 TestWebKitAPI 0x000000010607e7ca testing::Test::Run() + 154 10 TestWebKitAPI 0x000000010607f0cd testing::internal::TestInfoImpl::Run() + 189 11 TestWebKitAPI 0x000000010607f8fd testing::TestCase::Run() + 205 12 TestWebKitAPI 0x00000001060849e9 testing::internal::UnitTestImpl::RunAllTests() + 793 13 TestWebKitAPI 0x00000001060846c9 testing::UnitTest::Run() + 25 14 TestWebKitAPI 0x0000000105fe9870 TestWebKitAPI::TestsController::run(int, char**) + 48 (TestsController.cpp:51) 15 TestWebKitAPI 0x0000000105fe96e9 main + 121 (main.mm:35) 16 TestWebKitAPI 0x0000000105fe8ca4 start + 52 The assertion is here: bool isMainThread() { if (mainThreadEstablishedAsPthreadMain) { ASSERT(!mainThreadPthread); return pthread_main_np(); } ASSERT(mainThreadPthread); // Line 144 return pthread_equal(pthread_self(), mainThreadPthread); } That's a separate bug. I'll post the patch for this bug.
(In reply to comment #12) > (In reply to comment #11) > > (In reply to comment #10) > > > I guess I should work on reintroducing the changes I did related to bug 66521 > > > > I have a potential fix for the Mac. Will post a patch shortly. > > Well, I thought I had a fix. > > What I thought I was seeing in release builds is that FastMalloc.h was redefining the 'new' and 'delete' operators to use fastMalloc() and fastFree() when ENABLE(GLOBAL_FASTMALLOC_NEW) was true. > > This caused any use of 'new' and 'delete' in gtest headers to start using fastMalloc() and fastFree(), which contracted the use of malloc() and free() in gtest source code, so you ended up with crashes like the one in this bug. > > My initial solution was to do this in TestWebKitAPI/config.h (thereby turning off the redefining of 'new' and 'delete'): > > +#define ENABLE_GLOBAL_FASTMALLOC_NEW 0 > #include <wtf/Platform.h> > > This change fixed crashes in Release builds when a WTF::String() comparison fails (and when compiling with "-g -O0"), but I'm still seeing crashes on Debug builds. Investigating. Of course, the problem with this approach is that 'new' and 'delete' operators in JavaScriptCore headers now will NOT be redefined as fastMalloc() and fastFree(), so there will be mismatches when creating/deleting JSC objects.
(In reply to comment #14) > Of course, the problem with this approach is that 'new' and 'delete' operators in JavaScriptCore headers now will NOT be redefined as fastMalloc() and fastFree(), so there will be mismatches when creating/deleting JSC objects. I don't think TestWebKitAPI ever uses new to create JSC objects.
(In reply to comment #14) > Of course, the problem with this approach is that 'new' and 'delete' operators in JavaScriptCore headers now will NOT be redefined as fastMalloc() and fastFree(), so there will be mismatches when creating/deleting JSC objects. I beginning to see Dmitry's point in Bug 66521 about making gtest use FastMalloc.h as the only sane solution here.
(In reply to comment #15) > (In reply to comment #14) > > Of course, the problem with this approach is that 'new' and 'delete' operators in JavaScriptCore headers now will NOT be redefined as fastMalloc() and fastFree(), so there will be mismatches when creating/deleting JSC objects. > > I don't think TestWebKitAPI ever uses new to create JSC objects. Well, more generally speaking, any JavaScriptCore, WebCore or WebKit header that uses operator 'new' or operator 'delete' is at risk of allocating memory using the wrong allocator if we disable these redefinitions in config.h. (This assumes we want to be able to unit-test any class.)
(In reply to comment #16) > (In reply to comment #14) > > Of course, the problem with this approach is that 'new' and 'delete' operators in JavaScriptCore headers now will NOT be redefined as fastMalloc() and fastFree(), so there will be mismatches when creating/deleting JSC objects. > > I beginning to see Dmitry's point in Bug 66521 about making gtest use FastMalloc.h as the only sane solution here. Good :) Is that what are you shooting for now? Do you need help? I do not think USE_SYSTEM_MALLOC is going to cut it. Basically, since these are webkit/webcore unit tests, we do want to be able to test in the exact regimes ports compile. This includes running unit tests with global fast malloc/free. The testing framework should not require any specific set of ENABLE_/DISABLE_ defines (individual tests might though).
(In reply to comment #18) > (In reply to comment #16) > > I['m] beginning to see Dmitry's point in Bug 66521 about making gtest use FastMalloc.h as the only sane solution here. > > Good :) Is that what are you shooting for now? Do you need help? I have your patch from Bug 66521 working on the Apple Mac port. I need help testing the Windows vcproj changes (which I merged by hand). > I do not think USE_SYSTEM_MALLOC is going to cut it. Basically, since these are webkit/webcore unit tests, we do want to be able to test in the exact regimes ports compile. This includes running unit tests with global fast malloc/free. The testing framework should not require any specific set of ENABLE_/DISABLE_ defines (individual tests might though). I agree. I've finally reached the same conclusion. :) Duping this to Bug 66521. Fixing that also fixes this crash. *** This bug has been marked as a duplicate of bug 66521 ***
(In reply to comment #13) > (In reply to comment #12) > > This change fixed crashes in Release builds when a WTF::String() comparison fails (and when compiling with "-g -O0"), but I'm still seeing crashes on Debug builds. Investigating. > > LOL! Looks like threading isn't being initialized properly in TestWebKitAPI for a new test I wrote (an assertion is firing): > [...] > That's a separate bug. I'll post the patch for this bug. I filed Bug 75064 for this issue.