Mads, thanks a lot for review! I've reran tests and curiously release mode tests for ia32 failed. I don't know precise reason for failure yet (only threaded test fails), but that made me think if my approach is sane. For example consider the case of CallIC. Imagine that call to get property with interceptor only triggered GC, but didn't return the value. That might invalidate all the pointers. I don't know what would happen to receiver and name stored in previous stack frame (would they mended?), but mostly for sure holder stored on new frame won't be adjusted (is that correct) which seems problematic.
If my analysis is right, is there a simple way out? Storing handles in stubs didn't work when I first tried that (holder might come from new space, btw, why we don't allow storing handles to objects in new space?) I was considering compiling a function to check the rest (from holder to holder of cached result) and passing it into a runtime code (to be invoked after interceptor check), but that might be not the best solution. Any ideas are most appreciated. http://codereview.chromium.org/140069/diff/5032/6015 File src/ia32/stub-cache-ia32.cc (right): http://codereview.chromium.org/140069/diff/5032/6015#newcode437 Line 437: // Do tail-call to LoadPropertyByLookupResult On 2009/07/07 12:16:56, Mads Ager wrote: > End comment with period. Done. http://codereview.chromium.org/140069/diff/5032/6015#newcode492 Line 492: // Under EnterInternalFrame and push(holder) this refers to name On 2009/07/07 12:16:56, Mads Ager wrote: > End comment with period. Done. http://codereview.chromium.org/140069/diff/5032/6013 File test/cctest/test-api.cc (right): http://codereview.chromium.org/140069/diff/5032/6013#newcode5059 Line 5059: // Now it should be (hopefully) ICed and keep a reference On 2009/07/07 12:16:56, Mads Ager wrote: > Remove '(hopefully)'. :-) Done. http://codereview.chromium.org/140069/diff/5032/6013#newcode5205 Line 5205: // This test checks that if interceptor provides a function, On 2009/07/07 12:16:56, Mads Ager wrote: > It looks like this is testing the case where you use the cached function from > the prototype chain? Ctrl-C/Ctrl-V (or, to more precise, y/p). Sorry and thanks a lot for spotting this. http://codereview.chromium.org/140069 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
