Bug 81065 – WebKitTestRunner crashes when running release Tests

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Stephanie Lewis]

WebKitTestRunner crashes when running release due to the ostringstream object used to dump output.  When copying strings out of it it allocates the strings with FastMalloc and then tries to free them with system free.  I am moving WebKitTestRunner off of ostringstream.

Radar <rdar://problem/10944309>

[Alexey Proskuryakov]

This is confusing. Is it known that there is some deep problem preventing stringstream from working in WTR?

I don't think that we should be abandoning parts of C++ any time some misconfiguration in the project breaks such part (assuming it is a misconfiguration in the project).

[Geoffrey Garen]

> Is it known that there is some deep problem preventing stringstream from working in WTR?

Yes.

> I don't think that we should be abandoning parts of C++ any time some misconfiguration in the project breaks such part (assuming it is a misconfiguration in the project).

The problem isn't misconfiguration -- the problem is that C++ libraries that do some allocations in inlined functions and some allocations in out-of-line functions are incompatible with our FastMalloc configuration.

[Alexey Proskuryakov]

There is nothing magic in FastMalloc, we're just overriding global new and delete operators and using STL as designed. In a correctly configured project with a correct C++ implementation, this should just work. If we believe that we're doing it correctly, then we should file a bug against clang.

[Anders Carlsson]

Stephanie, what are the mismatched calls to operator new and operator delete? It seems to me that it's better to try to fix that problem instead of rewriting huge parts of WKTR.

[Stephanie Lewis]

I talked to Anders and we looked at the disassembly.  The call to allocate the string is inlined in the WebKitTestRunner file and so when it allocates it uses the FastMalloc override.  The destructor is not inlined, so it uses the system free.

Anders suggested just turning off FastMalloc for WebKitTestRunner which is what I'm going to do since I'm running into encoding differences replacing ostringstream.

[Mark Rowe (bdash)]

It is possible that could cause the opposite problem if WebKitTestRunner uses anything from WTF that inlines an allocation but not a deletion, as the code built in to JavaScriptCore would still be compiled with FastMalloc enabled.

[Stephanie Lewis]

Based on Mark's comment I went back to replacing ostringstream.  Uploading patch

[Stephanie Lewis]

Created attachment 132191 [details]
patch - replace ostringstream

[WebKit Review Bot]

Attachment 132191 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/WebKitTestRunner..." exit_code: 1
Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.h:43:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:198:  One line control clauses should not use braces.  [whitespace/braces] [4]
Tools/WebKitTestRunner/InjectedBundle/InjectedBundlePage.cpp:1229:  More than one command on the same line  [whitespace/newline] [4]
Total errors found: 3 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 132191 [details]
patch - replace ostringstream

Attachment 132191 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/11955982

[Geoffrey Garen]

Comment on attachment 132191 [details]
patch - replace ostringstream

r=me if you can get Windows to agree.

[Stephanie Lewis]

Created attachment 132379 [details]
second patch

verifying I fixed Windows

[WebKit Review Bot]

Attachment 132379 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/WebKitTestRunner..." exit_code: 1
Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.h:43:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 9 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Build Bot]

Comment on attachment 132379 [details]
second patch

Attachment 132379 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/11967454

[Stephanie Lewis]

Created attachment 132416 [details]
patch - this time with exports

This time with exports for Windows.

[Stephanie Lewis]

Committed http://trac.webkit.org/projects/webkit/changeset/111097