Summary: | Change malloc to fastMalloc and free to fastFree in Yarr's RegexInterpreter.cpp | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Zoltan Horvath <zoltan> | ||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED INVALID | ||||||
Severity: | Normal | CC: | barraclough, eric | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | PC | ||||||
OS: | All | ||||||
Attachments: |
|
Description
Zoltan Horvath
2009-07-31 04:21:13 PDT
Created attachment 33870 [details]
proposed patch
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 All reviewed patches have been landed. Closing bug. Unfortunately this change break the v8 tests (as run in the sunspider command-line harness), so reverted for now in r46778. Can you define "break"? :) 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. Weird. Was it on a MAC, wasn't it? 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. 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. (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 These allocations are now stack based, and as such this change is no longer valid. |