Bug 27874 - Change malloc to fastMalloc and free to fastFree in Yarr's RegexInterpreter.cpp
Summary: Change malloc to fastMalloc and free to fastFree in Yarr's RegexInterpreter.cpp
Status: RESOLVED INVALID
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-07-31 04:21 PDT by Zoltan Horvath
Modified: 2011-06-16 21:50 PDT (History)
2 users (show)

See Also:


Attachments
proposed patch (2.54 KB, patch)
2009-07-31 04:24 PDT, Zoltan Horvath
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zoltan Horvath 2009-07-31 04:21:13 PDT
Use fastMalloc and fastFree instead of malloc and free in RegexInterpreter.cpp's methods.
Comment 1 Zoltan Horvath 2009-07-31 04:24:22 PDT
Created attachment 33870 [details]
proposed patch
Comment 2 Eric Seidel (no email) 2009-07-31 15:29:17 PDT
Comment on attachment 33870 [details]
proposed patch

Clearing review flag on attachment: 33870

Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	JavaScriptCore/ChangeLog
	M	JavaScriptCore/yarr/RegexInterpreter.cpp
Committed r46643
	M	JavaScriptCore/yarr/RegexInterpreter.cpp
	M	JavaScriptCore/ChangeLog
r46643 = 2fe77a59e15a22979daaf74b11f4283d9b4f2186 (trunk)
No changes between current HEAD and refs/remotes/trunk
Resetting to the latest refs/remotes/trunk
http://trac.webkit.org/changeset/46643
Comment 3 Eric Seidel (no email) 2009-07-31 15:29:20 PDT
All reviewed patches have been landed.  Closing bug.
Comment 4 Gavin Barraclough 2009-08-04 14:36:35 PDT
Unfortunately this change break the v8 tests (as run in the sunspider command-line harness), so reverted for now in r46778.
Comment 5 Eric Seidel (no email) 2009-08-04 14:38:32 PDT
Can you define "break"? :)
Comment 6 Gavin Barraclough 2009-08-04 14:47:02 PDT
Long hang spinning the cpu, then a crash.  Smells like something going off the deep end of the stack, but I'm afraid I don't really have time to debug this regression further right now.
Comment 7 Zoltan Horvath 2009-08-05 00:17:18 PDT
Weird. Was it on a MAC, wasn't it?
Comment 8 Eric Seidel (no email) 2009-08-05 07:19:55 PDT
It sounds like either the conversion to fastMalloc/fastFree was incomplete here, or that by changing over to fastMalloc we've tripped some other subtle bug in the regexp engine.  The latter is bad news and requires further investigation.  I worry that the current use of malloc/free could just be papering over some underlying bug, given Gavin's above comment.
Comment 9 Gavin Barraclough 2009-08-05 12:45:06 PDT
It might be worth noting that these mallocs and frees will just go away at some point – these will be replaced with a stack-based allocator, though I don't have a timescale for this right now.
Comment 10 Gavin Barraclough 2009-08-05 14:07:10 PDT
(In reply to comment #7)
> Weird. Was it on a MAC, wasn't it?

This is on Mac, testing the YARR interpreter by setting:

#define ENABLE_YARR 1
#define ENABLE_YARR_JIT 0
Comment 11 Gavin Barraclough 2011-06-16 21:50:25 PDT
These allocations are now stack based, and as such this change is no longer valid.