TM: testConservativeGC fails with gczeal

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: gwagner, Assigned: Igor Bukanov)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 attachment, 1 obsolete attachment)

v2
1.16 KB, patch
gwagner
: review+
Details | Diff | Splinter Review
(Reporter)

Description

8 years ago
on current tip with gczeal = 2 enabled.

testConservativeGC
../../jsapi-tests/testConservativeGC.cpp:39:CHECK failed: !memcmp(&objCopy, JSVAL_TO_OBJECT(v2), sizeof(objCopy))
TEST-UNEXPECTED-FAIL | testConservativeGC | CHECK failed: !memcmp(&objCopy, JSVAL_TO_OBJECT(v2), sizeof(objCopy))
(Assignee)

Comment 1

8 years ago
> on current tip with gczeal = 2 enabled.

This is a JS_THREADSAFE build, right?
(Reporter)

Comment 2

8 years ago
It fails in a normal (no threadsafe) debug build for me.
(Assignee)

Comment 3

8 years ago
Created attachment 473498 [details] [diff] [review]
v1

This is a fallout from JSScope elimination. Now JSObject contains the shape field and that field can change under the GC. So the test cannot not use memcmp to check if the content of JSObject remains the same after the GC.
Assignee: general → igor
Attachment #473498 - Flags: review?(anygregor)
(Assignee)

Comment 4

8 years ago
(In reply to comment #0)
> on current tip with gczeal = 2 enabled.

How do you enable gczeal for tests? I had to do it via an explicit JS_SetGCZeal in tests.cpp.
Comment on attachment 473498 [details] [diff] [review]
v1

How about keeping the memcmp but copying objShape over first?

/be
(Assignee)

Comment 6

8 years ago
Created attachment 473687 [details] [diff] [review]
v2

This implements Brendan's idea.
Attachment #473498 - Attachment is obsolete: true
Attachment #473687 - Flags: review?(anygregor)
Attachment #473498 - Flags: review?(anygregor)
(Reporter)

Comment 7

8 years ago
Comment on attachment 473687 [details] [diff] [review]
v2

>diff --git a/js/src/jsapi-tests/testConservativeGC.cpp b/js/src/jsapi-tests/testConservativeGC.cpp
>--- a/js/src/jsapi-tests/testConservativeGC.cpp
>+++ b/js/src/jsapi-tests/testConservativeGC.cpp
>@@ -30,18 +30,31 @@ BEGIN_TEST(testConservativeGC)
>     JS_GC(cx);
> 
>     EVAL("var a = [];\n"
>          "for (var i = 0; i != 10000; ++i) {\n"
>          "a.push(i + 0.1, [1, 2], String(Math.sqrt(i)));\n"
>          "}", &tmp);
> 
>     JS_GC(cx);
> 
>-    CHECK(!memcmp(&objCopy,  JSVAL_TO_OBJECT(v2), sizeof(objCopy)));
>+    checkObjectFields(&objCopy,  JSVAL_TO_OBJECT(v2));

Why are there 2 spaces?
Attachment #473687 - Flags: review?(anygregor) → review+
(Reporter)

Comment 8

8 years ago
(In reply to comment #4)
> (In reply to comment #0)
> > on current tip with gczeal = 2 enabled.
> 
> How do you enable gczeal for tests? I had to do it via an explicit JS_SetGCZeal
> in tests.cpp.

I have a working space where it is hardcoded in the runtime init.
(Assignee)

Comment 9

8 years ago
http://hg.mozilla.org/tracemonkey/rev/a1f43f4ef565 - pushed with a fix to testIsAboutToBeFinalized.cpp with a workaround to make sure to overwrite machine registers.
Whiteboard: fixed-in-tracemonkey
http://hg.mozilla.org/mozilla-central/rev/a1f43f4ef565
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.