Closed Bug 585394 Opened 14 years ago Closed 14 years ago

random assertion on modules/plugin/test/reftest/pluginproblemui-direction-1.html due to non-null-terminated nsDependentCString

Categories

(Toolkit :: Crash Reporting, defect)

Product:
Toolkit
Component:
Crash Reporting
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED DUPLICATE of bug 584582

People

(Reporter: dbaron, Unassigned)

References

Details

(Keywords: intermittent-failure) 

We're seeing random assertions here:
REFTEST TEST-UNEXPECTED-FAIL | file:///home/cltbld/talos-slave/mozilla-central_fedora-debug_test-reftest/build/reftest/tests/modules/plugin/test/reftest/pluginproblemui-direction-1.html | assertion count 1 is more than expected 0 assertions

The most recent occurrence is:
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1281230511.1281231867.32694.gz
Rev3 Fedora 12 mozilla-central debug test reftest

and a bunch of prior occurrences are in bug 567367 comment 4, through bug 567367 comment 11 (incorrectly placed).


The log mentioned above (most recent) has the following assertion and stack:

###!!! ASSERTION: nsTDependentString must wrap only null-terminated strings: 'mData[mLength] == 0', file ../../../../dist/include/nsTDependentString.h, line 67
nsDependentCString::AssertValid [nsTDependentString.h:68]
nsDependentCString::nsDependentCString [nsTDependentString.h:92]
CrashReporter::WriteExtraData [toolkit/crashreporter/nsExceptionHandler.cpp:1406]
CrashReporter::WriteExtraForMinidump [toolkit/crashreporter/nsExceptionHandler.cpp:1431]
CrashReporter::OnChildProcessDumpRequested [toolkit/crashreporter/nsExceptionHandler.cpp:1467]
google_breakpad::CrashGenerationServer::ClientEvent [toolkit/crashreporter/google-breakpad/src/client/linux/crash_generation/crash_generation_server.cc:407]
google_breakpad::CrashGenerationServer::Run [toolkit/crashreporter/google-breakpad/src/client/linux/crash_generation/crash_generation_server.cc:287]
google_breakpad::CrashGenerationServer::ThreadMain [toolkit/crashreporter/google-breakpad/src/client/linux/crash_generation/crash_generation_server.cc:463]
libpthread.so.0 + 0x5ab5


This makes me think that the problem is that the relevant code in crashreporter:
    time_t crashTime = time(NULL);
    char crashTimeString[32];
    XP_TTOA(crashTime, crashTimeString, 10);

    WriteAnnotation(fd,
                    nsDependentCString("CrashTime"),
                    nsDependentCString(crashTimeString));
is leaving crashTimeString not explicitly null-terminated, and the nsDependentCString code is finding a null at some point during its construction that is no longer null by the time we reach AssertValid because the null is somewhere random on the stack that gets stomped on by calling AssertValid (though I admit that seems rather odd given the stack growth direction, though maybe I'm getting confused).  It does appear that XP_TTOA is my_itos, which is defined here: http://mxr.mozilla.org/mozilla-central/source/toolkit/crashreporter/google-breakpad/src/common/linux/linux_libc_support.h#128 and clearly does not null-terminate.
blocking2.0: --- → ?
It looks like the my_itos version of XP_TTOA is the only one of the XP_TTOA options that does not null-terminate.  That makes this Linux-only.
Please back out http://hg.mozilla.org/mozilla-central/rev/96f758f85b42
when this is fixed.  (It's easy to forget, since it's marked as 0-1 so it won't start failing when the assertion goes away!)
And, to be clear, I nominated this for blocking since it's reading stack memory and sending it as a crash annotation; that's a potential privacy violation and a potential (though pretty unlikely to have no null bytes on the stack, I'd think) crash.
This is bug 584582, and it's already blocking.
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → DUPLICATE
blocking2.0: ? → ---
(In reply to comment #3)
> Please back out http://hg.mozilla.org/mozilla-central/rev/96f758f85b42
> when this is fixed.  (It's easy to forget, since it's marked as 0-1 so it won't
> start failing when the assertion goes away!)

Done, but only because I have a script that reads reftest.list files and tells me when they refer to resolved bugs.

http://hg.mozilla.org/mozilla-central/rev/1066966425e1
Blocks: 438871
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.