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

Status: RESOLVED FIXED

(Core :: DMD, defect)

Description

[Andrew McCreight [:mccr8]]

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...)

Comment 1

[Nicholas Nethercote [:njn]]

I hit something similar yesterday when I set DMD but forgot to set the other env vars.

Comment 2

[Andrew McCreight [:mccr8]]

IIRC this happened when I ran without setting any environment variables, because I'm a pro like that.

Comment 3

[Nicholas Nethercote [:njn]]

Attachment: Beef up the "is DMD enabled?" and "is DMD running?" checks

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.

Comment 4

[Andrew McCreight [:mccr8]]

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?

Comment 5

[Nicholas Nethercote [:njn]]

> 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...

Comment 6

[Andrew McCreight [:mccr8]]

Yeah, that's true.  You could just say "the DMD Wiki page" or something. :)

Comment 7

[Nicholas Nethercote [:njn]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8936ac79e58a

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8936ac79e58a