YARR asserts when anonymous mmap returns address zero

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: gkw, Unassigned)

Tracking

(Blocks: 1 bug, {testcase, valgrind})

Trunk
x86_64
Mac OS X
testcase, valgrind
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: js-triage-needed, partly [valgrind bug])

Attachments

(3 attachments)

(Reporter)

Description

6 years ago
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)
(Reporter)

Updated

6 years ago
Severity: critical → normal
(Reporter)

Comment 1

6 years ago
Created attachment 608100 [details]
valgrind --trace-syscalls=yes --dsymutil=yes ./js testcase.js

trace syscalls log from Valgrind
(Reporter)

Comment 2

6 years ago
Created attachment 608105 [details]
Valgrind log with -d

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

Comment 3

6 years ago
> 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.
(Reporter)

Updated

6 years ago
Hardware: x86 → x86_64
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.
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.
(Reporter)

Comment 6

6 years ago
(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. :)
(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.
Summary: Valgrind shows SIGABRT with testcase even though testcase does not crash → YARR asserts when anonymous mmap returns address zero
(Reporter)

Updated

6 years ago
Whiteboard: js-triage-needed → js-triage-needed, partly valgrind bug
(Reporter)

Updated

6 years ago
Whiteboard: js-triage-needed, partly valgrind bug → js-triage-needed, partly [valgrind bug]
(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.
(Reporter)

Comment 9

5 years ago
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.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.