Closed Bug 1014249 Opened 6 years ago Closed 6 years ago

DMD crashes when trying to print out an error message when you fail to set DMD=1

Categories

(Core :: DMD, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: mccr8, Assigned: njn)

Details

Attachments

(1 file)

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...)
I hit something similar yesterday when I set DMD but forgot to set the other env vars.
IIRC this happened when I ran without setting any environment variables, because I'm a pro like that.
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: nobody → n.nethercote
Status: NEW → ASSIGNED
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+
> 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...
Yeah, that's true.  You could just say "the DMD Wiki page" or something. :)
https://hg.mozilla.org/mozilla-central/rev/8936ac79e58a
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.