502474 - valgrind - invalid read size 1 - nsXPTCVariant::IsPtrData() const (xptcall.h:110), invalid write in memset()

Status: RESOLVED INVALID

(Core :: XPConnect, defect)

Description

[Bob Clary [:bc] (inactive)]

linux trunk with jemalloc enabled.

http://test.bclary.com//tests/mozilla.org/js/js-test-driver-standards.html?test=e4x/GC/regress-324117.js;language=type;text/javascript;version=1.5

==15444== Invalid read of size 1
==15444==    at 0x4342DD0: nsXPTCVariant::IsPtrData() const (xptcall.h:110)
==15444==    by 0x4342CF0: invoke_copy_to_stack (xptcinvoke_gcc_x86_unix.cpp:55)
==15444==    by 0x4342DAA: NS_InvokeByIndex_P (xptcinvoke_gcc_x86_unix.cpp:69)
==15444==    by 0x43307A0: nsProxyObjectCallInfo::Run() (nsProxyEvent.cpp:181)
==15444==    by 0x43263E3: nsThread::ProcessNextEvent(int, int*) (nsThread.cpp:527)
==15444==    by 0x42B1D9E: NS_ProcessNextEvent_P(nsIThread*, int) (nsThreadUtils.cpp:230)
==15444==    by 0x559FEA7: nsBaseAppShell::Run() (nsBaseAppShell.cpp:170)
==15444==    by 0x582FCA2: nsAppStartup::Run() (nsAppStartup.cpp:193)
==15444==    by 0x4046A1E: XRE_main (nsAppRunner.cpp:3369)
==15444==    by 0x80497B6: main (nsBrowserApp.cpp:156)
==15444==  Address 0xae9f5bd is 13 bytes after a block of size 16 alloc'd
==15444==    at 0x8058AF5: arena_malloc_small (jemalloc.c:4057)
==15444==    by 0x8058F3F: arena_malloc (jemalloc.c:4117)
==15444==    by 0x805901E: imalloc (jemalloc.c:4129)
==15444==    by 0x805DA17: malloc (jemalloc.c:6160)
==15444==    by 0x435AA39: nsStringBuffer::Alloc(unsigned int) (nsSubstring.cpp:204)
==15444==    by 0x435C7FC: nsACString_internal::MutatePrep(unsigned int, char**, unsigned int*) (nsTSubstring.cpp:179)
==15444==    by 0x435C903: nsACString_internal::ReplacePrep(unsigned int, unsigned int, unsigned int) (nsTSubstring.cpp:224)
==15444==    by 0x435CCBC: nsACString_internal::Assign(char const*, unsigned int) (nsTSubstring.cpp:360)
==15444==    by 0x435CEE7: nsACString_internal::Assign(nsACString_internal const&) (nsTSubstring.cpp:421)
==15444==    by 0x5A07691: nsCString::nsCString(nsCString const&) (nsTString.h:86)
==15444==    by 0x5A12025: void nsTArrayElementTraits<nsCString>::Construct<nsDependentCString>(nsCString*, nsDependentCString const&) (nsTArray.h:193)
==15444==    by 0x5A11D48: void nsTArray<nsCString>::AssignRange<nsDependentCString>(unsigned int, unsigned int, nsDependentCString const*) (nsTArray.h:875)

not sure this is jemalloc or not, but we can with xpconnect. sensitive for now.

Comment 1

[Bob Clary [:bc] (inactive)]

Attachment: valgrind output

actually, there is more.

Invalid read of size 1
at 0x4342DD0: nsXPTCVariant::IsPtrData() const (xptcall.h:110)

Invalid read of size 1
at 0x4342DC4: nsXPTType::operator unsigned char() const (xptinfo.h:72)

Invalid read of size 4
at 0x4342D69: invoke_copy_to_stack (xptcinvoke_gcc_x86_unix.cpp:66)

Invalid read of size 4
0x4330938: nsProxyObjectCallInfo::CopyStrings(int) (nsProxyEvent.cpp:229)

Invalid write of size 1
at 0x4008378: memset (mc_replace_strmem.c:493)

Comment 2

[Brian Crowder]

Is this possibly happening simply because valgrind gets confused by jemalloc's magic?  Do we need better annotations for jemalloc?

Comment 3

[Nicholas Nethercote [inactive]]

> Is this possibly happening simply because valgrind gets confused by jemalloc's
> magic?  Do we need better annotations for jemalloc?

What magic is likely to confuse Valgrind?

It's possible jemalloc's annotations are wrong, but if they were I would expect lots and lots of false positives.  You could determine it for sure by building with --disable-jemalloc and see if the errors come up again.

In general, I'd recommend giving Valgrind the benefit of the doubt and investigating its warnings.

Comment 4

[Bob Clary [:bc] (inactive)]

two bugs where there are probably false positives which I see oodles and oodles of are bug 443965 and bug 457223. The one in this bug I only saw this once (so far).

Comment 5

[Johnny Stenback (:jst)]

peterv says he'll have a look at this one.

Comment 6

[Julian Seward [:jseward]]

Urr, custom allocators.  Me no like.

I would say, for this bug and all like it, if it can't be reproduced
in a --disable-jemalloc build, it should be closed as invalid.

Despite considerable efforts on annotating jemalloc, I don't think
that the results (w.r.t. Valgrind) are ever going to be as good as
using standard malloc/free, because

(1) we can't control the size of the redzones at the start & end of
    each block

(2) we can't control how long freed memory stays out of circulation
    before it gets reallocated

(3) stock valgrind releases are unable to describe invalid
    addresses in terms of recently-freed blocks, at least not without
    uncommitted and potentially-expensive hacks to Memcheck

(4) we can't force realloc to always-move, and hence can't guarantee
    to detect errors where Fx calls realloc but then continues to
    use the old address

(5) the (je)malloc/(je)free code necessarily runs on the simulated
    (guest) CPU, whereas when intercepting standard malloc/free, the
    work of running the allocator is done by the host cpu.  Given that
    the guest CPU is 10-30 x slower than the real one, when running
    Memcheck, this can be a significant performance loss in
    allocation-intensive code.

Recently I've been playing with running Win32 code on Valgrind, using
Wine as an intermediary.  Wine provides a malloc/free implementation
for the Windows app being run, and has to describe to Memcheck what it
is doing.  So I've seen these problems at first-hand.

IMO, we should spread the word that, although jemalloc is great for
production builds, it should not be used in any build intended to be
Memcheck'd.

There are fledgling efforts to use Helgrind, a thread error detector
in the Valgrind suite, on Fx (eg bug 544736).  Helgrind also needs to
intercept malloc/free and therefore is also likely to suffer if faced
with jemalloc.

Comment 7

[Bob Clary [:bc] (inactive)]

K, I've stopped using jemalloc on my builds. I'll mark invalid but leave hidden for now. Peter, if you disagree please reopen.