Closed
Bug 1014249
Opened 7 years ago
Closed 7 years ago
DMD crashes when trying to print out an error message when you fail to set DMD=1
Categories
(Core :: DMD, defect)
Core
DMD
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: mccr8, Assigned: n.nethercote)
Details
Attachments
(1 file)
10.28 KB,
patch
|
mccr8
:
review+
|
Details | Diff | Splinter Review |
I think StatusMsg tries to call malloc_ and fails if DMD=1 isn't set. Maybe this could just be a printf to stderr? This is in the |!gIsDMDRunning| stuff. (I'm running my own mangled up version so it is possible I did something else wrong...)
![]() |
Assignee | |
Comment 1•7 years ago
|
||
I hit something similar yesterday when I set DMD but forgot to set the other env vars.
Reporter | ||
Comment 2•7 years ago
|
||
IIRC this happened when I ran without setting any environment variables, because I'm a pro like that.
![]() |
Assignee | |
Comment 3•7 years ago
|
||
The problem is that when DMD is enabled (built) but not running, if you call DMDReportAndDump() (e.g. via a bookmark) then DMD tries to print a failure message. But the printing of that failure message calls DMD's own malloc wrapper, which hasn't been set up because DMD isn't running! So this patch fixes the crash by using alloca() instead of the malloc wrapper to allocate the transient memory needed for DMD's status messages. But I decided to also strengthen the general checking further. For example, currently about:memory decides if DMD is running by looking at the $DMD env var, but that's not sufficient -- you need to set other env vars like LD_PRELOAD as well. So, the patch does the following. - Uses alloca() in DMD's StatusMessage() function so it doesn't have to heap-allocate. - Adds reliable isDMDEnabled() and isDMDRunning() methods to nsIMemoryReporterManager. "Enabled" refers to being enabled at build-time. - Uses those methods to strengthen the checking in aboutMemory.js and DMDReportAndDump(). - Renames the existing dmd::IsEnabled() as dmd::IsRunning(), to fit the new terminology. - Shows a failure message in about:memory if DMD's dumping failed.
Attachment #8427443 -
Flags: review?(continuation)
![]() |
Assignee | |
Updated•7 years ago
|
Assignee: nobody → n.nethercote
Status: NEW → ASSIGNED
Reporter | ||
Comment 4•7 years ago
|
||
Comment on attachment 8427443 [details] [diff] [review] Beef up the "is DMD enabled?" and "is DMD running?" checks Review of attachment 8427443 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/aboutmemory/content/aboutMemory.js @@ +296,5 @@ > > const DMDEnabledDesc = "Run DMD analysis and save it to '" + DMDFile + "'.\n"; > + const DMDDisabledDesc = "DMD is not running. Please re-start with $DMD and " + > + "the other relevant environment variables set " + > + "appropriately."; Maybe have a reference to the DMD wiki page here?
Attachment #8427443 -
Flags: review?(continuation) → review+
![]() |
Assignee | |
Comment 5•7 years ago
|
||
> Maybe have a reference to the DMD wiki page here?
Hmm... I'm a bit reluctant to hardcode a URL like that into the product. In order to see that tool-tip they must have built with DMD enabled, in which case they're probably already aware of the wiki page...
Reporter | ||
Comment 6•7 years ago
|
||
Yeah, that's true. You could just say "the DMD Wiki page" or something. :)
![]() |
Assignee | |
Comment 7•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/8936ac79e58a
Comment 8•7 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/8936ac79e58a
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in
before you can comment on or make changes to this bug.
Description
•