738034 - YARR asserts when anonymous mmap returns address zero

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

RegExp("((\\2))").exec()

(without Valgrind testcase does not abort in any way)

I ran with valgrind --dsymutil=yes ./js

Using 64-bit js opt shell on Mac 10.7 with Valgrind r12455 on m-c changeset cea47dfc3fb7, the following error is shown:

==45879== Process terminating with default action of signal 6 (SIGABRT)
==45879==    at 0x5DF82A: __kill (in /usr/lib/system/libsystem_kernel.dylib)
==45879==    by 0x100284811: JSC::Yarr::interpret(JSC::Yarr::BytecodePattern*, unsigned short const*, unsigned int, unsigned int, int*) (YarrInterpreter.cpp:1865)
==45879==    by 0x100181D95: js::RegExpShared::execute(JSContext*, unsigned short const*, unsigned long, unsigned long*, js::MatchPairs**) (RegExpObject.cpp:304)
==45879==    by 0x10018653F: bool ExecuteRegExpImpl<js::RegExpShared>(JSContext*, js::RegExpStatics*, js::RegExpShared&, JSLinearString*, unsigned short const*, unsigned long, unsigned long*, js::RegExpExecType, JS::Value*) (RegExp.cpp:138)
==45879==    by 0x100185949: ExecuteRegExp(JSContext*, int (*)(JSContext*, unsigned int, JS::Value*), unsigned int, JS::Value*) (RegExp.cpp:624)
==45879==    by 0x100183C09: js::regexp_exec(JSContext*, unsigned int, JS::Value*) (RegExp.cpp:646)
==45879==    by 0x100081FB2: js::InvokeKernel(JSContext*, js::CallArgs, js::MaybeConstruct) (jscntxtinlines.h:314)
==45879==    by 0x100079810: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinterp.cpp:2746)
==45879==    by 0x10006C3BD: js::RunScript(JSContext*, JSScript*, js::StackFrame*) (jsinterp.cpp:480)
==45879==    by 0x1000826ED: js::ExecuteKernel(JSContext*, JSScript*, JSObject&, JS::Value const&, js::ExecuteType, js::StackFrame*, JS::Value*) (jsinterp.cpp:678)
==45879==    by 0x100082856: js::Execute(JSContext*, JSScript*, JSObject&, JS::Value*) (jsinterp.cpp:719)
==45879==    by 0x1000196DA: JS_ExecuteScript (jsapi.cpp:5249)

Comment 1

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

Attachment: valgrind --trace-syscalls=yes --dsymutil=yes ./js testcase.js

trace syscalls log from Valgrind

Comment 2

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

Attachment: Valgrind log with -d

valgrind -d --dsymutil=yes ./js testcase.js 2>&1

Comment 3

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

> Using 64-bit js opt shell on Mac 10.7 with Valgrind r12455 on m-c changeset
> cea47dfc3fb7, the following error is shown:

Just to clarify, turns out that the changeset listed above is from IonMonkey, but I tested on mozilla-central 9023803c5d64 and the issue still exists.

Comment 4

[Julian Seward [:jseward]]

I _believe_ (#include <disclaimer.h> etc) this is a bug in Yarr, or 
perhaps in its embedding in the JS engine.  What we have is it doing
CRASH at this point

YarrInterpreter.cpp:1354

    int interpret()   // I think this is Interpreter::interpret
    {
        allocatorPool = pattern->m_allocator->startAllocator();
        if (!allocatorPool)
            CRASH();

as a result of a mmap-anonymous in startAllocator() that legitimately,
if bizarrely, returns 0 when run on Valgrind.  I haven't checked what
startAllocator does, but I suspect that it detects that
0 != MAP_FAILED, so the allocation succeeded, but then this test here
is wrong.  I don't think we can assume any specific allocatorPool
value means the allocation failed; that info has to be communicated
out of band somehow -- perhaps as a non-page-aligned value, in the
same way that MAP_FAILED does.

That said, this does expose a scary bug in V on OSX, in that mmap
has been kludge-ly handled for as long as the port has existed -- 
V should never have allowed this mmap call to succeed and return zero.

Comment 5

[Julian Seward [:jseward]]

Attachment: "fix" for valgrind that stops it returning zero for anonymous mmap on OSX

Patching V thusly causes it to run the testcase without it SIGABRTing.

Comment 6

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

(In reply to Julian Seward from comment #5)
> Created attachment 609325 [details] [diff] [review]
> "fix" for valgrind that stops it returning zero for anonymous mmap on OSX
> 
> Patching V thusly causes it to run the testcase without it SIGABRTing.

This patch works wonderfully well, the SIGABRT error no longer shows up. :)

Comment 7

[Julian Seward [:jseward]]

(In reply to Julian Seward from comment #5)
> "fix" for valgrind that stops it returning zero for anonymous mmap on OSX

Committed as revision r12466.

Comment 8

[Julian Seward [:jseward]]

(In reply to Julian Seward from comment #7)
> > "fix" for valgrind that stops it returning zero for anonymous mmap on OSX
> Committed as revision r12466.

r12466 got backed out by r12813.  Whilst sounding plausible, r12466 causes
other apps (TextEdit) to fail on MacOS, and it was only added in order to work
around a bug in Yarr.  So, backing out.

Comment 9

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

Using Valgrind r13104, on m-c changeset 2937fd8e35a1, using:

valgrind --dsymutil=yes --smc-check=all-non-file ./js 738034.js

I no longer reproduce this error. Assuming fixed by the Yarr import upgrade in bug 740015 that landed only recently.