Reviewers: machenbach,
Description:
Fix invalid reuse of weak global handle in GetScriptWrapper.
This fixes a direct usage of a weak global handle in GetScriptWrapper
that just casted it to a strong local handle, while a subsequent GC
might clear it. Handlepocalypse anyone?
[email protected]
BUG=v8:2988
TEST=mjsunit/regress/regress-2988
Please review this at https://codereview.chromium.org/67273004/
SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge
Affected files (+12, -21 lines):
M src/handles.cc
A + test/mjsunit/regress/regress-2988.js
Index: src/handles.cc
diff --git a/src/handles.cc b/src/handles.cc
index
6fd047b0ab25562b5013c3daee4860847458ff8d..b86f19a498cdfc5fa1e0dfe942f9524db858f28f
100644
--- a/src/handles.cc
+++ b/src/handles.cc
@@ -242,9 +242,9 @@ static void ClearWrapperCache(v8::Isolate* v8_isolate,
Handle<JSValue> GetScriptWrapper(Handle<Script> script) {
if (script->wrapper()->foreign_address() != NULL) {
- // Return the script wrapper directly from the cache.
+ // Return a handle for the existing script wrapper from the cache.
return Handle<JSValue>(
- reinterpret_cast<JSValue**>(script->wrapper()->foreign_address()));
+
*reinterpret_cast<JSValue**>(script->wrapper()->foreign_address()));
}
Isolate* isolate = script->GetIsolate();
// Construct a new script wrapper.
@@ -255,10 +255,10 @@ Handle<JSValue> GetScriptWrapper(Handle<Script>
script) {
// The allocation might have triggered a GC, which could have called this
// function recursively, and a wrapper has already been created and
cached.
- // In that case, simply return the cached wrapper.
+ // In that case, simply return a handle for the cached wrapper.
if (script->wrapper()->foreign_address() != NULL) {
return Handle<JSValue>(
- reinterpret_cast<JSValue**>(script->wrapper()->foreign_address()));
+
*reinterpret_cast<JSValue**>(script->wrapper()->foreign_address()));
}
result->set_value(*script);
Index: test/mjsunit/regress/regress-2988.js
diff --git a/test/mjsunit/regress/regress-map-invalidation-1.js
b/test/mjsunit/regress/regress-2988.js
similarity index 78%
copy from test/mjsunit/regress/regress-map-invalidation-1.js
copy to test/mjsunit/regress/regress-2988.js
index
bcc6bbb615a1286248271bcf58e998fb9724154c..0311d2b76ded60d53a4627aa271d8c6d24bcdf36
100644
--- a/test/mjsunit/regress/regress-map-invalidation-1.js
+++ b/test/mjsunit/regress/regress-2988.js
@@ -25,24 +25,15 @@
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-// Flags: --allow-natives-syntax
+// Flags: --allow-natives-syntax --gc-global --throws
-var c = { x: 2, y: 1 };
+var f = eval("(function f() { throw 'kaboom'; })");
-function h() {
- %MigrateInstance(c);
- return 2;
-}
-%NeverOptimizeFunction(h);
-
-function f() {
- for (var i = 0; i < 100000; i++) {
- var n = c.x + h();
- assertEquals(4, n);
- }
- var o2 = [{ x: 2.5, y:1 }];
- return o2;
-}
+// Prepare that next MessageHandler::MakeMessageObject will result in
+// reclamation of existing script wrapper while weak handle is used.
+%FunctionGetScript(f);
+%SetAllocationTimeout(1000, 2);
+// This call throws to the console but the --throws flag passed to this
+// test will make sure we don't count it as an actual failure.
f();
-
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
