Closed
Bug 715750
Opened 13 years ago
Closed 12 years ago
Invalid read of size 4 with testcase at js::Sprint and maybe js_GC on the stack in Valgrind
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
WORKSFORME
People
(Reporter: gkw, Assigned: jseward)
Details
(Keywords: testcase, valgrind, Whiteboard: [valgrind bug])
Attachments
(2 files)
eval("(evalcx(''))".replace(""), newGlobal("new-compartment"))
throws a bunch of Valgrind "invalid read of size 4" errors on js opt shell from m-c changeset 44d992ccc97a without any CLI arguments.
s-s because Valgrind invalid reads aren't good, assuming everything is correctly working.
Example stacks:
==28284== Invalid read of size 4
==28284== at 0x838F04: _get_cpu_capabilities (in /usr/lib/system/libsystem_c.dylib)
==28284== by 0x834FCF: memmove (in /usr/lib/system/libsystem_c.dylib)
==28284== by 0x83D139: __udivmoddi4 (in /usr/lib/system/libsystem_c.dylib)
==28284== by 0xB590E: js::Sprint(js::Sprinter*, char const*, ...) (jsopcode.cpp:829)
==28284== by 0xBE784: Decompile(SprintStack*, unsigned char*, int) (jsopcode.cpp:2780)
==28284== by 0xB7BE9: DecompileCode(JSPrinter*, JSScript*, unsigned char*, unsigned int, unsigned int) (jsopcode.cpp:5440)
==28284== by 0xB7807: DecompileExpression(JSContext*, JSScript*, JSFunction*, unsigned char*) (jsopcode.cpp:5839)
==28284== by 0xB7269: js_DecompileValueGenerator (jsopcode.cpp:5737)
==28284== by 0x2E5CE: js_ReportValueErrorFlags(JSContext*, unsigned int, unsigned int, int, JS::Value const&, JSString*, char const*, char const*) (jscntxt.cpp:1219)
==28284== by 0x4C18F: js_ReportIsNotFunction(JSContext*, JS::Value const*, unsigned int) (jsfun.cpp:2416)
==28284== by 0x8F1D8: js::InvokeKernel(JSContext*, js::CallArgs, js::MaybeConstruct) (jsinterp.cpp:499)
==28284== by 0x7F9A8: js::Interpret(JSContext*, js::StackFrame*, js::InterpMode) (jsinterp.cpp:3414)
==28284== Address 0xffff0020 is not stack'd, malloc'd or (recently) free'd
or
==28284== Invalid read of size 4
==28284== at 0x7D1895: __commpage_gettimeofday (in /usr/lib/system/libsystem_c.dylib)
==28284== by 0x7D17FB: gettimeofday (in /usr/lib/system/libsystem_c.dylib)
==28284== by 0x12B579: PRMJ_Now (prmjtime.cpp:313)
==28284== by 0x180845: js::gcstats::Statistics::beginGC(JSCompartment*, js::gcstats::Reason) (Statistics.cpp:300)
==28284== by 0x4F3B5: js_GC(JSContext*, JSCompartment*, JSGCInvocationKind, js::gcstats::Reason) (Statistics.h:159)
==28284== by 0x2D76C: js_DestroyContext(JSContext*, JSDestroyContextMode) (jscntxt.cpp:641)
==28284== by 0x15DF8: JS_DestroyContext (jsapi.cpp:1171)
==28284== by 0x8856: main (js.cpp:4966)
==28284== Address 0xffff006c is not stack'd, malloc'd or (recently) free'd
|
||
Comment 1•13 years ago
|
||
Julian, does this look like cases where valgrind isn't trapping the system calls?
|
Assignee | |
Comment 2•13 years ago
|
||
Yeah, this has the same underlying cause as bug 710438. You might as well close it as a dup. The problem here and in bug 710438 is that memcheck is set up to intercept functions called "memcpy$VARIANT$sse3x" and "memcpy$VARIANT$sse42" etc but not plain "memcpy" (on MacOS, that is). Can you try the following patch (untested!) that also asks it to intercept the plain versions? Index: memcheck/mc_replace_strmem.c =================================================================== --- memcheck/mc_replace_strmem.c (revision 12325) +++ memcheck/mc_replace_strmem.c (working copy) @@ -849,7 +849,7 @@ MEMCPY(NONE, ZuintelZufastZumemcpy) #elif defined(VGO_darwin) - //MEMCPY(VG_Z_LIBC_SONAME, memcpy) + MEMCPY(VG_Z_LIBC_SONAME, memcpy) //MEMCPY(VG_Z_DYLD, memcpy) MEMCPY(VG_Z_LIBC_SONAME, memcpyZDVARIANTZDsse3x) /* memcpy$VARIANT$sse3x */ MEMCPY(VG_Z_LIBC_SONAME, memcpyZDVARIANTZDsse42) /* memcpy$VARIANT$sse42 */ @@ -981,7 +981,7 @@ MEMMOVE(VG_Z_LIBC_SONAME, memmove) #elif defined(VGO_darwin) - //MEMMOVE(VG_Z_LIBC_SONAME, memmove) + MEMMOVE(VG_Z_LIBC_SONAME, memmove) //MEMMOVE(VG_Z_DYLD, memmove)# MEMMOVE(VG_Z_LIBC_SONAME, memmoveZDVARIANTZDsse3x) /* memmove$VARIANT$sse3x */ MEMMOVE(VG_Z_LIBC_SONAME, memmoveZDVARIANTZDsse42) /* memmove$VARIANT$sse42 */
|
|
Reporter | |
Comment 3•13 years ago
|
||
> Yeah, this has the same underlying cause as bug 710438. You might as well
> close it as a dup.
Opening up since it seems to be a Valgrind issue and not s-s.Group: core-security
|
|
Reporter | |
Comment 5•13 years ago
|
||
> The problem here and in bug 710438 is that memcheck is set up to intercept
> functions called "memcpy$VARIANT$sse3x" and "memcpy$VARIANT$sse42" etc but
> not plain "memcpy" (on MacOS, that is). Can you try the following patch
> (untested!) that also asks it to intercept the plain versions?
That patch totally knocked out my MBP, first w/ a kernel panic when I ran `valgrind --dsymutil=yes ./js`, and the second time it just froze when I re-ran the same command after rebooting. I had to forcibly reboot both times.
I tried running `valgrind ls` and even it failed, albeit more gracefully.|
|
Assignee | |
Comment 6•13 years ago
|
||
(In reply to Gary Kwong [:gkw, :nth10sd] from comment #5) > That patch totally knocked out my MBP, first w/ a kernel panic when I ran Mmmhm, that's ungood. What about the older Mac Mini mentioned in bug 710438 (which I thought is what you were actually going to test on ..)
|
|
Reporter | |
Comment 7•13 years ago
|
||
Just to confirm: This bug: Occurs on Lion, in new Mac Mini / MBP Bug 710438: Occurs on Snow Leopard, in mid-2009 Mac Mini Your patch fixes bug 710438 but blows up Lion, so maybe Lion needs a different fix from Snow Leopard. :(
|
|
Assignee | |
Comment 8•13 years ago
|
||
Revised version of patch, in which we only replace plain memcpy and memmove on Snow Leopard but not on Lion. Can you give this a try? Index: memcheck/mc_replace_strmem.c =================================================================== --- memcheck/mc_replace_strmem.c (revision 12325) +++ memcheck/mc_replace_strmem.c (working copy) @@ -849,7 +849,9 @@ MEMCPY(NONE, ZuintelZufastZumemcpy) #elif defined(VGO_darwin) - //MEMCPY(VG_Z_LIBC_SONAME, memcpy) +# if DARWIN_VERS == DARWIN_10_6 + MEMCPY(VG_Z_LIBC_SONAME, memcpy) +# endif //MEMCPY(VG_Z_DYLD, memcpy) MEMCPY(VG_Z_LIBC_SONAME, memcpyZDVARIANTZDsse3x) /* memcpy$VARIANT$sse3x */ MEMCPY(VG_Z_LIBC_SONAME, memcpyZDVARIANTZDsse42) /* memcpy$VARIANT$sse42 */ @@ -981,7 +983,9 @@ MEMMOVE(VG_Z_LIBC_SONAME, memmove) #elif defined(VGO_darwin) - //MEMMOVE(VG_Z_LIBC_SONAME, memmove) +# if DARWIN_VERS == DARWIN_10_6 + MEMMOVE(VG_Z_LIBC_SONAME, memmove) +# endif //MEMMOVE(VG_Z_DYLD, memmove)# MEMMOVE(VG_Z_LIBC_SONAME, memmoveZDVARIANTZDsse3x) /* memmove$VARIANT$sse3x */ MEMMOVE(VG_Z_LIBC_SONAME, memmoveZDVARIANTZDsse42) /* memmove$VARIANT$sse42 */
|
|
Assignee | |
Comment 9•12 years ago
|
||
This is filed in Valgrind's bug tracker at https://bugs.kde.org/show_bug.cgi?id=285662 I can't reproduce this on a low end Mac Mini though.
|
|
Reporter | |
Comment 10•12 years ago
|
||
(In reply to Julian Seward from comment #9) > This is filed in Valgrind's bug tracker at > https://bugs.kde.org/show_bug.cgi?id=285662 > > I can't reproduce this on a low end Mac Mini though. My Mac Mini (Lion) is a quad core i7 server model 2011 one, 8Gb ram. My other Mac Mini (Snow Leopard) is a 2009 Core 2 Duo (dual core) one, 4Gb ram.
|
|
Reporter | |
Comment 11•12 years ago
|
||
This patch seems to no longer cause a kernel panic in Lion, but with the testcase in comment 0, I still get invalid reads: Index: memcheck/mc_replace_strmem.c =================================================================== --- memcheck/mc_replace_strmem.c (revision 12400) +++ memcheck/mc_replace_strmem.c (working copy) @@ -850,7 +850,9 @@ MEMCPY(NONE, ZuintelZufastZumemcpy) #elif defined(VGO_darwin) - //MEMCPY(VG_Z_LIBC_SONAME, memcpy) +# if DARWIN_VERS == DARWIN_10_6 + MEMCPY(VG_Z_LIBC_SONAME, memcpy) +# endif //MEMCPY(VG_Z_DYLD, memcpy) MEMCPY(VG_Z_LIBC_SONAME, memcpyZDVARIANTZDsse3x) /* memcpy$VARIANT$sse3x */ MEMCPY(VG_Z_LIBC_SONAME, memcpyZDVARIANTZDsse42) /* memcpy$VARIANT$sse42 */ @@ -982,7 +984,9 @@ MEMMOVE(VG_Z_LIBC_SONAME, memmove) #elif defined(VGO_darwin) - //MEMMOVE(VG_Z_LIBC_SONAME, memmove) +# if DARWIN_VERS == DARWIN_10_6 + MEMMOVE(VG_Z_LIBC_SONAME, memmove) +# endif //MEMMOVE(VG_Z_DYLD, memmove)# MEMMOVE(VG_Z_LIBC_SONAME, memmoveZDVARIANTZDsse3x) /* memmove$VARIANT$sse3x */ MEMMOVE(VG_Z_LIBC_SONAME, memmoveZDVARIANTZDsse42) /* memmove$VARIANT$sse42 */
|
|
Assignee | |
Comment 12•12 years ago
|
||
(In reply to Julian Seward from comment #2) > Yeah, this has the same underlying cause as bug 710438. You might as well > close it as a dup. This is wrong .. 710438 is unrelated to this. This seems to be related to accesses to high memory (0xFFFFxxxx) and is unrelated to function redirection. Will reopen 710438.
|
|
Assignee | |
Updated•12 years ago
|
Assignee: general → jseward
|
|
Assignee | |
Comment 13•12 years ago
|
||
Confirming as a bug in V. It could also have been caused by the OSX kernel giving us bogus information about memory segments, but that's not the case. The logic in VG_(get_changed_segments) doesn't correctly compute the changed segment set, leading Memcheck to incorrectly conclude that the addresses in the range 0xFFFF0000 to 0xFFFFF000 (in these traces) is inaccessible.
|
|
Assignee | |
Comment 14•12 years ago
|
||
A trip into the murky world of the OSX kernel moving memory mappings around under our feet. Anyway, fixed, r12422. Thanks for the help with finding a repro case.
|
|
Reporter | |
Comment 15•12 years ago
|
||
(In reply to Julian Seward from comment #14) > A trip into the murky world of the OSX kernel moving memory mappings > around under our feet. Anyway, fixed, r12422. Thanks for the help > with finding a repro case. WFM now. Resolved FIXED by a known Valgrind patch. You're welcome!
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
|
|
Reporter | |
Comment 16•12 years ago
|
||
WFM is a better resolution.
Resolution: FIXED → WORKSFORME
Whiteboard: js-triage-needed → valgrind bug
|
|
Reporter | |
Updated•12 years ago
|
Whiteboard: valgrind bug → [valgrind bug]
You need to log in
before you can comment on or make changes to this bug.
Description
•