Closed Bug 594138 Opened 10 years ago Closed 9 years ago

TM: testConservativeGC fails with gczeal

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: gwagner, Assigned: igor)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file, 1 obsolete file)

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))
> on current tip with gczeal = 2 enabled.

This is a JS_THREADSAFE build, right?
It fails in a normal (no threadsafe) debug build for me.
Attached patch v1 (obsolete) — Splinter Review
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)
(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
Attached patch v2Splinter Review
This implements Brendan's idea.
Attachment #473498 - Attachment is obsolete: true
Attachment #473687 - Flags: review?(anygregor)
Attachment #473498 - Flags: review?(anygregor)
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+
(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.
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
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.