826588 - Differential Testing: Getting different output on 64-bit Windows js shells involving lastIndex

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

r = RegExp("(){0}", "g")
r.test()
print(r.lastIndex)

prints a number that is different everytime, on js opt shell on m-c changeset ceef8a5c3ca1 without any CLI arguments. This only seems to happen on 64-bit builds.

Other platforms / OS'es or Win32 js shells seem to output 0 instead.

Jesse advises to set this s-s pending further investigation.

autoBisect is now running.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   116118:7711a36c2771
user:        Sean Stangl
date:        Wed Dec 12 17:42:02 2012 -0800
summary:     Bug 808245, Part 5/6 - Use MatchPairs for RegExp output. r=dvander

Comment 2

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

This is so bad that I've had to disable the compareJIT fuzzer on 64-bit Windows.

Comment 3

[Sean Stangl [:sstangl]]

This is a compiler bug on Win64. The problem is that the "length" parameter of executeMatchOnly() takes on a random value, but it has the correct value in the caller.

Printing |length| within executeMatchOnly() causes the test to pass.
Disabling optimizations around executeMatchOnly() does not fix it.

I'm not sure how to fix this.

Comment 4

[Sean Stangl [:sstangl]]

On further examination, it looks like MatchResult::MatchResult(EncodedMatchResult) assumes that (sizeof size_t) == 32. I'm not sure why this wouldn't break on x86_64 Linux, though.

Comment 5

[Sean Stangl [:sstangl]]

Unfortunately, the problem is that Yarr is built assuming the presence of an ABI that includes a secondary return register. Win64 does not have one, although returnReg2 is erroneously defined for the platform in yarr/YarrJIT.cpp.

Windows will happily let you return a struct of size 128 (inasmuch as it passes compilation). I don't see anything in the Win64 ABI that defines what happens in this situation, but some time with a debugger would sort that mess out. Probably just needs yet another shim.

Comment 6

[[e-mail]]

(In reply to Sean Stangl from comment #5)
> Windows will happily let you return a struct of size 128 (inasmuch as it
> passes compilation). I don't see anything in the Win64 ABI that defines what
> happens in this situation, but some time with a debugger would sort that
> mess out. Probably just needs yet another shim.

http://msdn.microsoft.com/en-us/library/7572ztz4.aspx

Unless you're returning __m128, __m128i, __m128d, floats, and doubles, which are returned in XMM0, then if the return value does not fit into 64 bits then the caller must pass a pointer to the return value.

Comment 7

[Sean Stangl [:sstangl]]

Gary, this should have been fixed by Makoto's recent patch in Bug 830676.

Comment 8

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

I verify that the assertion no longer occurs with the testcase. Setting in-testsuite? and assuming fixed by bug 830676.